Closed Bug 1256754 Opened 4 years ago Closed 3 years ago

Increase context menu items line-height when accessed through touch

Categories

(Firefox :: Theme, defect, P1)

Unspecified
Windows
defect

Tracking

()

VERIFIED FIXED
Firefox 55
Iteration:
55.5 - May 15
Tracking Status
firefox55 --- verified

People

(Reporter: designakt, Assigned: johannh)

References

Details

(Whiteboard: [photon-visual][p2])

Attachments

(2 files)

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.
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.
Blocks: 1158152
Component: General → Theme
See Also: → 1167299
Summary: Increase menu items line-height on touch enabled devices → Increase menu items line-height when accessed through touch
Whiteboard: [photon]
No longer blocks: photon-visual
Priority: -- → P2
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.
Flags: needinfo?(dao+bmo)
(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.
Flags: needinfo?(dao+bmo)
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
Whiteboard: [photon] → [photon-visual]
Depends on: 1344907
Flags: qe-verify+
QA Contact: ovidiu.boca
Whiteboard: [photon-visual] → [photon-visual][p2]
Priority: P2 → P1
Iteration: --- → 55.4 - May 1
QA Contact: ovidiu.boca → brindusa.tot
Assignee: nhnt11 → jhofmann
Iteration: 55.4 - May 1 → 55.5 - May 15
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.
Attachment #8864464 - Flags: feedback?(dao+bmo)
Attachment #8864464 - Flags: feedback?(dao+bmo) → feedback+
We decided to just spin off separate bugs for chrome context menu and toolbar menu items.
Summary: Increase menu items line-height when accessed through touch → Increase context menu items line-height when accessed through touch
Attachment #8864464 - Flags: review?(dao+bmo)
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.
Comment on attachment 8864464 [details]
Bug 1256754 - Add padding to contextmenu items when opened through touch.

https://reviewboard.mozilla.org/r/136152/#review139272
Attachment #8864464 - Flags: review?(dao+bmo)
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 :)
Flags: needinfo?(bbell)
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.
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.
Attachment #8864464 - Flags: review?(dao+bmo) → review-
Flags: needinfo?(bbell)
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
Attachment #8864464 - Flags: review?(dao+bmo) → review+
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d19f48b0f205
Add padding to contextmenu items when opened through touch. r=dao
https://hg.mozilla.org/mozilla-central/rev/d19f48b0f205
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
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 :(
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
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
(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.
You need to log in before you can comment on or make changes to this bug.