Page MenuHomePhabricator

Move hamburger code back to the top of the page
Closed, ResolvedPublic

Description

per explanation at T115132, almost 40% of hamburger opens are resulting in the html version of the menu page, which is an inferior page with no obvious navigation back for the user.

Until we identify the cause or redesign the html page, the code should be moved back to the top. Of these alternate paths, I propose they should be prioritized this way:

  1. move hamburger code back to the top
  2. identify the cause: this is only worthwhile if the speed gains in moving the code were significant
  3. redesign the html version: I would like to avoid this until the first option has been resolved.

Event Timeline

JKatzWMF raised the priority of this task from to Needs Triage.
JKatzWMF updated the task description. (Show Details)
JKatzWMF subscribed.
JKatzWMF added subscribers: Jdlrobson, KLans_WMF.

@Jdlrobson @KLans_WMF this task just dropped off the map. I don't know how. moving it to current sprint

Change 260345 had a related patch set uploaded (by Bmansurov):
Hamburger: wait until main menu has loaded

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

Copying my comment from the submitted patch: "This patch doesn't match any part of the associated task. I understand the reasoning for it, but I'm hesitant to +1 until it's been discussed on the task since disabling clicks (even if only during loading) is a really bad user experience."

That being said, I don't think it's necessarily a bad approach, but only if we do some kind of spike or research around how long this "unclickable" period is and if it's likely to affect users. One simple solution could be to log an event instead of just returning false each time a user clicks on the button too early. Then compare the number of early clicks to total (maybe unique) clicks.

Another potential solution could be to just put a minimal amount of code at the top to enable the sliding part and a spinner until the main menu has loaded. So user taps button -> drawer slides out with spinner -> main menu loads & spinner disappears.

Change 260746 had a related patch set uploaded (by Bmansurov):
Add main menu lite version to the top of the page

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

Change 260345 abandoned by Bmansurov:
Hamburger: wait until main menu has loaded

Reason:
https://gerrit.wikimedia.org/r/#/c/260746/

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

So I think we are overthinking this.
The motivation for this task was as a result of us moving mainMenu from top scripts to bottom scripts.
Adding position: top moves the main menu code to the back of the page and makes this go away.
Yes it does increase the weight of the JavaScript at the top of the page.

I think this is acceptable based on several things:

  • It should still load asynchronously when at the top of the page. It should not impact first paint.
  • We saw no big decrease in first paint when shifting it to the bottom. Impact of JS is minimal compared to HTML .
  • We're walking towards LOOT which will make it easier for us to move to the bottom in future if we want to
  • OOjs is tiny. Merging https://gerrit.wikimedia.org/r/237394 will impact top loaded js ever more (if anyone bothers to review that)

Thanks, Jon for the feedback. Your proposed solution doesn't solve the problem though. I've changed the position to top and throttled the net speed to 2G and after the page reload I immediately clicked on the hamburger button, instead of seeing the main menu I was taken to Special:MobileMenu. As you say, that's because ResourceLoader loads modules asynchronously now. So the question is, what's wrong with my approach, which loads lighter resources than your solution and solves the problem described in the description of the task.

Well immediately is not the fairest test :) and 2g is never going to be instant. It wasn't before and has no chance now

Moving to top should reduce the load time enough to help the majority of people.

Even though code is asynchronous if it's at the top it will start loading before the bottom so not sure what that's got to do with it.

In terms of your current solution if the js fails to load completely the menu becomes unusable and broken.

In terms of your current solution if the js fails to load completely the menu becomes unusable and broken.

Not really, the menu is usable without the mobile.mainMenu module. It lacks icons though, which will be loaded later on the page.

Also, the problem happens on a 3G connection too. I don't think we'll solve the problem by moving the module to the top even though it starts loading before the modules at the bottom.

Anyone else wants to join the discussion, @phuedx, @JKatzWMF?

@bmansurov can we provide some numbers here in terms of seconds difference by just moving to the top?

Realistically a user is not going to hit the hamburger menu straight away on the majority of page loads, but moving it to the bottom did obviously have an impact on clicks due to the drop in the graph.

Given this card came about due to us moving the code from top to bottom (not due to the async changes) I'm struggling to understand why that won't be sufficient to reverse that.

To be clear it doesn't have to be available instantly. We just need to load it quicker to avoid unnecessary visits to Special:MobileMenu
I suggest we do the simplest thing possible and then revisit if we still find many people are landing on that special page.

Change 261310 had a related patch set uploaded (by Bmansurov):
Load mobile.mainMenu at the top of the page

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

Change 260746 abandoned by Bmansurov:
Add main menu lite version to the top of the page

Reason:
https://gerrit.wikimedia.org/r/#/c/261310/ seems to be enough for our needs for now.

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

Change 263350 had a related patch set uploaded (by Bmansurov):
Load mobile.mainMenu at the top of the page

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

Change 263350 abandoned by Bmansurov:
Load mobile.mainMenu at the top of the page

Reason:
see the dependency instead

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

Jdlrobson added subscribers: Tbayer, bmansurov.

Change will be live on all wikis on 28th January.
We'll have to see if this reverses the trend on hamburger clicks seen since we moved it to the bottom on Oct 22
@Tbayer assigning to you in the hope you can check this (hopefully we'll start seeing positive impact in February) and sign off.

Change 261310 merged by jenkins-bot:
Load mobile.mainMenu at the top of the page

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

At our sprint prioritization and, if necessary, kickoff for sprint 65, let's figure out what level of self service we can do to verify the change is having the expected impact.

The work was done in sprint 64 so this task is theoretically completed. We should open a spike for the analysis.

The work was done in sprint 64 so this task is theoretically completed. We should open a spike for the analysis.

See T124675.