Make WordPress Core

Opened 13 years ago

Closed 12 years ago

Last modified 12 years ago

#18314 closed task (blessed) (fixed)

Merge admin CSS files and remove duplicates

Reported by: azaozz's profile azaozz Owned by:
Milestone: 3.3 Priority: normal
Severity: normal Version:
Component: Performance Keywords:
Focuses: Cc:

Description (last modified by azaozz)

Looking through wp-admin/css we have large amount of stylesheets that aren't optimized neither for loading nor for maintenance/consistency.

Generally we have two options: either to break our CSS into smaller chunks of dependent styles and load different amount on each screen or to "globalize" it all into wp-admin.css and wp-admin-rtl.css.

The second option would be (quite?) better for faster loading. Currently we concatenate and compress the admin css but since the files are different for most screens, the concatenated file has to be downloaded each time. If all admin styles are in one file it will be cached by the browser after the first time (in memory) and all subsequent loads will be faster.

To do:

  • continue removing duplicate and nearly duplicate styles from wp-admin.css
  • after that's in good shape, do the same for wp-admin-rtl.css

Please see comment:14

Attachments (15)

rwi-colors-rtl-combine.diff (7.9 KB) - added by ryanimel 13 years ago.
This patch combines the colors-classic.css with colors-classic-rtl.css, and same for the fresh colors. Also added some basic commenting.
18314.ie.patch (630 bytes) - added by SergeyBiryukov 13 years ago.
18314.db-upgrade.png (17.4 KB) - added by SergeyBiryukov 13 years ago.
18314.install-rtl.patch (475 bytes) - added by SergeyBiryukov 13 years ago.
login-body.patch (619 bytes) - added by azaozz 13 years ago.
Fix scrollbar on the login screen
18314.HXandApproveCombine.diff (1.5 KB) - added by kirasong 13 years ago.
Combine H1/H2/H3... and Approve/UnApprove styles
18314.hide-menu-ordering.diff (348 bytes) - added by duck_ 13 years ago.
18314.combineRemoveDup.diff (602 bytes) - added by kirasong 13 years ago.
Combine Duplicate #wp-fullscreen-save definitions and remove extra style from .post-com-count span
18314.fonts.patch (7.9 KB) - added by ocean90 12 years ago.
18314.combineColumnContainers.diff (566 bytes) - added by olleicua 12 years ago.
Combining rules under /* 2 columns main area */ in wp-admin.css
18314.wp-admin.css.onceOver.diff (9.5 KB) - added by olleicua 12 years ago.
various cleaning/combining in wp-admin/css/wp-admin.dev.css
18314.wphead-favorites-misc.diff (10.3 KB) - added by andrewryno 12 years ago.
tablenav_class_in_posts_subpannel_wp321.png (22.4 KB) - added by ramiy 12 years ago.
tablenav_class_in_posts_subpannel_wp330beta.png (22.8 KB) - added by ramiy 12 years ago.
18314.press-this.diff (2.9 KB) - added by andrewryno 12 years ago.
Use category/tag styles from postscreen with only minor changes

Download all attachments as: .zip

Change History (78)

#1 follow-up: @scribu
13 years ago

-1 on putting it all in wp-admin.dev.css.

Currently we concatenate and compress the admin css but since the files are different for most screens, the concatenated file has to be downloaded each time.

Ok, then let's serve all the files all the time, concatenated.

#2 @azaozz
13 years ago

  • Description modified (diff)

#3 in reply to: ↑ 1 @azaozz
13 years ago

Replying to scribu:

-1 on putting it all in wp-admin.dev.css...
Ok, then let's serve all the files all the time, concatenated.

What's the point of having 20 files then? Apart from making it harder to debug :)

Actually if we put all in wp-admin.css we can turn concatenation for admin css off, only keep compressing it.

Ugh, we actually have 33 files there, including the -rtl.css.

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

#4 @scribu
13 years ago

What's the point of having 20 files then? Apart from making it harder to debug :)

wp-admin.dev.css already has 4700+ LOC.

By that same token, what's the point in having separate PHP files then?

#5 @scribu
13 years ago

And I don't see how having separate CSS files would make it harder to debug.

