Page MenuHomePhabricator

Provide CSS class (hlist) to define horizontal lists in MediaWiki core
Closed, ResolvedPublic

Description

The English Wikipedia is using a class named "hlist" to allow creation of horizontal lists, which is very useful on navboxes. This kind of list exists on all wikis, so I'm following [[User:TheDJ]]'s suggestion and requesting something like that to be added to MW itself.

The existing code is at:
https://en.wikipedia.org/wiki/MediaWiki:Common.css


Whiteboard: gci2014
See Also:
T42297: Add support for horizontal lists (class="hlist") to mobile style sheet
T68336: mediawiki.hlist.css/js are not using localized punctuations

Details

Reference
bz40062

Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 22 2014, 1:08 AM
bzimport set Reference to bz40062.
bzimport added a subscriber: Unknown Object (MLST).
He7d3r created this task.Sep 6 2012, 8:53 PM
Qgil added a comment.Mar 16 2013, 6:31 AM

The feature sounds interesting in principle, but then looking at the hlist class... It is a noticeable chunk of CSS code that then we should maintain. And on the other hand if someone is interested in this non-standard feature it is very easy to add it to their own MediaWiki:Common.css.

CCing Brad just in case he has anything to say since this falls in the category of templates.

rahul14m93 wrote:

With Some guidance i feel i can make an attempt to solve this bug!

Qgil added a comment.Mar 19 2013, 8:04 PM

If I understand correctly, the task itself would be simple: copy the hlist class at https://en.wikipedia.org/wiki/MediaWiki:Common.css and insert it in the default MediaWiki:Common.css.

Maybe you can submit a patch for this in Gerrit, but the logical sequence would be to know first whether the MediaWiki maintainers are willing to accept this feature.

Qgil added a comment.Mar 19 2013, 8:10 PM

Adding Daniel Friesen as suggested by Platonides.

It would have to go in a css file, such as skins/common/shared.css Not at MediaWiki:Common.css
However, I'm not convinced about needing it. And once added, it's practically impossible to remove something.

rahul14m93 wrote:

It is an issue i have been facing in some of the previous bugs i fixed,in the end after alot of scrutiny the patch stays on hold till mass approval
As a naive person i feel adding this feature will enhance the navigation capabilities of a page

Qgil added a comment.Mar 19 2013, 8:28 PM

