Fix touch events for menu actions on mobile.

Review Request #14163 — Created Sept. 12, 2024 and submitted — Latest diff uploaded

Information

Review Board
release-7.x

Reviewers

Menu actions, such as the "Close" and "Update" review request actions,
were broken on mobile. A touch on an action item within the menu would
just close the menu instead of activating the action. This happened
because the touch handler for menu actions always just opened/closed the
menu regardless of whether touch was on an item or not.

This change fixes this, and updates a similar touch handler for the
unified banner. Previously, the unified banner touch handler would check
whether the touch was on a ink-c-menu__item-label. This was always
guaranteed to correctly determine whether the touch was on a menu item
because the label encompassed the entire element of the menu item.
However this could change in the future, and then we'd have a bug where
touches on menu items only work properly when the touch is exactly on
the label.

This change also alphabetically sorts a set of keys in the menu action
view JS file.

  • Tested opening/closing the "Close" and "Update" menus, and tapping
    on actions in them.
  • Tested opening/closing the unified banner's "Review" menu, and tapping
    on actions in it.

Diff Revision 2 (Latest)

orig
1
2

Commits

First Last Summary ID Author
Fix touch events for menu actions on mobile.
Menu actions, such as the "Close" and "Update" review request actions, were broken on mobile. A touch on an action item within the menu would just close the menu instead of activating the action. This happened because the touch handler for menu actions always just opened/closed the menu regardless of whether touch was on an item or not. This change fixes this, and updates a similar touch handler for the unified banner. Previously, the unified banner touch handler would check whether the touch was on a `ink-c-menu__item-label`. This was always guaranteed to correctly determine whether the touch was on a menu item because the label encompassed the entire element of the menu item. However this could change in the future, and then we'd have a bug where touches on menu items only work properly when the touch was exactly on the label.
f698bc473181c4fd0a56420f2e1c686e42d186ff Michelle Aubin
reviewboard/static/rb/js/common/actions/views/menuActionView.ts
reviewboard/static/rb/js/reviews/views/unifiedBannerView.ts
Loading...