From ee3b05cb5562496d12585addc9ac304ac3847caa Mon Sep 17 00:00:00 2001 From: Cyril Date: Tue, 16 Sep 2025 10:41:12 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(frontend)=20improve=20NVDA=20navigati?= =?UTF-8?q?on=20in=20DocShareModal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix NVDA focus and announcement issues in search modal combobox Signed-off-by: Cyril --- CHANGELOG.md | 7 ++- .../__tests__/app-impress/doc-editor.spec.ts | 14 +++-- .../app-impress/doc-member-create.spec.ts | 13 ++--- .../e2e/__tests__/app-impress/utils-share.ts | 4 +- .../__tests__/app-impress/utils-sub-pages.ts | 2 +- .../components/quick-search/QuickSearch.tsx | 54 ++++++++----------- .../quick-search/QuickSearchInput.tsx | 11 ++++ .../doc-share/components/DocShareModal.tsx | 8 +-- .../components/DocTreeItemActions.tsx | 2 +- 9 files changed, 60 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9103b8fb4a..6738ff5756 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,3 @@ -# Changelog All notable changes to this project will be documented in this file. @@ -27,6 +26,12 @@ and this project adheres to - 🐛(frontend) fix legacy role computation #1376 - 🐛(frontend) scroll back to top when navigate to a document #1406 +### Changed + +- ♿(frontend) improve accessibility: + - ♿improve NVDA navigation in DocShareModal #1396 + + ## [3.7.0] - 2025-09-12 ### Added diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts index c371c2c4e1..aedb30d270 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts @@ -757,15 +757,21 @@ test.describe('Doc Editor', () => { await expect(searchContainer.getByText(docChild2)).toBeVisible(); await expect(searchContainer.getByText(randomDoc)).toBeHidden(); - // use keydown to select the second result + await page.keyboard.press('ArrowDown'); await page.keyboard.press('ArrowDown'); await page.keyboard.press('Enter'); - const interlink = page.getByRole('link', { - name: 'child-2', + // Wait for the search container to disappear, indicating selection was made + await expect(searchContainer).toBeHidden(); + + // Wait for the interlink to be created and rendered + const editor = page.locator('.ProseMirror.bn-editor'); + + const interlink = editor.getByRole('link', { + name: docChild2, }); - await expect(interlink).toBeVisible(); + await expect(interlink).toBeVisible({ timeout: 10000 }); await interlink.click(); await verifyDocName(page, docChild2); diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts index 12d1b83bc5..2addaa68c8 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts @@ -26,9 +26,8 @@ test.describe('Document create member', () => { await page.getByRole('button', { name: 'Share' }).click(); - const inputSearch = page.getByRole('combobox', { - name: 'Quick search input', - }); + const inputSearch = page.getByTestId('quick-search-input'); + await expect(inputSearch).toBeVisible(); // Select user 1 and verify tag @@ -118,9 +117,7 @@ test.describe('Document create member', () => { await page.getByRole('button', { name: 'Share' }).click(); - const inputSearch = page.getByRole('combobox', { - name: 'Quick search input', - }); + const inputSearch = page.getByTestId('quick-search-input'); const [email] = randomName('test@test.fr', browserName, 1); await inputSearch.fill(email); @@ -168,9 +165,7 @@ test.describe('Document create member', () => { await page.getByRole('button', { name: 'Share' }).click(); - const inputSearch = page.getByRole('combobox', { - name: 'Quick search input', - }); + const inputSearch = page.getByTestId('quick-search-input'); const email = randomName('test@test.fr', browserName, 1)[0]; await inputSearch.fill(email); diff --git a/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts b/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts index 5c2a5a33db..761e6a3400 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts @@ -23,9 +23,7 @@ export const addNewMember = async ( response.status() === 200, ); - const inputSearch = page.getByRole('combobox', { - name: 'Quick search input', - }); + const inputSearch = page.getByTestId('quick-search-input'); // Select a new user await inputSearch.fill(fillText); diff --git a/src/frontend/apps/e2e/__tests__/app-impress/utils-sub-pages.ts b/src/frontend/apps/e2e/__tests__/app-impress/utils-sub-pages.ts index 6e3f7e513e..16bf849dfb 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/utils-sub-pages.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/utils-sub-pages.ts @@ -64,7 +64,7 @@ export const clickOnAddRootSubPage = async (page: Page) => { const rootItem = page.getByTestId('doc-tree-root-item'); await expect(rootItem).toBeVisible(); await rootItem.hover(); - await rootItem.getByRole('button', { name: /add subpage/i }).click(); + await rootItem.getByTestId('doc-tree-item-actions-add-child').click(); }; export const navigateToPageFromTree = async ({ diff --git a/src/frontend/apps/impress/src/components/quick-search/QuickSearch.tsx b/src/frontend/apps/impress/src/components/quick-search/QuickSearch.tsx index 9c0ad62f45..89a5417290 100644 --- a/src/frontend/apps/impress/src/components/quick-search/QuickSearch.tsx +++ b/src/frontend/apps/impress/src/components/quick-search/QuickSearch.tsx @@ -1,11 +1,5 @@ import { Command } from 'cmdk'; -import { - PropsWithChildren, - ReactNode, - useEffect, - useRef, - useState, -} from 'react'; +import { PropsWithChildren, ReactNode, useId, useRef, useState } from 'react'; import { hasChildrens } from '@/utils/children'; @@ -49,32 +43,23 @@ export const QuickSearch = ({ children, }: PropsWithChildren) => { const ref = useRef(null); - const [selectedValue, setSelectedValue] = useState(''); + const listId = useId(); + const NO_SELECTION_VALUE = '__none__'; + const [userInteracted, setUserInteracted] = useState(false); + const [selectedValue, setSelectedValue] = useState(NO_SELECTION_VALUE); + const isExpanded = userInteracted; - // Auto-select first item when children change - useEffect(() => { - if (!children) { - setSelectedValue(''); - return; + const handleValueChange = (val: string) => { + if (userInteracted) { + setSelectedValue(val); } + }; - // Small delay for DOM to update - const timeoutId = setTimeout(() => { - const firstItem = ref.current?.querySelector('[cmdk-item]'); - if (firstItem) { - const value = - firstItem.getAttribute('data-value') || - firstItem.getAttribute('value') || - firstItem.textContent?.trim() || - ''; - if (value) { - setSelectedValue(value); - } - } - }, 50); - - return () => clearTimeout(timeoutId); - }, [children]); + const handleUserInteract = () => { + if (!userInteracted) { + setUserInteracted(true); + } + }; return ( <> @@ -84,9 +69,9 @@ export const QuickSearch = ({ label={label} shouldFilter={false} ref={ref} - value={selectedValue} - onValueChange={setSelectedValue} tabIndex={0} + value={selectedValue} + onValueChange={handleValueChange} > {showInput && ( {inputContent} )} - + {children} diff --git a/src/frontend/apps/impress/src/components/quick-search/QuickSearchInput.tsx b/src/frontend/apps/impress/src/components/quick-search/QuickSearchInput.tsx index e67f6ca88d..ff43576820 100644 --- a/src/frontend/apps/impress/src/components/quick-search/QuickSearchInput.tsx +++ b/src/frontend/apps/impress/src/components/quick-search/QuickSearchInput.tsx @@ -16,6 +16,9 @@ type Props = { placeholder?: string; children?: ReactNode; withSeparator?: boolean; + listId?: string; + onUserInteract?: () => void; + isExpanded?: boolean; }; export const QuickSearchInput = ({ loading, @@ -24,6 +27,9 @@ export const QuickSearchInput = ({ placeholder, children, withSeparator: separator = true, + listId, + onUserInteract, + isExpanded, }: Props) => { const { t } = useTranslation(); const { spacingsTokens } = useCunninghamTheme(); @@ -57,14 +63,19 @@ export const QuickSearchInput = ({ { e.stopPropagation(); + onUserInteract?.(); }} + onKeyDown={() => onUserInteract?.()} value={inputValue} role="combobox" placeholder={placeholder ?? t('Search')} onValueChange={onFilter} maxLength={254} + data-testid="quick-search-input" /> {separator && } diff --git a/src/frontend/apps/impress/src/features/docs/doc-share/components/DocShareModal.tsx b/src/frontend/apps/impress/src/features/docs/doc-share/components/DocShareModal.tsx index da9194684c..c214b1ed16 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-share/components/DocShareModal.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-share/components/DocShareModal.tsx @@ -135,8 +135,9 @@ export const DocShareModal = ({ doc, onClose, isRootDoc = true }: Props) => { isOpen closeOnClickOutside data-testid="doc-share-modal" - aria-describedby="doc-share-modal-title" + aria-labelledby="doc-share-modal-title" size={isDesktop ? ModalSize.LARGE : ModalSize.FULL} + aria-modal="true" onClose={onClose} title={ @@ -160,13 +161,13 @@ export const DocShareModal = ({ doc, onClose, isRootDoc = true }: Props) => { > { )} {canViewAccesses && ( { setInputValue(str); onFilter(str); diff --git a/src/frontend/apps/impress/src/features/docs/doc-tree/components/DocTreeItemActions.tsx b/src/frontend/apps/impress/src/features/docs/doc-tree/components/DocTreeItemActions.tsx index 5d2285e786..1ab8b2c84c 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-tree/components/DocTreeItemActions.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-tree/components/DocTreeItemActions.tsx @@ -181,7 +181,7 @@ export const DocTreeItemActions = ({ }); }} color="primary" - aria-label={t('Add subpage')} + data-testid="doc-tree-item-actions-add-child" >