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.
Dashboard.css has excessive blank lines and does not have alphabetized properties.
Patch fixes these problems and organizes selectors into a better order.
Comment | File | Size | Author |
---|---|---|---|
#8 | dashboard-css-standards.patch | 3.36 KB | TR |
#1 | dashboard-css-reroll.patch | 3.27 KB | TR |
dashboard-css.patch | 3.07 KB | Jody Lynn | |
Comments
Comment #1
TR CreditAttribution: TR commentedPatch no longer applies after #766624: Dashboard lacks rtl styling got committed. Here's a re-roll against D8. This new patch applies to both D8 and D7.
NO STYLES ARE CHANGED by this patch. As stated by the OP, excessive blank lines were removed and properties and selectors were reordered to conform with coding standards.
Comment #2
TR CreditAttribution: TR commentedTagging.
Comment #3
Jody LynnI went through the patch and confirmed it's just reordering and whitespace changes.
Comment #4
droplet CreditAttribution: droplet commented- non-prefix property should be the last one.
- missing RTL style?
11 days to next Drupal core point release.
Comment #5
TR CreditAttribution: TR commentedThe CSS coding standards found at http://drupal.org/node/302199 do not say anything about browser-specific styles such as -moz-border-radius and -webkit-border-radius, so this is really a matter of personal preference/style. The patch orders the styles alphabetically, as required by the standards, with the browser-specific styles appearing directly after the generic style they override:
Given that there's no standard for where the "-" styles should go, I think this is a reasonable choice as it groups these browser-specific styles together with the generic style, and it visually implies that the browser-specific styles are subordinate to and more specific than the generic style. @droplet, are there technical reasons why border-radius should follow the browser-specific styles, or is this just your preference? If there are technical reasons, I suggest they should be added to http://drupal.org/node/302199.
RTL styles are in dashboard-rtl.css
Any RTL issues with the dashboard should be dealt with in a separate issue - this is one is just about coding standards.
If what you're asking is why isn't dashboard-rtl.css being corrected for coding standards at the same time, the answer is that it doesn't need any changes - it already conforms to the standards.
Comment #6
droplet CreditAttribution: droplet commented@TR,
not the personal preference.
border-radius is W3C standard
-prefix-border-radius is browsers style (dev or testing property, it may change under different version)
W3C standard should always override the others. (exclude it's a hack to fix the problem under specific browsers)
e.g. (it's not a real case)
Firefox 4 supports border-radius
while released Firefox 1000, HTML 7 is coming out. border-radius is different than before. Firefox 1000 try to test something new on -moz-border-radius.
What we want is W3C effect :)
Comment #7
stevetweeddale CreditAttribution: stevetweeddale commentedChris Coyier has a nice post explaining the reason for putting the non-prefixed version last. <short answer>vendor implementations don't always conform to the official spec, with regards to how they implement things or the order of their arguments, and so we'd want the non-prefixed rule to be the last that 'trumps' the others.</short answer>
Browsers will use latterly-declared values when several are declared for the same parameter, just like CSS rules trump preceding ones of identical specificity. They consider vendor prefixed/non-prefixed to be the same parameter, and so prefer the latter. In this case that's not a problem, because the prefixed ones do the same as the official spec (apply 5px radius to each corner).
However, if the prefixed ones come last and operate differently to the non-prefixed, then browser will use it's 'own' (read: potentially wrong) implementation, even after it's started to support the official spec in the un-prefixed option…because you've essentially explicitly told it to!
l'd add it to the standards doc but I don't have permissions.
Comment #8
TR CreditAttribution: TR commented@stevetweeddale: Thanks for that excellent reference.
That's what I didn't know - I had thought that the prefixed properties were more specific and therefore, like CSS rules, automatically took precedence over the less-specific standard properties regardless of ordering. But that's wrong. I also didn't know browsers considered these the same property.
I re-rolled the patch to put the non-prefixed property after the prefixed properties, and I'll open up an issue to add this recommendation to the coding standards doc.
I did add one additional change to dashboard.css. The Drupal CSS standards say that color values should be upper case, e.g. #FFF instead of #fff. dashboard.css had some upper case and some lower case, so this revised patch changes them all to upper case. Patch applies cleanly to both D8 and D7.
Comment #9
stevetweeddale CreditAttribution: stevetweeddale commentedYeah, vendor-prefixes are something of a necessary but confusing/annoying evil.
Patch looks great to me though. Good job!
Man, looks like there's also quite a few redundant id's in there (or at least to my eyes). Does anyone know of a reason there might be selectors like
#dashboard div#dashboard_main
in there? Over say, just#dashboard_main
? Is it just on the assumption that we might end up in the broken situation of multiple #dashboard_main id's on a page, perhaps due to contrib/custom code?Comment #10
Dries CreditAttribution: Dries commentedCommitted to 8.x. Moving to 7.x for webchick to consider.