Chinese
Shawarma
Pizza
@@ -69,4 +70,38 @@ const KeyboardNavigationTemplate = () => html`
Button 4
`;
+const DynamicSlotsTemplate = () => {
+ let counter = 2;
+
+ // Function to dynamically add radio buttons to the radio group
+ const onAddButtonClick = () => {
+ const radioGroup = document.querySelector('pie-radio-group[data-test-id="radio-group-1"]');
+
+ if (radioGroup) {
+ const newRadio = document.createElement('pie-radio');
+ newRadio.setAttribute('value', counter.toString());
+ newRadio.textContent = `Option ${counter}`;
+
+ radioGroup.appendChild(newRadio);
+
+ counter++;
+ }
+ };
+
+ return html`
+
+ Option 1
+
+
+
+
+
+ Add another option
+
+
+ `;
+};
+
export const KeyboardNavigation = createStory(KeyboardNavigationTemplate, defaultArgs)();
+export const DynamicSlots = createStory(DynamicSlotsTemplate, defaultArgs)();
+
diff --git a/packages/components/pie-radio-group/src/index.ts b/packages/components/pie-radio-group/src/index.ts
index 47dbe72979..b02e234538 100644
--- a/packages/components/pie-radio-group/src/index.ts
+++ b/packages/components/pie-radio-group/src/index.ts
@@ -130,11 +130,22 @@ export class PieRadioGroup extends FormControlMixin(RtlMixin(LitElement)) implem
* @param {Event} e - The slotchange event.
* @private
*/
- private _handleSlotChange (e: { target: HTMLSlotElement }): void {
+ private _handleLabelSlotChange (e: { target: HTMLSlotElement }): void {
const childNodes = e.target.assignedNodes({ flatten: true });
this._hasLabel = childNodes.length > 0;
}
+ /**
+ * Ensures all newly added radio buttons are not tabbable and inherit the name property
+ */
+ private _handleRadioSlotChange (): void {
+ // Make all (including any newly added) radio buttons impossible to tab to
+ // This is because by default, we are able to tab to each individual radio button.
+ // This is not the behaviour we want, so applying -1 tabindex prevents it.
+ this._slottedChildren.forEach((radio) => radio.setAttribute('tabindex', '-1'));
+ this._applyNameToChildren();
+ }
+
/**
* Renders the label element inside a legend, wrapping the slot content.
* @returns {TemplateResult } The template for the label slot.
@@ -142,8 +153,16 @@ export class PieRadioGroup extends FormControlMixin(RtlMixin(LitElement)) implem
*/
private _renderWrappedLabel (): TemplateResult {
return this._hasLabel
- ? html``
- : html``;
+ ? html``
+ : html``;
+ }
+
+ private _applyNameToChildren () : void {
+ this._slottedChildren.forEach((radio) => {
+ if (this.name) {
+ radio.setAttribute('name', this.name);
+ }
+ });
}
protected updated (_changedProperties: PropertyValues): void {
@@ -158,13 +177,10 @@ export class PieRadioGroup extends FormControlMixin(RtlMixin(LitElement)) implem
if (_changedProperties.has('status')) {
this._handleStatus();
}
- }
- protected firstUpdated (): void {
- // Make all radios impossible to tab to
- // This is because by default, we are able to tab to each individual radio button.
- // This is not the behaviour we want, so applying -1 tabindex prevents it.
- this._slottedChildren.forEach((radio) => radio.setAttribute('tabindex', '-1'));
+ if (_changedProperties.has('name')) {
+ this._applyNameToChildren();
+ }
}
connectedCallback (): void {
@@ -178,7 +194,13 @@ export class PieRadioGroup extends FormControlMixin(RtlMixin(LitElement)) implem
this.addEventListener('focusout', this._handleFocusOut, { signal });
this.addEventListener('keydown', this._handleKeyDown, { signal });
+
+ // Warning! One edge case bug we've noticed is if the radio group is the first focusable element in the document,
+ // and you shift + tab out of it, when tabbing back in, it will focus the last radio instead of the first.
+ // Given it is quite an edge case, we will leave it for now.
document.addEventListener('keydown', this._updateShiftTabState.bind(this), { signal });
+
+ this._applyNameToChildren();
}
/**
@@ -342,7 +364,7 @@ export class PieRadioGroup extends FormControlMixin(RtlMixin(LitElement)) implem
aria-describedby=${hasAssistiveText ? assistiveTextId : nothing}
class="${classMap(classes)}">
${this._renderWrappedLabel()}
-
+
${hasAssistiveText ? html`
=> {
@@ -59,19 +66,65 @@ const expectFocusedRadioToBeChecked = async (page: Page, expect: Expect): Promis
test.describe('PieRadioGroup - Component tests new', () => {
let pageObject;
- test.describe('Keyboard navigation', () => {
- test.describe('Tab', () => {
- test.beforeEach(async ({ page }) => {
+ test.describe('props', () => {
+ test.describe('Name', () => {
+ test('Name prop is passed down to all slotted radio buttons', async ({ page }) => {
+ const expectedName = 'radio-group-1';
pageObject = new BasePage(page, 'radio-group');
await pageObject.load();
+
+ const radioSelectors = Object.keys(keyboardNavigationStorySelectors.radioGroup1.radios).map(Number) as Array;
+
+ await Promise.all(radioSelectors.map(async (radioSelectorKey) => {
+ const selector = keyboardNavigationStorySelectors.radioGroup1.radios[radioSelectorKey];
+ const radio = page.getByTestId(selector);
+
+ await expect(radio).toHaveAttribute('name', expectedName);
+ }));
});
+ test('Name prop is passed down to any dynamically added radio buttons', async ({ page }) => {
+ const expectedName = 'radio-group-1';
+
+ pageObject = new BasePage(page, 'radio-group--dynamic-slots');
+ await pageObject.load();
+
+ const addOptionsBtn = page.getByTestId(dynamicSlotsStorySelectors.addOptionBtn);
+
+ // Create 3 new options
+ await addOptionsBtn.click();
+ await addOptionsBtn.click();
+ await addOptionsBtn.click();
+
+ // Ensure correct number of radios
+ const radioButtons = page.getByTestId(dynamicSlotsStorySelectors.radioGroup1.self).locator('pie-radio');
+ await expect(radioButtons).toHaveCount(4);
+
+ const radioButtonsCount = await radioButtons.count();
+
+ const validateRadioButtonName = async (index: number) => {
+ const radio = radioButtons.nth(index);
+ await expect(radio).toHaveAttribute('name', expectedName);
+ };
+
+ await Promise.all(Array.from({ length: radioButtonsCount }, (_, i) => validateRadioButtonName(i)));
+ });
+ });
+ });
+
+ test.describe('Keyboard navigation', () => {
+ test.beforeEach(async ({ page }) => {
+ pageObject = new BasePage(page, 'radio-group');
+ await pageObject.load();
+ });
+
+ test.describe('Tab', () => {
test('Tab and no option selected focuses the first radio', async ({ page }) => {
// Tab 2 times to go button 1 -> Radio group 1
await page.keyboard.press('Tab');
await page.keyboard.press('Tab');
- const radio = page.getByTestId(selectors.radioGroup1.radios[1]);
+ const radio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[1]);
await expect(radio).toBeFocused();
});
@@ -83,7 +136,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]);
+ const radio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup2.radios[4]);
await expect(radio).toBeFocused();
});
@@ -94,7 +147,7 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('Tab');
await page.keyboard.press('Tab');
- const button = page.getByTestId(selectors.button2);
+ const button = page.getByTestId(keyboardNavigationStorySelectors.button2);
await expect(button).toBeFocused();
});
@@ -114,7 +167,7 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('Shift+Tab');
- const radio = page.getByTestId(selectors.radioGroup1.radios[5]);
+ const radio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[5]);
await expect(radio).toBeFocused();
});
@@ -130,7 +183,7 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('Shift+Tab');
- const radio = page.getByTestId(selectors.radioGroup2.radios[4]);
+ const radio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup2.radios[4]);
await expect(radio).toBeFocused();
});
@@ -144,7 +197,7 @@ test.describe('PieRadioGroup - Component tests new', () => {
await page.keyboard.press('Shift+Tab');
await page.keyboard.press('Shift+Tab');
- const button = page.getByTestId(selectors.button1);
+ const button = page.getByTestId(keyboardNavigationStorySelectors.button1);
await expect(button).toBeFocused();
});
@@ -164,13 +217,13 @@ 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]);
+ const lastRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
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]);
+ const secondToLastRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[4]);
await expect(secondToLastRadio).toBeFocused();
await expectFocusedRadioToBeChecked(page, expect);
});
@@ -183,13 +236,13 @@ 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]);
+ const lastRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
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]);
+ const secondToLastRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[4]);
await expect(secondToLastRadio).toBeFocused();
await expectFocusedRadioToBeChecked(page, expect);
});
@@ -206,14 +259,14 @@ 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]);
+ const lastRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
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]);
+ const firstRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[1]);
await expect(firstRadio).toBeFocused();
await expectFocusedRadioToBeChecked(page, expect);
});
@@ -230,14 +283,14 @@ 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]);
+ const lastRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
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]);
+ const firstRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[1]);
await expect(firstRadio).toBeFocused();
await expectFocusedRadioToBeChecked(page, expect);
});
@@ -250,7 +303,7 @@ 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]);
+ const secondRadioButtonGroup1 = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[2]);
await expect(secondRadioButtonGroup1).toBeFocused();
await expectFocusedRadioToBeChecked(page, expect);
@@ -261,7 +314,7 @@ 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]);
+ const lastRadioButtonGroup2 = page.getByTestId(keyboardNavigationStorySelectors.radioGroup2.radios[5]);
await expect(lastRadioButtonGroup2).toBeFocused();
await expectFocusedRadioToBeChecked(page, expect);
@@ -295,14 +348,14 @@ 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]);
+ const lastRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
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]);
+ const firstRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[1]);
await expect(firstRadio).toBeFocused();
await expectFocusedRadioToBeChecked(page, expect);
});
@@ -315,13 +368,13 @@ 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]);
+ const lastRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
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]);
+ const secondToLastRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[4]);
await expect(secondToLastRadio).toBeFocused();
await expectFocusedRadioToBeChecked(page, expect);
});
@@ -334,13 +387,13 @@ 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]);
+ const lastRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
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]);
+ const secondToLastRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[4]);
await expect(secondToLastRadio).toBeFocused();
await expectFocusedRadioToBeChecked(page, expect);
});
@@ -357,14 +410,14 @@ 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]);
+ const lastRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[5]);
await expect(lastRadio).toBeFocused();
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]);
+ const firstRadio = page.getByTestId(keyboardNavigationStorySelectors.radioGroup1.radios[1]);
await expect(firstRadio).toBeFocused();
await expectFocusedRadioToBeChecked(page, expect);
});
diff --git a/packages/tools/pie-monorepo-utils/changeset-snapshot/test-aperture.js b/packages/tools/pie-monorepo-utils/changeset-snapshot/test-aperture.js
index 5f76b37877..e3847749f0 100644
--- a/packages/tools/pie-monorepo-utils/changeset-snapshot/test-aperture.js
+++ b/packages/tools/pie-monorepo-utils/changeset-snapshot/test-aperture.js
@@ -1,29 +1,40 @@
/* eslint-disable camelcase */
module.exports = async ({ github, context }, execa) => {
- await execa.command('yarn changeset:version --snapshot snapshot-release', { stdio: 'inherit' });
+ // Helper function to handle errors and post GitHub comments
+ const handleError = async (message, error) => {
+ console.error(`${message}:`, error);
+ await github.rest.issues.createComment({
+ issue_number: context.issue.number,
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ body: `${message}:\n\`\`\`\n${error.message || error}\n\`\`\``,
+ });
+ };
- const releaseProcess = execa.command('yarn changeset:publish --no-git-tags --snapshot --tag snapshot-release');
- releaseProcess.stdout.pipe(process.stdout);
+ try {
+ await execa.command('yarn changeset:version --snapshot snapshot-release', { stdio: 'inherit' });
- const { stdout } = await releaseProcess;
+ const releaseProcess = execa.command('yarn changeset:publish --no-git-tags --snapshot --tag snapshot-release');
+ releaseProcess.stdout.pipe(process.stdout);
- const newTags = Array
- .from(stdout.matchAll(/New tag:\s+([^\s\n]+)/g))
- .map(([, tag]) => tag)
- .filter((tag) => !/^wc-.+$|pie-(monorepo|docs|storybook)/.test(tag));
+ const { stdout } = await releaseProcess;
- // Extract the snapshot version from one of the tags
- const [snapshotVersion] = newTags[0].match(/\d{14}$/);
+ const newTags = Array
+ .from(stdout.matchAll(/New tag:\s+([^\s\n]+)/g))
+ .map(([, tag]) => tag)
+ .filter((tag) => !/^wc-.+$|pie-(monorepo|docs|storybook)/.test(tag));
- // Extract package names by removing version and scope from the tags
- const packageNames = newTags.map((tag) => `@justeattakeaway/${tag.match(/pie-[\w-]+/)[0]}`);
+ // Extract the snapshot version from one of the tags
+ if (newTags.length === 0) {
+ throw new Error('No changed packages found! Please make sure you have added a changeset entry for the packages you would like to snapshot.');
+ }
- let body;
+ const [snapshotVersion] = newTags[0].match(/\d{14}$/);
+ const packageNames = newTags.map((tag) => `@justeattakeaway/${tag.match(/pie-[\w-]+/)[0]}`);
- if (newTags.length > 0) {
const multiple = newTags.length > 1;
- body = `@${context.actor} Your snapshot${multiple ? 's have' : ' has'} been published to npm!\n\nTest the snapshot${multiple ? 's' : ''} by updating your \`package.json\` with the newly-published version${multiple ? 's' : ''}:\n`;
+ let body = `@${context.actor} Your snapshot${multiple ? 's have' : ' has'} been published to npm!\n\nTest the snapshot${multiple ? 's' : ''} by updating your \`package.json\` with the newly-published version${multiple ? 's' : ''}:\n`;
if (multiple) {
body += `> [!NOTE]\n> If you have more than one of these packages installed, we suggest using the new snapshots for all of them to help avoid version conflicts.\n\n${newTags.map((tag) => `\`\`\`sh\nyarn up ${tag} --mode=update-lockfile\n\`\`\``).join('\n')}\nThen finally:\n\`\`\`sh\nyarn install\n\`\`\``;
@@ -45,23 +56,17 @@ module.exports = async ({ github, context }, execa) => {
},
});
} catch (error) {
- console.error(`Failed to dispatch workflow: ${error.message}`);
- await github.rest.issues.createComment({
- issue_number: context.issue.number,
- owner: context.repo.owner,
- repo: context.repo.repo,
- body: `Failed to trigger PIE Aperture workflow: ${error.message}`,
- });
+ await handleError('Failed to dispatch workflow', error);
}
- } else {
- body = 'No changed packages found! Please make sure you have added a changeset entry for the packages you would like to snapshot.';
- }
- // Create a GitHub comment with the update instructions
- await github.rest.issues.createComment({
- issue_number: context.issue.number,
- owner: context.repo.owner,
- repo: context.repo.repo,
- body,
- });
+ // Create a GitHub comment with the update instructions
+ await github.rest.issues.createComment({
+ issue_number: context.issue.number,
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ body,
+ });
+ } catch (error) {
+ await handleError('An unexpected error occurred during the snapshot release process', error);
+ }
};