WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 3 months ago

#20570 closed enhancement (maybelater)

Tidying Admin CSS, bringing it up to CSS Code Standards (work in progress)

Reported by: georgestephanis Owned by:
Milestone: Priority: normal
Severity: minor Version: 3.4
Component: Administration Keywords: work-in-progress
Focuses: ui Cc:

Description (last modified by SergeyBiryukov)

as per http://codex.wordpress.org/CSS_Coding_Standards, this is an attempt to bring the internal admin stylesheets up to CSS Coding Standards

Some of the changes being made are ...

  • Properly intenting top: left: right: bottom: attributes two tabs when following a position:absolute position:fixed or position:relative declaration
  • Alphabetizing css properties
  • Removing unnecessary browser prefixes
  • Placing browser prefixed css properties before the non-browser prefixed versions
  • Removing extraneous spaces
  • Applying a consistent ordering of browser-prefixed properties (based on length of prefix to get an aesthetically pleasing diagonal line, rather than jagged, inconsistent line endings -- yes, I know it's silly, but it's better than randomness, and alphabetical isn't as useful)
  • Applying a uniform method to attribute selectors and url references by wrapping double-quotes around the values (there was no prior standard and a motley assortment of usages)
  • Changing colors to 3-character shorthand ( 888888 becomes 888 )
  • Changing colors to lower case ( EEE becomes eee )

and probably a couple others that aren't coming to mind right now.

Attachments (2)

20570.draft.diff (79.5 KB) - added by georgestephanis 2 years ago.
First draft -- more revisions pending
hex-not-name.patch (3.2 KB) - added by bananastalktome 2 years ago.

Download all attachments as: .zip

Change History (32)

georgestephanis2 years ago

First draft -- more revisions pending

comment:1 SergeyBiryukov2 years ago

  • Description modified (diff)

comment:2 ocean902 years ago

Are these standards now final?

comment:3 georgestephanis2 years ago

There's some bits and pieces that aren't specified in it -- I've got a poll running at http://stephanis.info/wp-css-coding-standards/ to try and get a consensus.

As far as whether it is final, I'm not certain. If not, I'd like to get them solidified one direction or another, so we don't have three ways of doing things.

If possible, I'd also like to include in the standards which properties we encourage browser-prefixing, as well as which ones we discourage.

border-radius and box-shadow have progressed to a point where neither really need the -moz- and -webkit- browser prefixing any longer, IMHO.

comment:4 ocean902 years ago

Related: #19114

comment:5 azaozz2 years ago

Some of the so proposed "coding standards" seem pretty weird imho. Keep in mind that most people look at the CSS code through Firebug and friends, making the formatting somewhat redundant:

  • "Properly indenting top: left: right: bottom: attributes...". Why? It's not more readable.
  • "Alphabetizing css properties". What's the point? We don't alphabetize function names/placement in PHP and JS...
  • "Removing unnecessary browser prefixes". Sounds good as long as no currently user browser is left behind.
  • "Placing browser prefixed css properties before the non-browser prefixed versions". This is more of a requirement than code style. As far as I remember there was a case when this didn't work properly.
  • "Removing extraneous spaces". As long as that doesn't impact readability. Perhaps leaving two empty lines between sections.
  • "Applying a consistent ordering of browser-prefixed properties". Meaning -moz- should always go above -webkit-? Same as alphabetizing, don't see the point.

Also thinking there should be another one: "Each selector should be on a new line".

As far as I understand it "Coding Standards" means "to make the code readable in a consistent way". Alphabetizing usually goes against that. It seems better if the css properties are grouped by functionality rather than alphabetized. For example top: and left: should be right after position:, padding: should be next to margin:, etc.

comment:6 follow-up: georgestephanis2 years ago

I'm not particularly attached to alphabetizing -- if we want to ditch it, it's fine by me. I started down this rabbit hole looking to address the inconsistency in whether the value should be quoted in [type="number"] or not, and just got sucked in.

It's kinda like this => http://cl.ly/2j2g2f2Q0t191E1Z2a3n

comment:7 in reply to: ↑ 6 azaozz2 years ago

Replying to georgestephanis:

Ah, I see...

Yeah, agree that discussing coding standards on trac seems better than relaying on people editing the wiki directly. After all most people that will have to comply with the standards seem more likely to comment here :)

Right, we need to follow the specs for always quoting [type="..."] and url("..."). Also when using hex color values, they seem easier to read when lower case and shortened. Thinking we still need some of the -webkit- stuff for the mobile versions of Safari.

comment:8 DrewAPicture2 years ago

  • Cc xoodrew@… added

Also not really on board with alphabetizing. I agree with @azaozz that several of these are a bit weird and somewhat unnecessary.

As for using shorthand with property values, some are easy to navigate and others aren't. There seems to be some contention that shorthand is more difficult to read. I personally like it better.

Maybe should also add something about how to handle indenting @media queries and sub-selectors since they've been sneaking into core in the last couple of releases.

comment:9 sabreuse2 years ago

  • Cc sabreuse@… added

comment:10 follow-up: bananastalktome2 years ago

  • Cc bananastalktome@… added

Patch which changes css color names to their hex counterparts (let me know if I got any wrong, and even if this patch is not used at least fix .wp-dialog in wp-includes/css/editor.dev.css since an invalid color value is used)

comment:11 follow-ups: helenyhou2 years ago

