Page MenuHomePhabricator

Core should provide default page status indicator CSS inside the `interface` feature
Closed, ResolvedPublic

Description

To better support page status indicators, for new skins, a skin needs a one-line call to $this->getIndicators(); in its template class (the class which extends BaseTemplate) in the appropriate location, usually after the site notice but before the <h1> element and the appropriate CSS styling for it.

The problem: there are no default styles for page status indicators whatsoever, which in practice means having to adapt the CSS yourself or copy it over from e.g. MonoBook. This is not ideal as it results in unnecessary code duplication and makes it harder to update these styles, given that (based on a few quick tests) the styles used by MonoBook work in many other skins besides that skin and thus they really should be default, core-provided styles.

In the odd edge case if a skin loads another skin's CSS, such as how Nimbus loads MonoBook's skins.monobook.styles ResourceLoader module (and thus essentially depends on MonoBook), page status indicator support needs merely that one line of PHP.

@matmarex pointed out that many skins in gerrit implement support for page status indicators differently from each other. To be specific, the skins which have styles for either .mw-indicators or .mw-indicator are:

Most of these styles are very different, with one exception - they all float right.

All of the aforementioned skins should be tested to ensure that the new default styles don't cause any regressions and whatever custom, skin-specific CSS is needed should be kept, but things obviously duplicating this new set of default rules should be thrown away.

This feature should be implemented as a brand new ResourceLoader module independent of the pre-existing mediawiki.skinning.* modules because due to various problems with those modules, not all skins use those modules (specifically I'm referring to mediawiki.skinning.elements, mediawiki.skinning.content and mediawiki.skinning.interface -- all of these modules include the file resources/src/mediawiki.skinning/elements.css which is often the file that causes the most problems, forcing skin developers to manually include the content.css and/or interface.css files or totally re-implement everything from scratch). This new RL module should be loaded by OutputPage where appropriate.

Acceptance criteria

  • A style will be added to the existing interface feature. Add the following rule to resources/src/mediawiki.skinning/interface.less :
.mw-indicators { float: right; }

Event Timeline

I feel like that last paragraph should be a bug all on its own.

Jdlrobson added a subscriber: Jdlrobson.

Is there anything truly generic across all skins for page indicators or is the status quo acceptable of deferring to the skin for all indicator styling?
if so I would suggest adding an interface-indicators feature to the ResourceLoaderSkinModule class that defaults to false to allow skins to use it.

We'll be looking at indicators in the context of Vector shortly (T248761)so if I understand this better I can probably help drive some changes.

Is there anything truly generic across all skins for page indicators or is the status quo acceptable of deferring to the skin for all indicator styling?

Doing a code search would suggest that the only truly skin-independent style is float: right. line-height: 1.5em seems quite common, along with font-size: 95%. These last two are copied from MonoBook. Specifying inline-block (and vendor-prefixed variants) for .mw-indicator adds another two/three CSS rules.

That's five, of which two are MonoBook specific and might not be desirable for all skins.

All skins also appear to hide .mw-indicators when @media is print.

Assuming we'd want to include all of that, we're looking at a whopping 5 lines of CSS. Is that worth creating a new feature for? I'm not really convinced.

Assuming we'd want to include all of that, we're looking at a whopping 5 lines of CSS. Is that worth creating a new feature for? I'm not really convinced.

That's how I feel. Especially given Vector is about to not use those styles. However, adding to the existing interface might make sense if we want interface to evolve to mean "something monobook/classic vector like"

However, adding to the existing interface might make sense if we want interface to evolve to mean "something monobook/classic vector like"

That's what the documentation of that feature currently says: Essentially this level is for styles that are common to MonoBook clones., so that seems a good compromise.

It would help address some issues with T89981: Split the `legacy` feature on ResourceLoaderSkinModule into non-legacy manageable and reusable parts and deprecate use of the feature as well.

Jdlrobson renamed this task from Core should provide default page status indicator CSS to Core should provide default page status indicator CSS inside the `interface` feature.Jan 20 2021, 8:00 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a project: MW-1.36-release.

Can we perhaps aim to get this in for the 1.36 release?

Change 674531 had a related patch set uploaded (by Mainframe98; owner: Mainframe98):
[mediawiki/core@master] Include indicator styling in the interface feature

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

Change 674531 merged by jenkins-bot:
[mediawiki/core@master] Include indicator styling in the interface feature

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

Skins can make sure of the interface module.

Created T278362 and T278364 for Vector and Monobook as follow ups.

Thanks @Mainframe98 for making the skinning system a little better!