If anything, it would make it easier.

#6 @azaozz
13 years ago

If only PHP could cascade like CSS... :)

Apart from slowing down the browser loading, having many smaller CSS files has several drawbacks: much harder to reuse styles, harder to track duplicates, harder to maintain proper "cascading", etc. I think we can eliminate about 1/5 to 1/4 of the total CSS LOC if we do proper removal of duplicate styles.

#7 follow-up: @scribu
13 years ago

I'm totally for removing duplicate styles.

much harder to reuse styles

How so? There's reusing and then there's clumping together unrelated elements.

harder to track duplicates

Maybe, but not if we split the files by components: general layout, list tables, form elements etc.

harder to maintain proper "cascading"

Huh? Please give an example.

#8 in reply to: ↑ 7 @azaozz
13 years ago

Replying to scribu:

much harder to reuse styles

How so? There's reusing and then there's clumping together unrelated elements.

CSS doesn't care whether elements are related or not. If we repeat

border-width: 1px;
border-style: solid;

in 20-30 places, that's duplication.

harder to track duplicates

Maybe, but not if we split the files by components: general layout, list tables, form elements etc.

So if a style is defined in list-tables.css and you're adding something on the Add New post screen that can use it, what do you do? Do you add the selector to list-tables.css and "break the flow" or do you define (copy) the style to add-new-post.css?

harder to maintain proper "cascading"

Huh? Please give an example.

See above. If we start mixing what CSS applies on which screen, it can get messier.

Generally the question is: is it easier to maintain one large css file or 20 small files (that will always be outputted together)? IMHO one large file is easier.

#9 @sabreuse
13 years ago

  • Cc sabreuse@… added

#10 @andrewryno
13 years ago

I would like to get started on this. Could we agree to at least merge the *-rtl.css files into the main .css files (with using the .rtl body class)?

This would make it easier to edit RTL styles because you could include the RTL style directly after the non-RTL style (grouping them together). I can provide a patch tomorrow if we would like to start with that.

#11 @nacin
13 years ago

There's something like 36k across the RTL files. It'd be better to combine them all into one rtl.css file.

#12 @azaozz
13 years ago

In [18577]:

Merge most admin css files, first run, see #18314

#13 @azaozz
13 years ago

In [18578]:

Merged install.css with install-rtl.css (it is used only when installing so it makes sense to be a separate stylesheet), merged farbtastic.css with farbtastic-rtl.css, see #18314

#14 follow-up: @azaozz
13 years ago

  • Description modified (diff)

There are still quite a few duplicates or nearly duplicates in wp-admin.css. All are welcome to help slim it down. The only thing we need to be careful about is to apply any uncommited patches to it before starting to hunt for duplicates. It's hard to merge conflicting css patches as each conflict has to be tested separately.

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

@ryanimel
13 years ago

This patch combines the colors-classic.css with colors-classic-rtl.css, and same for the fresh colors. Also added some basic commenting.

#15 @ryanimel
13 years ago

  • Cc ryan@… added

#16 @ramoonus
13 years ago

this makes script_debug in wp-config obsolete?

#17 @azaozz
13 years ago

In [18579]:

Merge colors*.css with colors*-rtl.css, props ryanimel, see #18314

#18 @SergeyBiryukov
13 years ago

[18577] introduced the dependency between ie.css and wp-admin.css. It breaks the layout of database upgrade page (see the screenshot), since wp-admin.css shouldn't be included there.

18314.ie.patch removes the dependency.

#19 @holizz
13 years ago

  • Cc tom@… added

#20 follow-up: @SergeyBiryukov
13 years ago

