Page MenuHomePhabricator

Related Article cards are not shown on Timeless skin
Open, Needs TriagePublic

Description

Related article cards are not shown with the Timeless skin, but with Vector and Minerva skins as RelatedArticles disables all skins by default and requires them to provide their own styles consistent with the skin's appearance.

To enable it the config should be updated and some styles should be provided.

Examples:

Changes

  • Add skinStyles styling for the ext.relatedArticles.readMore module in RelatedArticles
  • Config change
  • Feel free to add to the default whitelist in RelatedArticles extension as a follow up when this is done
$wgRelatedArticlesFooterWhitelistedSkins = [ 'minerva', 'vector', 'timeless' ];

Event Timeline

Change 393080 had a related patch set uploaded (by Jack Phoenix; owner: Jack Phoenix):
[mediawiki/extensions/RelatedArticles@master] [WIP] Use mw.util to get the element ID for skin-agnosticness

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

ashley added a subscriber: ashley.Nov 23 2017, 3:57 PM

So RelatedArticles appears to have a configuration variable named $wgRelatedArticlesFooterWhitelistedSkins and by default only minerva is whitelisted in the extension's extension.json. This explains why on the aforementioned German Wikivoyage page the related articles stuff doesn't show up if using Cologne Blue or Modern.

Furthermore I'm a bit suspicious about the code in /extensions/RelatedArticles/resources/ext.relatedArticles.readMore.bootstrap/index.js as it certainly looks somewhat skin-specific and making assumptions about class names and IDs. My aforementioned untested patch attempts to work around that by utilizing mw.util.$content appropriately.

In general I feel that whitelists are a Bad Thing™ and should be avoided.

Jdlrobson added a subscriber: Jdlrobson.

There's two things going on here:

  1. $wgRelatedArticlesFooterWhitelistedSkins needs to include 'timeless' to whitelist the skin for use with this feature.
  2. Timeless needs to provide some styling.

This should require no changes for RelatedArticles.
Using mw.util.$content seems like a good idea but isn't needed to fix this. All you need is:

$wgRelatedArticlesFooterWhitelistedSkins = [ 'minerva', 'vector', 'timeless' ];

The whitelist is intentionally done this way to allow skins to opt into this experience at their leisure as we provide minimal styling and skins should provide skinStyles.

The current experience is broken and will needs styling before enabling:

ashley moved this task from Backlog to Features on the Timeless board.Jun 24 2018, 5:34 PM
Isarra moved this task from Features to Compatibility on the Timeless board.Dec 5 2018, 10:28 PM
Isarra added a subscriber: Isarra.Wed, Jun 19, 10:38 PM

So if I am understanding this correctly, the issue here is that this extension doesn't know where to add its content in the dom because skins vary and it's doing so via js?

Why not just use the SkinAfterContent hook to insert an empty placeholder after the content with a known id, add the stuff to that? Like flagged revisions, I think.

(It looks like the broken styles in the example are because it got added to the wrong element there - those seem to be the content header styles, but it looks like this shouldn't even be in the content to begin with based on the linked example?)

...also why is it headers at all?

So it looks like the headers styling issue has since been fixed, regardless of why, and the reason why it's not using that hook is because Minerva doesn't support that hook and this extension is apparently primarily for Minerva, based on the default whitelist only including Minerva.

...Minerva should really support that hook. Filed T226143.

Change 518032 had a related patch set uploaded (by Isarra; owner: Isarra):
[mediawiki/extensions/RelatedArticles@master] Use SkinAfterContent hook to place cards in the DOM

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

All right, so experimenting with using the hook, it looks like it works pretty much as expected in most skins: Minerva (once support is added) is unchanged, Timeless, Modern, and other skins behave as expected, etc.

The main problems seem to be CologneBlue, MonoBook, and Vector:

  • CologneBlue just doesn't style the DataAfterContent properly in general: on short pages anything there typically winds up overlapping the sidebar (PendingChanges also has this issue). Filed T226201, but we probably don't particularly care for the sake of this regardless.
  • Vector and MonoBook work just fine, they're totally usable and it looks okay, it's just in a different place in them now due to how they place the DataAfterContent in their skin structure:

I'm going to go out on a limb and guess whoever was behind this design placed the related article cards outside the content block for a reason, however, and frankly I think I agree - as a designer myself, I've likewise moved the entire DataAfterContent block outside of the content block in quite a few skins because in terms of related content, it tends to just make more sense this way for what it is and the way extensions use it. We should consider doing this in MonoBook/Vector as well: T226199

Change 518100 had a related patch set uploaded (by Isarra; owner: Isarra):
[mediawiki/skins/MonoBook@master] Style RelatedArticle cards

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

Change 518102 had a related patch set uploaded (by Isarra; owner: Isarra):
[mediawiki/skins/Vector@master] Style RelatedArticle cards

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

Sorry, apparently this is a site request because apparently it's only enabled for some skins some places regardless. I misunderstood that.

Anyway, the patch pile here is thus:

At which point we should basically be exactly where we started in terms of what people actually see, but such that RelatedArticles should mostly just work with whatever if enabled. May also need to discuss/look into how this will affect pending changes and anything else using the same hook.

No patches needed to wmf-config aside from one to address this task specifically and, er, actually enable RelatedArticles for Timeless (and possibly other skins) on whichever project(s) were wanting it, probably WikiVoyage, based on the example.