fix(overlays): show focus indicators only during keyboard navigation and improve focus management by brandyscarney · Pull Request #31165 · ionic-team/ionic-framework

brandyscarney · 2026-05-22T16:22:48Z

Issue number: internal

What is the current behavior?

The focus indicator is displayed on the option when opening certain interfaces by clicking on the Select component:

Modal	Popover

What is the new behavior?

Hide the focus indicator when the Select component is opened using pointer events
Display the focus indicator when the Select component is opened using keyboard navigation (Space or Enter)
Cycle focus through the Select elements (radios, handle, cancel buttons) in the correct order and allow Shift + Tab to go backwards in order
Removes the native focus from Checkbox and does not allow focusable when inside of an item (this matches Radio's behavior)
Adds e2e tests for focus functionality in both Select and Modal to avoid future regressions

Does this introduce a breaking change?

Yes
No

Other information

vercel · 2026-05-22T16:22:54Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
ionic-framework	Ready	Preview, Comment	Jun 12, 2026 10:31pm

brandyscarney · 2026-06-01T16:32:24Z

@@ -114,10 +114,6 @@ slot[name="end"]::slotted(*) {

 // Item in Select Modal
 // --------------------------------------------------
-:host(.in-select-modal) {


This was added to hide the focused state.

brandyscarney · 2026-06-01T16:35:05Z

@@ -15,11 +15,6 @@ ion-item {
  --border-width: 0;
 }

-ion-item.ion-focused::part(native)::after {


This was added to hide the focused state.

brandyscarney · 2026-06-01T17:10:29Z

The ionic theme wasn't added to this test so now it is generating screenshots.

brandyscarney · 2026-06-01T17:11:32Z

The focus indicator is no longer displayed when opening with a mouse click.

brandyscarney · 2026-06-01T17:11:39Z

The focus indicator is no longer displayed when opening with a mouse click.

brandyscarney · 2026-06-01T17:12:01Z

The focus indicator is no longer displayed when opening with a mouse click.

brandyscarney · 2026-06-01T17:12:06Z

The focus indicator is no longer displayed when opening with a mouse click.

brandyscarney · 2026-06-01T17:13:42Z

@@ -8,7 +8,7 @@ import { configs, test } from '@utils/test/playwright';
 * does not. The overlay rendering is already tested in the respective
 * test files.
 */
-configs({ directions: ['ltr'] }).forEach(({ title, config, screenshot }) => {
+configs({ modes: ['ios', 'md', 'ionic-md'], directions: ['ltr'] }).forEach(({ title, config, screenshot }) => {


This now checks for the ionic theme and captures screenshots to avoid future focus behavior regressions.

brandyscarney · 2026-06-02T13:55:41Z

This was removed intentionally because we don't want to show the native focus when we are adding our own. We need to add some ion-focused styles to checkbox for ios and md to fix this. Should I make a follow-up?

Probably? Just so we don't forget it in the future

Yes, let's create a ticket and then can you link it to the checkbox modular migration ticket? If it's not done by then, we can just add it during the migration.

I actually created FW-7585 already for this #31165 (comment) which requests the same thing. I linked it to the modular migration ticket.

brandyscarney · 2026-06-02T14:11:36Z

-      test('holding Enter to open a popover should not immediately dismiss it', async ({ page, skip }, testInfo) => {
-        // TODO (ROU-5437)
-        skip.browser('webkit', 'Safari 16 only allows text fields and pop-up menus to be focused.');
-


I updated the keyboard presses to this:

-await page.keyboard.press('Enter'); +await pageUtils.pressKeys('Enter');

This handles Safari tabbing so we are able to check Safari again in most of these tests.

ShaneK

This is looking really good! I tested the functionality and the changes here seem to work fine, but I noticed some things and just wanted a bit of clarification. Good work so far, though!

ShaneK

This looks great to me! Thanks for fixing that issue and addressing the questions I left before! Just a very minor non-blocking question

ShaneK · 2026-06-15T13:49:33Z

    });
+
+    test('it should preserve the last arrow-focused radio when tabbing', async ({ page, pageUtils }) => {
+      await page.goto('/src/components/modal/test/sheet', config);


Is this useful since you call page.setContent right after it? I thought setContent fully replaced the page's content, but I may be mistaken and am just curious

I'm under the same assumption. I'm assuming it was from a copy and paste.

thetaPC

Questions:

Multiple-select interfaces: if a checked option is not the first one, should that checked option receive focus when the overlay is opened with the keyboard, rather than the first option?

Issues:

Multiple-select on popover: I can only Tab to the first two options.
Multiple-select on popover: when I Tab into the second option and press Space to select it, it selects the first option instead.
Action sheet: when I Tab to open it, I do not see the focus indicator on the wrapper the way I do with the alert. This happens both with no value and with a value selected.

Comments:

The action sheet and alert behave differently from each other, and I find the wrapper ring surprising in general. I would have expected the focus indicator to be on the selected option when an overlay opens with a value, not on the wrapper. You mentioned the wrapper ring is the intended behavior: is that intended based on web standards, or is it inherited from the existing overlay focus-trap?

thetaPC · 2026-06-15T17:36:13Z

  private isFocusable(): boolean {
    const focusableChild = this.el.querySelector('.ion-focusable');
-    return this.canActivate() || focusableChild !== null;
+    // An item is focusable when it can receive keyboard focus: when it is


Is the colon supposed to be a comma?

Suggested change

// An item is focusable when it can receive keyboard focus: when it is

// An item is focusable when it can receive keyboard focus, when it is

thetaPC · 2026-06-15T17:37:56Z

    });
+
+    test('it should preserve the last arrow-focused radio when tabbing', async ({ page, pageUtils }) => {
+      await page.goto('/src/components/modal/test/sheet', config);


I'm under the same assumption. I'm assuming it was from a copy and paste.

thetaPC · 2026-06-15T17:46:00Z

Do we need a snapshot of the entire screen? It seems that a snapshot of the overlay would work. It would be great if we can get in the habit of only capturing the overlay if that's all we need. It limits the need to update snapshots if the main page changes.

thetaPC · 2026-06-15T17:50:26Z

+        await expect(firstOption).toBeFocused();
+        await expect(firstOption).toHaveClass(/ion-focused/);
+
+        await expect(alert).toHaveScreenshot(screenshot(`select-basic-alert-opened-focused`));


Do we need a snapshot when you've already tested for focus? The snapshots seem redundant in my opinion especially since we have Alert state tests.

thetaPC · 2026-06-15T17:51:18Z

+        await expect(selectedOption).toBeFocused();
+        await expect(selectedOption).toHaveClass(/ion-focused/);
+
+        await expect(alert).toHaveScreenshot(screenshot(`select-basic-alert-opened-with-value-focused`));


Do we need a snapshot when you've already tested for focus? The snapshots seem redundant in my opinion especially since we have Alert state tests.

thetaPC · 2026-06-15T18:00:05Z

+        await expect(secondOption.locator('ion-radio')).toBeFocused();
+        await expect(secondOption).toHaveClass(/ion-focused/);

+        await expect(popover).toHaveScreenshot(screenshot(`select-basic-popover-opened-focused`));


Do we need a snapshot when you've already tested for focus? The snapshots seem redundant in my opinion especially since we have Select Popover state tests.

thetaPC · 2026-06-15T18:00:23Z

+        await expect(orangesOption.locator('ion-radio')).toBeFocused();
+        await expect(orangesOption).toHaveClass(/ion-focused/);
+
+        await expect(popover).toHaveScreenshot(screenshot(`select-basic-popover-opened-with-value-focused`));


Do we need a snapshot when you've already tested for focus? The snapshots seem redundant in my opinion especially since we have Select Popover state tests.

thetaPC · 2026-06-15T18:02:57Z

+        await expect(firstOption).not.toHaveClass(/ion-focused/);
+      });
+
+      test('it should scroll to selected option when opened', async ({ page }) => {


This test feels like it should live under select-popover instead.

thetaPC · 2026-06-15T18:05:14Z

+        await expect(secondOption.locator('ion-radio')).toBeFocused();
+        await expect(secondOption).toHaveClass(/ion-focused/);
+
+        await expect(modal).toHaveScreenshot(screenshot(`select-basic-modal-opened-focused`));


Do we need a snapshot when you've already tested for focus? The snapshots seem redundant in my opinion especially since we have Select Modal state tests.

thetaPC · 2026-06-15T18:05:35Z

+        await expect(orangesOption.locator('ion-radio')).toBeFocused();
+        await expect(orangesOption).toHaveClass(/ion-focused/);
+
+        await expect(modal).toHaveScreenshot(screenshot(`select-basic-modal-opened-with-value-focused`));


Do we need a snapshot when you've already tested for focus? The snapshots seem redundant in my opinion especially since we have Select Modal state tests.

fix(overlays): properly focus elements in a sheet modal

641d05e

brandyscarney requested a review from a team as a code owner May 22, 2026 16:22

brandyscarney requested a review from thetaPC May 22, 2026 16:22

brandyscarney marked this pull request as draft May 22, 2026 16:22

github-actions Bot added the package: core @ionic/core package label May 22, 2026

vercel Bot deployed to Preview May 22, 2026 16:28 View deployment

style: lint

3278a01

vercel Bot deployed to Preview May 22, 2026 16:58 View deployment

brandyscarney added 4 commits May 29, 2026 14:10

fix(item): add the focus styles for ionic theme

808c193

fix(select): hide the focus ring when initially opening an interface

ceecf4d

test(select): add ionic theme to basic test

a3377a3

chore(): add updated snapshots

17983b1

vercel Bot deployed to Preview June 1, 2026 14:44 View deployment

brandyscarney added 2 commits June 1, 2026 11:42

test(select): add tests to verify interface focused behavior on click

add3c34

chore(): add updated snapshots

31665fa

brandyscarney force-pushed the FW-6922 branch from 3fb8b4c to 31665fa Compare June 1, 2026 15:49

vercel Bot deployed to Preview June 1, 2026 15:55 View deployment

brandyscarney commented Jun 1, 2026

View reviewed changes

brandyscarney added 4 commits June 1, 2026 18:27

fix(select): suppress the focus visible for alert only

1aa5b7c

fix(checkbox): hide the native focus state to match radio

641419f

test(select): add tests for keyboard focus behavior and remove skips

395987c

chore(): add updated snapshots

ea55440

chore(): add updated snapshots

4bd07bf

vercel Bot deployed to Preview June 1, 2026 23:36 View deployment

brandyscarney commented Jun 2, 2026

View reviewed changes

Merge branch 'next' into FW-6922

7abfe13

vercel Bot deployed to Preview June 2, 2026 14:42 View deployment

chore(): add updated snapshots

1d0b267

vercel Bot deployed to Preview June 2, 2026 15:00 View deployment

chore(): revert snapshots

29e7e8e

vercel Bot deployed to Preview June 2, 2026 15:06 View deployment

fix(overlays): properly navigate between action sheet options

a11a5c9

vercel Bot deployed to Preview June 2, 2026 17:53 View deployment

brandyscarney changed the title ~~fix(overlays): properly focus elements in a sheet modal~~ fix(overlays): show focus indicators only during keyboard navigation and improve focus management Jun 2, 2026

brandyscarney marked this pull request as ready for review June 2, 2026 18:27

ShaneK requested changes Jun 3, 2026

View reviewed changes

Comment thread core/src/utils/overlays.ts Outdated

Comment thread core/src/components/checkbox/checkbox.tsx

Comment thread core/src/components/select/test/basic/select.e2e.ts

brandyscarney added 2 commits June 12, 2026 15:01

refactor(utils): update focus trap to be component-agnostic

9e200ed

docs(component-guide): add a section on the new overlay focus markers

272c7f7

vercel Bot deployed to Preview June 12, 2026 19:19 View deployment

Merge branch 'next' into FW-6922

f4a33db

vercel Bot deployed to Preview June 12, 2026 21:38 View deployment

vercel Bot deployed to Preview June 12, 2026 21:45 View deployment

brandyscarney and others added 2 commits June 12, 2026 18:00

refactor(item): remove old styles

7951ac3

chore(): add updated snapshots

26048b9

brandyscarney force-pushed the FW-6922 branch from 72030b2 to 26048b9 Compare June 12, 2026 22:00

brandyscarney requested a review from ShaneK June 12, 2026 22:02

vercel Bot deployed to Preview June 12, 2026 22:06 View deployment

chore(): add updated snapshots

86edcb5

vercel Bot deployed to Preview June 12, 2026 22:31 View deployment

ShaneK approved these changes Jun 15, 2026

View reviewed changes

thetaPC requested changes Jun 15, 2026

View reviewed changes

	// An item is focusable when it can receive keyboard focus: when it is
	// An item is focusable when it can receive keyboard focus, when it is

Conversation

brandyscarney commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the current behavior?

What is the new behavior?

Does this introduce a breaking change?

Other information

Uh oh!

vercel Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brandyscarney Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaneK Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brandyscarney Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaneK left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ShaneK left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thetaPC left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brandyscarney commented May 22, 2026 •

edited

Loading

vercel Bot commented May 22, 2026 •

edited

Loading

brandyscarney Jun 2, 2026 •

edited

Loading

ShaneK Jun 15, 2026 •

edited

Loading

brandyscarney Jun 2, 2026 •

edited

Loading

ShaneK left a comment •

edited

Loading