Page MenuHomePhabricator

Create an abstraction for the message box components (warningbox, errorbox etc)
Closed, ResolvedPublic

Description

Various projects not using OOUI are creating HTML which contains the class warningbox and relies on styles in core. This is going to make it very difficult to migrate all these extensions in future.

See here:

Can I suggest we create a helper function messageBox in core and update everything generating warning boxes to use it.

This will help with standardisation efforts.

MobileFrontend has a simple abstraction here which can be used for guidance:
https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/a352e235190df48248734d7dc74c23079831e500/includes/MobileUI.php#L85

Related Objects

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 2 2017, 9:49 PM
Jdlrobson renamed this task from Create an abstraction for the warningbox/errorbox components to Create an abstraction for the message box components (warningbox, errorbox etc).
Jdlrobson updated the task description. (Show Details)
Jdlrobson triaged this task as High priority.Jun 2 2017, 9:52 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson moved this task from To Triage to Needs Analysis on the Readers-Web-Backlog board.

Html class seems a good place for this to live?

We have abstractions in MobileFrontend:
https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/includes/MobileUI.php

Annoyingly we are copy/pasting these into other extensions now (see T169569)
Maybe it's time to put that in core?

Prtksxna added a subscriber: Prtksxna.EditedSep 27 2017, 1:21 PM

Not exactly the same, but there are warning and errorboxes in OOUI too. See the FieldLayout with error message for example. These are tied to a single form element, not the entire form. Are we looking at both?

Jdlrobson added a subscriber: Esanders.

The plan is to have a legacy method for generating these boxes. I'll add this with support from frontend standards groups and will include @Esanders to ensure that it's compatible with OOUI down the road. Design changes are out of scope for the time being.

Jdlrobson moved this task from Inbox to Next up on the User-Jdlrobson board.

Change 381291 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Provide message/warning/error box abstraction

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

Patch is up ^ I'd appreciate some thoughts particularly about:

  1. How to handle those wrapWikiMsg usages
  2. Whether we should allow additional ids and classes on the first path standardised message/warning/success boxes.
Jdlrobson moved this task from Next up to Blocked on the User-Jdlrobson board.Sep 28 2017, 7:04 PM
Jdlrobson moved this task from Blocked to Focus on the User-Jdlrobson board.Oct 3 2017, 9:26 PM
Jdlrobson moved this task from Focus to Next up on the User-Jdlrobson board.

Feedback appreciated @Esanders and @Volker_E
There is some feedback from @Legoktm and I am fine with that (although it creates more work for me), but I want to check we're okay with the proposal before continuing.

Jdlrobson moved this task from Next up to Blocked on the User-Jdlrobson board.Oct 10 2017, 6:44 PM

I've responded to the feedback and am waiting for another round of review.

@Jdlrobson Just saw the comment about “additional ids and classes on the first path standardised”
What do you mean by first path and what use cases to you have in mind?

additional ids and classes on the first path standardised”

What I meant was there are some cases where warningbox class is also accompanied by other classes and an id. I'm not sure we want to encourage that so I've purposely not made that possible in the first "pass" of standardisation.

Change 381291 merged by jenkins-bot:
[mediawiki/core@master] Provide message/warning/error box abstraction

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

Change 391626 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Add unit tests for Html helper methods and change messageBox visibility

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

Jdlrobson moved this task from Blocked to Done on the User-Jdlrobson board.Nov 16 2017, 8:00 PM

Change 391626 merged by jenkins-bot:
[mediawiki/core@master] Add unit tests for Html helper methods and change messageBox visibility

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

Jdlrobson closed this task as Resolved.Nov 22 2017, 9:34 PM

Done...? T127405 captures the remaining work.

Tgr added a subscriber: Tgr.Feb 2 2018, 6:47 PM

This is a great idea but IMO we should be vary of making Html the grab-bag utility class of all things that generate HTML. Maybe there should be a WikimediaUI or similar class for generating HTML for a specific design style, and Html should be preserved for methods which take an exact specification of HTML code in some structured form and turn it into a string? (Yeah I know I'm a little late raising this issue.)

Yeh that was the original hope, but OOUI is basically this and that's why it's not happened. The problem is OOUI adoption is much harder than using PHP helper methods in a lot of places in our codebase.

In fact we have one in both MobileFrontend and Minerva:

When I've tried to upstream this I've encountered much apathy so would happily review patches that split out the functionality from Html into a separate MediaWiki class.