• Add server side render
  • Does it use MenuBuilder? Are three any changes to menubuilder needed?
  • How will we show / hide for specific pages? Is that covered by menubuilder entirely?
  • Is there any client side code needed?
• Add overflow button to toolbar
• Add styling
  • Do not worry about scrolling, my friend
• Add accessibility attributes

Question for Alex: what are the expectations for scrolling?

Outcome is to update the task, maybe prototype.

@alexhollender - discussed this during grooming and noted that we would like to add the scrolling behavior for the overflow menu to this task as well

https://adoring-clarke-7bb0cd.netlify.com/demo-new.html

Each item on the menu will be tracked in the same schema where main menu tracking occurs

T216152: AMC Navigation - add new links to main menu with click tracking currently contains a good explanation of wiring up items in the MainMenu to the MobileWebMainMenuClickTracking instrumentation:

Analytics

• MainMenuClickTracking schema ID exists in MobileFrontend. We should move that to Minerva as this is confusing but apart from that all code is in Minerva.
• All the code for communicating with EventLogging lives in Minerva in [resources/skins.minerva.scripts/menu/MainMenu.js](https://github.com/wikimedia/mediawiki-skins-MinervaNeue/blob/799f426f5b3601712fdce69a48b7b6fb1173c4cc/resources/skins.minerva.scripts/menu/MainMenu.js#L42-L53) and [resources/skins.minerva.scripts/menu/schema.js](https://github.com/wikimedia/mediawiki-skins-MinervaNeue/blob/799f426f5b3601712fdce69a48b7b6fb1173c4cc/resources/skins.minerva.scripts/menu/schema.js)
• 50% of clicks in the main menu are sampled (per wgMinervaSchemaMainMenuClickTrackingSampleRate)
• Every link inside the MainMenu will be logged provided it has a data-event-name attribute

https://adoring-clarke-7bb0cd.netlify.com/demo-new.html

To expand on that a little bit, I did some preliminary research on the front-end implementation of this feature. The parts of this feature that I focused on were: the HTML/CSS & accessibility.

The HTML in the above demo is essentially a nested list:

<ul class="page-actions">
  <li>languages</li>
  <li>watchstar</li> 
  <li>edit</li> 
  <li> 
    <button>overflow menu icon</button>
    <ul>
      <li>first overflow menu item</li>
      <li>second overflow menu item</li>
    </ul>
  </li>
</ul>

I think this HTML structure is beneficial because it allows us to achieve the show/hide behaviour with CSS and maintain semantically correct DOM order.

Placing the overflow menu immediately after the overflow icon/button allows us to use the adjacent sibling selector the change the visibility of the menu, based on the focus state of the button (similar to the "checkbox hack"):

.overflow-menu { display: none; }
.overflow-button:focus + .overflow-menu { display: block; }

As far as I can tell, this seems to work on mobile devices well. The item is initially hidden, then when clicking the button it’s made visible, and remains visible while scrolling, and then when clicking elsewhere on the screen, it’s made hidden again.

The area where this CSS approach falls down though, is accessibility. Since screen-reader/keyboard-users depend on focus-state to navigate, so hiding the menu when the button looses focus makes it impossible to navigate via keyboard. To rectify this, javascript is indeed required to keep the menu open (and maybe on a blur event, close the menu?).

Javascript is also required to improve screen-reader accessibility. The aria-expanded attribute should be placed on the button to announce the state of the menu (expanded/collapsed).

So in terms of HTML/CSS & a11y, those are pretty much my findings from that prototype.

Change 501418 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/MinervaNeue@master] WIP: add secondary page actions menu in AMC mode

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

@alexhollender, still working through this but the following would help:

