refactor(item): do not automatically delegate focus (#29091)

resolves #21982

BREAKING CHANGE:

- Item no longer automatically delegates focus to the first focusable element. While most developers should not need to make any changes to account for this update, usages of `ion-item` with interactive elements such as form controls (inputs, textareas, etc) should be evaluated to verify that interactions still work as expected.

BREAKING.md

@@ -22,6 +22,7 @@ This is a comprehensive list of the breaking changes introduced in the major ver
   - [Content](#version-8x-content)
   - [Datetime](#version-8x-datetime)
   - [Input](#version-8x-input)
+  - [Item](#version-8x-item)
   - [Modal](#version-8x-modal)
   - [Nav](#version-8x-nav)
   - [Picker](#version-8x-picker)
@@ -168,6 +169,10 @@ For more information on the dynamic font, refer to the [Dynamic Font Scaling doc
 - `accept` has been removed from the `ion-input` component. This was previously used in conjunction with the `type="file"`. However, the `file` value for `type` is not a valid value in Ionic Framework.
 - The `legacy` property and support for the legacy syntax, which involved placing an `ion-input` inside of an `ion-item` with an `ion-label`, have been removed. For more information on migrating from the legacy input syntax, refer to the [Input documentation](https://ionicframework.com/docs/api/input#migrating-from-legacy-input-syntax).
 
+<h4 id="version-8x-item">Item</h4>
+
+- Item no longer automatically delegates focus to the first focusable element. While most developers should not need to make any changes to account for this update, usages of `ion-item` with interactive elements such as form controls (inputs, textareas, etc) should be evaluated to verify that interactions still work as expected.
+
 <h4 id="version-8x-modal">Modal</h4>
 
 - Detection for Capacitor <= 2 with applying status bar styles has been removed. Developers should ensure they are using Capacitor 3 or later when using the card modal presentation.

core/src/components/input/test/item/input.e2e.ts

@@ -45,3 +45,30 @@ configs().forEach(({ title, screenshot, config }) => {
     });
   });
 });
+
+configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
+  test.describe(title('input: item functionality'), () => {
+    test('clicking padded space within item should focus the input', async ({ page }) => {
+      await page.setContent(
+        `
+        <ion-item>
+          <ion-input label="Input"></ion-input>
+        </ion-item>
+      `,
+        config
+      );
+      const itemNative = page.locator('.item-native');
+      const input = page.locator('ion-input input');
+
+      // Clicks the padded space within the item
+      await itemNative.click({
+        position: {
+          x: 5,
+          y: 5,
+        },
+      });
+
+      await expect(input).toBeFocused();
+    });
+  });
+});

core/src/components/item/item.scss

@@ -176,7 +176,8 @@
 // Item: Interactive
 // --------------------------------------------------
 
-:host(.item-has-interactive-control) {
+// Inputs and textareas do not need the cursor, but other components like checkbox or toggle do.
+:host(.item-control-needs-pointer-cursor) {
   cursor: pointer;
 }
 

core/src/components/item/item.tsx

@@ -32,9 +32,7 @@ import type { CounterFormatter } from './item-interface';
     ios: 'item.ios.scss',
     md: 'item.md.scss',
   },
-  shadow: {
+  shadow: true,
-    delegatesFocus: true,
-  },
 })
 export class Item implements ComponentInterface, AnchorInterface, ButtonInterface {
   private labelColorStyles = {};
@@ -359,7 +357,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
 
   private getFirstInteractive() {
     const controls = this.el.querySelectorAll<HTMLElement>(
-      'ion-toggle:not([disabled]), ion-checkbox:not([disabled]), ion-radio:not([disabled]), ion-select:not([disabled])'
+      'ion-toggle:not([disabled]), ion-checkbox:not([disabled]), ion-radio:not([disabled]), ion-select:not([disabled]), ion-input:not([disabled]), ion-textarea:not([disabled])'
     );
     return controls[0];
   }
@@ -425,10 +423,19 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
                */
               const clickedWithinShadowRoot = this.el.shadowRoot!.contains(target);
               if (clickedWithinShadowRoot) {
+                /**
+                 * For input/textarea clicking the padding should focus the
+                 * text field (thus making it editable). For everything else,
+                 * we want to click the control so it activates.
+                 */
+                if (firstInteractive.tagName === 'ION-INPUT' || firstInteractive.tagName === 'ION-TEXTAREA') {
+                  (firstInteractive as HTMLIonInputElement | HTMLIonTextareaElement).setFocus();
+                } else {
                   firstInteractive.click();
                 }
               }
             }
+          }
         },
       };
     }
