Fix up mobile interaction for menu actions.

Review Request #13061 — Created May 23, 2023 and submitted

Information

Review Board
release-6.x

Reviewers

Interacting with actions located in menus was broken in a few different
ways on mobile. Some of these problems were baked into the menu
implementation from the very beginning, and some are a bit more recent.

To fix these issues, this change does several things:

  • Hidden menus were being set to have visibility: hidden and
    opacity: 0, but this still can allow them to be interacted with.
    When in mobile mode, various action menus would overlap, so a click on
    something in the "Review" menu was accidentally activating things in
    the "Close" menu.
  • Interaction with a lot of the menus relied entirely on the :focus
    and :hover states. This kind of worked sometimes with the way that
    mobile browsers deal with touch events, but it wasn't reliable, and
    barely worked at all with mobile mode in desktop browser devtools.
    I've added explicit touch event handlers to improve the interaction.
  • The "Review" menu in particular is very modal, and takes up a lot of
    the screen when working on mobile. I've added an event overlay over
    the rest of the page so that taps outside of the menu area will
    dismiss the menu.
  • Exercised all of the menus and actions using both mobile and desktop
    modes.
  • Ran python tests
  • Ran js tests
Summary ID
Fix up mobile interaction for menu actions.
Interacting with actions located in menus was broken in a few different ways on mobile. Some of these problems were baked into the menu implementation from the very beginning, and some are a bit more recent. To fix these issues, this change does several things: - Hidden menus were being set to have `visibility: hidden` and `opacity: 0`, but this still can allow them to be interacted with. When in mobile mode, various action menus would overlap, so a click on something in the "Review" menu was accidentally activating things in the "Close" menu. - Interaction with a lot of the menus relied entirely on the `:focus` and `:hover` states. This kind of worked sometimes with the way that mobile browsers deal with touch events, but it wasn't reliable, and barely worked at all with mobile mode in desktop browser devtools. I've added explicit touch event handlers to improve the interaction. - The "Review" menu in particular is very modal, and takes up a lot of the screen when working on mobile. I've added an event overlay over the rest of the page so that taps outside of the menu area will dismiss the menu. Testing Done: - Exercised all of the menus and actions using both mobile and desktop modes. - Ran python tests - Ran js tests
fdf990c09cd60eaf93d767bcf944b402e54d2e7f
Description From Last Updated

We don't take in e.

chipx86chipx86

Should use our standard multi-line comment style.

chipx86chipx86

Missing a VersionAdded.

chipx86chipx86

Missing a VersionAdded.

chipx86chipx86

Can we call this _onClick, like other event handlers?

chipx86chipx86
maubin
  1. Ship It!
  2. 
      
chipx86
  1. Are the event handler moves part of this change, or another? They feel like a different body of work.

    1. You mean moving things into their own action view subclasses? Part of this work, since they need the special touch event handler.

  2. We don't take in e.

  3. Should use our standard multi-line comment style.

  4. Missing a VersionAdded.

  5. Missing a VersionAdded.

  6. Can we call this _onClick, like other event handlers?

  7. 
      
david
maubin
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (1a73d94)
Loading...