flake8
passed.
JSHint
passed.
Fix touch events for menu actions on mobile.
Review Request #14163 — Created Sept. 12, 2024 and submitted
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 aink-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.
| Summary | ID |
|---|---|
| f698bc473181c4fd0a56420f2e1c686e42d186ff |
| Description | From | Last Updated |
|---|---|---|
|
Hmm, this is cute, but I worry that it's not super robust. For example, a custom menu item might add … |
 david
|
-
-
Hmm, this is cute, but I worry that it's not super robust. For example, a custom menu item might add links or other elements that don't have a classname that starts with this.
Can we instead do something like this?
const $target = $(e.target); if (!($target.hasClass('ink-c-menu__item') || $target.parents('.ink-c-menu__item')) { ... } $(e.target).parents('.ink-c-menu__item')
