#26669 closed enhancement (fixed)
wp-admin.css should be broken up into modules
Reported by: | jorbin | Owned by: | helen |
---|---|---|---|
Milestone: | 3.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Administration | Keywords: | has-patch |
Focuses: | ui | Cc: |
Description (last modified by )
This is a follow up #22862.
There helen stated:
Nah, let's address autoprefixing and concat separately and in between cycles / the very start of a new one - too much churn to do it right now without killing everybody's patches.
This ticket adds concatenation. And right now we are at the start of a new cycle, so I figured why not.
Attachments (13)
Change History (37)
#4
follow-up:
↓ 5
@
11 years ago
The one concern I have about this is it makes it much more difficult to submit a CSS patch via core.svn. An alternative approach could be to have all of these files in both core.svn and develop.svn, then also include a single wp-admin.min.css file in the built version. SCRIPT_DEBUG would load these individual files in both build and src.
Seems like if we have separate files, then each should be commented, and the TOC should be removed.
TinyMCE can just be removed, that is covered by wp-includes/css/editor.css.
It will be easier to examine any future patches if wp-admin.css just got comments added throughout, as in: /* this would start wp-admin-buttons.css, forms.css, etc. */
, rather than actually moving code around. Or if the CSS files are admitted entirely for now, in lieu of the grunt/script-loader/etc changes. The TOC can be patched with /* this would be split into buttons.css, */
or something, that way we can discuss organization.
#5
in reply to:
↑ 4
@
11 years ago
Replying to nacin:
The one concern I have about this is it makes it much more difficult to submit a CSS patch via core.svn. An alternative approach could be to have all of these files in both core.svn and develop.svn, then also include a single wp-admin.min.css file in the built version. SCRIPT_DEBUG would load these individual files in both build and src.
Seems like if we have separate files, then each should be commented, and the TOC should be removed.
That makes a lot of sense. I've switched it so SCRIPT_DEBUG always loads all the files. I've kept TOC since I think having a reference of what is where makes sense. Though it's entirely possible that it could easily get stale and may make more sense to remove it.
This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.
11 years ago
This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.
11 years ago
#9
@
11 years ago
I think we should probably make sure wp-login.php-specific styles are separate and usable separately or at least investigate what to do there: #12506.
#10
@
11 years ago
- Focuses ui added
The above patch divides wp-admin.css up and also fixes #12506.
Note, that I tried to include a blank wp-login.css file in src/wp-admin/css , but it doesn't seem to actually get added when I apply my only patch. That should also be a part of it.
This ticket was mentioned in IRC in #wordpress-dev by jorbin_work. View the logs.
11 years ago
This ticket was mentioned in IRC in #wordpress-dev by jorbin_work. View the logs.
11 years ago
This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.
11 years ago
#14
@
11 years ago
26669.3.diff is a quick split proposal using comments in wp-admin.css, a lot of it based on jorbin's work. Some thoughts:
- The number of times you will see some markers repeated (common, forms, etc.) is a pretty clear indicator of the mess we're in.
- I'm leaving plugins-related CSS as part of list tables as well as common (the plugin details view).
- Themes is separate from appearance, due to the amount of CSS and assuming further changes due to continued work to align the manage and install screens.
- Not sure if users-tools-settings should be separate, or just put into common for now. it's a fairly small amount of CSS.
- Press This has two blobs, one currently indicated as part of settings (see above), the other as part of common. Not sure what to do with it.
This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.
11 years ago
#18
@
11 years ago
- Milestone changed from Awaiting Review to 3.9
A few things here:
- Do not attempt to load the changeset in Trac or link to it directly. At last count, the number of altered lines was 213,216, due to using
svn cp
to create each of the new CSS files with history. wp-admin.css
uses@import
for each of the modules/components/whatever we call them, cssjanus creates RTL versions of each, andwp-admin-rtl.css
is created, calling the generated RTL files.- The hit of multiple HTTP requests only applies with
SCRIPT_DEBUG
on -clean-css
(viacssmin
) inlines@import
ed files in the minified versions. - This approach works out well for us because it does not involve script loader changes, it utilizes existing tooling,
core.svn
anddevelop.svn
can continue to both be used for patches, and web inspectors handle CSS@import
s already - no need to worry about whether or not source maps are supported yet.
Please open new tickets for any issues discovered, as it may not be clear whether it is a consequence of this split or the preceding colors.css merge in #18380.
#19
follow-up:
↓ 20
@
11 years ago
Using @import and cssmin meets all our needs
@import can be problematic for plugins that include their own admin color schemes because wp-content
might not always be relative to wp-admin
.
Helen: we've talked in the past about just copying dependent styles directly into the plugin's color scheme(s), but that comes with added overhead of needing to merge core styling changes between versions. Because it changes frequently (and drastically every few years) I was hoping for an approach that enqueues the structural bits, and separates out the individual color schemes.
Any comment or recommendation on how best to proceed, and does this point warrant its own ticket?
#20
in reply to:
↑ 19
;
follow-up:
↓ 21
@
11 years ago
Replying to johnjamesjacoby:
Using @import and cssmin meets all our needs
@import can be problematic for plugins that include their own admin color schemes because
wp-content
might not always be relative towp-admin
.
@import is just being used by wp-admin/css/wp-admin.css at this point and unless plugins are hacking core, I'm confused as to how this will affect them. For people running without SCRIPT_DEBUG set to true, this creates zero change for them.
#21
in reply to:
↑ 20
;
follow-up:
↓ 22
@
11 years ago
Replying to jorbin:
@import is just being used by wp-admin/css/wp-admin.css at this point and unless plugins are hacking core, I'm confused as to how this will affect them. For people running without SCRIPT_DEBUG set to true, this creates zero change for them.
The commit message (though clever, and quoting Children's Story is always appreciated) wasn't super clear (to me) about what was switched to using @import. After reading Helen's summary, it wasn't much more clear what the impact would be for plugins that include their own color schemes -- something that has been historically difficult (see: almost impossible without including your own image assets, and without pulling in colors-fresh for 3.8.)
Showing compassion for plugin developers that want to do good and ensure limited/no breakage (especially when 200k line changesets come in) goes a long way towards keeping up-stream contributions flowing in. I'm hoping the next time someone asks a similar question, it's met with a more positive and helpful response.
Having done some cursory testing with bbPress's green schemes, I'll sign off that all appears a-okay. I'll create a new ticket if I see any oddities. Nice work, Helen.
#22
in reply to:
↑ 21
@
11 years ago
Replying to johnjamesjacoby:
Having done some cursory testing with bbPress's green schemes, I'll sign off that all appears a-okay. I'll create a new ticket if I see any oddities. Nice work, Helen.
Thanks, glad to hear that things are looking okay. I readily admit that (a) I got too excited about my commit message and just committing the blasted thing and failed to make a plain English summary until later, and (b) because this particular changeset only affects core development, I didn't think to allay plugin/theme dev concerns with something so large that you can't even view it. But trust me: this has zero impact on plugin and theme development. :)
All the CSS changes move existing CSS into separated files and break up the media queries so the cascade isn't broken. The Gruntfile.js changes can be found in 26669-gruntfile.2.diff. Finally, a few lines were removed from $_old_files
, because some newly created CSS files have the same name as files that existed before the 3.3 merge, so we don't want to copy the new files over and then have the install process just stupidly delete them again (it's happened before). Happy to diff that file alone for the curious. Here's what changed:
M Gruntfile.js A + src/wp-admin/css/about.css A + src/wp-admin/css/admin-menu.css A + src/wp-admin/css/common.css A + src/wp-admin/css/dashboard.css A + src/wp-admin/css/edit.css A + src/wp-admin/css/forms.css A + src/wp-admin/css/l10n.css A + src/wp-admin/css/list-tables.css A + src/wp-admin/css/login.css A + src/wp-admin/css/media.css A + src/wp-admin/css/nav-menus.css A + src/wp-admin/css/press-this.css A + src/wp-admin/css/revisions.css A + src/wp-admin/css/themes.css A + src/wp-admin/css/widgets.css M src/wp-admin/css/wp-admin.css M src/wp-admin/includes/update-core.php
The above patch doesn't change any of our css, all it does is move it into smaller more manageable chunks and adds the grunt pieces along with the special src magic inside wp-includes/script-loader.php