Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
- Copy and paste the the stylesheet(s) below into the css lint tool at http://csslint.net and test.
- Fix any warnings or errors the tool finds.
- Patch Drupal 8 locally and make sure the css changes have not broken anything visually.
- Create patch and upload for the testbot.
Files: modules/field_ui/field_ui.css (and field_ui-rtl.css)
Comment | File | Size | Author |
---|---|---|---|
#17 | 1662986-17.patch | 1021 bytes | swentel |
#16 | 1662986-16.patch | 413 bytes | swentel |
#12 | beforepatch.png | 57.89 KB | markgifford |
#12 | afterpatch.png | 57.89 KB | markgifford |
#8 | drupal-csslint-field_ui-1662986-8.patch | 3.36 KB | barraponto |
Comments
Comment #1
_Cedric_ CreditAttribution: _Cedric_ commentedNeeds review
Comment #2
RobLoachMissed a couple.
Comment #3
_Cedric_ CreditAttribution: _Cedric_ commentedLast css fixes, needs review
Comment #4
droplet CreditAttribution: droplet commented<table id="field-display-overview" class="field-ui-overview
any reason not to use this class ?
Comment #5
Albert Volkman CreditAttribution: Albert Volkman commentedNot that I can see.
Comment #6
swentel CreditAttribution: swentel commentedThe refresh button at the bottom of the manage display is still there, that's because the button is not inside the table, so the class doesn't work for that.
See screenshot
Comment #7
drupee CreditAttribution: drupee commentedHey Guys,
Is this issue already fixed?
I have cloned a recent version of git repo. I neither see any warnings nor any 'Refresh' button.
Can you please elaborate this issue if it is still there?
Comment #8
barraponto CreditAttribution: barraponto commentedLooks good, issue related by @swentel in #6 is gone as of #1792600: Refactor field_ui so common behavior for fields and display overview screens are extracted. Rerolling to apply cleanly.
Comment #9
xjmCSS changes need manual testing in all browsers and all affected core themes:
Post before-and-after screenshots for one browser/theme.
Comment #10
xjmComment #11
markgifford CreditAttribution: markgifford commentedComment #12
markgifford CreditAttribution: markgifford commentedBefore patch applied:
After patch applied:
No visible difference, all good. Tested in Chrome on Ubuntu btw
Comment #13
markgifford CreditAttribution: markgifford commentedForgot to remove tags and change status after posting screenshots
Comment #14
catchCommitted/pushed to 8.x, thanks!
Comment #15
swentel CreditAttribution: swentel commentedSo we missed the 'Manage display' screen, seriously people, see #6.
You now see the 'Refresh' button that should be hidden.
Comment #16
swentel CreditAttribution: swentel commentedHere's the patch.
Comment #17
swentel CreditAttribution: swentel commentedOk, better approach by using element-invisible.
Comment #18
wuinfo - Bill Wu CreditAttribution: wuinfo - Bill Wu commentedRule of css lint: Don't use IDs in selectors.
Comment #19
yched CreditAttribution: yched commented#17 works.
Comment #20
webchickCommitted and pushed to 8.x. Thanks!