Skip to content

Commit

Permalink
feat(pie-radio-group): DSW-2634 announce radio button index in groups
Browse files Browse the repository at this point in the history
  • Loading branch information
@jamieomaguire
jamieomaguire committed Jan 7, 2025
1 parent d7370ee commit 934a5f2
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 51 deletions.
6 changes: 6 additions & 0 deletions .changeset/two-boxes-lie.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@justeattakeaway/pie-radio-group": minor
"@justeattakeaway/pie-radio": minor
---

[Fixed] - Ensure screenreaders correctly announce radio index in groups
2 changes: 2 additions & 0 deletions packages/components/pie-radio-group/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ export class PieRadioGroup extends FormControlMixin(RtlMixin(LitElement)) implem

private _focusAndClickOption (option: HTMLInputElement): void {
option.focus();
console.log(document.activeElement);

Check warning on line 306 in packages/components/pie-radio-group/src/index.ts

View workflow job for this annotation

GitHub Actions / lint-js 

Unexpected console statement
// This is quite hacky, but it ensures the radio elements correct emit a real change event.
// Simply setting option.checked as true would require re-architecture of both this component and the radio button
// to ensure that property changes are observed and correctly propagated up.
Expand Down Expand Up @@ -334,6 +335,7 @@ export class PieRadioGroup extends FormControlMixin(RtlMixin(LitElement)) implem

return html`
<fieldset
role="radiogroup"
tabindex="0"
name=${ifDefined(name)}
?disabled=${disabled}
Expand Down
112 changes: 69 additions & 43 deletions packages/components/pie-radio-group/test/component/pie-radio-group-new.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { test, expect } from '@playwright/test';
import {
test, expect, type Page, type Expect,
} from '@playwright/test';
import { BasePage } from '@justeattakeaway/pie-webc-testing/src/helpers/page-object/base-page.ts';
import { type PieRadio } from '@justeattakeaway/pie-radio';

// The structure of this object reflects the structure of our test Story
const selectors = {
Expand Down Expand Up @@ -29,6 +32,30 @@ const selectors = {
button4: 'btn-4',
};

// Returns the checked state of the currently focused pie-radio on the page
// Will error if the active element on the page is not a pie-radio instance
const expectFocusedRadioToBeChecked = async (page: Page, expect: Expect): Promise<void> => {
const result = await page.evaluate(() => {
const queryResult: { checked: boolean, error?: boolean } = {
checked: false,
};

if (document.activeElement?.nodeName === 'PIE-RADIO') {
queryResult.checked = (document.activeElement as PieRadio)?.checked;
} else {
queryResult.error = true;
}

return queryResult;
});

if (result.error) {
throw new Error('The currently focused element was not an instance of PIE-RADIO.');
}

await expect(result.checked).toBe(true);
};

test.describe('PieRadioGroup - Component tests new', () => {
let pageObject;

Expand All @@ -44,8 +71,7 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('Tab');
await page.keyboard.press('Tab');

const radio = page.getByTestId(selectors.radioGroup1.radios[1]).locator('input');

const radio = page.getByTestId(selectors.radioGroup1.radios[1]);
await expect(radio).toBeFocused();
});

Expand All @@ -57,7 +83,7 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('Tab');
await page.keyboard.press('Tab');

const radio = page.getByTestId(selectors.radioGroup2.radios[4]).locator('input');
const radio = page.getByTestId(selectors.radioGroup2.radios[4]);

await expect(radio).toBeFocused();
});
Expand Down Expand Up @@ -88,7 +114,7 @@ test.describe('PieRadioGroup - Component tests new', () => {

await page.keyboard.press('Shift+Tab');

const radio = page.getByTestId(selectors.radioGroup1.radios[5]).locator('input');
const radio = page.getByTestId(selectors.radioGroup1.radios[5]);

await expect(radio).toBeFocused();
});
Expand All @@ -104,7 +130,7 @@ test.describe('PieRadioGroup - Component tests new', () => {

await page.keyboard.press('Shift+Tab');

const radio = page.getByTestId(selectors.radioGroup2.radios[4]).locator('input');
const radio = page.getByTestId(selectors.radioGroup2.radios[4]);

await expect(radio).toBeFocused();
});
Expand Down Expand Up @@ -138,15 +164,15 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('ArrowLeft');

// Ensure it's gone backwards and selected the last radio
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]).locator('input');
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
await expect(lastRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);

// Ensure it's gone backwards once more and selected the second to last radio
await page.keyboard.press('ArrowLeft');
const secondToLastRadio = page.getByTestId(selectors.radioGroup1.radios[4]).locator('input');
const secondToLastRadio = page.getByTestId(selectors.radioGroup1.radios[4]);
await expect(secondToLastRadio).toBeFocused();
await expect(secondToLastRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);
});

test('Up Arrow goes backwards through radios and loops', async ({ page }) => {
Expand All @@ -157,15 +183,15 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('ArrowUp');

// Ensure it's gone backwards and selected the last radio
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]).locator('input');
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
await expect(lastRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);

// Ensure it's gone backwards once more and selected the second to last radio
await page.keyboard.press('ArrowUp');
const secondToLastRadio = page.getByTestId(selectors.radioGroup1.radios[4]).locator('input');
const secondToLastRadio = page.getByTestId(selectors.radioGroup1.radios[4]);
await expect(secondToLastRadio).toBeFocused();
await expect(secondToLastRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);
});

test('Right Arrow goes forwards through radios and loops', async ({ page }) => {
Expand All @@ -180,16 +206,16 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('ArrowRight');

// Ensure we are on the last radio
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]).locator('input');
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
await expect(lastRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);

// Press Arrow Right 1 more time to get back to the first radio
await page.keyboard.press('ArrowRight');

const firstRadio = page.getByTestId(selectors.radioGroup1.radios[1]).locator('input');
const firstRadio = page.getByTestId(selectors.radioGroup1.radios[1]);
await expect(firstRadio).toBeFocused();
await expect(firstRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);
});

test('Down Arrow goes forwards through radios and loops', async ({ page }) => {
Expand All @@ -204,16 +230,16 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('ArrowDown');

// Ensure we are on the last radio
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]).locator('input');
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
await expect(lastRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);

// Press Arrow Down 1 more time to get back to the first radio
await page.keyboard.press('ArrowDown');

const firstRadio = page.getByTestId(selectors.radioGroup1.radios[1]).locator('input');
const firstRadio = page.getByTestId(selectors.radioGroup1.radios[1]);
await expect(firstRadio).toBeFocused();
await expect(firstRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);
});

test('Arrow key selection in one Group does not affect the other', async ({ page }) => {
Expand All @@ -224,9 +250,9 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('ArrowRight');

// Ensure it's gone forwards and selected the second radio
const secondRadioButtonGroup1 = page.getByTestId(selectors.radioGroup1.radios[2]).locator('input');
const secondRadioButtonGroup1 = page.getByTestId(selectors.radioGroup1.radios[2]);
await expect(secondRadioButtonGroup1).toBeFocused();
await expect(secondRadioButtonGroup1).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);

// Move to next radio group
await page.keyboard.press('Tab');
Expand All @@ -235,9 +261,9 @@ test.describe('PieRadioGroup - Component tests new', () => {

// Press right to move from the selected radio to the next
await page.keyboard.press('ArrowRight');
const lastRadioButtonGroup2 = page.getByTestId(selectors.radioGroup2.radios[5]).locator('input');
const lastRadioButtonGroup2 = page.getByTestId(selectors.radioGroup2.radios[5]);
await expect(lastRadioButtonGroup2).toBeFocused();
await expect(lastRadioButtonGroup2).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);

// Press Shift + Tab 3 times to get back into the first radio group
await page.keyboard.press('Shift+Tab');
Expand All @@ -246,7 +272,7 @@ test.describe('PieRadioGroup - Component tests new', () => {

// Ensure the previously selected radio in the first group is focused and still selected
await expect(secondRadioButtonGroup1).toBeFocused();
await expect(secondRadioButtonGroup1).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);
});
});

Expand All @@ -269,16 +295,16 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('ArrowLeft');

// Ensure we are on the last radio
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]).locator('input');
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
await expect(lastRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);

// Press Arrow Left 1 more time to get back to the first radio
await page.keyboard.press('ArrowLeft');

const firstRadio = page.getByTestId(selectors.radioGroup1.radios[1]).locator('input');
const firstRadio = page.getByTestId(selectors.radioGroup1.radios[1]);
await expect(firstRadio).toBeFocused();
await expect(firstRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);
});

test('Up Arrow goes backwards through radios and loops', async ({ page }) => {
Expand All @@ -289,15 +315,15 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('ArrowUp');

// Ensure it's gone backwards and selected the last radio
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]).locator('input');
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
await expect(lastRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);

// Ensure it's gone backwards once more and selected the second to last radio
await page.keyboard.press('ArrowUp');
const secondToLastRadio = page.getByTestId(selectors.radioGroup1.radios[4]).locator('input');
const secondToLastRadio = page.getByTestId(selectors.radioGroup1.radios[4]);
await expect(secondToLastRadio).toBeFocused();
await expect(secondToLastRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);
});

test('Right Arrow goes backwards through radios and loops', async ({ page }) => {
Expand All @@ -308,15 +334,15 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('ArrowRight');

// Ensure it's gone backwards and selected the last radio
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]).locator('input');
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
await expect(lastRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);

// Ensure it's gone backwards once more and selected the second to last radio
await page.keyboard.press('ArrowRight');
const secondToLastRadio = page.getByTestId(selectors.radioGroup1.radios[4]).locator('input');
const secondToLastRadio = page.getByTestId(selectors.radioGroup1.radios[4]);
await expect(secondToLastRadio).toBeFocused();
await expect(secondToLastRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);
});

test('Down Arrow goes forwards through radios and loops', async ({ page }) => {
Expand All @@ -331,16 +357,16 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('ArrowDown');

// Ensure we are on the last radio
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]).locator('input');
const lastRadio = page.getByTestId(selectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
await expect(lastRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);

// Press Arrow Down 1 more time to get back to the first radio
await page.keyboard.press('ArrowDown');

const firstRadio = page.getByTestId(selectors.radioGroup1.radios[1]).locator('input');
const firstRadio = page.getByTestId(selectors.radioGroup1.radios[1]);
await expect(firstRadio).toBeFocused();
await expect(firstRadio).toBeChecked();
await expectFocusedRadioToBeChecked(page, expect);
});
});
});
Expand Down
9 changes: 5 additions & 4 deletions packages/components/pie-radio/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export class PieRadio extends FormControlMixin(RtlMixin(LitElement)) implements
super.connectedCallback();
this._abortController = new AbortController();
const { signal } = this._abortController;

this.setAttribute('role', 'radio');
this.addEventListener('pie-radio-group-disabled', (e: CustomEventInit) => { this._disabledByParent = e.detail.disabled; }, { signal });
}

Expand Down Expand Up @@ -95,9 +95,9 @@ export class PieRadio extends FormControlMixin(RtlMixin(LitElement)) implements
this._handleFormAssociation();
}

public focus () {
this._radio.focus();
}
// public focus () {
// this._radio.focus();
// }

/**
* (Read-only) returns a ValidityState with the validity states that this element is in.
Expand Down Expand Up @@ -151,6 +151,7 @@ export class PieRadio extends FormControlMixin(RtlMixin(LitElement)) implements
return html`
<label class=${classMap(classes)} for="radioId">
<input
tabindex="-1"
class="c-radio-input"
type="radio"
id="radioId"
Expand Down
26 changes: 22 additions & 4 deletions packages/components/pie-radio/src/radio.scss
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
@use '@justeattakeaway/pie-css/scss' as p;

// :host(:focus) {
// background: red;
// }

@mixin radio-interactive-state($bg-color) {
.c-radio-input {
&:hover:not(:checked, :disabled) {
Expand Down Expand Up @@ -50,6 +54,8 @@

@include radio-interactive-state('dt-color-interactive-brand');



&.c-radio--disabled {
--radio-cursor: not-allowed;
}
Expand All @@ -61,6 +67,10 @@
@include radio-interactive-state('dt-color-support-error');
}

// &:focus .c-radio-input {
// @include p.focus;
// }

.c-radio-input {
appearance: none;
display: block;
Expand All @@ -75,9 +85,9 @@
transition: background-color var(--dt-motion-timing-100) var(--radio-motion-easing);
flex-shrink: 0;

&:focus {
@include p.focus;
}
// &:focus {
// @include p.focus;
// }

// The filled circle before checking the radio
&:before {
Expand Down Expand Up @@ -147,4 +157,12 @@
--radio-bg-color--checked: var(--dt-color-disabled-01);
}
}
}
}

:host(:focus) {
outline: none;

.c-radio .c-radio-input {
@include p.focus;
}
}

0 comments on commit 934a5f2

Please sign in to comment.