Due to [18578], we need to set <body class="rtl"> in install.php (also in setup-config.php, but that should be adressed in #18180) if is_rtl().

Added a patch for that.

#22 follow-up: @SergeyBiryukov
13 years ago

Yep, 18314.ie.patch Download should fix this.

(Didn't mean to edit this comment. Seems there's a bug on trac as the comment numbers are not right on this ticket.)

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

#23 follow-up: @dd32
13 years ago

In [18622]:

Mark the install page as rtl for styling purposes. Props SergeyBiryukov. See #18314 and #18180

#13 @azaozz
13 years ago

In [18719]:

Remove dependency from ie.css, props SergeyBiryukov, see #18314

#14 follow-up: @azaozz
13 years ago

In [18728]:

Merge all border-radius: 3px, see #18314

#15 @SergeyBiryukov
13 years ago

Related: ticket:18197:18197.remove-userinfo.2.patch (removes obsolete user info styles)

#16 @azaozz
13 years ago

In [18729]:

Add the borders to the border-radius merge, see #18314

@azaozz
13 years ago

Fix scrollbar on the login screen

#17 @azaozz
13 years ago

In [18737]:

Remove the scrollbar from the login screen, see #18314

#18 @scribu
13 years ago

Related: #18740

@kirasong
13 years ago

Combine H1/H2/H3... and Approve/UnApprove styles

#19 @azaozz
13 years ago

In [18744]:

Remove some duplicates and combine some css selectors, props DH-Shredder, see #18314

#20 follow-up: @duck_
13 years ago

18314.hide-menu-ordering.diff will hide the menu item ordering arrows when JS is enabled. I was wondering why this style is restricted to body.nav-menus-php when others are not, e.g. .js .input-with-default-title.

Also [18577] dropped the CSS reset entirely. It was previously in global.css which is no more. One problem this has caused can be seen on the nav menus page: expanded menu items have a gap between the header and the drop down holding the settings. In Chrome this is caused by the 1em margin on dl elements.

@kirasong
13 years ago

Combine Duplicate #wp-fullscreen-save definitions and remove extra style from .post-com-count span

#21 @nacin
13 years ago

In [18759]:

Combine some CSS definitions. props DH-Shredder, see #18314.

#22 in reply to: ↑ 20 ; follow-up: @azaozz
13 years ago

Replying to duck_:

I was wondering why this style is restricted to body.nav-menus-php when others are not...

Probably because the class .item-order is used on other screens too and this interferes with them.

Also [18577] dropped the CSS reset entirely.

CSS reset is necessary/good to have when supporting older browsers (IE6, FF2, etc.) as they have wildly different default styling. All newer browsers have very similar styling, so instead of resetting everything to some kind of zero values we only need to define global styles for the elements we need.

You're right we are missing DL from the UL and OL style :)

#23 in reply to: ↑ 22 ; follow-up: @duck_
13 years ago

Replying to azaozz:

Replying to duck_:

I was wondering why this style is restricted to body.nav-menus-php when others are not...

Probably because the class .item-order is used on other screens too and this interferes with them.

I though that too, but searching for "item-order" I couldn't find any other instances. As of [18755] that .item-order styling is redundant and can be removed anyway.

#24 in reply to: ↑ 23 @azaozz
13 years ago

Replying to duck_:

As of [18755] that .item-order styling is redundant and can be removed anyway.

Patches for anything like that are always welcome (anything like that = styling that is redundant because of another change but wasn't removed with the other commit).

#25 @duck_
13 years ago

In [18770]:

Remove .js .item-order style made redundant by r18755. See #18314.

#26 @azaozz
13 years ago

In [18772]:

Fix line-height inside postbox and stuffbox, see #18314

#27 @ryan
13 years ago

  • Type changed from enhancement to task (blessed)

#28 @ocean90
12 years ago

18314.fonts.patch will combine the font-families and removes some unused lines.

#29 @azaozz
12 years ago

In [18872]:

Combine font-family, remove few unused bits, props ocean90, see #18314

@olleicua
12 years ago

Combining rules under /* 2 columns main area */ in wp-admin.css

#30 follow-up: @olleicua
12 years ago

  • Cc olleicua added

This is my first WordPress patch so please let me know if there is something wrong with it: 18314.combineColumnContainers.diff
~Peace

Last edited 12 years ago by olleicua (previous) (diff)

#31 in reply to: ↑ 14 @olleicua
12 years ago

Replying to azaozz:

There are still quite a few duplicates or nearly duplicates in wp-admin.css. All are welcome to help slim it down. The only thing we need to be careful about is to apply any uncommited patches to it before starting to hunt for duplicates. It's hard to merge conflicting css patches as each conflict has to be tested separately.

Is there an easy way to see which patches have been committed besides actually reading each diff and looking to see if the changes are already there?
~Peace

Last edited 12 years ago by olleicua (previous) (diff)

#32 @azaozz
12 years ago

In [18903]:

Combine few styles for column containers, props olleicua, see #18314

#33 in reply to: ↑ 30 ; follow-up: @azaozz
12 years ago

Replying to olleicua:

This is my first WordPress patch so please let me know if there is something wrong with it...

No, all is good, thanks.

Is there an easy way to see which patches have been committed besides actually reading each diff and looking to see if the changes are already there?

Sure, if you add your email to Trac preferences, you will get a notification of all subsequent comments on tickets you commented on including all commit messages.

#34 @dd32
12 years ago

I'm seeing a few styling issues, not sure if it's related to this ticket or not:

  • IE: Add new Media buttons above TinyMCE has a thick border
  • General: Iframes seem to have an inset edge (The default?), whereas previously they were styled similar to textarea's (With regard to the border at least)

#35 @dd32
12 years ago

looks like iframe lost it's border:0 as it wasn't brought over from the reset in global.css, Spotting a few other a > img with borders elsewhere too (add_menu_page() items which don't specify a icon)

As for where I'm getting the iframe from, i'm using the 'Debug Bar Console' plugin which uses a inline iframe.

#36 @scribu
12 years ago

Probably related: #18901

@olleicua
12 years ago

various cleaning/combining in wp-admin/css/wp-admin.dev.css

#37 @olleicua
12 years ago

Did a once over of wp-admin.css trying to clean things up a bit. There's probably a lot I missed.

#38 in reply to: ↑ 33 @olleicua
12 years ago

Thanks

Replying to azaozz:

Replying to olleicua:

No, all is good, thanks.

Sure, if you add your email to Trac preferences, you will get a notification of all subsequent comments on tickets you commented on including all commit messages.

Last edited 12 years ago by olleicua (previous) (diff)

#39 @azaozz
12 years ago

In [18939]:

More CSS cleanup, props olleicua, see #18314

#40 @andrewryno
12 years ago

18314.wphead-favorites-misc.diff gets rid of a lot of #wphead rules since the new admin bar was put in (leaves in stuff for Press This), #favorite-actions and the old editor buttons (new rules in editor-buttons.css).

#41 follow-up: @azaozz
12 years ago

In [18975]:

RTL, IE7 and IE7 RTL fixes, add class="ie8" to the html tag, see #18314

#42 @azaozz
12 years ago

In [18976]:

Clean up styles for #wphead, #favorite-actions and the old editor buttons, props andrewryno, see #18314

#43 in reply to: ↑ 41 ; follow-up: @duck_
12 years ago

Replying to azaozz:

In [18975]:

RTL, IE7 and IE7 RTL fixes, add class="ie8" to the html tag, see #18314

[18977] Fix notices in wp_iframe() and iframe_header() by referencing $wp_htmltag_class as a global. See r18975 and #18314

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

#44 in reply to: ↑ 43 @azaozz
12 years ago

Replying to duck_:

Thanks, meant to do that before committing :)

#45 @scribu
12 years ago

Related: #18966

#46 @ramiy
12 years ago

Some changes break the RTL admin.

wp 3.2.1:

wp 3.3 beta1-18972:

@andrewryno
12 years ago

Use category/tag styles from postscreen with only minor changes

#47 @andrewryno
12 years ago

attachment:18314.press-this.diff adds an ID to the press-this sidebar so it can use the styles for the regular post screen and then only changes the dimensions so it still fits within the window. Also gets rid of some other styles that are already defined for the main post screen.

#48 @azaozz
12 years ago

In [19001]:

Press This CSS cleanup, props andrewryno, see #18314

#49 @azaozz
12 years ago

In [19090]:

Some more css cleanup, see #18314

#50 @azaozz
12 years ago

In [19116]:

More css cleanup, fix styles in the Edit Image popup then the editor is on the front end see #18314

#51 @azaozz
12 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Close as fixed for now. More to come in 3.4.

#52 @azaozz
12 years ago

In [19227]:

Fix styling in the MCE help popup, see #18314

Note: See TracTickets for help on using tickets.