@@ -441,6 +448,13 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
     const fillValue = fill || 'none';
     const inList = hostContext('ion-list', this.el) && !hostContext('ion-radio-group', this.el);
 
+    /**
+     * Inputs and textareas do not need to show a cursor pointer.
+     * However, other form controls such as checkboxes and radios do.
+     */
+    const firstInteractiveNeedsPointerCursor =
+      firstInteractive !== undefined && !['ION-INPUT', 'ION-TEXTAREA'].includes(firstInteractive.tagName);
+
     return (
       <Host
         aria-disabled={ariaDisabled}
@@ -454,7 +468,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
             [`item-lines-${lines}`]: lines !== undefined,
             [`item-fill-${fillValue}`]: true,
             [`item-shape-${shape}`]: shape !== undefined,
-            'item-has-interactive-control': firstInteractive !== undefined,
+            'item-control-needs-pointer-cursor': firstInteractiveNeedsPointerCursor,
             'item-disabled': disabled,
             'in-list': inList,
             'item-multiple-inputs': this.multipleInputs,

core/src/components/menu/menu.tsx

@@ -1,6 +1,7 @@
 import type { ComponentInterface, EventEmitter } from '@stencil/core';
 import { Build, Component, Element, Event, Host, Listen, Method, Prop, State, Watch, h } from '@stencil/core';
 import { getTimeGivenProgression } from '@utils/animation/cubic-bezier';
+import { focusFirstDescendant, focusLastDescendant } from '@utils/focus-trap';
 import { GESTURE_CONTROLLER } from '@utils/gesture';
 import { shoudUseCloseWatcher } from '@utils/hardware-back-button';
 import type { Attributes } from '@utils/helpers';
@@ -19,8 +20,6 @@ const iosEasing = 'cubic-bezier(0.32,0.72,0,1)';
 const mdEasing = 'cubic-bezier(0.0,0.0,0.2,1)';
 const iosEasingReverse = 'cubic-bezier(1, 0, 0.68, 0.28)';
 const mdEasingReverse = 'cubic-bezier(0.4, 0, 0.6, 1)';
-const focusableQueryString =
-  '[tabindex]:not([tabindex^="-"]), input:not([type=hidden]):not([tabindex^="-"]), textarea:not([tabindex^="-"]), button:not([tabindex^="-"]), select:not([tabindex^="-"]), .ion-focusable:not([tabindex^="-"])';
 
 /**
  * @part container - The container for the menu content.
@@ -398,31 +397,9 @@ export class Menu implements ComponentInterface, MenuI {
     return menuController._setOpen(this, shouldOpen, animated);
   }
 
-  private focusFirstDescendant() {
-    const { el } = this;
-    const firstInput = el.querySelector(focusableQueryString) as HTMLElement | null;
-
-    if (firstInput) {
-      firstInput.focus();
-    } else {
-      el.focus();
-    }
-  }
-
-  private focusLastDescendant() {
-    const { el } = this;
-    const inputs = Array.from(el.querySelectorAll<HTMLElement>(focusableQueryString));
-    const lastInput = inputs.length > 0 ? inputs[inputs.length - 1] : null;
-
-    if (lastInput) {
-      lastInput.focus();
-    } else {
-      el.focus();
-    }
-  }
-
   private trapKeyboardFocus(ev: Event, doc: Document) {
     const target = ev.target as HTMLElement | null;
+
     if (!target) {
       return;
     }
@@ -439,13 +416,15 @@ export class Menu implements ComponentInterface, MenuI {
        * Wrap the focus to either the first or last element.
        */
 
+      const { el } = this;
+
       /**
        * Once we call `focusFirstDescendant`, another focus event
        * will fire, which will cause `lastFocus` to be updated
        * before we can run the code after that. We cache the value
        * here to avoid that.
        */
-      this.focusFirstDescendant();
+      focusFirstDescendant(el);
 
       /**
        * If the cached last focused element is the same as the now-
@@ -454,7 +433,7 @@ export class Menu implements ComponentInterface, MenuI {
        * last descendant.
        */
       if (this.lastFocus === doc.activeElement) {
-        this.focusLastDescendant();
+        focusLastDescendant(el);
       }
     }
   }

