Page MenuHomePhabricator

Split theme-night.less into component files
Open, In Progress, LowPublic

Description

Feature summary (what you would like to be able to do and where):

Currently, theme-night.less has about 10 different templates/template groups that it is overriding styles for as we transition to night mode.

To enable local admins to work template-by-template to restore color control locally (and deal with caching effects gracefully), each of these invocations for a specific template should be split up into their own theme-night-component.less file.

This file would still be needed to support night theme generically. P62659 is a paste with a suggested revision to html.skin-theme-clientpref-night, with -os left as an exercise to the reader.

After working on the paste, I don't think it makes sense to add them to the already-existing LESS files in this directory since those each may be loading in different locations.

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):

  • disabling metadata styles (see original request in T365318)

Benefits (why should this be implemented?):

Event Timeline

And then going forward, guidance should be to add any new coloring work to appropriate component files.

The main reason this was created in this way was for French who wanted to the night mode styles but not the infobox styles, and keeping these rules together make sense - as they are essentially a recommended night mode reset file at this point. I'd want to hear from @Lofhi on how this would impact French first.

Is there any reason you cannot copy the rules in https://github.com/wikimedia/mediawiki-extensions-WikimediaMessages/blob/master/modules/ext.wikimediamessages.styles/theme-night.less into MediaWiki:Common.css and disable the night mode styles https://www.mediawiki.org/wiki/Extension:WikimediaMessages#Disabling_styles ? That would allow you to then cherry pick the ones you want and the ones you don't to the relevant places?

and deal with caching effects gracefully

Could you expand on the caching effects you are worried about? Disabling the message should only be a problem for 5 mins and provided you copy to MediaWiki:Common.css first, would never be a problem in production.

The main reason this was created in this way was for French who wanted to the night mode styles but not the infobox styles, and keeping these rules together make sense - as they are essentially a recommended night mode reset file at this point. I'd want to hear from @Lofhi on how this would impact French first.

I understood this ticket as a request to remove the global theme-night.less file in order to create small files like infobox-theme-night.less. In this case, I think I have no comment, because frwiki (and maybe others projects) would still be able to disable (temporally for the good fingers crossed) infobox styles (infobox.less) but keep the infobox dark styles fixes (infobox-theme-night.less).

This would perhaps be a welcome granularity?

The main reason this was created in this way was for French who wanted to the night mode styles but not the infobox styles, and keeping these rules together make sense - as they are essentially a recommended night mode reset file at this point. I'd want to hear from @Lofhi on how this would impact French first.

Which is another reasonable reason not to put things in with the other overrides:

@Izno wrote:

After working on the paste, I don't think it makes sense to add them to the already-existing LESS files in this directory since those each may be loading in different locations.

This is a bit of a reference to the hatnote dark background that's referenced in T365087 since the alternative hatnote coloring (relative to en.wp) is applied only in mobile (I just caught that some wikis have coloring of their own here unlike en.wp, so another reason to separate it out), but also slightly the one for metadata also now pointing here.

Is there any reason you cannot copy the rules in https://github.com/wikimedia/mediawiki-extensions-WikimediaMessages/blob/master/modules/ext.wikimediamessages.styles/theme-night.less into MediaWiki:Common.css and disable the night mode styles https://www.mediawiki.org/wiki/Extension:WikimediaMessages#Disabling_styles ? That would allow you to then cherry pick the ones you want and the ones you don't to the relevant places?

