fix(overlays): don't force the focus ring on pointer redirects

core/src/utils/focus-trap.ts

@@ -1,5 +1,20 @@
+import { isKeyboardMode } from '@utils/focus-visible';
 import { focusVisibleElement } from '@utils/helpers';
 
+/**
+ * Focuses an element a focus trap is redirecting focus to. Only draws the
+ * keyboard focus indicator when the user is navigating with a keyboard, so a
+ * redirect caused by a tap or click does not leave the element looking as
+ * though it was tabbed to.
+ */
+export const focusRedirectedElement = (el: HTMLElement) => {
+  if (isKeyboardMode()) {
+    focusVisibleElement(el);
+  } else {
+    el.focus();
+  }
+};
+
 /**
  * This query string selects elements that
  * are eligible to receive focus. We select
@@ -94,7 +109,7 @@ const focusElementInContext = <T extends HTMLElement>(
     if (radioGroup) {
       radioGroup.setFocus();
     } else {
-      focusVisibleElement(elementToFocus);
+      focusRedirectedElement(elementToFocus);
     }
   } else {
     // Focus fallback element instead of letting focus escape

core/src/utils/focus-visible.ts

@@ -18,6 +18,7 @@ const FOCUS_KEYS = [
 export interface FocusVisibleUtility {
   destroy: () => void;
   setFocus: (elements: Element[]) => void;
+  isKeyboardMode: () => boolean;
 }
 
 let focusVisibleUtility: FocusVisibleUtility | null = null;
@@ -46,10 +47,39 @@ export const focusElements = (elements: Element[]) => {
   focusVisible.setFocus(elements);
 };
 
+/**
+ * Reports whether the most recent interaction on the page was keyboard-driven.
+ *
+ * Check this before drawing the keyboard focus indicator programmatically.
+ *
+ * @returns `true` while the user is navigating with a keyboard, and before the
+ * first interaction on the page.
+ */
+export const isKeyboardMode = () => getOrInitFocusVisibleUtility().isKeyboardMode();
+
+/**
+ * Watches how the user is interacting with the page and marks the focused
+ * element with `ion-focused`, so the keyboard focus indicator is only drawn
+ * while the user navigates with a keyboard.
+ *
+ * @param rootEl Scopes the utility to this element's shadow root, so it only
+ * reacts to interactions inside it. Omit it to listen on the document.
+ * @returns `setFocus` to mark elements focused programmatically, and `destroy`
+ * to detach the listeners.
+ */
 export const startFocusVisible = (rootEl?: HTMLElement): FocusVisibleUtility => {
   let currentFocus: Element[] = [];
+
+  /*
+   * Starts as `true` so an element focused before the user has interacted,
+   * such as one focused on page load, still draws an indicator.
+   */
   let keyboardMode = true;
 
+  /*
+   * `ref` is where the listeners go and `root` is the element focus falls back
+   * to once it leaves everything inside `ref`.
+   */
   const ref = rootEl ? rootEl.shadowRoot! : document;
   const root = rootEl ? rootEl : document.body;
 
@@ -63,12 +93,25 @@ export const startFocusVisible = (rootEl?: HTMLElement): FocusVisibleUtility =>
     setFocus([]);
   };
 
+  /*
+   * Only the keys that move focus keep the indicator on. Any other key means
+   * the user is typing into the focused element rather than navigating, so the
+   * indicator is dropped.
+   */
   const onKeydown = (ev: Event) => {
     keyboardMode = FOCUS_KEYS.includes((ev as KeyboardEvent).key);
     if (!keyboardMode) {
       setFocus([]);
     }
   };