• Please provide the assets wanted.
• Do we need a vertical scrollbar in the menu itself? I explored this briefly and I have an idea for how we could do it but it's not a great experience. Basically, I set the maximum height to the viewport height and if the menu exceeds it, it adds a scroll bar. It's not a great experience because the maximum height is not adjusted with the scroll offset. That is, the menu initially fits on screen but once you start scrolling, it no longer can fit, which feels really inconsistent. I think it's fine to let this overflow without scrollbar. I can show you what I mean if it's helpful.
• Where does the download link go?
  1. Is it the same link shown on the production mobile site's toolbar? If so, this can only be shown for the JavaScript render, not the server render.
  2. Or is the Vector download PDF link (E.g., https://en.wikipedia.org/w/index.php?title=Special:ElectronPdf&page=Dog&action=show-download-screen)?
  3. The Vector print link (E.g., https://en.wikipedia.org/w/index.php?title=Dog&printable=yes)?
  4. Or something else?
• Where does the uploads link go? https://en.wikipedia.org/wiki/Special:Log?type=upload&user=Jdlrobson&page=&wpdate=&tagfilter=&subtype=?
• I'm not planning to explore "in other projects" for this task since it will require new UI.

Per our discussion:

Please provide the assets wanted.

Alex to provide. Please also include the Wikibase, citation, and user page icons if possible.

Do we need a vertical scrollbar in the menu itself?

Alex is still experimenting.

Where does the download link go?

The existing download button moves from the toolbar into the overflow menu. However, given that there is a visible delay between when the page is initially rendered on device and when the download button appears, progressively enhancing the desktop download link seems preferable. That is, rendering the desktop's "Download as PDF" link on the server and replacing this with the client download link when supported. Unfortunately, this data appears to come from the ElectronPdfService and Collections extensions and the API isn't very clean. I was kind of hoping this would be a drop-in but I don't think it is. @pmiazga, is this something we should use?

There's another link on desktop, "Printable version", that also sounded like a promising backup. It's easy to access in the code too, $tpl->data['nav_urls']['print']. This basically adds a &printable=yes parameter to the URL like https://en.wikipedia.org/w/index.php?title=Dog&printable=yes. Unfortunately, the presentation on mobile is identical because it doesn't magically kick change the CSS media type to print rendering.

@alexhollender, ...So I'm leaning more towards zapping the download button into place like we currently do even though the experience isn't great :[

Where does the uploads link go?

This is in now: https://en.m.wikipedia.org/wiki/Special:Uploads/Jdlrobson

Alex's demo here: https://mobile-contributions.firebaseapp.com/nav4.html
Current patch here (be sure to enable AMC):

• https://readers-web-stephen.wmflabs.org/wiki/Foobarbaz
• https://readers-web-stephen.wmflabs.org/wiki/User:Stephen

Following up on the remaining open topics:

1. Menu scrolling behavior

Regarding when there is not enough vertical space to display the entire menu, either because the phone is in landscape mode, or has a shorter screen.

For JS users we will do the following:

set a max-height on the menu, taking into account the offset from top of screen	allow the menu to scroll internally	set a min-height of 220px so that the menu never gets too short

For non-JS users we will do the following:

Not set a max-height	defer to scrolling the page to access the additional menu items

2. Menu item icons

I've updated the task description to indicate which OOUI icon to use for each menu item

@Niedzielski the Minerva print button is handled uniquely. First, after injecting button, it logs shownPrintButton (printing is not always available, there is no need to inject the button if you cannot print). Then when clicked - it doesn't add printable=yes to the URL, what it does, after it's clicked first it logs the click, then tries to load images on the page (the one that is lazyloaded), then calls window.print().

The Menu instrumentation will not be able to track that stuff as it is too complicated. What we can do is to track when a user clicks a menu item, nothing else. In this case, you'll need to add an entry that is not visible by default, then bind all actions on the frontend, and make it visible. MenuElement already knows how to handle those elements - when MenuElement is created with isJSOnly - the menu element will have jsonly class, which makes it hidden on non-js clients.

IMHO some menu entries should be added/shipped only with JS module, that makes those menu elements interactive (deciding whether to show the button, tracking button actions). Same will happen with the Share button, it's not available on all browsers, also it has special instrumentation that cannot be handled by the instrumentation built into the Menu system.

Change 501418 abandoned by Niedzielski:
WIP: add secondary page actions submenu in AMC mode

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

Change 501418 restored by Niedzielski:
WIP: add secondary page actions submenu in AMC mode

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

I've revised the images and scrolling behavior. The changes are most visible on user pages. E.g., https://readers-web-stephen.wmflabs.org/wiki/User:Hello.

All feedback is addressed. Latest changes are up https://readers-web-stephen.wmflabs.org/wiki/User:Hello. Open questions on download icon behavior and toolbar layout to be discussed tomorrow.

Change 503092 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Fix: don't overwrite additional icon classes

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

All feedback is addressed. Latest changes are up https://readers-web-stephen.wmflabs.org/wiki/User:Hello.

In T216418#5098537, @alexhollender wrote:

Following up on the remaining open topics:

1. Menu scrolling behavior

Regarding when there is not enough vertical space to display the entire menu, either because the phone is in landscape mode, or has a shorter screen.

For JS users we will do the following:

set a max-height on the menu, taking into account the offset from top of screen	allow the menu to scroll internally	set a min-height of 220px so that the menu never gets too short

For non-JS users we will do the following:

Not set a max-height	defer to scrolling the page to access the additional menu items

2. Menu item icons

I've updated the task description to indicate which OOUI icon to use for each menu item

@alexhollender Regarding the "set min-height of 220px so that the menu never gets too short" spec, can you please clarify what we should do if the height of the menu fits within the viewport but is less than 220px? Should we still apply the min-height: 220px CSS rule or let the menu fit to its contents in that scenario? I ask this because if we apply the CSS rule in this scenario, there will can be white space at the bottom of the menu as pictured below; is that okay?

Change 503092 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Fix: don't overwrite additional icon classes

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

@nray well that height was set based on the assumption of the minimum number of items that would appear in that menu. Do you know if the case in the screenshot you posted above is a real one, or something that would only occur on staging. I also notice that there is no history icon in that screenshot, which seems peculiar.

In T216418#5125379, @alexhollender wrote:

@nray well that height was set based on the assumption of the minimum number of items that would appear in that menu. Do you know if the case in the screenshot you posted above is a real one, or something that would only occur on staging. I also notice that there is no history icon in that screenshot, which seems peculiar.

Sorry, yeah Stephen filled me in on this info after I posted this question. This screenshot was of my local machine where I try to keep the extensions down to a minimum so production would have more menu items.

Change 501418 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Update: add secondary page actions submenu in AMC mode

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

When testing I noticed couple things:

when AMC mode is on, a user signed up for the AMC experience, and the MinervaOverflowInPageActions is set to false, icons get organized in a different way:

As a non-amc user I see:

User Pages are bit messy. It would be awesome to clean it up:

User does not exist view:

User exists, but doesn't have a user page:

User exists, has a user page:

When download icon gets injected, FOUC is a bit more visible now

There is something we should reconsider (it's not possible to avoid), the FOUC when we inject download icon is a bit more visible now. There is a script that injects a new Icon if the print feature is available, that has to be done on the JS side. Previously the icon was just added before the "Watch", it didn't cause any reflow - a new icon jumped into empty space. Now when the AMC mode is on, all icons are nicely spaced (same distance between each icon), when we add a new icon (Download button) only far left and far right icon stay in the same place, all icons in the middle have to "readjust" and that is a bit more visible.

The feature is available to test on https://reading-web-staging.wmflabs.org/

@pmiazga thanks for the feedback. I think it makes sense for @Niedzielski to respond to the first few points because I don't know which of those cases are unique to the staging environment, and which might actually occur in production.

User does not exist view

I would expect you to see something like what renders here: https://en.m.wikipedia.org/wiki/User:Asdas2382138asdasd

User exists, but doesn't have a user page:

if you are pointing out that there is no toolbar in this case, see T220114

When download icon gets injected, FOUC is a bit more visible now

We decided to put the download icon at the bottom of the menu, I'm not sure if that change was live yet when you tested.

@Niedzielski I've only been able to find one bug thus far. Sometimes when navigating to the Permanent link page from the overflow menu I see a flash of the menu on the Permanent link page. I can't reproduce this consistently, and I have not yet seen it on iPhone/Safari (only desktop & Android/Chrome).

video of menu flash	screenshot from 0:03 seconds
menu flash.mov1 MBDownload

Regarding the discussion/talk page - would it be possible to include the History page icon/action in the toolbar? Perhaps I should make a subtask for T213352 for that. Apologies for not catching this earlier.

Looks like this is in design review?

Change 506526 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/MinervaNeue@master] Fix: AMC overflow menu initial CSS and SSR icons

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

I've only been able to find one bug thus far. Sometimes when navigating to the Permanent link page from the overflow menu I see a flash of the menu on the Permanent link page. I can't reproduce this consistently, and I have not yet seen it on iPhone/Safari (only desktop & Android/Chrome).

Thank you for spotting this bug. This was a flaw (actually two) in my original code.

Regarding the discussion/talk page - would it be possible to include the History page icon/action in the toolbar? Perhaps I should make a subtask for T213352 for that. Apologies for not catching this earlier.

I think a new task would be best. Work has already started to transition the overflow menu to Piotr's new menu system and I don't want to make more changes that will need to be migrated or wait until that work is done to resolve this task.

In T216418#5129152, @alexhollender wrote:

@Niedzielski I've only been able to find one bug thus far. Sometimes when navigating to the Permanent link page from the overflow menu I see a flash of the menu on the Permanent link page. I can't reproduce this consistently, and I have not yet seen it on iPhone/Safari (only desktop & Android/Chrome).

video of menu flash	screenshot from 0:03 seconds
menu flash.mov1 MBDownload

Regarding the discussion/talk page - would it be possible to include the History page icon/action in the toolbar? Perhaps I should make a subtask for T213352 for that. Apologies for not catching this earlier.

@alexhollender regarding the menu flash bug, what chrome version are you using (desktop) just out of curiousity? No matter how hard I try, I cannot reproduce the issue you are seeing, but I did notice this 5 year old chrome bug ticket here (still active!) that seems related to the issue https://bugs.chromium.org/p/chromium/issues/detail?id=332189

^^ fyi, I'm on chrome Version 74.0.3729.108

@nray Version 74.0.3729.108 (Official Build) (64-bit)

Update: I've successfully replicated the flash that alex was seeing on my android phone (Pixel 2)! Ironically, it happened for me on the page "Nothing to Hide" (http://readers-web-staging.wmflabs.org/w/index.php?title=Nothing_to_Hide_(book)&mobileaction=toggle_view_mobile)

Change 506526 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Fix: AMC overflow menu initial CSS and SSR icons

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

Needs QA steps and staging changes!

@Edtadros - just wanted to note that in addition to beta we should also QA this in production once it's out