| • Nirzar | |
| Dec 1 2016, 6:18 AM |
| F8549048: Simulator Screen Shot Jun 28, 2017, 12.54.28 PM.png | |
| Jun 28 2017, 7:55 PM |
| F8503765: Screen Shot 2017-06-21 at 3.50.17 PM.png | |
| Jun 21 2017, 10:52 PM |
| F8503768: Screen Shot 2017-06-21 at 3.51.37 PM.png | |
| Jun 21 2017, 10:52 PM |
the navigation drawer doesn't take enough % width of the device screen.
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
View the navigation drawer in a number of browsers and note any inconsistencies
Change 359494 had a related patch set uploaded (by Bmansurov; owner: Bmansurov):
[mediawiki/extensions/MobileFrontend@master] Adjust the main menu width
Change 361013 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Increase navigation menu size for mobile devices.
^ @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 width | Navigation drawer size | Shield width |
| 100 | 0.75 * 100 = 75 | 0.25 * 200 = 25 |
| 200 | 0.75 * 200 = 150 | 0.25 * 200 = 50 |
| 320 | 0.75 * 320 = 240 | 0.25 * 320 = 80 |
| 400 | 0.75 * 400 = 300 | 0.25 * 400 = 100 |
| 500 | 0.75 * 500 = 375 | 0.25 * 500 = 125 |
| 600 | 0.75 * 600 = 450 | 0.25 * 600 = 150 |
| 700 | 0.75 * 700 = 525 | 0.25 * 700 = 175 |
| 719 | 0.75 * 719 = 539 | 0.25 * 719 = 180 |
| 720 | 600px | 120 |
| 800 | 600px | 200 |
| 1000 | 600px | 400 |
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)
Change 359494 abandoned by Bmansurov:
Adjust the main menu width
Reason:
See I41759ccf629e36f8a498e0ad9411a86d2cb93572
Change 361013 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Increase navigation menu size for mobile devices.
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.
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
