diff --git a/.changeset/keyboard-navigation-option-groups.md b/.changeset/keyboard-navigation-option-groups.md new file mode 100644 index 000000000000..8f8994ebaf1b --- /dev/null +++ b/.changeset/keyboard-navigation-option-groups.md @@ -0,0 +1,13 @@ +--- +"@db-ux/core-components": patch +"@db-ux/ngx-core-components": patch +"@db-ux/react-core-components": patch +"@db-ux/v-core-components": patch +"@db-ux/wc-core-components": patch +--- + +- fix(custom-select): keyboard navigation for option groups in single-select mode: + - Fixes a keyboard accessibility issue where users could not navigate to options in subsequent option groups using arrow keys in single-select mode. + - Now, all options are accessible via keyboard regardless of group boundaries. + + diff --git a/packages/components/src/components/custom-select/custom-select.lite.tsx b/packages/components/src/components/custom-select/custom-select.lite.tsx index e03a32315d21..08123955e24c 100644 --- a/packages/components/src/components/custom-select/custom-select.lite.tsx +++ b/packages/components/src/components/custom-select/custom-select.lite.tsx @@ -297,47 +297,74 @@ export default function DBCustomSelect(props: DBCustomSelectProps) { event.key === 'ArrowDown' || event.key === 'ArrowRight' ) { - if (listElement?.nextElementSibling) { - listElement?.nextElementSibling - ?.querySelector('input') - ?.focus(); - } else { + // Find next element with input, skipping group titles + let nextElement = + listElement?.nextElementSibling; + while (nextElement) { + const nextInput = + nextElement.querySelector('input'); + if (nextInput) { + nextInput.focus(); + break; + } + nextElement = + nextElement.nextElementSibling; + } + + if (!nextElement) { // We are on the last checkbox we move to the top checkbox state.handleFocusFirstDropdownCheckbox( activeElement ); } } else { - if (listElement?.previousElementSibling) { - listElement?.previousElementSibling - ?.querySelector('input') - ?.focus(); - } else if ( - detailsRef.querySelector( - `input[type="checkbox"]` - ) !== activeElement - ) { - // We are on the top list checkbox but there is a select all checkbox as well - state.handleFocusFirstDropdownCheckbox( - activeElement - ); - } else { - // We are on the top checkbox, we need to move to the search - // or to the last checkbox - const search = getSearchInput(detailsRef); - if (search) { - delay(() => { - search.focus(); - }, 100); + // Find previous element with input, skipping group titles + let prevElement = + listElement?.previousElementSibling; + while (prevElement) { + const prevInput = + prevElement.querySelector('input'); + if (prevInput) { + prevInput.focus(); + break; + } + prevElement = + prevElement.previousElementSibling; + } + + if (!prevElement) { + // Check if we have a "select all" checkbox (only relevant for multi-select) + const selectAllCheckbox = + detailsRef.querySelector( + `input[type="checkbox"]` + ); + if ( + selectAllCheckbox && + selectAllCheckbox !== activeElement + ) { + // We are on the top list checkbox but there is a select all checkbox as well + state.handleFocusFirstDropdownCheckbox( + activeElement + ); } else { - const checkboxList: HTMLInputElement[] = - Array.from( - detailsRef?.querySelectorAll( - `input[type="checkbox"],input[type="radio"]` - ) - ); - if (checkboxList.length) { - checkboxList.at(-1)?.focus(); + // We are on the top checkbox, we need to move to the search + // or to the last checkbox + const search = + getSearchInput(detailsRef); + if (search) { + delay(() => { + search.focus(); + }, 100); + } else { + const checkboxList: HTMLInputElement[] = + Array.from( + detailsRef?.querySelectorAll( + `input[type="checkbox"],input[type="radio"]` + ) + ); + if (checkboxList.length) { + checkboxList.at(-1)?.focus(); + } } } } diff --git a/packages/components/src/components/custom-select/custom-select.spec.tsx b/packages/components/src/components/custom-select/custom-select.spec.tsx index aef5f781dfcb..c6f7f49896af 100644 --- a/packages/components/src/components/custom-select/custom-select.spec.tsx +++ b/packages/components/src/components/custom-select/custom-select.spec.tsx @@ -5,6 +5,22 @@ import { DBCustomSelect } from './index'; // @ts-ignore - vue can only find it with .ts as file ending import { DEFAULT_VIEWPORT } from '../../shared/constants.ts'; +// Helper function to wait for focus changes instead of using hard-coded timeouts +const waitForFocusChange = async ( + page: any, + expectedValue: string, + timeout = 5000 +) => { + await page.waitForFunction( + (expected: any) => { + const activeElement = document.activeElement as HTMLInputElement; + return activeElement && activeElement.value === expected; + }, + expectedValue, + { timeout } + ); +}; + const comp: any = ( ); +const optionGroupsComp: any = ( + +); + const tagSelectWithCustomRemoveTexts: any = ( { expect(accessibilityScanResults.violations).toEqual([]); }); + test('option groups should be accessible', async ({ page, mount }) => { + await mount(optionGroupsComp); + const accessibilityScanResults = await new AxeBuilder({ page }) + .include('.db-custom-select') + .analyze(); + + expect(accessibilityScanResults.violations).toEqual([]); + }); }; const testAction = () => { @@ -93,7 +132,7 @@ const testAction = () => { await expect(summary).not.toContainText('Option 1'); await page.keyboard.press('Tab'); await page.keyboard.press('ArrowDown'); - await page.waitForTimeout(1000); // wait for focus to apply + await waitForFocusChange(page, 'Option 1'); await page.keyboard.press('Space'); await expect(summary).toContainText('Option 1'); }); @@ -104,7 +143,7 @@ const testAction = () => { await expect(summary).not.toContainText('Option 1'); await page.keyboard.press('Tab'); await page.keyboard.press('ArrowDown'); - await page.waitForTimeout(1000); // wait for focus to apply + await waitForFocusChange(page, 'Option 1'); await page.keyboard.press('Space'); await page.keyboard.press('Escape'); await expect(summary).toContainText('Option 1'); @@ -139,7 +178,7 @@ const testAction = () => { await expect(summary).not.toContainText('Option 1'); await page.keyboard.press('Tab'); await page.keyboard.press('ArrowDown'); - await page.waitForTimeout(1000); // wait for focus to apply + await waitForFocusChange(page, 'Option 1'); await page.keyboard.press('Enter'); await expect(summary).toContainText('Option 1'); // For single select, dropdown should be closed after Enter @@ -152,7 +191,7 @@ const testAction = () => { await expect(summary).not.toContainText('Option 1'); await page.keyboard.press('Tab'); await page.keyboard.press('ArrowDown'); - await page.waitForTimeout(1000); // wait for focus to apply + await waitForFocusChange(page, 'Option 1'); await page.keyboard.press('Enter'); // For multiple select, dropdown should remain open after Enter await expect(component.locator('details')).toHaveAttribute('open'); @@ -160,24 +199,87 @@ const testAction = () => { await expect(summary).toContainText('Option 1'); }); - test('custom removeTagsTexts should correspond to correct options', async ({ mount }) => { + test('option groups keyboard navigation: should navigate between option groups correctly', async ({ + page, + mount + }) => { + const component = await mount(optionGroupsComp); + + // Open the dropdown and focus first option + await page.keyboard.press('Tab'); + await page.keyboard.press('ArrowDown'); + await waitForFocusChange(page, 'G1:Option 1'); + + // Should be focused on G1:Option 1 + const focused1 = await page.evaluate( + () => (document.activeElement as HTMLInputElement)?.value + ); + expect(focused1).toBe('G1:Option 1'); + + // Navigate to G1:Option 2 + await page.keyboard.press('ArrowDown'); + await waitForFocusChange(page, 'G1:Option 2'); + const focused2 = await page.evaluate( + () => (document.activeElement as HTMLInputElement)?.value + ); + expect(focused2).toBe('G1:Option 2'); + + // CRITICAL TEST: Navigate from G1:Option 2 to G2:Option 1 + // This should skip the "Option group 2" title and focus on G2:Option 1 + // This is the core issue that was fixed in #4920 + await page.keyboard.press('ArrowDown'); + await waitForFocusChange(page, 'G2:Option 1'); + const focused3 = await page.evaluate( + () => (document.activeElement as HTMLInputElement)?.value + ); + expect(focused3).toBe('G2:Option 1'); // This was previously broken + + // Continue to G2:Option 2 + await page.keyboard.press('ArrowDown'); + await waitForFocusChange(page, 'G2:Option 2'); + const focused4 = await page.evaluate( + () => (document.activeElement as HTMLInputElement)?.value + ); + expect(focused4).toBe('G2:Option 2'); + + // Test reverse navigation + await page.keyboard.press('ArrowUp'); + await waitForFocusChange(page, 'G2:Option 1'); + const focused5 = await page.evaluate( + () => (document.activeElement as HTMLInputElement)?.value + ); + expect(focused5).toBe('G2:Option 1'); + + await page.keyboard.press('ArrowUp'); + await waitForFocusChange(page, 'G1:Option 2'); + const focused6 = await page.evaluate( + () => (document.activeElement as HTMLInputElement)?.value + ); + expect(focused6).toBe('G1:Option 2'); + }); + + test('custom removeTagsTexts should correspond to correct options', async ({ + mount + }) => { const component = await mount(tagSelectWithCustomRemoveTexts); - + // Should have tags for Blue and Green (selected values) const tags = component.locator('.db-tag'); await expect(tags).toHaveCount(2); - + // Get the remove buttons and their tooltip text - const removeButtons = component.locator('.db-tag .db-tab-remove-button'); + const removeButtons = component.locator( + '.db-tag .db-tab-remove-button' + ); await expect(removeButtons).toHaveCount(2); - + // The first selected option is 'Blue' (index 1 in options array) // Should have 'Remove Blue Color' tooltip const firstRemoveButton = removeButtons.first(); const firstTooltip = firstRemoveButton.locator('.db-tooltip'); await expect(firstTooltip).toContainText('Remove Blue Color'); - - // The second selected option is 'Green' (index 2 in options array) + + // The second selected option is 'Green' (index 2 in options array) // Should have 'Remove Green Color' tooltip const secondRemoveButton = removeButtons.last(); const secondTooltip = secondRemoveButton.locator('.db-tooltip');