• 
      

    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. Show all issues

      We don't take in e.

    3. Show all issues

      Should use our standard multi-line comment style.

    4. Show all issues

      Missing a VersionAdded.

    5. Show all issues

      Missing a VersionAdded.

    6. Show all issues

      Can we call this _onClick, like other event handlers?

    7. 
        
    david
    maubin
    1. Ship It!
    2. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-6.x (1a73d94)