core/src/components/menu/test/basic/index.html

@@ -35,9 +35,7 @@
         </ion-header>
         <ion-content>
           <ion-list>
-            <ion-item>
             <ion-button id="start-menu-button">Button</ion-button>
-            </ion-item>
             <ion-item>Menu Item</ion-item>
             <ion-item>Menu Item</ion-item>
             <ion-item>Menu Item</ion-item>

core/src/components/popover/popover.tsx

@@ -1,18 +1,11 @@
 import type { ComponentInterface, EventEmitter } from '@stencil/core';
 import { Component, Element, Event, Host, Method, Prop, State, Watch, h } from '@stencil/core';
+import { focusFirstDescendant } from '@utils/focus-trap';
 import { CoreDelegate, attachComponent, detachComponent } from '@utils/framework-delegate';
 import { addEventListener, raf, hasLazyBuild } from '@utils/helpers';
 import { createLockController } from '@utils/lock-controller';
 import { printIonWarning } from '@utils/logging';
-import {
+import { BACKDROP, dismiss, eventMethod, prepareOverlay, present, setOverlayId } from '@utils/overlays';
-  BACKDROP,
-  dismiss,
-  eventMethod,
-  focusFirstDescendant,
-  prepareOverlay,
-  present,
-  setOverlayId,
-} from '@utils/overlays';
 import { isPlatform } from '@utils/platform';
 import { getClassMap } from '@utils/theme';
 import { deepReady, waitForMount } from '@utils/transition';
@@ -512,7 +505,7 @@ export class Popover implements ComponentInterface, PopoverInterface {
      * descendant inside of the popover.
      */
     if (this.focusDescendantOnPresent) {
-      focusFirstDescendant(this.el, this.el);
+      focusFirstDescendant(el);
     }
 
     unlock();

core/src/components/textarea/test/item/textarea.e2e.ts

@@ -45,3 +45,30 @@ configs().forEach(({ title, screenshot, config }) => {
     });
   });
 });
+
+configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
+  test.describe(title('textarea: item functionality'), () => {
+    test('clicking padded space within item should focus the textarea', async ({ page }) => {
+      await page.setContent(
+        `
+        <ion-item>
+          <ion-textarea label="Textarea"></ion-textarea>
+        </ion-item>
+      `,
+        config
+      );
+      const itemNative = page.locator('.item-native');
+      const textarea = page.locator('ion-textarea textarea');
+
+      // Clicks the padded space within the item
+      await itemNative.click({
+        position: {
+          x: 5,
+          y: 5,
+        },
+      });
+
+      await expect(textarea).toBeFocused();
+    });
+  });
+});

core/src/utils/focus-trap.ts

