8000 test(radio): re-enable keyboard navigation (#28747) · ionic-team/ionic-framework@204a861 · GitHub
[go: up one dir, main page]

Skip to content

Commit 204a861

Browse files
authored
test(radio): re-enable keyboard navigation (#28747)
Issue number: internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> The keyboard navigation tests for radio were disable due to flakiness with Safari when it came to the CI. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Re-enabled the tests. Debugging was done with a saved artifact. The artifact didn't provide a clear reason of why it flakes. But it did seem that the test was tabbing before the Safari page finished loading. I've added a `waitFor()` to verify that the radios have rendered. This was done for Safari only to prevent any additional wait time. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> There is no great way to test this since it only flakes on GitHub.
1 parent dc1dd9c commit 204a861

File tree

2 files changed

+33
-5
lines changed

2 files changed

+33
-5
lines changed

core/src/components/radio/test/a11y/radio.e2e.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,8 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
1515
});
1616
});
1717

18-
// TODO(FW-5715): re-enable test
19-
test.skip(title('radio: keyboard navigation'), () => {
20-
test.beforeEach(async ({ page }) => {
18+
test.describe(title('radio: keyboard navigation'), () => {
19+
test.beforeEach(async ({ page, browserName }) => {
2120
await page.setContent(
2221
`
2322
<ion-app>
@@ -59,6 +58,21 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
5958
`,
6059
config
6160
);
61+
62+
if (browserName === 'webkit') {
63+
const radio = page.locator('#first-group ion-radio').first();
64+
/**
65+
* Sometimes Safari does not focus the first radio.
66+
* This is a workaround to ensure the first radio is focused.
67+
*
68+
* Wait for the first radio to be rendered before tabbing.
69+
* This is necessary because the first radio may not be rendered
70+
* when the page first loads.
71+
*
72+
* This would cause the first radio to be skipped when tabbing.
73+
*/
74+
await radio.waitFor();
75+
}
6276
});
6377

6478
test('tabbing should switch between radio groups', async ({ page, pageUtils }) => {

core/src/components/radio/test/legacy/a11y/radio.e2e.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,23 @@ import { configs, test } from '@utils/test/playwright';
66
*/
77
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
88
test.describe(title('radio: a11y'), () => {
9-
test.beforeEach(async ({ page, skip }) => {
10-
skip.browser('webkit', 'Tabbing is flaky in Safari');
9+
test.beforeEach(async ({ page, browserName }) => {
1110
await page.goto(`/src/components/radio/test/legacy/a11y`, config);
11+
12+
if (browserName === 'webkit') {
13+
const radio = page.locator('#first-group ion-radio').first();
14+
/**
15+
* Sometimes Safari does not focus the first radio.
16+
* This is a workaround to ensure the first radio is focused.
17+
*
18+
* Wait for the first radio to be rendered before tabbing.
19+
* This is necessary because the first radio may not be rendered
20+
* when the page first loads.
21+
*
22+
* This would cause the first radio to be skipped when tabbing.
23+
*/
24+
await radio.waitFor();
25+
}
1226
});
1327
test('tabbing should switch between radio groups', async ({ page, pageUtils }) => {
1428
const firstGroupRadios = page.locator('#first-group ion-radio');

0 commit comments

Comments
 (0)
0