I am also not a big fan of alphabetizing and a couple other points, but I know I've seen... disagreements from the past about these coding standards that never seem to have resolved. There is definitely a need to actually come to an agreement. Have already talked briefly to georgestephanis about this in #wordpress-ui - perhaps we could actually hold a meeting during the regularly scheduled time and at least start talking it through? And perhaps also grab the Automatticians/Theme Team who are also (to my knowledge) working on their coding standards?

Let's hold off on patches for now, though. They will quickly become stale and are not useful without a decision. This is the sort of work that really needs to happen pretty much directly after a stable version drops and I think we'll find that there is a general CSS cleanup needed, during which we can also look at including standards fixing. It'll muck up some of the revision history/blame, but so it goes.

Also, bananastalktome - note that your patch doesn't always use the shortened hex value. We should probably fix that value for .wp-dialog, though...

comment:12 in reply to: ↑ 10 ; follow-up: SergeyBiryukov2 years ago

Replying to bananastalktome:

at least fix .wp-dialog in wp-includes/css/editor.dev.css since an invalid color value is used)

I guess it's better to create a separate ticket for that.

comment:13 SergeyBiryukov2 years ago

+1 to lower case and shortened colors and always quoting [type="..."] and url("...").

−1 to additional tab for top: left: right: bottom: and alphabetizing properties.

comment:14 in reply to: ↑ 12 bananastalktome2 years ago

Replying to SergeyBiryukov:

Replying to bananastalktome:

at least fix .wp-dialog in wp-includes/css/editor.dev.css since an invalid color value is used)

I guess it's better to create a separate ticket for that.

created (#20574)

comment:15 bananastalktome2 years ago

In looking through the stylesheets, I noticed some discrepancies with the text-shadow property. In some instances, we have things like text-shadow: 0 -1px 0 #333; and other times we have text-shadow: #fff 0 1px 0; (color first vs color last). The w3c validator (http://jigsaw.w3.org/css-validator/validator) appears to normalize the css to using color first when I pasted in a few different snippets from the WordPress css, but I have seen sources online showing both ways as correct. Is there a preference on this (and is it worthy of another patch to normalize)?

comment:16 in reply to: ↑ 11 bananastalktome2 years ago

Replying to helenyhou:

Also, bananastalktome - note that your patch doesn't always use the shortened hex value. We should probably fix that value for .wp-dialog, though...

Good call, it was an oversight on my part. Is this worth me revisiting to fix, or should I just hold off until some consensus is reached on whether this should be a rule?

comment:17 follow-up: georgestephanis2 years ago

I think we'll wait until CSS is fairly stable for a couple weeks (probably right after the release of 3.4) to do a larger overhaul -- if it is to happen at all. In the mean time, our focus would be best spent on building a consensus with regard to what principles to use in the overhaul.

comment:18 in reply to: ↑ 17 ; follow-up: azaozz2 years ago

Replying to helenyhou:
Replying to georgestephanis:

I wouldn't call them "disagreements", rather ideas and personal preferences :)

Really like the idea to talk things over in #wordpress-ui, next meetup there is tomorrow (Tuesday) at 18:00 UTC, right?

Yes, most of the admin CSS is still in need of a general overhaul/cleanup, it's too much to try to do all at once, so I see it more like an ongoing process. Would be great if we have written CSS coding standards before attempting the next round, probably early in 3.5.

comment:19 in reply to: ↑ 18 helenyhou2 years ago

Replying to azaozz:

Yep, Tuesdays at 18:00 UTC is what it says, so let's go with that. Shall I post on Make UI about it? I have a couple other things I want to run by the group as well, and it might be worth putting a call out to more prospective contributors in the HTML/CSS area and also do a little discussion about testing RTL and colors and such so it doesn't get left for the very very end like usual.

comment:20 azaozz2 years ago

Sounds good. Yes, the UI group seems to be busiest near a new release, but perhaps can plan some tasks that would be better done early in a release cycle, like general css handling in core, RTL, etc.

Last edited 2 years ago by azaozz (previous) (diff)

comment:21 in reply to: ↑ 11 koopersmith2 years ago

Replying to helenyhou:

I am also not a big fan of alphabetizing and a couple other points, but I know I've seen... disagreements from the past about these coding standards that never seem to have resolved. There is definitely a need to actually come to an agreement.

Let's hold off on patches for now, though. They will quickly become stale and are not useful without a decision. This is the sort of work that really needs to happen pretty much directly after a stable version drops and I think we'll find that there is a general CSS cleanup needed, during which we can also look at including standards fixing.

I couldn't agree more. Let's start up the meetings again and try to come to a consensus on an updated CSS style guide early next cycle.

comment:23 nacin2 years ago

  • Type changed from defect (bug) to enhancement

comment:25 helenyhou19 months ago

#22097 was marked as a duplicate.

comment:27 SergeyBiryukov16 months ago

  • Component changed from General to Administration
  • Keywords ui-focus added

comment:28 helen9 months ago

Anybody interested in this should probably take a look at MP6 and get involved through there for now.

comment:29 ocean903 months ago

  • Keywords close added

Not sure what we should do with this ticket, helen?

comment:30 helen3 months ago

  • Keywords close removed
  • Milestone Awaiting Review deleted
  • Resolution set to maybelater
  • Status changed from new to closed

I don't know. I don't see any realistic path to this, as lovely as it sounds. Let's maybelater this, maybe over time as we do things like split files, introduce reusable components, and take advantage of tooling, we can naturally ease in some sanity.

Note: See TracTickets for help on using tickets.