From 2ed0ada9237b3f4dbf5959746ce2d1744936eebe Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Wed, 14 Feb 2024 12:33:11 -0500 Subject: [PATCH] fix(overlays): focus is returned to last focus element when focusing toast (#28950) Issue number: resolves #28261 --------- ## What is the current behavior? When moving focus from a focus-trapped overlay to a toast, focus is moved back to the overlay. This is the correct behavior as focus should never leave a focus-trapped overlay (unless the overlay is dismissed or focus is moved to a _new_ top-most overlay). However, the way we return focus is a bit unexpected because it always returns focus to the last focusable element in the overlay. This means that if you were focused on the first focusable element, presented the toast, and then focused the toast, focus might not be moved back to that first focusable element. In the case of the linked issue, this was causing an unexpected scroll so that the last focused element could be in view. ## What is the new behavior? - This fix adds an exception for `ion-toast` (as it is the only overlay that is **not** focus trapped) that ensures that focus is moved back to the last focus element. ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information Dev build: `7.7.1-dev.11707253408.186eea70` Note: We don't recommend this pattern in general because it would be impossible for a screen reader user to focus the toast. However, we can at least improve the experience for developers who continue to implement this pattern by returning focus in a more predictable manner. Docs: https://github.com/ionic-team/ionic-docs/pull/3432 Testing: Reviewers should manually test the following behaviors: 1. Create a modal with 2 buttons. Have one of the buttons present a toast. Open the toast and verify that you can still Tab to cycle through the buttons in the modal. 2. Create a modal with 2 buttons. Have one of the buttons present a toast. Open the toast. Move focus to the toast and verify that you can still Tab to cycle through the buttons in the modal (once focus is returned to the modal). --- core/src/components/select/select.tsx | 6 +- core/src/utils/helpers.ts | 2 +- core/src/utils/overlays.ts | 93 ++++++++++---- core/src/utils/test/overlays/overlays.e2e.ts | 120 +++++++++++++++++++ 4 files changed, 196 insertions(+), 25 deletions(-) diff --git a/core/src/components/select/select.tsx b/core/src/components/select/select.tsx index 9fe26349bb1..e15ae27e653 100644 --- a/core/src/components/select/select.tsx +++ b/core/src/components/select/select.tsx @@ -2,7 +2,7 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core'; import { Component, Element, Event, Host, Method, Prop, State, Watch, h, forceUpdate } from '@stencil/core'; import type { LegacyFormController, NotchController } from '@utils/forms'; import { compareOptions, createLegacyFormController, createNotchController, isOptionSelected } from '@utils/forms'; -import { findItemLabel, focusElement, getAriaLabel, renderHiddenInput, inheritAttributes } from '@utils/helpers'; +import { findItemLabel, focusVisibleElement, getAriaLabel, renderHiddenInput, inheritAttributes } from '@utils/helpers'; import type { Attributes } from '@utils/helpers'; import { printIonWarning } from '@utils/logging'; import { actionSheetController, alertController, popoverController } from '@utils/overlays'; @@ -329,7 +329,7 @@ export class Select implements ComponentInterface { ); if (selectedItem) { - focusElement(selectedItem); + focusVisibleElement(selectedItem); /** * Browsers such as Firefox do not @@ -355,7 +355,7 @@ export class Select implements ComponentInterface { 'ion-radio:not(.radio-disabled), ion-checkbox:not(.checkbox-disabled)' ); if (firstEnabledOption) { - focusElement(firstEnabledOption.closest('ion-item')!); + focusVisibleElement(firstEnabledOption.closest('ion-item')!); /** * Focus the option for the same reason as we do above. diff --git a/core/src/utils/helpers.ts b/core/src/utils/helpers.ts index 3dff5a5e657..81ffb4efa92 100644 --- a/core/src/utils/helpers.ts +++ b/core/src/utils/helpers.ts @@ -262,7 +262,7 @@ export const findItemLabel = (componentEl: HTMLElement): HTMLIonLabelElement | n return null; }; -export const focusElement = (el: HTMLElement) => { +export const focusVisibleElement = (el: HTMLElement) => { el.focus(); /** diff --git a/core/src/utils/overlays.ts b/core/src/utils/overlays.ts index 8ccb015d8d8..11ec2f56698 100644 --- a/core/src/utils/overlays.ts +++ b/core/src/utils/overlays.ts @@ -22,7 +22,13 @@ import type { import { CoreDelegate } from './framework-delegate'; import { OVERLAY_BACK_BUTTON_PRIORITY } from './hardware-back-button'; -import { addEventListener, componentOnReady, focusElement, getElementRoot, removeEventListener } from './helpers'; +import { + addEventListener, + componentOnReady, + focusVisibleElement, + getElementRoot, + removeEventListener, +} from './helpers'; import { printIonWarning } from './logging'; let lastOverlayIndex = 0; @@ -131,38 +137,55 @@ export const createOverlay = ( */ const focusableQueryString = '[tabindex]:not([tabindex^="-"]):not([hidden]):not([disabled]), input:not([type=hidden]):not([tabindex^="-"]):not([hidden]):not([disabled]), textarea:not([tabindex^="-"]):not([hidden]):not([disabled]), button:not([tabindex^="-"]):not([hidden]):not([disabled]), select:not([tabindex^="-"]):not([hidden]):not([disabled]), .ion-focusable:not([tabindex^="-"]):not([hidden]):not([disabled]), .ion-focusable[disabled="false"]:not([tabindex^="-"]):not([hidden])'; +const isOverlayHidden = (overlay: Element) => overlay.classList.contains('overlay-hidden'); +/** + * Focuses the first descendant in an overlay + * that can receive focus. If none exists, + * the entire overlay will be focused. + */ export const focusFirstDescendant = (ref: Element, overlay: HTMLIonOverlayElement) => { - let firstInput = ref.querySelector(focusableQueryString) as HTMLElement | null; - - const shadowRoot = firstInput?.shadowRoot; - if (shadowRoot) { - // If there are no inner focusable elements, just focus the host element. - firstInput = shadowRoot.querySelector(focusableQueryString) || firstInput; - } + const firstInput = ref.querySelector(focusableQueryString) as HTMLElement | null; - if (firstInput) { - focusElement(firstInput); - } else { - // Focus overlay instead of letting focus escape - overlay.focus(); - } + focusElementInOverlay(firstInput, overlay); }; -const isOverlayHidden = (overlay: Element) => overlay.classList.contains('overlay-hidden'); - +/** + * Focuses the last descendant in an overlay + * that can receive focus. If none exists, + * the entire overlay will be focused. + */ const focusLastDescendant = (ref: Element, overlay: HTMLIonOverlayElement) => { const inputs = Array.from(ref.querySelectorAll(focusableQueryString)) as HTMLElement[]; - let lastInput = inputs.length > 0 ? inputs[inputs.length - 1] : null; + const lastInput = inputs.length > 0 ? inputs[inputs.length - 1] : null; + + focusElementInOverlay(lastInput, overlay); +}; + +/** + * Focuses a particular element in an overlay. If the element + * doesn't have anything focusable associated with it then + * the overlay itself will be focused. + * This should be used instead of the focus() method + * on most elements because the focusable element + * may not be the host element. + * + * For example, if an ion-button should be focused + * then we should actually focus the native + + + + + + `, + config + ); + + const modal = page.locator('ion-modal'); + const showToastTrigger = page.locator('#show-toast'); + + const toast = page.locator('ion-toast'); + const toastButton = toast.locator('button'); + + const ionToastDidPresent = await page.spyOnEvent('ionToastDidPresent'); + + // Show overlay + await modal.evaluate((el: HTMLIonModalElement) => el.present()); + + // Click trigger to open toast + await showToastTrigger.click(); + + // Wait for toast to be presented + await ionToastDidPresent.next(); + + // Verify trigger in overlay is focused + await expect(showToastTrigger).toBeFocused(); + + // Click a button in the toast and therefore attempt to move focus + await toastButton.click(); + + // Verify trigger in overlay is still focused + await expect(showToastTrigger).toBeFocused(); + }); + + test('focusing toast from a scoped overlay should return focus to the last focused element', async ({ + page, + skip, + }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/28261', + }); + skip.browser('webkit', 'WebKit does not consider buttons to be focusable'); + + await page.setContent( + ` + + + + + `, + config + ); + + const actionSheet = page.locator('ion-action-sheet'); + const showToastButton = page.locator('#show-toast'); + + const toast = page.locator('ion-toast'); + const toastButton = toast.locator('button'); + + const ionToastDidPresent = await page.spyOnEvent('ionToastDidPresent'); + + // Show overlay + await actionSheet.evaluate((el: HTMLIonActionSheetElement) => el.present()); + + // Click button to open toast + await showToastButton.click(); + + // Wait for toast to be presented + await ionToastDidPresent.next(); + + // Verify button in overlay is focused + await expect(showToastButton).toBeFocused(); + + // Click a button in the toast and therefore attempt to move focus + await toastButton.click(); + + await page.pause(); + + // Verify button in overlay is still focused + await expect(showToastButton).toBeFocused(); + }); test('should not return focus to another element if focus already manually returned', async ({ page, skip,