Page MenuHomePhabricator

increase the width of navigation drawer
Closed, ResolvedPublic1 Story Points

Description

the navigation drawer doesn't take enough % width of the device screen.

  • the dismiss area - .shield should be narrower. not less than 50px though
  • for some languages, the menu items are longer so increasing this width will help too

The width should be 75% of device screen with a max width of 600px; the shield on right should be 25%
Notifications drawer not impacted.

zeplin: https://zpl.io/Z2cpOSU

Testing Criteria

View the navigation drawer in a number of browsers and note any inconsistencies

Details

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : masterIncrease navigation menu size for mobile devices.
mediawiki/extensions/MobileFrontend : masterAdjust the main menu width

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 1 2016, 6:18 AM
Nirzar triaged this task as Low priority.Dec 1 2016, 6:18 AM
jhobs set the point value for this task to 1.
jhobs moved this task from Incoming to Triaged but Future on the Readers-Web-Backlog board.
jhobs added a subscriber: jhobs.

This can probably even be Normal if you want @Nirzar.

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptDec 1 2016, 9:31 PM
Jdlrobson raised the priority of this task from Low to Normal.Apr 17 2017, 11:54 PM
Jdlrobson added a project: patch-welcome.
Jdlrobson removed Volker_E as the assignee of this task.May 3 2017, 3:39 PM
Jdlrobson added subscribers: Volker_E, Jdlrobson.

Doesn't look like Volker's work on this right now.

Jdlrobson updated the task description. (Show Details)Jun 14 2017, 5:42 PM

Change 359494 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/extensions/MobileFrontend@master] Adjust the main menu width

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

the mocks don't mention what should happen on tablet mode. @Nirzar and @Volker_E can you confirm this is what you want?:

Before:

After:

with a max width of 600px

@Volker_E / @Nirzar his also seems a bit arbitary. did you mean @deviceWidthTablet (720px) ?

Change 361013 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Increase navigation menu size for mobile devices.

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

^ @bmansurov so here's what I came up with when I had a go at this. What am I missing about this solution? Am I overlooking something?

@Jdlrobson, in your solution there are a couple of cases where the calculation doesn't satisfy the requirements. For example the width of the navigation drawer is set at 600px for screen widths of 720px and above. According to the requirements though, 600px should be set when the screen width is 800px and above. So for a tablet with a width of 720px, the navigation drawer width is taking up 83% of the space, and not 75%.

Similarly, for mobile devices (whose screen widths are less than 720px) the navigation drawer is taking up 70% of the screen width, while it should take 75% of the width according to the requirements. If you change this value to 75%, then for screen widths of 200px and less, the dismiss area will be less than 50px, contrary to what's being asked.

Having said that, I think it's an acceptable solution given that no JS is involved. So from the technical perspective I'd go with your solution, but from the perspective of satisfying the requirements exactly I'd go with the alternative solution.

I'll happily merge your patch if @Nirzar is fine with your approach.

Not sure where I got the 70% from. I've switched it to 75% but I think my solution meets the description as written.

According to the requirements though, 600px should be set when the screen width is 800px and above.

That's not quite what it says. It says "with a max width of 600px". It doesn't mention when this should kick in.

The only issue with my solution is that the shield width will be under 50px for browsers under 200px but that's easily fixed or overlooked - this should be very rare. I don't know how many phones try to cramp our site into that much space AND run JavaScript.

Screen widthNavigation drawer sizeShield width
1000.75 * 100 = 750.25 * 200 = 25
2000.75 * 200 = 1500.25 * 200 = 50
3200.75 * 320 = 2400.25 * 320 = 80
4000.75 * 400 = 3000.25 * 400 = 100
5000.75 * 500 = 3750.25 * 500 = 125
6000.75 * 600 = 4500.25 * 600 = 150
7000.75 * 700 = 5250.25 * 700 = 175
7190.75 * 719 = 5390.25 * 719 = 180
720600px120
800600px200
1000600px400

Why are we showing 600px drawer width for screen width 720px with your solution? Shouldn't it be 540px (= 0.75 * 720)?

Jdlrobson added a comment.EditedJun 23 2017, 7:31 PM

Why are we showing 600px drawer width for screen width 720px with your solution? Shouldn't it be 540px (= 0.75 * 720)?

Mathematically, yes, but as I pointed out earlier the number seems a little arbitrary and I'm not sure whether this is truly a big problem as it keeps the solution clean. If we want to keep it 75% all the way to 800px (75% of 800 is 600px) we can update the media query to kick in at 800px which feels a little messy but possible (We can rename @menuWidthTablet to @menuWidthMaximum) and make the media query @menuWidthMaximum / 0.75)

This needs input from @Nirzar and @Volker_E

This comment was removed by Jdlrobson.

a min....... thanks for holding it. is the current patch deployed anywhere?

doesn't have to be exact 75% the reading-web-staging looks great!!

Change 359494 abandoned by Bmansurov:
Adjust the main menu width

Reason:
See I41759ccf629e36f8a498e0ad9411a86d2cb93572

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

bmansurov removed bmansurov as the assignee of this task.Jun 26 2017, 8:44 PM
bmansurov added a subscriber: bmansurov.

Change 361013 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Increase navigation menu size for mobile devices.

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

ovasileva updated the task description. (Show Details)Jun 27 2017, 5:13 PM

This needs testing criteria/notes before we assign it to @ABorbaWMF.

Looks good on. Tested against beta. Tried it on a number of devices/browsers. On iPhone 4 it takes up most of the screen but functions properly.

Jdlrobson reassigned this task from ABorbaWMF to Nirzar.Jun 28 2017, 6:42 PM

please sign off!

On iPhone 4 it takes up most of the screen but functions properly.

Just for documentation: on iphone 4 and 5 (narrow width) iphones, the dismiss area is larger than 44px so it falls under guidelines.

otherwise

Macro votecat: looks  good

Nirzar closed this task as Resolved.Jun 28 2017, 7:55 PM