Markus Jaritz [:designakt] (UX) Reporter
	Description • 8 years ago

More and more Windows devices provide a touch screen. However, our line-height in lists is rather narrow to easily touch the option one wants. This is the case for context menus (e.g. opening a link in a new tab) or on panels such as bookmarks or history.

Increasing the line-height on touch devices by 20% should make them easier to touch.

	:Gijs (he/him)
	Comment 1 • 8 years ago

I'm fairly sure win10 does this only when you access such (context) menus through the use of touch, not all the time. I imagine this is because it'd be actively annoying when you're using the mouse, because you'd have to mouse longer distances. Also, just generally when navigating long menus on small screen devices such as netbooks this would be problematic, because the menu takes up a lot more space.

	Marco Mucci [:MarcoM]
	Comment 2 • 7 years ago

Hi Dao, you removed this bug from blocking the Firefox Visual Refresh meta.  Want to confirm if this bug is still part of the Photon project?  Thanks.

	Dão Gottwald [:dao]
	Comment 3 • 7 years ago

(In reply to Marco Mucci [:MarcoM] from comment #2)
> Hi Dao, you removed this bug from blocking the Firefox Visual Refresh meta. 
> Want to confirm if this bug is still part of the Photon project?  Thanks.

It's still part of it, just moved further down in the dependency tree via bug 1352356.

	Johann Hofmann [:johannh] Assignee
	Comment 5 • 7 years ago

Comment on attachment 8864464 [details]
Bug 1256754 - Add padding to contextmenu items when opened through touch.

After a lot of back and forth I ended up with this rather simple solution. A couple of things I'm not sure about:

- This only increases the size of the page content context menu. I wasn't really able to find a hook into opening the contextmenu from chrome
- I realize that this CSS selector technically isn't very performant, but I'm not sure if there's a better way to do it.
- We might want to increase the size of menu items in other popups as well, though I can we can make a followup bug for that.

	Johann Hofmann [:johannh] Assignee
	Comment 6 • 7 years ago

We decided to just spin off separate bugs for chrome context menu and toolbar menu items.

	Dão Gottwald [:dao]
	Comment 7 • 7 years ago
mozreview-review

Comment on attachment 8864464 [details]
Bug 1256754 - Add padding to contextmenu items when opened through touch.

https://reviewboard.mozilla.org/r/136152/#review139202

::: browser/themes/windows/browser.css:2038
(Diff revision 1)
>  }
>  
>  %include ../shared/contextmenu.inc.css
>  
> +#contentAreaContextMenu[touchmode] menu,
> +#contentAreaContextMenu[touchmode] menuitem {

Are you intentionally targetting the back, forward, reload, stop and bookmark items here? I believe these are already touch-friendly without extra padding.

	Dão Gottwald [:dao]
	Comment 8 • 7 years ago
mozreview-review

Comment on attachment 8864464 [details]
Bug 1256754 - Add padding to contextmenu items when opened through touch.

https://reviewboard.mozilla.org/r/136152/#review139272

	Johann Hofmann [:johannh] Assignee
	Comment 9 • 7 years ago

Bryan, do you have spec for how large these context menu items should be? I currently set it to 8px vertical padding but that was just by intuition.

(In reply to Dão Gottwald [::dao] from comment #7)
> Are you intentionally targetting the back, forward, reload, stop and
> bookmark items here? I believe these are already touch-friendly without
> extra padding.

I'm forwarding this question to Bryan :)

	Johann Hofmann [:johannh] Assignee
	Comment 11 • 7 years ago

After talking to Shorlander this is what we came up with. Makes the total sizing be around ~44px, which is the recommended size for windows touch context menu items apparently.

	Dão Gottwald [:dao]
	Comment 12 • 7 years ago
mozreview-review

Comment on attachment 8864464 [details]
Bug 1256754 - Add padding to contextmenu items when opened through touch.

https://reviewboard.mozilla.org/r/136152/#review140262

::: browser/themes/windows/browser.css:2023
(Diff revision 2)
> +#contentAreaContextMenu[touchmode] menuitem:not(.menuitem-iconic) {
> +  padding-top: 12px;
> +  padding-bottom: 12px;
> +}
> +
> +#contentAreaContextMenu[touchmode] .menuitem-iconic {

.menuitem-icon is used more broadly and therefore not right for this selector. Please use #context-navigation > menuitem instead.

You can remove :not(.menuitem-iconic) from the other rule's selector.

	Dão Gottwald [:dao]
	Comment 14 • 7 years ago
mozreview-review

Comment on attachment 8864464 [details]
Bug 1256754 - Add padding to contextmenu items when opened through touch.

https://reviewboard.mozilla.org/r/136152/#review140268

::: browser/themes/windows/browser.css:2024
(Diff revision 3)
> +#contentAreaContextMenu[touchmode] menuitem {
> +  padding-top: 12px;
> +  padding-bottom: 12px;
> +}
> +
> +#contentAreaContextMenu[touchmode] #context-navigation > menuitem {

Please use the child selector:
#contentAreaContextMenu[touchmode] > #context-navigation > menuitem

	Pulsebot
	Comment 16 • 7 years ago

Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d19f48b0f205
Add padding to contextmenu items when opened through touch. r=dao

	Carsten Book [:Tomcat]
	Comment 17 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/d19f48b0f205

	Will
	Comment 18 • 7 years ago

Thank you so much for doing this! I use Firefox on a SP4 everyday and you might not realize how much this has improved things! Please do the same to URL bar suggestions! URL bar suggestions are too close together for touch screen use and I frequently tap the wrong suggestion :(

	ovidiu boca[:Ovidiu]
	Comment 19 • 7 years ago

Tested this issue with FF Nightly 55.0a1(2017-06-08) on Windows 8.1 x64 with touch screen and on Microsoft Surface Pro 2 with Windows 10 Preview build and I can confirm the fix. I will mark this as a verified fix.

	Will
	Comment 20 • 7 years ago

I thought I should post this here: when you long press on items in the Library window (AKA bookmark manager/history manager) , the menu items aren't spaced out like they are elsewhere

	:Gijs (he/him)
	Comment 21 • 7 years ago

(In reply to Will from comment #20)
> I thought I should post this here: when you long press on items in the
> Library window (AKA bookmark manager/history manager) , the menu items
> aren't spaced out like they are elsewhere

I filed bug 1428909 for this.