Page MenuHomePhabricator

Message boxes classes should carry `mw-`
Open, MediumPublic

Description

'resources/src/mediawiki.skinning/messageBoxes.css' classes are from early times in MediaWiki core, where CSS class architecture wasn't yet a thing.

The classes should be turned into

  • messagebox => mw-message-box
  • errorbox => mw-error-box
  • warningbox => mw-warning-box

Without strong opinion if *-box is the right naming scheme, we're using *-container in a related, but slightly different context.
There would be also the possibility to use mw-error and mw-error--inline assuming that user notice boxes are the default way of exposing them.

Event Timeline

Change 651848 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/core@master] message boxes: Use CSS classes adhering to class naming scheme

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

Actually most of these uses are much older than the introduction of .messagebox in e15e45b38670 in September 2019, and just clash with the class in MW core. In fact, this change will fix what was broken in 1fd66a67edcd (see also mw:Topic:Vcxxk5h088xsv9va). But if you want to check everything, it’s not just stylesheets and scripts. My naïve regexp (doesn’t check for context) found more than 200,000 pages Wikimedia-wide. This certainly contains many false positives, but still I think the true positives (i.e. pages using these classes) are probably over 100,000. Fortunately most of these should be the legacy (non-MediaWiki) .messagebox clas, which doesn’t need fixings (.errorbox and .warningbox together account for “only” less than 4000), but they should still be checked if we want to make sure nothing breaks. Is it possible to introduce a feature flag to define the legacy class names, and progressively turn it off across Wikimedia as projects fix their uses? There should still be a hard deadline, of course, when the feature flag goes away, but this way it’s possible to remove the clashing classes on a wiki as soon as it’s fixed, follow the progress, and find the wikis that need help.

I feel like the word order should be changed to be increasing specificity, so <namespace>-<feature>-<modifier> instead of <namespace>-<modifier>-<feature>.

That would be something like mw-box-message, mw-box-warning

I feel like the word order should be changed to be increasing specificity, so <namespace>-<feature>-<modifier> instead of <namespace>-<modifier>-<feature>.

That would be something like mw-box-message, mw-box-warning

It seems to me that then all the classes will have to be rewritten, although we do not have a complete guide on how to name the classes: most of the classes use the reverse order :)
mw-head-base
mw-page-base
etc

I confused, sorry :)

Change 651848 merged by jenkins-bot:
[mediawiki/core@master] message boxes: Use CSS classes adhering to class naming scheme

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

Needs a user-notice for gadgets and user styles/scripts so they are aware these classes are being replaced.

When will this be in production? Next week?

Something like this for Tech News?

The CSS classes for message boxes will change. .messagebox should use .mw-message-box. .errorbox should use .mw-error-box. .warningbox should use .mw-warning-box. .successbox should use .mw-success-box. You should update your gadgets and scripts if necessary. This is to make the class names work with the MediaWiki CSS class architecture.

I feel like the word order should be changed to be increasing specificity, so <namespace>-<feature>-<modifier> instead of <namespace>-<modifier>-<feature>.

That would be something like mw-box-message, mw-box-warning

I've not had a reply to this. It should be addressed either way before the release.

I may be mistaken, but isn't the issue with the eventual removal (not the deprecation here) that there are, as noted in T270796#6714564, a huge number of uses of these in active use? If there are really hundreds of thousands of edits to be made, I'm not sure saying "the old class names... don't work..." is entirely accurate.

Again, I could be mistaken.

Is it possible to introduce a feature flag to define the legacy class names, and progressively turn it off across Wikimedia as projects fix their uses? There should still be a hard deadline, of course, when the feature flag goes away, but this way it’s possible to remove the clashing classes on a wiki as soon as it’s fixed, follow the progress, and find the wikis that need help.

I haven’t got an answer for this, either. This change has a quite large impact given the number of places where these classes are used, please be careful. I’d appreciate even a No, this would be impossible/too much work to implement/we can’t handle the progressive turnoff answer much more than the current ignorance.

@Tacsipacsi the old classes will continue to work. We have not removed them yet. We will remove them at a later date as @Amorymeltzer says and only do that when enough scripts have been updated.

