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.

Changes between revision 1 and 2

orig
1
2

Commits

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.
b4b9e8502c4da8808976887c495a3eb12743766c Michelle Aubin
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...