+
+  /*
+   * The indicator does not always belong to the element that took focus. The
+   * composed path is walked so every `ion-focusable` ancestor is marked too,
+   * which is how an `ion-item` draws the indicator for a checkbox slotted into
+   * it, since a checkbox in an item drops the class itself. The composed path
+   * is used because it reaches hosts across shadow boundaries.
+   */
   const onFocusin = (ev: Event) => {
     if (keyboardMode && ev.composedPath !== undefined) {
       const toFocus = ev.composedPath().filter((el: any) => {
@@ -81,20 +124,30 @@ export const startFocusVisible = (rootEl?: HTMLElement): FocusVisibleUtility =>
       setFocus(toFocus);
     }
   };
+
+  /*
+   * Focus landing back on `root` means it left every focusable element, so
+   * nothing should stay marked. Focus moving between elements is left alone
+   * because `onFocusin` marks the new one.
+   */
   const onFocusout = () => {
     if (ref.activeElement === root) {
       setFocus([]);
     }
   };
 
-  ref.addEventListener('keydown', onKeydown);
+  /*
+   * Capture phase, so the mode is current for the overlay focus trap, which
+   * intercepts Tab in its own capture listener.
+   */
+  ref.addEventListener('keydown', onKeydown, true);
   ref.addEventListener('focusin', onFocusin);
   ref.addEventListener('focusout', onFocusout);
   ref.addEventListener('touchstart', pointerDown, { passive: true });
   ref.addEventListener('mousedown', pointerDown);
 
   const destroy = () => {
-    ref.removeEventListener('keydown', onKeydown);
+    ref.removeEventListener('keydown', onKeydown, true);
     ref.removeEventListener('focusin', onFocusin);
     ref.removeEventListener('focusout', onFocusout);
     ref.removeEventListener('touchstart', pointerDown);
@@ -104,5 +157,6 @@ export const startFocusVisible = (rootEl?: HTMLElement): FocusVisibleUtility =>
   return {
     destroy,
     setFocus,
+    isKeyboardMode: () => keyboardMode,
   };
 };

core/src/utils/overlays.ts

@@ -1,5 +1,10 @@
 import { doc } from '@utils/browser';
-import { focusFirstDescendant, focusLastDescendant, focusableQueryString } from '@utils/focus-trap';
+import {
+  focusFirstDescendant,
+  focusLastDescendant,
+  focusRedirectedElement,
+  focusableQueryString,
+} from '@utils/focus-trap';
 import type { BackButtonEvent } from '@utils/hardware-back-button';
 import { shouldUseCloseWatcher } from '@utils/hardware-back-button';
 import { printIonError, printIonWarning } from '@utils/logging';
@@ -294,7 +299,7 @@ const focusElementInOverlay = (hostToFocus: HTMLElement | null | undefined, over
   }
 
   if (elementToFocus) {
-    focusVisibleElement(elementToFocus);
+    focusRedirectedElement(elementToFocus);
   } else {
     // Focus overlay instead of letting focus escape
     overlay.focus();

core/src/utils/test/overlays/index.html

@@ -45,6 +45,9 @@
           <ion-button id="create-and-present-toast" onclick="createAndPresentToast()"
             >Create and Present Toast</ion-button
           >
+          <ion-button id="create-and-present-focus-trap" onclick="createAndPresentFocusTrapModal()"
+            >Create and Present Focus Trap Modal</ion-button
+          >
         </ion-content>
       </div>
     </ion-app>
@@ -128,6 +131,61 @@
         await toast.present();
       };
 
+      /*
+        Presents a modal for checking the focus trap's redirect by hand. Both
+        routes move focus to the input behind the modal, which the trap
+        redirects back to the checkbox. Only the keyboard route should leave an
+        indicator there.
+      */
+      const createAndPresentFocusTrapModal = async () => {
+        const div = document.createElement('div');
+        div.innerHTML = `
+          <ion-content class="ion-padding">
+            <ion-checkbox>Dark Mode</ion-checkbox>
+
+            <ion-button id="modal-move-focus">Move focus outside</ion-button>
+
+            <p>Pointer: click the button above. Expect false below.</p>
+
+            <p>Keyboard: reopen the modal, press Tab twice to reach the button, then Enter. Expect true below.</p>
+
+            <p id="focus-trap-status">indicator on the checkbox: not checked yet</p>
+
+            <p>
+              The indicator is a faint wash, so the reading above is the reliable
+              check. Reopen the modal between the two routes, since a click leaves
+              the focus utility out of keyboard mode.
+            </p>
+          </ion-content>
+        `;
+
+        const report = () => {
+          const checkbox = div.querySelector('ion-checkbox');
+          const status = div.querySelector('#focus-trap-status');
+          status.textContent = `indicator on the checkbox: ${checkbox.classList.contains('ion-focused')}`;
+        };
+
+        /*
+          The reading is deferred a frame because `setFocus` is async, so the
+          trap has not redirected focus back yet when it returns.
+        */
+        const moveFocusOutside = () => {
+          document.querySelector('#root-input').setFocus();
+          requestAnimationFrame(report);
+        };
+
+        const moveFocusButton = div.querySelector('ion-button#modal-move-focus');
+        moveFocusButton.onclick = moveFocusOutside;
+
+        const modal = await modalController.create({
+          component: div,
+        });
+
+        await modal.present();
+
+        return modal;
+      };
+
       const createNestedOverlayModal = async () => {
         const div = document.createElement('div');
         div.innerHTML = `

core/src/utils/test/overlays/overlays.e2e.ts

@@ -529,5 +529,64 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
       await expect(wrapper).toHaveAttribute('role', 'dialog');
       await expect(wrapper).toBeFocused();
     });
+
+    /*
+     * The focus trap redirects focus back into the overlay when focus lands
+     * outside of it. The indicator should only follow that redirect during
+     * keyboard navigation.
+     *
+     * It lands on the `ion-item` because a toggle inside an item has the item
+     * draw the indicator on its behalf. `ion-app` is required to apply the
+     * focused styles.
+     */
+    const redirectContent = `
+      <ion-app>
+        <ion-button id="open-modal">Show Modal</ion-button>
+        <div tabindex="0">Outside Element</div>
+        <ion-modal trigger="open-modal">
+          <ion-content>
+            <ion-item>
+              <ion-toggle>Notifications</ion-toggle>
+            </ion-item>
+          </ion-content>
+        </ion-modal>
+      </ion-app>
+    `;
+
+    test('should not show a focus indicator when focus is redirected after a pointer interaction', async ({ page }) => {
+      await page.setContent(redirectContent, config);
+
+      const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
+      const item = page.locator('ion-modal ion-item');
+
+      // Opening with a click leaves the focus utility in pointer mode.
+      await page.locator('ion-button#open-modal').click();
+      await ionModalDidPresent.next();
+
+      await page.locator('ion-app > div[tabindex="0"]').evaluate((el: HTMLElement) => el.focus());
+      await page.waitForChanges();
+
+      await expect(item).not.toHaveClass(/ion-focused/);
+    });
+
+    test('should show a focus indicator when focus is redirected during keyboard navigation', async ({
+      page,
+      pageUtils,
+    }) => {
+      await page.setContent(redirectContent, config);
+
+      const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
+      const item = page.locator('ion-modal ion-item');
+
+      await page.locator('ion-button#open-modal').click();
+      await ionModalDidPresent.next();
+
+      // Shift turns keyboard mode back on without moving focus.
+      await pageUtils.pressKeys('Shift');
+      await page.locator('ion-app > div[tabindex="0"]').evaluate((el: HTMLElement) => el.focus());
+      await page.waitForChanges();
+
+      await expect(item).toHaveClass(/ion-focused/);
+    });
   });
 });