fix(overlays): show focus indicators only during keyboard navigation and improve focus management#31165
fix(overlays): show focus indicators only during keyboard navigation and improve focus management#31165brandyscarney wants to merge 18 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| @@ -114,10 +114,6 @@ slot[name="end"]::slotted(*) { | |||
|
|
|||
| // Item in Select Modal | |||
| // -------------------------------------------------- | |||
| :host(.in-select-modal) { | |||
There was a problem hiding this comment.
This was added to hide the focused state.
| @@ -15,11 +15,6 @@ ion-item { | |||
| --border-width: 0; | |||
| } | |||
|
|
|||
| ion-item.ion-focused::part(native)::after { | |||
There was a problem hiding this comment.
This was added to hide the focused state.
There was a problem hiding this comment.
The ionic theme wasn't added to this test so now it is generating screenshots.
There was a problem hiding this comment.
The focus indicator is no longer displayed when opening with a mouse click.
There was a problem hiding this comment.
The focus indicator is no longer displayed when opening with a mouse click.
There was a problem hiding this comment.
The focus indicator is no longer displayed when opening with a mouse click.
There was a problem hiding this comment.
The focus indicator is no longer displayed when opening with a mouse click.
| @@ -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 }) => { | |||
There was a problem hiding this comment.
This now checks for the ionic theme and captures screenshots to avoid future focus behavior regressions.
| 'checkbox-checked': checked, | ||
| 'checkbox-disabled': disabled, | ||
| 'ion-focusable': true, | ||
| // Focus styling should not apply when the checkbox is in an item | ||
| 'ion-focusable': !inItem, |
There was a problem hiding this comment.
This matches radio behavior. Otherwise when a checkbox in an item has focus (such as inside of a select popover) the item and checkbox will both show a focus indicator.
There was a problem hiding this comment.
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?
| 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.'); | ||
|
|
There was a problem hiding this comment.
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.
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:
What is the new behavior?
Shift + Tabto go backwards in orderDoes this introduce a breaking change?
Other information