Sub-issue of #1190252: [573] Use csslint as a weapon to beat the crappy CSS out of Drupal core

Inline with the CSS cleanup efforts of the HTML5 initiative, using CSSLint at http://csslint.net provides a quick way to code-sniff our css and tweak styles.

  1. Copy and paste the the stylesheet(s) below into the css lint tool at http://csslint.net and test.
  2. Fix any warnings or errors the tool finds.
  3. Patch Drupal 8 locally and make sure the css changes have not broken anything visually.
  4. Create patch and upload for the testbot.

Files: modules/locale/locale.css

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

droplet’s picture

Status: Active » Needs review
FileSize
2.77 KB
-#locale-translate-edit-form tr .form-type-item abbr.ajax-changed {
+#locale-translate-edit-form .form-type-item abbr.ajax-changed {
   position: absolute;
 }

abbr.ajax-changed is a globally issue to every AJAX-change.

RobLoach’s picture

Status: Needs review » Needs work

After applying the patch....

rob@Computron /var/www/drupal/8 $ node ../../csslint/release/npm/cli.js core/modules/locale/

csslint: No errors in core/modules/locale//locale.admin-rtl.css.

csslint: There are 11 problems in core/modules/locale//locale.admin.css.

locale.admin.css
1: warning at line 5, col 3
Using width with padding-right can sometimes make elements larger than you expect.
padding-right: 1em; /* LTR */

locale.admin.css
2: warning at line 19, col 22
Values of 0 shouldn't have units specified.
padding: 3.5ex 0 0 0em; /* LTR */

locale.admin.css
3: warning at line 21, col 1
Don't use IDs in selectors.
#locale-translate-edit-form th{

locale.admin.css
4: warning at line 25, col 1
Don't use IDs in selectors.
#locale-translate-edit-form .form-item{

locale.admin.css
5: warning at line 28, col 1
Don't use IDs in selectors.
#locale-translate-edit-form td {

locale.admin.css
6: warning at line 32, col 1
Don't use IDs in selectors.
#locale-translate-edit-form .changed {

locale.admin.css
7: warning at line 36, col 1
Don't use IDs in selectors.
#locale-translate-edit-form .form-type-item abbr.ajax-changed {

locale.admin.css
8: warning at line 36, col 45
Element (abbr.ajax-changed) is overqualified, just use .ajax-changed without element name.
#locale-translate-edit-form .form-type-item abbr.ajax-changed {

locale.admin.css
9: warning at line 44, col 1
2 IDs in the selector, really?
#locale-translate-edit-form #edit-strings .locale-translate-edit-table {

locale.admin.css
10: warning at line 48, col 43
Don't use adjoining classes.
#locale-translate-edit-form #edit-strings .locale-translate-edit-table.changed {

locale.admin.css
11: warning at line 48, col 1
2 IDs in the selector, really?
#locale-translate-edit-form #edit-strings .locale-translate-edit-table.changed {

droplet’s picture

Seems like I uploaded a wrong patch.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
2.85 KB

rob@Computron /var/www/drupal/8 $ node ../../csslint/release/npm/cli.js core/modules/locale/

csslint: No errors in core/modules/locale//locale-rtl.css.

csslint: No errors in core/modules/locale//locale.admin-rtl.css.

csslint: No errors in core/modules/locale//locale.admin.css.

droplet’s picture

FileSize
3.72 KB
-#locale-translate-edit-form tr .form-type-item abbr.ajax-changed {
+.locale-translate-edit-form tr .form-type-item .ajax-changed {
   position: absolute;
 }

no position: absolute; for following message

"* Changes made in this table will not be saved until the form is submitted."

droplet’s picture

FileSize
2.85 KB

my editor su*

star-szr’s picture

Risse’s picture

Issue tags: -html5

#6: locale2.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +html5

The last submitted patch, locale2.patch, failed testing.

Risse’s picture

Updated patch to work with latest dev.

locale.admin.css = CSS lint found 0 errors and 0 warnings.
locale.rtl.css = CSS lint found 0 errors and 0 warnings.

Risse’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, locale-css-cleanup-1663130-10.patch, failed testing.

Risse’s picture

Status: Needs work » Needs review
FileSize
2.91 KB

Well, that was a stupid mistake. New try!

barraponto’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
2.92 KB

Wonderful, works as expected. Just added spaces before { to meet css styles.

xjm’s picture

CSS changes need manual testing in all browsers and all affected core themes:

  1. Test pages where the relevant classes are used without the patch applied.
  2. Apply the patch, clear the site and browser caches, and test again. Confirm that there are no changes or regressions.

Post before-and-after screenshots for one browser/theme.

xjm’s picture

Status: Reviewed & tested by the community » Needs work
dnotes’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
80.58 KB
80.58 KB
81.16 KB
81.31 KB

I've confirmed that there are no changes. Screenshots attached. Setting back to RTBC since manual testing is done.

xjm’s picture

Thanks @dnotes!

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome work! Committed and pushed to 8.x. :)

Automatically closed -- issue fixed for 2 weeks with no activity.

frederickjh’s picture

Removing Needs screenshots tag. Marked Closed. Issue queue cleanup for those searching for issues needing screenshots.