Skip to content

✨(frontend) improve accessibility of search modal for screen readers #1275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 14, 2025
Merged

✨(frontend) improve accessibility of search modal for screen readers #1275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ and this project adheres to
- ⚡️(frontend) improve accessibility:
- #1248
- #1235
- #1275
- #1255
- #1262
- #1244
Expand Down
8 changes: 4 additions & 4 deletions src/frontend/apps/e2e/__tests__/app-impress/doc-editor.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ test.describe('Doc Editor', () => {

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

const selectVisibility = page.getByLabel('Visibility', { exact: true });
const selectVisibility = page.getByTestId('doc-visibility');

// When the visibility is changed, the ws should close the connection (backend signal)
const wsClosePromise = webSocket.waitForEvent('close');
Expand Down Expand Up @@ -561,7 +561,7 @@ test.describe('Doc Editor', () => {

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

await page.getByLabel('Visibility', { exact: true }).click();
await page.getByTestId('doc-visibility').click();

await page
.getByRole('menuitem', {
Expand All @@ -573,7 +573,7 @@ test.describe('Doc Editor', () => {
page.getByText('The document visibility has been updated.'),
).toBeVisible();

await page.getByLabel('Visibility mode').click();
await page.getByTestId('doc-access-mode').click();
await page.getByRole('menuitem', { name: 'Editing' }).click();

// Close the modal
Expand Down Expand Up @@ -655,7 +655,7 @@ test.describe('Doc Editor', () => {

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

await page.getByLabel('Visibility mode').click();
await page.getByTestId('doc-access-mode').click();
await page.getByRole('menuitem', { name: 'Reading' }).click();

// Close the modal
Expand Down
2 changes: 1 addition & 1 deletion src/frontend/apps/e2e/__tests__/app-impress/doc-header.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ test.describe('Doc Header', () => {

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

await page.getByLabel('Visibility', { exact: true }).click();
await page.getByTestId('doc-visibility').click();

await page
.getByRole('menuitem', {
Expand Down
7 changes: 6 additions & 1 deletion src/frontend/apps/e2e/__tests__/app-impress/doc-tree.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,10 @@ test.describe('Doc Tree: Inheritance', () => {
test.use({ storageState: { cookies: [], origins: [] } });

test('A child inherit from the parent', async ({ page, browserName }) => {
// test.slow() to extend timeout since this scenario chains Keycloak login + redirects,
// doc creation/navigation and async doc-tree loading (/documents/:id/tree), which can exceed 30s (especially in CI).
test.slow();

await page.goto('/');
await keyCloakSignIn(page, browserName);

Expand All @@ -271,7 +275,7 @@ test.describe('Doc Tree: Inheritance', () => {
await verifyDocName(page, docParent);

await page.getByRole('button', { name: 'Share' }).click();
const selectVisibility = page.getByLabel('Visibility', { exact: true });
const selectVisibility = page.getByTestId('doc-visibility');
await selectVisibility.click();

await page
Expand Down Expand Up @@ -307,6 +311,7 @@ test.describe('Doc Tree: Inheritance', () => {
await expect(page.locator('h2').getByText(docChild)).toBeVisible();

const docTree = page.getByTestId('doc-tree');
await expect(docTree).toBeVisible({ timeout: 10000 });
await expect(docTree.getByText(docParent)).toBeVisible();
});
});
33 changes: 19 additions & 14 deletions src/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ test.describe('Doc Visibility', () => {

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

const selectVisibility = page.getByLabel('Visibility', { exact: true });
const selectVisibility = page.getByTestId('doc-visibility');

await expect(selectVisibility.getByText('Private')).toBeVisible();

Expand All @@ -51,13 +51,13 @@ test.describe('Doc Visibility', () => {
await selectVisibility.click();
await page.getByLabel('Connected').click();

await expect(page.getByLabel('Visibility mode')).toBeVisible();
await expect(page.getByTestId('doc-access-mode')).toBeVisible();

await selectVisibility.click();

await page.getByLabel('Public', { exact: true }).click();

await expect(page.getByLabel('Visibility mode')).toBeVisible();
await expect(page.getByTestId('doc-access-mode')).toBeVisible();
});
});

Expand Down Expand Up @@ -205,7 +205,7 @@ test.describe('Doc Visibility: Public', () => {
await verifyDocName(page, docTitle);

await page.getByRole('button', { name: 'Share' }).click();
const selectVisibility = page.getByLabel('Visibility', { exact: true });
const selectVisibility = page.getByTestId('doc-visibility');
await selectVisibility.click();

await page
Expand All @@ -218,8 +218,8 @@ test.describe('Doc Visibility: Public', () => {
page.getByText('The document visibility has been updated.'),
).toBeVisible();

await expect(page.getByLabel('Visibility mode')).toBeVisible();
await page.getByLabel('Visibility mode').click();
await expect(page.getByTestId('doc-access-mode')).toBeVisible();
await page.getByTestId('doc-access-mode').click();
await page
.getByRole('menuitem', {
name: 'Reading',
Expand Down Expand Up @@ -289,7 +289,7 @@ test.describe('Doc Visibility: Public', () => {
await verifyDocName(page, docTitle);

await page.getByRole('button', { name: 'Share' }).click();
const selectVisibility = page.getByLabel('Visibility', { exact: true });
const selectVisibility = page.getByTestId('doc-visibility');
await selectVisibility.click();

await page
Expand All @@ -302,7 +302,7 @@ test.describe('Doc Visibility: Public', () => {
page.getByText('The document visibility has been updated.'),
).toBeVisible();

await page.getByLabel('Visibility mode').click();
await page.getByTestId('doc-access-mode').click();
await page.getByLabel('Editing').click();

await expect(
Expand Down Expand Up @@ -358,7 +358,7 @@ test.describe('Doc Visibility: Authenticated', () => {
await verifyDocName(page, docTitle);

await page.getByRole('button', { name: 'Share' }).click();
const selectVisibility = page.getByLabel('Visibility', { exact: true });
const selectVisibility = page.getByTestId('doc-visibility');
await selectVisibility.click();
await page
.getByRole('menuitem', {
Expand Down Expand Up @@ -410,7 +410,7 @@ test.describe('Doc Visibility: Authenticated', () => {
await verifyDocName(page, docTitle);

await page.getByRole('button', { name: 'Share' }).click();
const selectVisibility = page.getByLabel('Visibility', { exact: true });
const selectVisibility = page.getByTestId('doc-visibility');
await selectVisibility.click();
await page
.getByRole('menuitem', {
Expand Down Expand Up @@ -495,6 +495,7 @@ test.describe('Doc Visibility: Authenticated', () => {
page,
browserName,
}) => {
test.slow();
await page.goto('/');
await keyCloakSignIn(page, browserName);

Expand All @@ -508,7 +509,7 @@ test.describe('Doc Visibility: Authenticated', () => {
await verifyDocName(page, docTitle);

await page.getByRole('button', { name: 'Share' }).click();
const selectVisibility = page.getByLabel('Visibility', { exact: true });
const selectVisibility = page.getByTestId('doc-visibility');
await selectVisibility.click();
await page
.getByRole('menuitem', {
Expand All @@ -521,7 +522,7 @@ test.describe('Doc Visibility: Authenticated', () => {
).toBeVisible();

const urlDoc = page.url();
await page.getByLabel('Visibility mode').click();
await page.getByTestId('doc-access-mode').click();
await page.getByLabel('Editing').click();

await expect(
Expand All @@ -539,13 +540,17 @@ test.describe('Doc Visibility: Authenticated', () => {
const otherBrowser = BROWSERS.find((b) => b !== browserName);
await keyCloakSignIn(page, otherBrowser!);

await expect(page.getByTestId('header-logo-link')).toBeVisible();
await expect(page.getByTestId('header-logo-link')).toBeVisible({
timeout: 10000,
});

await page.goto(urlDoc);

await verifyDocName(page, docTitle);
await page.getByRole('button', { name: 'Share' }).click();
await page.getByRole('button', { name: 'Copy link' }).click();
await expect(page.getByText('Link Copied !')).toBeVisible();
await expect(page.getByText('Link Copied !')).toBeVisible({
timeout: 10000,
});
});
});
6 changes: 2 additions & 4 deletions src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const updateShareLink = async (
linkReach: LinkReach,
linkRole?: LinkRole | null,
) => {
await page.getByRole('button', { name: 'Visibility', exact: true }).click();
await page.getByTestId('doc-visibility').click();
await page.getByRole('menuitem', { name: linkReach }).click();

const visibilityUpdatedText = page
Expand All @@ -55,9 +55,7 @@ export const updateShareLink = async (
await expect(visibilityUpdatedText).toBeVisible();

if (linkRole) {
await page
.getByRole('button', { name: 'Visibility mode', exact: true })
.click();
await page.getByTestId('doc-access-mode').click();
await page.getByRole('menuitem', { name: linkRole }).click();
await expect(visibilityUpdatedText).toBeVisible();
}
Expand Down
3 changes: 3 additions & 0 deletions src/frontend/apps/impress/src/components/DropButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export interface DropButtonProps {
isOpen?: boolean;
onOpenChange?: (isOpen: boolean) => void;
label?: string;
testId?: string;
}

export const DropButton = ({
Expand All @@ -57,6 +58,7 @@ export const DropButton = ({
onOpenChange,
children,
label,
testId,
}: PropsWithChildren<DropButtonProps>) => {
const { themeTokens } = useCunninghamTheme();
const font = themeTokens['font']?.['families']['base'];
Expand All @@ -79,6 +81,7 @@ export const DropButton = ({
ref={triggerRef}
onPress={() => onOpenChangeHandler(true)}
aria-label={label}
data-testid={testId}
$css={css`
font-family: ${font};
${buttonCss};
Expand Down
3 changes: 3 additions & 0 deletions src/frontend/apps/impress/src/components/dropdown-menu/DropdownMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export type DropdownMenuProps = {
topMessage?: string;
selectedValues?: string[];
afterOpenChange?: (isOpen: boolean) => void;
testId?: string;
};

export const DropdownMenu = ({
Expand All @@ -52,6 +53,7 @@ export const DropdownMenu = ({
topMessage,
afterOpenChange,
selectedValues,
testId,
}: PropsWithChildren<DropdownMenuProps>) => {
const { spacingsTokens, colorsTokens } = useCunninghamTheme();
const [isOpen, setIsOpen] = useState(opened ?? false);
Expand Down Expand Up @@ -100,6 +102,7 @@ export const DropdownMenu = ({
onOpenChange={onOpenChange}
label={label}
buttonCss={buttonCss}
testId={testId}
button={
showArrow ? (
<Box
Expand Down
4 changes: 3 additions & 1 deletion src/frontend/apps/impress/src/components/quick-search/QuickSearchInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ export const QuickSearchInput = ({
$gap={spacingsTokens['2xs']}
$padding={{ horizontal: 'base', vertical: 'sm' }}
>
{!loading && <Icon iconName="search" $variation="600" />}
{!loading && (
<Icon iconName="search" $variation="600" aria-hidden="true" />
)}
{loading && (
<div>
<Loader size="small" />
Expand Down
6 changes: 5 additions & 1 deletion src/frontend/apps/impress/src/features/docs/doc-editor/components/DocEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,11 @@ export const DocVersionEditor = ({
causes={error.cause}
icon={
error.status === 502 ? (
<Text className="material-icons" $theme="danger">
<Text
className="material-icons"
$theme="danger"
aria-hidden={true}
>
wifi_off
</Text>
) : undefined
Expand Down
6 changes: 5 additions & 1 deletion src/frontend/apps/impress/src/features/docs/doc-share/components/DocShareModalFooter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ export const DocShareModalFooter = ({
fullWidth={false}
onClick={copyDocLink}
color="tertiary"
icon={<span className="material-icons">add_link</span>}
icon={
<span className="material-icons" aria-hidden={true}>
add_link
</span>
}
>
{t('Copy link')}
</Button>
Expand Down
6 changes: 4 additions & 2 deletions src/frontend/apps/impress/src/features/docs/doc-share/components/DocVisibility.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ export const DocVisibility = ({ doc }: DocVisibilityProps) => {
$gap={canManage ? spacingsTokens['3xs'] : spacingsTokens['base']}
>
<DropdownMenu
label={t('Visibility')}
testId="doc-visibility"
label={t('Document visibility')}
arrowCss={css`
color: ${colorsTokens['primary-800']} !important;
`}
Expand Down Expand Up @@ -170,6 +171,7 @@ export const DocVisibility = ({ doc }: DocVisibilityProps) => {
<Box $direction="row" $align="center" $gap={spacingsTokens['3xs']}>
{docLinkReach !== LinkReach.RESTRICTED && (
<DropdownMenu
testId="doc-access-mode"
disabled={!canManage}
showArrow={true}
options={linkRoleOptions}
Expand All @@ -180,7 +182,7 @@ export const DocVisibility = ({ doc }: DocVisibilityProps) => {
)
: undefined
}
label={t('Visibility mode')}
label={t('Document access mode')}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why switch from visibility to access ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was for better clarity, as we’re switching from “edition” to “read-only” it’s essentially an access mode.
I’m open to other suggestions if this wording isn’t ideal :)

>
<Text $weight="initial" $variation="600">
{linkModeTranslations[docLinkRole]}
Expand Down
Loading
Loading