I feel like the word order should be changed to be increasing specificity, so <namespace>-<feature>-<modifier> instead of <namespace>-<modifier>-<feature>.

That would be something like mw-box-message, mw-box-warning

I've not had a reply to this. It should be addressed either way before the release.

My take: Based on the CSS code mw-message-box, mw-warningbox and mw-success-box are all separate features, not modifiers. I'm not sure why a mw-warning-box class also carries a mw-message-box class, as that provides no obvious benefit and seems like a bug to me. All boxes are styled identically without that class.

If the plan is to make warning a modifier of message box on the long term, than yes, it should probably be mw-message-box-warning and the styles should be updated to reflect that.
e.g.

.mw-message-box,
.mw-error-box,
.mw-warning-box,
.mw-success-box,
.messagebox, /* Deprecated since v1.36 */
.errorbox, /* Deprecated since v1.36 */
.warningbox, /* Deprecated since v1.36 */
.successbox { /* Deprecated since v1.36 */

should become

.mw-message-box,
.messagebox, /* Deprecated since v1.36 */
.errorbox, /* Deprecated since v1.36 */
.warningbox, /* Deprecated since v1.36 */
.successbox { /* Deprecated since v1.36 */

@Johan let's hold off on the user notice for now, until this matter has been settled.
Worse case in that we can't resolve this I will revert Monday before the train cut off.

mw-message-box, mw-warningbox and mw-success-box are all separate features, not modifiers

In OOUI they are all MessageWidgets with a type=error/info/warning. In our styleguide they are all listed under one component: https://design.wikimedia.org/style-guide/components/messages.html

Given their commonalities, I would imagine any future library would also implement them as one widget.

(I can see how a modifier-feature order reads better in English due to our typical adjective-noun word order, but I don't think that should be a consideration here)

Change 658250 had a related patch set uploaded (by Esanders; owner: VolkerE):
[mediawiki/core@master] Revert "message boxes: Use CSS classes adhering to class naming scheme"

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

Change 658250 merged by jenkins-bot:
[mediawiki/core@master] Revert "message boxes: Use CSS classes adhering to class naming scheme"

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

@Esanders @Jdlrobson
Two points, somewhat taking parts of both your criticism and proposals into account.
As the code tells us by

  1. the module name and
  2. by the code, they have a lot in common. They are (user system) messages. The feature is a message box, the modifier is a warning.

In the optimal world, we would have

CSS:

.mw-message-box {
	color: #000;
	-webkit-box-sizing: border-box;
	-moz-box-sizing: border-box;
	box-sizing: border-box;
	margin-bottom: 16px;
	border: 1px solid;
	padding: 12px 24px;
	word-wrap: break-word;
	/* Standard property is `overflow-wrap` */
	overflow-wrap: break-word;
	overflow: hidden;

	// 
}

.mw-message-box--notice {
	background-color: #eaecf0;
	border-color: #a2a9b1;
}
// 

HTML:

<div class="mw-message-box mw-message-box--notice></div>

The main issue I see is, that we haven't clarified yet, if we support BEM-like notation style in MediaWiki core, which would need a wider conversation among front-end devs in Foundation and Movement. I don't want to steer ahead with such without, as we have already so many competing, half-baked standards in CSS class naming.
Second priority would be amendment of the class names across our codebase, clearly.

A feature definition like box doesn't make sense to me, it doesn't speak for itself and could be badly misused due to confusion as well.

I think messagebox+modifier if fine too, I was just amending the original proposal.

ovasileva triaged this task as Medium priority.Feb 15 2021, 4:46 PM

This looks good. I suggest making this change as part of the 1.36 release. We should make sure we include messagebox and mw-message-box classes for now, to keep compatibility, as we did before and we'll run a user notice, to ensure that it's clear these classes and associated styles will be removed. Any on-wiki templates using these classes should probably not share styles with interface warnings, so this would be a good opportunity to get article-specific message boxes defined on wiki. This would also have the benefit of those finally displaying correctly on skins like Minerva which only load message box styles on pages that need them.

Sometime after 1.36 release we can remove the original classes.

While these are not prefixed, they create opportunities for broken gadgets that we'd rather avoid.