(In reply to comment #6)

As a naive person i feel adding this feature will enhance the navigation
capabilities of a page

While this might be true in some cases, it is definitely true that it takes a simple copy & paste to have this feature in your own MediaWiki.

For the MediaWiki maintainers it means one feature more that they have to oversee, and this is why they are very cautious about including new features. Think also that the general trend in MediaWiki is to have a simpler core and then more features added through extensions, gadgets, templates and what not.

This class might be too specific to go into core. Things like the parenthesis used inside the css are usually supposed to be i18nable in core but won't be in this css.

Would also be nice to see an example of hlist in use.

'Master' code now lives at [[mw:Snippets/Horizontal lists]]. The code is pretty stable now.

99% of all navboxes on enwiki use hlist, so there are plenty of examples.

I'm starting to think that this should be a Gadget. And we should depend it on things like letting gadgets that load without JS and implementing a gadget repository.

(In reply to comment #11)

I'm starting to think that this should be a Gadget. And we should depend it
on
things like letting gadgets that load without JS and implementing a gadget
repository.

aka [[mw:Gadgets 2.0#Shared gadgets]].

  • Bug 51692 has been marked as a duplicate of this bug. ***

I think some of the rules are useful as generic ones, looking through many of the rules many appear to be specific to certain pages and contexts e.g. talk pages and those rules should only be loaded on talk pages rather than given to all users.

Change 96071 had a related patch set uploaded by Mayankmadan:
Adding hlist module to mediawiki

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

Change 96071 merged by jenkins-bot:
Adding hlist module to mediawiki

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

Okay – so what we have now is that the code is in core. Now, if we want to, we should probably add that module on all pages, or it could just be loaded by wikis which want that. I'll leave this for someone else to decide (and leave this bug open for now).

Agh, it's in "core" now.

Was my comment about internationalization issues with hardcoded punctuation just ignored.

Now we have a module in core with another a super generic css class name (which btw only barely escapes conflicting with Microformats like hCard done with css classes since hListing uses hlisting instead of 'hlist'). Hardcoding skin names assuming that's fine. Assuming that it's going to be of use to the widespread and varied MediaWiki community. And added in a way that it would need JavaScript to use the css (addModules loads CSS via JS and addModuleStyles can't be used without bugs on a module you're using addModules to load JS).

If a Gadget or site css is so unacceptable can't these styles originally intended for WMF sites, not simply any MediaWiki site be put into an extension instead?!

(In reply to comment #19)

Was my comment about internationalization issues with hardcoded punctuation
just ignored.

I'm afraid it was (I didn't notice it myself). But there's nothing
stopping us from localizing the punctuation in CSS (at least for the
saner browsers). With LESS we could probably define a function that
would process MediaWiki messages from CSS (utter madness!, but should
work :P).

Now we have a module in core with another a super generic css class
name (which btw only barely escapes conflicting with Microformats
like hCard done with css classes since hListing uses hlisting
instead of 'hlist').

Applying a 'mw-' prefix here seemed silly to me, as that's not an
interface class. The name can, naturally, still be changed, since
nothing depends on it yet, if you have a better concise one.

Hardcoding skin names assuming that's fine.

Other modules do that too (although it's not much justification).
Edokter, what are those line-height rules for? They don't seem needed
to me.

Assuming that it's going to be of use to the widespread and varied
MediaWiki community.

Why not? Is, say, tablesorter not usable for the "widespread
community"? That one stems from en.wp too, AFAIR.

And added in a way that it would need JavaScript to use the css
(addModules loads CSS via JS and addModuleStyles can't be used
without bugs on a module you're using addModules to load JS).

It's currently not loaded in either way anywhere. You could use either
addModuleStyles (in which case you don't fully support IE<9, but new
browsers get the styles perfectly well), addModules (in which case you
do support even IE6, but require JS), or both (to get the best of both
worlds).

(In reply to comment #20)

(In reply to comment #19)

And added in a way that it would need JavaScript to use the css
(addModules loads CSS via JS and addModuleStyles can't be used
without bugs on a module you're using addModules to load JS).

It's currently not loaded in either way anywhere. You could use either
addModuleStyles (in which case you don't fully support IE<9, but new
browsers get the styles perfectly well), addModules (in which case you
do support even IE6, but require JS), or both (to get the best of both
worlds).

You can't use addModules and addModuleStyles on the same module. This will currently lead to a double loading of the CSS (can't find the bug number). The current practice is to put the JS into a module separate module.

(In reply to comment #21)

You can't use addModules and addModuleStyles on the same module. This will
currently lead to a double loading of the CSS (can't find the bug number).
The current practice is to put the JS into a module separate module.

Fair enough, but that's a minor technical issue.

(In reply to comment #20)

Other modules do that too (although it's not much justification).
Edokter, what are those line-height rules for? They don't seem needed
to me.

I can't remember why I added it. Something to do with the line-height not inheriting properly when a smaller font-size is used (as in navbox). The rule seems to have no effect anymore, So I'm removing it.

(In reply to comment #23)

I can't remember why I added it. Something to do with the line-height not
inheriting properly when a smaller font-size is used (as in navbox). The rule
seems to have no effect anymore, So I'm removing it.

Can you submit a patch for that?

Please revert this and iterate further in a branch per Daniel's concerns.

Also, adding it to core with the same class name as lots of wikis have locally is problematic. Especially given (at least older version of it), don't coop very well when it is effectively run twice. And even now, the local versions across wikis (both within Wikimedia and outside) are likely not all the same version.

This is why 'collapsible' was added in core as 'mw-collapsible'. By requiring things to be (ever to slightly) migrated, it will also give us the opportunity to change the semantics to something more manageable if we want to without breaking existing content that hasn't been migrated yet. Of course, if the current implementation is want we want to maintain in the future forward, it can stay as is and backwards compatible, no problem.

(In reply to comment #25)

Please revert this and iterate further in a branch per Daniel's concerns.

The module currently is just "present", it's not loaded anywhere, so I don't think keeping it committed and improving is worse than reverting it completely. If you feel it is, revert yourself and I won't block that.

Also, adding it to core with the same class name as lots of wikis have
locally is problematic. (…)

Hmmmm. Okay, I might not have thought this out as well as I thought I did.

(I have two pending patches related to this, https://gerrit.wikimedia.org/r/96239 and https://gerrit.wikimedia.org/r/99805 . Any reviews, especially from the naysayers ;), would be appreciated.)

FYI: those two patches were merged in december.

(In reply to Helder from comment #28)

FYI: those two patches were merged in december.

Could somebody (Matma Rex?) elaborate what is left to do here?

(In reply to Andre Klapper from comment #29)

(In reply to Helder from comment #28)

FYI: those two patches were merged in december.

Could somebody (Matma Rex?) elaborate what is left to do here?

If there is work left to do, would anybody be willing to be a mentor for this task in Google Code-in 2014? If yes, could you add it to
https://www.mediawiki.org/wiki/Google_Code-in_2014#Proposed_tasks until
Sunday (we can still improve the description until December 1st)?
If time is spare I can also do that - I just need a statement who
would mentor it.

See https://lists.wikimedia.org/pipermail/wikitech-l/2014-October/079264.html for more information. Don't hesitate to ask if you have questions!

I believe this is done.

Krinkle reopened this task as Open.Mar 1 2017, 5:27 AM

I believe this is done.

adding it to core with the same class name as lots of wikis have locally is problematic. Especially given (at least older version of it), don't coop very well when it is effectively run twice. And even now, the local versions across wikis (both within Wikimedia and outside) are likely not all the same version.

This was not addressed. And from what I can see, wikis are still defining their own hlist CSS/JS. So this module is effectively still unused (aside ApiHelp.php apparently using it now). As such, given it's not being used, this isn't resolved since that wasn't strictly the purpose of this task.

Also, it seems MobileFrontend uses the "hlist" class name in two places, but doesn't load this module or define its styles anywhere. Presumably a name clash, with the assumption enough wikis define it locally in a similar-enough way.

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptMar 1 2017, 5:27 AM
Krinkle raised the priority of this task from Lowest to Normal.Mar 1 2017, 5:27 AM
Krinkle added a project: Readers-Web-Backlog.
Krinkle removed a subscriber: wikibugs-l-list.
pmiazga added a subscriber: pmiazga.Mar 2 2017, 3:44 PM

Styles are already added to MediaWikiCore, the only remaining work is to clean up MobileFrontend styles (for example this line https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/resources/skins.minerva.content.styles/lists.less#L5)

With regards to MobileFrontend - mediawiki.hlist.css has much more exhaustive selectors than are necessary for Minerva and contains many resets which are unnecessary as it uses a reset stylesheet.
It also uses hlist-separated to add separators rather than having them by default. We'd have to reset all the existing rules in a skin style to make this work.

Not sure what approach to take here.

Krinkle updated the task description. (Show Details)Mar 11 2017, 1:10 AM

Regardless of how we proceed, let's re-focus on the success criteria for this task. One of the use cases would be that wikis like English Wikipedia no longer need to define (duplicate, and maintain) "hlist" styles in their MediaWiki:Common.css. This, however, presents two problems:

  1. How would the core module be loaded? Given it's not a gadget, there is no place for a "default-loaded" definition to go on individual wikis. We also don't want to load it by default everywhere as part of core. And given it's a styles module (not a JS module) lazy loading through Common.js would cause a FOUC.
  2. Migration strategy. Depending on how we load it, the core implementation may need to be renamed as otherwise both definitions will apply simultaneously, which will result in style collisions and layout issues. While a suitable replacement, it probably isn't side-by-side compatible.

Migration aside, problem #1 may alone be reason enough to consider the possibility that the current approach won't solve the problem in an adequate manner given the current shape of the platform. But, if there are any ideas for making it work, I'd love to see it happen. Otherwise, we should consider removing the core module, and add minimal versions to ApiHelp and MobileFrontend separately (with their own class name).

As for the original use case, global gadgets could be a suitable solution. It'd be defined centrally, and individual wikis can enable it by default. Much like the "site-styles" gadget that exists on mediawiki.org.

It's a shame the parser doesn't support this in my opinion which would solve all of these problems by loading the css when the wikitext uses it.
e.g.

* Foo * Bar * Test

Editors shouldn't need to think about classes and when to apply them.
Does markdown have horizontal list support?

How would the core module be loaded?

Add a parser tag/function that loads the named RL module?

Qgil removed a subscriber: Qgil.Mar 13 2017, 5:04 PM
Jdlrobson lowered the priority of this task from Normal to Low.May 2 2017, 5:00 PM

Change 356773 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Allow skins to control style of mediawiki.hlist

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

Change 356775 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Minerva should use mediawiki.hlist

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

Change 356773 merged by jenkins-bot:
[mediawiki/core@master] Allow skins to control style of mediawiki.hlist

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

Change 366423 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Minerva should use mediawiki.hlist

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

Change 356775 abandoned by Jdlrobson:
Minerva should use mediawiki.hlist

Reason:
Moved to https://gerrit.wikimedia.org/r/366423 Minerva should use mediawiki.hlist

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

Change 366423 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Minerva should use mediawiki.hlist

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

Change 366892 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Add sensible defaults to mediawiki.hlist module

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

MinervaNeue is now using this module. Here's some notes from an exchange on the previous review with @Krinkle

In what way are the hlist meant to render differently in Minerva from the default? It seems like they doing the same thing: Fold the list into a sequence of inline elements, with bullet separators between them.

Yep there's definitely some overlap. In particular there are lots of css rules in MediaWiki:Mobile.css on enwiki that I'd like to consolidate.

The difference seems rather arbitrary:

Instead of using a space before/after within the "content", Minerva hard-codes a px margin/padding. Arguably, the code approach scales better here, and possibly better for usability (assuming the margin won't be interpreted as a space when the content is copied, resulting in "Foo*Bar" instead of "Foo * Bar").
it's a little arbitrary but I think it should be up to the skin to control. For instance a fictional pirate skin might want to put a skull and crossbones between them :)

There is something about a linkColor that I don't quite understand.

Could you elaborate? The idea is to make the colour the same as the link.

The main difference I do understand is that this version doesn't add support for older browsers. Which makes it an optimisation only. Is that the rationale?

Partly, but as I point out there are more subtle tweaks. These could arguably be done anyway using overrides but I think it's better to do either/or.

If so, I can still support this, but t would be nice if the rest of the implementation is made consistent for maintainability. + a big FIXME comment saying to manually keep this in sync + tech debt task.

That's my plan. I'd like to update the core module to have some defaults (expressed in stylesheets again) with the customisations/older browser support in skinStyles overridden by Minerva. Does this make sense?

I've submitted the above patch to make these a little more sensible.

Change 366892 merged by jenkins-bot:
[mediawiki/core@master] mediawiki.hlist: Add sensible defaults

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

What's remaining with this task?
Porting over remaining styles from MediaWiki:Common.css / Community-Relations-Support engagement that it's available??

Change 374421 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Enable mediawiki.hlist on mobile

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

Change 374421 merged by jenkins-bot:
[mediawiki/core@master] Enable mediawiki.hlist on mobile

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

Jdlrobson closed this task as Resolved.Aug 28 2017, 9:34 PM
Jdlrobson claimed this task.

This task is getting confusing so I've spun out T174399 to work out next steps.