Valid question. From my point of view these overrides are there by WMF to hack around slow communities, or those with quick communities who lack appropriate support from interface administrators (usually meaning none, or the stewards, who from my observation don't spend a ton of time on the fleet "stack" as it were; and global interface editors don't spend a ton of time here either despite the role existing for them :'). From that perspective I'd like to keep them in WMF land to own (and/or to be cast out at some point in the future when the handful of groups like English or French have added native support in their local styles which can then be pointed to).

But the second point here is that I'd like to be able to go item by item, placing things where appropriate in their order, rather than plopping the whole blob into Common.css, and then moving them, as I attempted to explain in the original above.

@Izno wrote:

To enable local admins to work template-by-template to restore color control locally

Could you expand on the caching effects you are worried about? Disabling the message should only be a problem for 5 mins and provided you copy to MediaWiki:Common.css first, would never be a problem in production.

It did take close to 5-10 minutes for dark-theme-mainpage to turn off when I did that yesterday. But this was less CSS cache in browser comment which is what I suspect you're thinking and more PHP/output cache, since I need to roll out styles for N template(groups), which is probably 30 or 40 million transclusions which don't all gracefully update at the drop of a CSS hat. It took a couple weeks when I TemplateStyled navboxes for transclusions to be recorded in various systems onwiki.

I understood this ticket as a request to remove the global theme-night.less file in order to create small files like infobox-theme-night.less

Correct.

Jdlrobson triaged this task as Medium priority.Jun 3 2024, 3:05 PM

The more I look at it, the more I am in favour of this proposal. Currently it is a huge hodgepodge of many different overrides in there. As an interface admin, I would like to disable .infobox overrides since infobox colouring is TemplateStyles-based in Russian Wikipedia, but I literally cannot because it is either all or nothing in the file. .navbox overrides, for example, are not so easily fixable, so I would like to not have to define all of them myself. Please split the files into chunks that can be disabled one by one.

Also, merged .metadata task should still be fixed by removing the overrides from .metadata class, not by splitting it into a separate thing; .metadata class is used in a bunch of different places with different styling purposes, so it should not have been added to the override CSS in the first place.

Btw, [bgcolor] thing is not really wise because not all of the [bgcolor] values actually take effect, for example if they are in a table with a .wikitable class, they do not.

https://www.mediawiki.org/wiki/Manual:Interface/IDs_and_classes#Content

.metadata
See Help:Extension:Media Viewer#How can I disable Media Viewer for unrelated images? and Reading/Multimedia/Media Viewer/Template compatibility#Marking up article templates as ignored.

Another argument as to why this poor-advised style rewrite should be removed. @Jdlrobson is anything blocking the .metadata thing in particular, since you merged T365318: Dark/night mode shouldn't reset .metadata styles into this?

Dark mode is being enabled on wikis next week. metadata is used on non-English Wikipedias so it shouldn't be removed as it would create more work for those wikis. Here's an example on French that would be broken by the. removal: https://fr.wikipedia.org/wiki/Mod%C3%A8le:Attention

But, I have no objections to moving these rules to a separate module e.g. theme-night-metadata if that's helpful. It's just not a priority for my team right now, but you are welcome to post a patch and I'll make sure to take a look.

French Wikipedia has other classes in there. .bandeau-container for example. Why it has to be .metadata? The choice for a technical class makes it so when someone wants to adapt .ambox styling to the dark theme, they have to do this:
https://ru.wikipedia.org/?diff=137874758

This is just one example I pulled up quickly. .bandeau-container is a selector that applies only on French Wikipedia, but these rules are meant to apply broadly to all our projects (even if they are not as helpful to some wikis).

If you use global search, there are other 10k uses of metadata in templates and modules alone:
https://global-search.toolforge.org/?q=metadata%5B+%22%27%5D&regex=1&namespaces=10%2C828&title=

We're deploying this week so we're not going to remove these rules as that would risk unnecessarily break accessibility in templates on some of the projects we are shipping to (we've made lots of assumptions based on how wikis are rendering right now).

But, repeating myself I am perfectly fine with us splitting this up, to support projects disabling them on a per-project level.

Not sure if this is the right place for this but I was referred to this task by @Izno. Is it possible to locally disable all the overrides containing the hacky selector [style*="background"]? To be clear, I want things as broken as possible to make it easy to identify which templates need to be updated.

@Ioaxxere you can add the following code as a user script, to undo the overrides. Hope this helps!

(function inlineStyleToCSS() {
const addCSS = (node, className, styles) => {
    node.classList.add(className);
    mw.util.addCSS(`.${className} {
        ${styles}
        }`);
};

document.querySelectorAll('[style]').forEach((node, i) => {
    const styles = node.getAttribute('style');
    addCSS(node, `inline-style-${i}`, styles);
    node.setAttribute('data-style', styles);
    node.removeAttribute('style');
});

document.querySelectorAll('[bgcolor]').forEach((node, i) => {
    const bgcolor = node.getAttribute('bgcolor');
    addCSS(node, `inline-bgcolor-${i}`, ` background-color: ${bgcolor}`);
    node.setAttribute('data-bgcolor', bgcolor);
    node.removeAttribute('bgcolor');
} );
}() );
bwang lowered the priority of this task from Medium to Low.Sep 11 2024, 5:20 PM
bwang moved this task from Incoming to Tracking on the Web-Team-Backlog board.

Change #1099369 had a related patch set uploaded (by Saint Johann; author: Saint Johann):

[mediawiki/extensions/WikimediaMessages@master] Split night mode hacks by their purpose

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

stjn added a subscriber: bwang.

Just published a patch that would hopefully resolve this task so I’ll claim it. With the way I structured it, this would allow us in Russian Wikipedia to remove some of the stuff in https://ru.wikipedia.org/wiki/MediaWiki:Gadget-darkModeFixes.css that isn’t wiki-specific and enable theme-night back. Other wikis, in turn, could remove parts of night theme hacks that they fixed with changes to relevant template/site CSS.

It is split like this:

  • theme-night
  • theme-night-hatnote
  • theme-night-infobox
  • theme-night-metadata
  • theme-night-navbox
  • theme-night-sidebox

If anyone has comments on this structure, leave them here or in the patch.

As an aside, both this task and future additions to night theme hacks would need to be announced in User-notice once this gets resolved. For the current task, it is something like:

[Advanced item] Adjustments for the night theme were split into separate submodules. Administrators on your wiki can now disable adjustments in [[MediaWiki:Wikimedia-styles-exclude]] by submodule. Adjustments should only be disabled if contrast issues they fix would be fixed locally.


And for future files for night theme hacks, something like:

[Advanced item] New night theme adjustments were added for [purpose here]. They can be disabled in your wiki in [[MediaWiki:Wikimedia-styles-exclude]] by adding theme-night-[ID] ID if they are not needed.

Thanks for the patch. We have one deploy window before the end of the year and many people who know this code are away for it, so I would recommend we hold off on this change until January.

Jdlrobson changed the task status from Open to In Progress.Wed, Dec 4, 10:28 PM

Nothing should be changed in the code itself apart from splitting it into separate files and adding them as identifiers into .php file. I simply modeled this patch on a patch in T366380, so it should not require a significant knowledge of this code in particular if merging that patch was fine. Basically, someone just needs to check that I didn’t lose anything in the process while moving selectors around. @Izno checked some files already, I think. Are you sure this is not doable till January?