From 78cd9ae4a90bc29132ceeeb792a14a5fc0162813 Mon Sep 17 00:00:00 2001 From: Elliott Marquez Date: Thu, 5 Oct 2023 22:17:29 -0700 Subject: [PATCH] fix(menu): fix menu tapping behaviors on iOS and do not close on anchor click fixes #5036 fixes #5015 iOS has some really bad behaviors when it comes to clicking and focus. clicking on the body on a non-clickable item (like anything outside a menu) does not tirgger a click event that bubbles to menu. Also, clicking on a button does not cause it to be focused, thus depriving us of information on focusout's related target. Therefore we have to assume the anchor is the trigger. PiperOrigin-RevId: 571227552 --- menu/internal/menu.ts | 75 ++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/menu/internal/menu.ts b/menu/internal/menu.ts index 2390fba22a9..c9617938286 100644 --- a/menu/internal/menu.ts +++ b/menu/internal/menu.ts @@ -209,7 +209,6 @@ export abstract class Menu extends LitElement { * The event path of the last window pointerdown event. */ private pointerPath: EventTarget[] = []; - private isPointerDown = false; private readonly openCloseAnimationSignal = createAnimationSignal(); private readonly listController = new ListController({ @@ -303,7 +302,7 @@ export abstract class Menu extends LitElement { super(); if (!isServer) { this.internals.role = 'menu'; - this.addEventListener('keydown', this.listController.handleKeydown); + this.addEventListener('keydown', this.handleKeydown); // Capture so that we can grab the event before it reaches the menu item // istelf. Specifically useful for the case where typeahead encounters a // space and we don't want the menu item to close the menu. @@ -358,6 +357,22 @@ export abstract class Menu extends LitElement { this.setAttribute('aria-hidden', 'true'); } + override update(changed: PropertyValues) { + if (changed.has('open')) { + if (this.open) { + window.addEventListener( + 'pointerup', this.onWindowPointerup, {capture: true}); + window.addEventListener('pointerdown', this.onWindowPointerdown); + } else { + window.removeEventListener( + 'pointerup', this.onWindowPointerup, {capture: true}); + window.removeEventListener('pointerdown', this.onWindowPointerdown); + } + } + + super.update(changed); + } + protected override render() { return this.renderSurface(); } @@ -411,25 +426,23 @@ export abstract class Menu extends LitElement { } private readonly handleFocusout = async (event: FocusEvent) => { - if (this.stayOpenOnFocusout || !this.open) { + const anchorEl = this.anchorElement!; + // Do not close if we focused out by clicking on the anchor element. We + // can't assume anchor buttons can be the related target because of iOS does + // not focus buttons. + if (this.stayOpenOnFocusout || !this.open || + this.pointerPath.includes(anchorEl)) { return; } if (event.relatedTarget) { // Don't close the menu if we are switching focus between menu, - // md-menu-item, and md-list - if (isElementInSubtree(event.relatedTarget, this)) { + // md-menu-item, and md-list or if the anchor was click focused. + if (isElementInSubtree(event.relatedTarget, this) || + isElementInSubtree(event.relatedTarget, anchorEl)) { return; } - - const anchorEl = this.anchorElement!; - const wasAnchorClickFocused = - isElementInSubtree(event.relatedTarget, anchorEl) && - this.isPointerDown; - if (wasAnchorClickFocused) { - return; - } - } else if (this.isPointerDown && this.pointerPath.includes(this)) { + } else if (this.pointerPath.includes(this)) { // If menu tabindex == -1 and the user clicks on the menu or a divider, we // want to keep the menu open. return; @@ -752,34 +765,24 @@ export abstract class Menu extends LitElement { return animationEnded; } - override connectedCallback() { - super.connectedCallback(); - if (!isServer) { - window.addEventListener('click', this.onWindowClick, {capture: true}); - window.addEventListener('pointerdown', this.onWindowPointerdown); - window.addEventListener('pointerup', this.onWindowPointerup); - } - } - - override disconnectedCallback() { - super.disconnectedCallback(); - if (!isServer) { - window.removeEventListener('click', this.onWindowClick, {capture: true}); - window.removeEventListener('pointerdown', this.onWindowPointerdown); - window.removeEventListener('pointerup', this.onWindowPointerup); - } + private handleKeydown(event: KeyboardEvent) { + // At any key event, the pointer interaction is done so we need to clear our + // cached pointerpath. This handles the case where the user clicks on the + // anchor, and then hits shift+tab + this.pointerPath = []; + this.listController.handleKeydown(event); } private readonly onWindowPointerdown = (event: PointerEvent) => { - this.isPointerDown = true; this.pointerPath = event.composedPath(); }; - private readonly onWindowPointerup = () => { - this.isPointerDown = false; - }; - - private readonly onWindowClick = (event: MouseEvent) => { + /** + * We cannot listen to window click because Safari on iOS will not fire a + * click event on window if the item clicked is not a "clickable" item such as + * + */ + private readonly onWindowPointerup = (event: Event) => { if (!this.open) { return; }