Page MenuHomePhabricator

Drop !important-s from .night-mode-strip-all-colors-when-safe()
Closed, DeclinedPublicFeature

Description

Feature summary (what you would like to be able to do and where):
Remove the !important flags from ext.wikimediamessages.styles/mixins.less.

Use case(s) (list the steps that you performed to discover that problem, and describe the actual underlying problem which you want to solve. Do not describe only a solution):
Affected templates such as en:TM:Ambox have color coding that is overridden by this style.
Tackling the !important head-on is suboptimal since it can cause visual glitches in other templates that transclude the ambox and break users' CSS customizations.
This proposal was inspired by Izno's comments at Template talk:Ambox#Fix stripe and bg color in dark mode

Benefits (why should this be implemented?):
The current specificity is already enough for all of the remaining use cases of the strip-colors style as far am I'm aware and would allow for much better fixes to restore functionality in affected templates.

Event Timeline

Change #1178671 had a related patch set uploaded (by Aaron liu; author: Aaron liu):

[mediawiki/extensions/WikimediaMessages@master] feat(night mode styles): Drop !important-s from strip-all-colors mixin

https://gerrit.wikimedia.org/r/1178671

I am not really certain it is valid to remove !important from everything. What would be unequivocally fine would be to remove it from the border declaration, since borders are never going to cause issues with unreadable text (maybe some white against dark grey background lines but that's not a readability issue, just a style one), or perhaps for that reason, simply removing the line with border altogether. (nb I would personally prefer T365330 finally get sorted.)

I presume the LESS file here is being used in more places than just en.wp amboxes, which is wherefrom this request originates.

Though the border color is indeed the biggest issues, the ambox styles and many other need to change the background color as well.

I'd want to see a scenario where this'd break things with an existing template first, since (0,3,1)/(0,3,2) is high enough IMO.

The !importants are there to account for backgrounds using inline styles. You cannot solely count selectors. You also have to account for non-enwiki as well who may not have the same general disposition away from using inline styles in these kinds of templates, even if we can go and hunt down the handful that do on enwiki and fix those (and some have indeed been fixed).

Jdlrobson-WMF subscribed.

We will not be making any changes to these styles in the foreseeable future since they are in use across all wikis and any change here might break one wiki and we have no way to know ie. far too risky.

Disable the styles you don't need via https://www.mediawiki.org/wiki/Extension:WikimediaMessages#Disabling_styles and provide your own rules if the defaults do not fit.

I'm open to tasks that make disabling easier (e.g. reorganization) but given nobody in the WMF is actively focused on dark mode and related code right now, changes there might take some time.

Thank you, that makes a lot of sense.

We will not be making any changes to these styles in the foreseeable future since they are in use across all wikis and any change here might break one wiki and we have no way to know ie. far too risky.

I know you're still recovering from not-working ( :) ). I remain of the opinion that this block doesn't need to be opinionated about borders. People aren't going to be flashbanged by random 1px lines that are most often grey (metadata where this same block I think is used), and the colors in the ambox border actually look really nice on dark mode. I don't think it can be removed either after looking a bit (navbox uses it which I think probably has some relevant borders that would actually flashbang people) but I'm not totally convinced the !important must remain for that specific line.

I'm open to tasks that make disabling easier (e.g. reorganization) but given nobody in the WMF is actively focused on dark mode and related code right now, changes there might take some time.

Well, I know of two of those that do exist. :) T365330 is the one that matters and which mostly needs some help getting agreement about the particular issue that I've brought up with the current proposed patch there. And your return to it I guess since you had your own -1 on the patch.

I can be convinced about removing borders since they won't result in unreadable text that seems less risky. T365330: Split theme-night.less into component files
That said, since here is no team focused on dark mode right now, so I'm still a little hesitant about any change that may have unexpected consequences so I will definitely be very cautious about anything merged in this area without hearing the same thing from multiple communities.

Change #1178671 abandoned by Aaron liu:

[mediawiki/extensions/WikimediaMessages@master] feat(night mode styles): Drop !important-s from strip-all-colors mixin

Reason:

see reasoning in ticket

https://gerrit.wikimedia.org/r/1178671