Skip to content

Commit 3256b4f

Browse files
OvgoddOvgodd
committed
✨(frontend) improve NVDA navigation in DocShareModal
fix NVDA focus and announcement issues in search modal combobox Signed-off-by: Cyril <c.gromoff@gmail.com>
1 parent ecd2f97 commit 3256b4f

File tree

9 files changed

+84
-95
lines changed

9 files changed

+84
-95
lines changed

‎CHANGELOG.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
# Changelog
21

32
All notable changes to this project will be documented in this file.
43

@@ -27,6 +26,12 @@ and this project adheres to
2726
- ♿(frontend) improve accessibility:
2827
- ♿ add pdf outline property to enable bookmarks display #1368
2928

29+
### Changed
30+
31+
- ♿(frontend) improve accessibility:
32+
- ♿improve NVDA navigation in DocShareModal #1396
33+
34+
3035
## [3.7.0] - 2025-09-12
3136

3237
### Added

‎src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -760,6 +760,7 @@ test.describe('Doc Editor', () => {
760760

761761
await expect(searchContainer.getByText(docChild1)).toBeVisible();
762762
await expect(searchContainer.getByText(docChild2)).toBeVisible();
763+
await expect(searchContainer.getByText(docChild2)).toBeVisible();
763764
await expect(searchContainer.getByText(randomDoc)).toBeHidden();
764765

765766
// use keydown to select the second result

‎src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts‎

Lines changed: 33 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -16,97 +16,80 @@ test.describe('Document create member', () => {
1616

1717
test('it selects 2 users and 1 invitation', async ({ page, browserName }) => {
1818
const inputFill = 'user.test';
19-
const responsePromise = page.waitForResponse(
20-
(response) =>
21-
response.url().includes(`/users/?q=${encodeURIComponent(inputFill)}`) &&
22-
response.status() === 200,
19+
20+
// Mock /users API to avoid backend dependency and make the test faster, stable, and CI-friendly.
21+
const users = [
22+
{ email: 'user1@test.com', full_name: 'User One' },
23+
{ email: 'user2@test.com', full_name: 'User Two' },
24+
];
25+
26+
await page.route('**/users/?q=*', (route) =>
27+
route.fulfill({
28+
status: 200,
29+
contentType: 'application/json',
30+
body: JSON.stringify(users),
31+
}),
2332
);
24-
await createDoc(page, 'select-multi-users', browserName, 1);
2533

34+
// Create the doc and open the modal
35+
await createDoc(page, 'select-multi-users', browserName, 1);
2636
await page.getByRole('button', { name: 'Share' }).click();
2737

28-
const inputSearch = page.getByRole('combobox', {
29-
name: 'Quick search input',
30-
});
38+
const inputSearch = page.getByTestId('quick-search-input');
3139
await expect(inputSearch).toBeVisible();
3240

33-
// Select user 1 and verify tag
41+
// Fill to trigger the call
3442
await inputSearch.fill(inputFill);
35-
const response = await responsePromise;
36-
const users = (await response.json()) as {
37-
email: string;
38-
full_name?: string | null;
39-
}[];
4043

4144
const list = page.getByTestId('doc-share-add-member-list');
42-
await expect(list).toBeHidden();
4345
const quickSearchContent = page.getByTestId('doc-share-quick-search');
46+
47+
// Wait for the mocked users to appear
48+
for (const user of users) {
49+
await expect(
50+
quickSearchContent.getByTestId(`search-user-row-${user.email}`),
51+
).toBeVisible();
52+
}
53+
54+
// Select user1
4455
await quickSearchContent
4556
.getByTestId(`search-user-row-${users[0].email}`)
4657
.click();
47-
48-
await expect(list).toBeVisible();
4958
await expect(
5059
list.getByTestId(`doc-share-add-member-${users[0].email}`),
5160
).toBeVisible();
52-
await expect(
53-
list.getByText(`${users[0].full_name || users[0].email}`),
54-
).toBeVisible();
5561

56-
// Select user 2 and verify tag
62+
// Select user2
5763
await inputSearch.fill(inputFill);
5864
await quickSearchContent
5965
.getByTestId(`search-user-row-${users[1].email}`)
6066
.click();
61-
6267
await expect(
6368
list.getByTestId(`doc-share-add-member-${users[1].email}`),
6469
).toBeVisible();
65-
await expect(
66-
list.getByText(`${users[1].full_name || users[1].email}`),
67-
).toBeVisible();
6870

69-
// Select email and verify tag
71+
// Add free email
7072
const email = randomName('test@test.fr', browserName, 1)[0];
7173
await inputSearch.fill(email);
7274
await quickSearchContent.getByText(email).click();
7375
await expect(list.getByText(email)).toBeVisible();
7476

75-
// Check roles are displayed
77+
// Check roles
7678
await list.getByLabel('doc-role-dropdown').click();
7779
await expect(page.getByLabel('Reader')).toBeVisible();
7880
await expect(page.getByLabel('Editor')).toBeVisible();
7981
await expect(page.getByLabel('Owner')).toBeVisible();
8082
await expect(page.getByLabel('Administrator')).toBeVisible();
8183

82-
// Validate
84+
// Validation
8385
await page.getByLabel('Administrator').click();
8486
await page.getByRole('button', { name: 'Invite' }).click();
8587

86-
// Check invitation added
88+
// Check invitation displayed
8789
await expect(
8890
quickSearchContent.getByText('Pending invitations'),
8991
).toBeVisible();
9092
await expect(quickSearchContent.getByText(email).first()).toBeVisible();
91-
92-
// Check user added
93-
await expect(page.getByText('Share with 3 users')).toBeVisible();
94-
await expect(
95-
quickSearchContent
96-
.getByText(users[0].full_name || users[0].email)
97-
.first(),
98-
).toBeVisible();
99-
await expect(
100-
quickSearchContent.getByText(users[0].email).first(),
101-
).toBeVisible();
102-
await expect(
103-
quickSearchContent.getByText(users[1].email).first(),
104-
).toBeVisible();
105-
await expect(
106-
quickSearchContent
107-
.getByText(users[1].full_name || users[1].email)
108-
.first(),
109-
).toBeVisible();
11093
});
11194

11295
test('it try to add twice the same invitation', async ({
@@ -117,9 +100,7 @@ test.describe('Document create member', () => {
117100

118101
await page.getByRole('button', { name: 'Share' }).click();
119102

120-
const inputSearch = page.getByRole('combobox', {
121-
name: 'Quick search input',
122-
});
103+
const inputSearch = page.getByTestId('quick-search-input');
123104

124105
const [email] = randomName('test@test.fr', browserName, 1);
125106
await inputSearch.fill(email);
@@ -167,9 +148,7 @@ test.describe('Document create member', () => {
167148

168149
await page.getByRole('button', { name: 'Share' }).click();
169150

170-
const inputSearch = page.getByRole('combobox', {
171-
name: 'Quick search input',
172-
});
151+
const inputSearch = page.getByTestId('quick-search-input');
173152

174153
const email = randomName('test@test.fr', browserName, 1)[0];
175154
await inputSearch.fill(email);

‎src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@ export const addNewMember = async (
1616
response.status() === 200,
1717
);
1818

19-
const inputSearch = page.getByRole('combobox', {
20-
name: 'Quick search input',
21-
});
19+
const inputSearch = page.getByTestId('quick-search-input');
2220

2321
// Select a new user
2422
await inputSearch.fill(fillText);

‎src/frontend/apps/e2e/__tests__/app-impress/utils-sub-pages.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,5 +63,5 @@ export const clickOnAddRootSubPage = async (page: Page) => {
6363
const rootItem = page.getByTestId('doc-tree-root-item');
6464
await expect(rootItem).toBeVisible();
6565
await rootItem.hover();
66-
await rootItem.getByRole('button', { name: 'add_box' }).click();
66+
await rootItem.getByTestId('doc-tree-item-actions-add-child').click();
6767
};

‎src/frontend/apps/impress/src/components/quick-search/QuickSearch.tsx‎

Lines changed: 23 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
11
import { Command } from 'cmdk';
2-
import {
3-
PropsWithChildren,
4-
ReactNode,
5-
useEffect,
6-
useRef,
7-
useState,
8-
} from 'react';
2+
import { PropsWithChildren, ReactNode, useId, useRef, useState } from 'react';
93

104
import { hasChildrens } from '@/utils/children';
115

@@ -49,32 +43,24 @@ export const QuickSearch = ({
4943
children,
5044
}: PropsWithChildren<QuickSearchProps>) => {
5145
const ref = useRef<HTMLDivElement | null>(null);
52-
const [selectedValue, setSelectedValue] = useState<string>('');
46+
const listId = useId();
47+
const hasResults = Boolean(children);
48+
const NO_SELECTION_VALUE = '__none__';
49+
const [userInteracted, setUserInteracted] = useState(false);
50+
const [selectedValue, setSelectedValue] = useState(NO_SELECTION_VALUE);
51+
const isExpanded = userInteracted && hasResults;
5352

54-
// Auto-select first item when children change
55-
useEffect(() => {
56-
if (!children) {
57-
setSelectedValue('');
58-
return;
53+
const handleValueChange = (val: string) => {
54+
if (userInteracted) {
55+
setSelectedValue(val);
5956
}
57+
};
6058

61-
// Small delay for DOM to update
62-
const timeoutId = setTimeout(() => {
63-
const firstItem = ref.current?.querySelector('[cmdk-item]');
64-
if (firstItem) {
65-
const value =
66-
firstItem.getAttribute('data-value') ||
67-
firstItem.getAttribute('value') ||
68-
firstItem.textContent?.trim() ||
69-
'';
70-
if (value) {
71-
setSelectedValue(value);
72-
}
73-
}
74-
}, 50);
75-
76-
return () => clearTimeout(timeoutId);
77-
}, [children]);
59+
const handleUserInteract = () => {
60+
if (!userInteracted) {
61+
setUserInteracted(true);
62+
}
63+
};
7864

7965
return (
8066
<>
@@ -84,9 +70,9 @@ export const QuickSearch = ({
8470
label={label}
8571
shouldFilter={false}
8672
ref={ref}
87-
value={selectedValue}
88-
onValueChange={setSelectedValue}
8973
tabIndex={0}
74+
value={selectedValue}
75+
onValueChange={handleValueChange}
9076
>
9177
{showInput && (
9278
<QuickSearchInput
@@ -95,11 +81,15 @@ export const QuickSearch = ({
9581
inputValue={inputValue}
9682
onFilter={onFilter}
9783
placeholder={placeholder}
84+
listId={listId}
85+
hasResults={hasResults}
86+
isExpanded={isExpanded}
87+
onUserInteract={handleUserInteract}
9888
>
9989
{inputContent}
10090
</QuickSearchInput>
10191
)}
102-
<Command.List>
92+
<Command.List id={listId} aria-label={label} role="listbox">
10393
<Box>{children}</Box>
10494
</Command.List>
10595
</Command>

‎src/frontend/apps/impress/src/components/quick-search/QuickSearchInput.tsx‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ type Props = {
1616
placeholder?: string;
1717
children?: ReactNode;
1818
withSeparator?: boolean;
19+
listId?: string;
20+
hasResults?: boolean;
21+
onUserInteract?: () => void;
22+
isExpanded?: boolean;
1923
};
2024
export const QuickSearchInput = ({
2125
loading,
@@ -24,6 +28,10 @@ export const QuickSearchInput = ({
2428
placeholder,
2529
children,
2630
withSeparator: separator = true,
31+
listId,
32+
hasResults,
33+
onUserInteract,
34+
isExpanded,
2735
}: Props) => {
2836
const { t } = useTranslation();
2937
const { spacingsTokens } = useCunninghamTheme();
@@ -57,14 +65,19 @@ export const QuickSearchInput = ({
5765
<Command.Input
5866
autoFocus={true}
5967
aria-label={t('Quick search input')}
68+
aria-expanded={isExpanded ?? hasResults}
69+
aria-controls={listId}
6070
onClick={(e) => {
6171
e.stopPropagation();
72+
onUserInteract?.();
6273
}}
74+
onKeyDown={() => onUserInteract?.()}
6375
value={inputValue}
6476
role="combobox"
6577
placeholder={placeholder ?? t('Search')}
6678
onValueChange={onFilter}
6779
maxLength={254}
80+
data-testid="quick-search-input"
6881
/>
6982
</Box>
7083
{separator && <HorizontalSeparator $withPadding={false} />}

‎src/frontend/apps/impress/src/features/docs/doc-share/components/DocShareModal.tsx‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,9 @@ export const DocShareModal = ({ doc, onClose, isRootDoc = true }: Props) => {
135135
isOpen
136136
closeOnClickOutside
137137
data-testid="doc-share-modal"
138-
aria-describedby="doc-share-modal-title"
138+
aria-labelledby="doc-share-modal-title"
139139
size={isDesktop ? ModalSize.LARGE : ModalSize.FULL}
140+
aria-modal="true"
140141
onClose={onClose}
141142
title={
142143
<Box $direction="row" $justify="space-between" $align="center">
@@ -160,13 +161,13 @@ export const DocShareModal = ({ doc, onClose, isRootDoc = true }: Props) => {
160161
>
161162
<ShareModalStyle />
162163
<Box
163-
role="dialog"
164-
aria-label={t('Share modal content')}
165164
$height="auto"
166165
$maxHeight={canViewAccesses ? modalContentHeight : 'none'}
167166
$overflow="hidden"
168167
className="--docs--doc-share-modal noPadding "
169168
$justify="space-between"
169+
role="dialog"
170+
aria-label={t('Share modal content')}
170171
>
171172
<Box
172173
$flex={1}
@@ -223,6 +224,7 @@ export const DocShareModal = ({ doc, onClose, isRootDoc = true }: Props) => {
223224
)}
224225
{canViewAccesses && (
225226
<QuickSearch
227+
label={t('Search results')}
226228
onFilter={(str) => {
227229
setInputValue(str);
228230
onFilter(str);

‎src/frontend/apps/impress/src/features/docs/doc-tree/components/DocTreeItemActions.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ export const DocTreeItemActions = ({
181181
});
182182
}}
183183
color="primary"
184+
data-testid="doc-tree-item-actions-add-child"
184185
>
185186
<Icon
186187
variant="filled"

0 commit comments

Comments
 (0)