Opened 11 years ago
Closed 3 years ago
#26350 closed defect (bug) (fixed)
!important audit
Reported by: | azaozz | Owned by: | |
---|---|---|---|
Milestone: | 5.8 | Priority: | high |
Severity: | major | Version: | 3.8 |
Component: | Administration | Keywords: | |
Focuses: | ui, css, performance | Cc: |
Description
After the MP6 merge many CSS rules became "very !important". Some are so !important that overwrite other !important...
Looking at wp-admin.css: in trunk there are 65 !important
compared to 22 in 3.7.
Attachments (4)
Change History (48)
#2
in reply to:
↑ 1
@
11 years ago
Hi Rhomanu,
I also want to look into this but do not want to make the work twice, any proposal how to divide it, any progress at your site so far?
#3
@
11 years ago
- Milestone changed from 3.8 to Future Release
Yeah, each use of !important spreads exponentially. Every time it needs to be overridden, new !important has to be used.
As much as I'd like to get rid of all !important for 3.8, there is simply not enough time to do it... Hoping we can do this in 3.9.
#6
@
10 years ago
26350.diff: Is anything really that !important? and if so, why? This patch removes all !important declaration, please restore where appropriate, with !justification.
#7
@
10 years ago
Refresh against trunk.
Left in one !important where the purpose was indicated in a doc block.
#8
@
9 years ago
26350.3.diff is a refresh of 26350.2.diff and includes the 17 new instances of !important that have been added in the last 4 months.
This ticket was mentioned in Slack in #design by voldemortensen. View the logs.
8 years ago
#10
@
8 years ago
- Keywords needs-testing added
This is great work, @MikeHansenMe and @adamsilverstein, thank you. We'd like to help with any review and revision of !important free CSS for WP Admin. We have quite a bit of experience in writing resilient !important free code for plugins. How can my team help finish this revision for core?
#11
@
8 years ago
@FolioVision Thanks for your offer of help.
Essentially we need to go thru each use of !important
and see how its being used, try removing it and determine any consequences, then remove any that can be and document the reason for the remaining uses.
Using svn or git blame type tools we should be able to track the commit that introduced each usage (although this may be difficult given the wholesale restructure the css has undergone in the past); alternately using inspection and code hunting we should be able to determine where the class in question is used and possibly determine why it was added. Its possible many are useful (since we strive to support older browsers), or that they are extraneous - I really don't know.
My farcical proposal was just to 'remove them all' and see what breaks but in reality we probably need to exercise more diligence. If we can't determine a purpose in each case I think we should remove them and see what breaks, however we still need to spend some time trying to determine why they were added. Where we do determine they have a purpose we should add some inline comments in the CSS to indicate why the use is required or desired.
I know this is a somewhat tedious task, however its something we really should undertake and we can take them out little by little so don't feel like you have to check them all to contribute.
Thanks again for your offer of assistance.
#12
@
8 years ago
Hi Adam,
Why don't we try to break this task into segments as @bassgang suggested in the early days? It would be great to remove the !important
tags for one section of WP Admin successfully and then move on to another section. I'm worried about the idea of doing a huge amount of work to then have it ignored and slide out of date without action (commit).
Is there a particular WP Admin section which would benefit more than others from the absence of !important
tags in your opinion?
Thanks, Alec
#13
@
5 years ago
Agreed with @FolioVision, especially since wp-admin.css
is now broken into multiple files using @import
. Working on a fix for wp-admin/css/common.css
on the WCEU contributor day right now.
#14
@
5 years ago
For reference, I just created this ticket for wp-admin/css/common.css
: https://core.trac.wordpress.org/ticket/47569#ticket
#15
@
5 years ago
Thanks for picking up the banner on this, @foack. Cleaning up Admin CSS is a noble mission, if a thankless one.
#16
@
5 years ago
It was a good starting point for me to get involved in the WP development :) Thanks, @FolioVision!
#17
@
5 years ago
- Keywords needs-testing removed
It's good to start by focusing on the 21 instances in the common.css
file, to edit them on ticket:47569.
They are at the beginning of this list of 331 !important styles in WP 5.3.2 core:
https://docs.google.com/spreadsheets/d/1WCuUXfm_WSx7wYFlZ3h8_yLaLxoganyBcPys0TKfoRY/edit?usp=sharing
Column D gives any explanatory notes that are already in each file. If you have something to add to columns E through G, you can use comments within the spreadsheet.
I already marked "leave" for a few of the ones I think are worth keeping (word-wrap:normal
for screen-reader-text and skip link styles, as well as call-to-action links without rounded corners when the no-border-radius
class is added).
Bundled plugins and themes (currently Twenty Sixteen through Twenty Twenty) are also included, at the bottom, for review later.
This ticket was mentioned in Slack in #core by marybaum. View the logs.
5 years ago
#22
@
4 years ago
- Keywords has-patch needs-refresh added
The most recent patch doesn't apply cleanly to trunk
anymore. Marking this for a refresh. Also, would like to see this addressed by #core-css in Slack, as they have plans for an audit in #49582.
This ticket was mentioned in Slack in #core by david.baumwald. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by whyisjake. View the logs.
4 years ago
#25
@
4 years ago
- Milestone changed from 5.5 to 5.6
Going to punt to 5.6 as the beta period is ending.
This ticket was mentioned in Slack in #core-css by kirstyburgoine. View the logs.
4 years ago
This ticket was mentioned in Slack in #core-css by kirstyburgoine. View the logs.
4 years ago
This ticket was mentioned in Slack in #core-css by hellofromtonya. View the logs.
4 years ago
#30
@
4 years ago
- Keywords needs-patch added; 2nd-opinion has-patch needs-refresh removed
- Milestone changed from 5.6 to 5.7
This is part of the #core-css team's audit, and another fix should be achievable in the 5.7 cycle.
This ticket was mentioned in Slack in #core-css by kirstyburgoine. View the logs.
4 years ago
This ticket was mentioned in Slack in #core-css by danfarrow. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by sergey. View the logs.
4 years ago
This ticket was mentioned in Slack in #core-css by danfarrow. View the logs.
4 years ago
#35
@
4 years ago
In #core-css much progress has been made on the automated CSS audit tool.
The tool generates an HTML report of the results of several audits run on WP core, including an audit of all uses of !important
.
The `!important` audit report can be viewed here.
It would be great to get feedback on this part of the audit in relation to this ticket.
Thank you!
This ticket was mentioned in Slack in #core-css by danfarrow. View the logs.
4 years ago
#37
@
4 years ago
In the wp-includes/css/dist/block-editor-style.min.css
there is a
.components-button.block-editor-block-switcher__no-switcher-icon:disabled .block-editor-block-icon.has-colors{color:#1e1e1e!important}
Which turns all the icons in the editor for blocks black (dark gray). Not sure why, as I don't see this in the GB on github 🤷🏼♂️
This is grating because when you are building your own blocks and icons, they are turned black, even though you set the color to be white for instance (when you want to switch one block to another).
We have fills in the icons set to currentColor
so that the background of the icon can come through and it looks seamless in the block picker.
@
4 years ago
Image that shows how impoortant in the core css overrides the color of the icon in the editor
#38
@
4 years ago
- Milestone changed from 5.7 to 5.8
@dingo_d Could you create a GitHub issue for the custom icons (or should I)? The !important
had been necessary to change the style when those buttons are in the disabled state. GB16390 added a gray background color and then GB19344 removed it.
Milestone edited to 5.8 for creating any patches after the audit conducted during 5.7.
#39
@
4 years ago
Sure, go ahead and open the issue, I'm a bit busy atm with work stuff so I don't have that much time unfortunately :/
#41
@
3 years ago
@sabernhardt it looks like GB29130 was resolved, are there any additional tasks to fully resolve the audit that @azaozz opened in this ticket?
#42
@
3 years ago
GB21930 was a separate issue, but this is the new location for the CSS audit:
https://wordpress.github.io/css-audit/public/wp-admin#important
That audit does not identify further specific changes to make.
If this ticket should stay open, it might be better in the Future Release milestone. Either open or closed, it would be good to add a comment here whenever a ticket is opened for a smaller related task.
#43
follow-up:
↓ 44
@
3 years ago
@azaozz sounds like this is probably safe to close now that the related audit has completed and no further changes have been identified unless you feel there's still more to track here?
#44
in reply to:
↑ 43
@
3 years ago
- Keywords needs-patch removed
- Resolution set to fixed
- Status changed from new to closed
Replying to JeffPaul:
Right, time to close this ticket. Fixing/removing redundant !important
would be better in separate tickets, case by case.
The WordPress admin CSS audit can be found at https://wordpress.github.io/css-audit/public/wp-admin. Thanks to everybody that made this possible!
66 in 3.8.
Agreed, it's not a good practice : Using these force the use of another !important to overwrite a rule, and then it's expanding... While it's just a simple problem of CSS declaration weight.
But it will be long to fix... I'll try a bit tonight (I must find how to contribute without blocking others' work), but there's many CSS files to check.