Page MenuHomePhabricator

Catch up with minerva header changes (id & star)
Closed, ResolvedPublic

Description

I7e989a3d4553eb3357598a5cad3ccebf51dc9fae (for T212216) changed the order of id and page action HTML in minerva's header. This breaks what we did during T211600.

Looking at the changes the mentioned ticket envisions (see screenshots) we should be careful how/if we reuse the "default header" as implementing a delta to the default behavior is ill fated without a good understanding of possible states, influencing factors.

The change is in master, is visible on beta and reproducible locally.

There was another change apparently, now (2019-03-01) the actions overlap the id

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 12 2019, 12:04 PM

Change 490038 had a related patch set uploaded (by Jakob; owner: Jakob):
[mediawiki/extensions/Wikibase@master] Catch up with minerva header changes (id & star)

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

Change 490038 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Catch up with minerva header changes (id & star)

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

Pablo-WMDE updated the task description. (Show Details)Mar 1 2019, 2:11 PM
Pablo-WMDE added a subscriber: Tarrow.

Apologies for the breaking change. Didn't realize Wikidata made such significant header modifications.
The changes were made to give more flexibility to the page-actions menu (and potentially the header as well) and fix some of the unintuitive HTML structure.

The changes consisted of the following:

  1. Remove the absolute positioning of the page-actions menu. https://github.com/wikimedia/mediawiki-skins-MinervaNeue/commit/95b139d7569bc328bb0a34adde34e653ec06c604
  2. Add a .page-heading element to combine the page heading and tagline (this change looks similar to what the .wikibase-title element does, maybe that element could be replaced now in favour of modifying the .page-heading element?). https://github.com/wikimedia/mediawiki-skins-MinervaNeue/commit/f92b1171589527ad7d8fd94f78145291cff2d1b1
  3. Refactor the page-actions menu markup. This change changes the HTML of the page-actions menu itself, improving a11y and layout flexibility https://github.com/wikimedia/mediawiki-skins-MinervaNeue/commit/864a1766a7e547c4a75f64ee4fe730b47fb885d9

Please let me know if you have any questions about these changes.

@Jdrewniak Thanks for getting back to us about this, and for the impressive work you put into moving this to a more 2019-ish implementation.
Substantial renovation of legacy code should indeed not be blocked by (questionable) extensions built on top of it - sure way to stall innovation. The tectonic shifts just took us by surprise because we had word to the contrary - but all is well.
I have a question regarding the "page-actions menu markup" - this introduces indistinguishable lis (only the links one step down know the "role") and even
hard-wires their order. Should the need come up to specify a different order, something that could be conveniently done on the style level nowadays, we'd be lacking the selectors to implement this. Would it not be beneficial to allow action identification already on the list item level?
Of course, this is not a completely hypothetical question. For wikibase termbox we are moving the "edit pen" into the individual sections (there is no "page edit") and the hard wiring of order causes us trouble (T215494#5040282 ff.) with the "download action" eventually inconsistently positioned to the right of the "watchlist star" (the only other action). Of course we would also be happy about any other solution suggestion.

Thanks for your time!

@Pablo-WMDE Good point! Adding ID's on the li level seems useful and cleans up that :nth-child in the jQuery, and probably make the CSS more resilient too. This shouldn't be too difficult to add, I'll try to get something out soon.

Change 498821 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Add IDs to page-action menu items.

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

Change 498821 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Add IDs to page-action menu items.

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

@Jdrewniak Thanks for your swift action in https://gerrit.wikimedia.org/r/c/mediawiki/skins/MinervaNeue/+/498821 - we are now looking into controling the consistent positioning of e.g. #page-actions-watch via these ids.
One thing that struck me was that there are JS-based (legacy/prototypical?) ways of actions being added (e.g. downloadPageAction) that do not use an offical minerva API to achieve this but write straight to the DOM and consequently do not profit from this change (i.e. still miss the id ATM). Are there plans to standardize such behavior?

Again, thank you very much!

@Pablo-WMDE thanks for your interest here. As you probably guessed, the reason downloadPageAction and others do not use an official Minerva API, is because there is none!

We are considering this as a feature, but we haven't prioritized or written it up yet. We're in the progress of adding further modifications to the page-actions menu (adding an addition "overflow" menu T216418 ) and I think whatever public API we create should take this additional menu into account, so I don't think we'll get around to it until that menu is done.

That being said, we're only aware of one external consumer who would use such an API (the MobileMaps gadget) would Wikidata also use this feature?

Hello @Jdrewniak,

thanks for getting back to us. As far as I am aware wikidata would hardly ever (disclaimer: not a product/UX person) have a scenario for adding a button but what happens is that we made the .page-heading more compact and moved all the actions to one side. Also there is no concept of a whole page edit in wikidata so there is no global edit action (only happens on sections). Have a look: https://wikidata.beta.wmflabs.org/w/index.php?title=Item:Q11&useformat=mobile
I mentioned the API idea because of the way the downloadPageAction is implemented (forcing itself onto a certain position of the list of action siblings) and centralizing the insertion logic seemed like a reasonable answer - but arguably properly modeling this would also be quite a challenge but with limited gain.

Cheers

Pablo-WMDE closed this task as Resolved.Apr 17 2019, 1:16 PM