Rewrite module page CSS inline with our CSS standards

Adding initial patch for feedback.

@crazyrohila Thanks for working on this!

1. +++ b/core/modules/locale/src/Form/TranslationStatusForm.php
   @@ -167,7 +170,9 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   +    // Add "package-group-list" class to form that can be reused on
   +    // module list page.
   +    $form ['#attributes']['class'][] = 'list-item-group';
   

   The comment and class name don't match

2. +++ b/core/modules/system/css/system.admin.css
   @@ -79,21 +79,7 @@ small .admin-link:after {
   +.list-item-group label {
   

   Let's use .list-item-group__label instead

3. +++ b/core/modules/system/css/system.admin.css
   @@ -116,46 +102,52 @@ small .admin-link:after {
   +.list-item__checkbox {
   

   The checkbox is a modifier of the list-item__column, so you could write list-item__column--checkbox instead

4. +++ b/core/modules/system/css/system.admin.css
   @@ -116,46 +102,52 @@ small .admin-link:after {
   +.list-item__title {
   

   Following the previous logic, this would be .list-item__column--title

5. +++ b/core/modules/system/css/system.admin.css
   @@ -116,46 +102,52 @@ small .admin-link:after {
   +  .list-item__description {
   

   And .list-item__column--description

6. +++ b/core/modules/system/src/Form/ModulesListForm.php
   @@ -232,7 +232,8 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   +    // Add "list-item-group" class to form.
   

   Let's leave out this comment as adding an attribute to the renderable array is very common.

7. +++ b/core/modules/system/src/Form/ModulesListForm.php
   @@ -232,7 +232,8 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   +    $form ['#attributes']['class'][] = 'list-item-group';
   

   There's an unexpected space after $form

8. +++ b/core/modules/system/src/Form/ModulesListForm.php
   @@ -261,6 +262,9 @@ protected function buildRow(array $modules, Extension $module, $distribution) {
   +    // Add "list-item" class to rows.
   

   Let's leave out this comment as well, adding an attribute to a renderable array is very common.

9. +++ b/core/modules/system/system.admin.inc
   @@ -225,26 +225,26 @@ function theme_system_modules_details($variables) {
   +    $row[] = array('class' => array('checkbox', 'list-item__column', 'list-item__checkbox'), 'data' => drupal_render($module['enable']));
   

   The original class .checkbox can be removed, as it is no longer used.

10. +++ b/core/themes/seven/css/components/modules-page.css
    @@ -17,31 +14,22 @@
    -.system-modules tr.even,
    -.system-modules tr.odd,
    

    These styling is removed but never reapplied. This changes the module overview on the 'Extend' page quite a bit.

@idebr, Thanks for review.

9. .checkbox was used to apply text-align: center that was being applied by system.theme.css (td.checkbox). I have removed checkbox class and applied text-align: center to list-item__column--checkbox.

10. Selector changed to `.list-item`.

1-8.
Updated code.

@crazyrohila Thanks for working on this issue! I did a manual test to check for visual regressions as well as a code review:

1. +++ b/core/modules/locale/src/Form/TranslationStatusForm.php
   @@ -167,7 +170,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   +    $form ['#attributes']['class'][] = 'list-item-group';
   

   There is a space after $form that can be removed.

2. +++ b/core/modules/system/css/system.admin.css
   @@ -79,21 +79,7 @@ small .admin-link:after {
   -.system-modules label,
   

   This selector was also used for the label 'Search' above the module filter on the 'Extend' page, so now this label has been reduced in size. Let's add a new selector for this label to make sure we don't introduce a usability regression.

3. +++ b/core/modules/system/src/Form/ModulesUninstallForm.php
   @@ -154,7 +154,8 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   -
   +    // Add "list-item-group" class to form.
   +    $form ['#attributes']['class'][] = 'list-item-group';
   

   Adding an attribute is very common, so I don't think a comment is necessary here. Let's remove it from the patch.

4. +++ b/core/modules/system/system.admin.inc
   @@ -225,26 +225,26 @@ function theme_system_modules_details($variables) {
   +    $col2 = '<label id="' . $id . '" for="' . $module['enable']['#id'] . '" class="list-item-group__label module-name table-filter-text-source">' . drupal_render($module['name']) . '</label>';
   

   Are the classes module-name and table-filter-text-source still needed? If these are only used for javascript, the class names should be prefixes with .js- according to the css coding standards.

5. +++ b/core/modules/system/system.admin.inc
   @@ -225,26 +225,26 @@ function theme_system_modules_details($variables) {
   -    $description .= '<div class="requirements">';
   -    $description .= '<div class="admin-requirements">' . t('Machine name: !machine-name', array('!machine-name' => '<span dir="ltr" class="table-filter-text-source">' . $key . '</span>')) . '</div>';
   +    $description .= '<div class="list-item__requirements">';
   

   The selector .system-modules .requirements in system.admin.css should be updated to the new selector.

6. +++ b/core/modules/system/system.admin.inc
   @@ -264,7 +264,7 @@ function theme_system_modules_details($variables) {
   +    $row[] = array('class' => array('list-item__column', 'list-item__column--description', 'expand'), 'data' => $col4);
   

   Is the class 'expand' necessary here? If it is only used for javascript, the class should be prefixed with .js- per the CSS coding standards.

1,2,3. Done
4. Yes, class was being used by js, so added .js- to class name.
5. This is already done in last patch.
6. Class was not in use, removed.

Class names are generic now and being used by other pages (eg. translation-update, modules uninstall), I think we should change file name too. current file name is modules-page.css. I am not sure if breaking this file into component files can be covered in this issue or we should create new one.

@crazyrohila Thanks, this is looking great! The module page is done, but I found one regression on the Locale translation status page (/admin/reports/translations) with manual testing:

+++ b/core/themes/seven/css/components/modules-page.css
@@ -17,31 +14,22 @@
-.system-modules tr.even,
-.system-modules tr.odd,
-.locale-translation-status-form tr.even,
-.locale-translation-status-form tr.odd {
-  background: #f3f4ee;
+.list-item {

The locale translation status (/admin/reports/translations) has no .list-item on its rows, so this styling is never applied:

Added list-item class to rows of table.

I noticed some visual problems with translation table, when we have some data there. but I think that is out of scope of this issue. attached screenshots.

Screenshot...

updated for latest HEAD.

I have added languages, but all are showing "updated" or "No update found". So I am not able to check this manually and not sure why this testInterface() Test is failing.

Can anyone point somethings here?