@@ -0,0 +1,86 @@
+import { focusVisibleElement } from '@utils/helpers';
+
+/**
+ * This query string selects elements that
+ * are eligible to receive focus. We select
+ * interactive elements that meet the following
+ * criteria:
+ * 1. Element does not have a negative tabindex
+ * 2. Element does not have `hidden`
+ * 3. Element does not have `disabled` for non-Ionic components.
+ * 4. Element does not have `disabled` or `disabled="true"` for Ionic components.
+ * Note: We need this distinction because `disabled="false"` is
+ * valid usage for the disabled property on ion-button.
+ */
+export 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])';
+
+/**
+ * Focuses the first descendant in a context
+ * that can receive focus. If none exists,
+ * a fallback element will be focused.
+ * This fallback is typically an ancestor
+ * container such as a menu or overlay so focus does not
+ * leave the container we are trying to trap focus in.
+ *
+ * If no fallback is specified then we focus the container itself.
+ */
+export const focusFirstDescendant = <R extends HTMLElement, T extends HTMLElement>(ref: R, fallbackElement?: T) => {
+  const firstInput = ref.querySelector<HTMLElement>(focusableQueryString);
+
+  focusElementInContext(firstInput, fallbackElement ?? ref);
+};
+
+/**
+ * Focuses the last descendant in a context
+ * that can receive focus. If none exists,
+ * a fallback element will be focused.
+ * This fallback is typically an ancestor
+ * container such as a menu or overlay so focus does not
+ * leave the container we are trying to trap focus in.
+ *
+ * If no fallback is specified then we focus the container itself.
+ */
+export const focusLastDescendant = <R extends HTMLElement, T extends HTMLElement>(ref: R, fallbackElement?: T) => {
+  const inputs = Array.from(ref.querySelectorAll<HTMLElement>(focusableQueryString));
+  const lastInput = inputs.length > 0 ? inputs[inputs.length - 1] : null;
+
+  focusElementInContext(lastInput, fallbackElement ?? ref);
+};
+
+/**
+ * Focuses a particular element in a context. If the element
+ * doesn't have anything focusable associated with it then
+ * a fallback element will be focused.
+ *
+ * This fallback is typically an ancestor
+ * container such as a menu or overlay so focus does not
+ * leave the container we are trying to trap focus in.
+ * 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 <button>
+ * element inside of ion-button's shadow root, not
+ * the host element itself.
+ */
+const focusElementInContext = <T extends HTMLElement>(
+  hostToFocus: HTMLElement | null | undefined,
+  fallbackElement: T
+) => {
+  let elementToFocus = hostToFocus;
+
+  const shadowRoot = hostToFocus?.shadowRoot;
+  if (shadowRoot) {
+    // If there are no inner focusable elements, just focus the host element.
+    elementToFocus = shadowRoot.querySelector<HTMLElement>(focusableQueryString) || hostToFocus;
+  }
+
+  if (elementToFocus) {
+    focusVisibleElement(elementToFocus);
+  } else {
+    // Focus fallback element instead of letting focus escape
+    fallbackElement.focus();
+  }
+};

core/src/utils/overlays.ts

@@ -1,4 +1,5 @@
 import { doc } from '@utils/browser';
+import { focusFirstDescendant, focusLastDescendant, focusableQueryString } from '@utils/focus-trap';
 import type { BackButtonEvent } from '@utils/hardware-back-button';
 import { shoudUseCloseWatcher } from '@utils/hardware-back-button';
 
@@ -129,45 +130,8 @@ export const createOverlay = <T extends HTMLIonOverlayElement>(
   return Promise.resolve() as any;
 };
 
-/**
- * This query string selects elements that
- * are eligible to receive focus. We select
- * interactive elements that meet the following
- * criteria:
- * 1. Element does not have a negative tabindex
- * 2. Element does not have `hidden`
- * 3. Element does not have `disabled` for non-Ionic components.
- * 4. Element does not have `disabled` or `disabled="true"` for Ionic components.
- * Note: We need this distinction because `disabled="false"` is
- * valid usage for the disabled property on ion-button.
- */
-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) => {
-  const firstInput = ref.querySelector(focusableQueryString) as HTMLElement | null;
-
-  focusElementInOverlay(firstInput, overlay);
-};
-
-/**
- * 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[];
-  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
@@ -282,7 +246,7 @@ const trapKeyboardFocus = (ev: Event, doc: Document) => {
         return;
       }
 
-      const overlayWrapper = overlayRoot.querySelector('.ion-overlay-wrapper');
+      const overlayWrapper = overlayRoot.querySelector<HTMLElement>('.ion-overlay-wrapper');
       if (!overlayWrapper) {
         return;
       }
@@ -370,7 +334,7 @@ const trapKeyboardFocus = (ev: Event, doc: Document) => {
       const lastFocus = lastOverlay.lastFocus;
 
       // Focus the first element in the overlay wrapper
-      focusFirstDescendant(lastOverlay, lastOverlay);
+      focusFirstDescendant(lastOverlay);
 
       /**
        * If the cached last focused element is the
@@ -382,7 +346,7 @@ const trapKeyboardFocus = (ev: Event, doc: Document) => {
        * last focus to equal the active element.
        */
       if (lastFocus === doc.activeElement) {
-        focusLastDescendant(lastOverlay, lastOverlay);
+        focusLastDescendant(lastOverlay);
       }
       lastOverlay.lastFocus = doc.activeElement as HTMLElement;
     }