Skip to content

Fix export format select keyboard focus #2366

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

Open
web3blind wants to merge 3 commits into
base: main
Choose a base branch
from
+34 −11
Open

Fix export format select keyboard focus #2366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
35a296d
Fix export format select keyboard focus
web3blind May 29, 2026
e55c225
Merge remote-tracking branch 'upstream/main' into fix/export-format-s…
devarticle May 31, 2026
bd1b73e
test: guard export format select focus state
devarticle May 31, 2026
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
5 changes: 5 additions & 0 deletions src/frontend/apps/e2e/__tests__/app-impress/doc-export.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ test.describe('Doc Export', () => {
),
).toBeVisible();
await expect(page.getByRole('combobox', { name: 'Format' })).toBeVisible();
const formatSelectToggle = page.locator(
'.--docs--modal-export-content .c__select__inner__actions__open',
);
await expect(formatSelectToggle).toHaveAttribute('aria-hidden', 'true');
await expect(formatSelectToggle).toHaveAttribute('tabindex', '-1');
await expect(
page.getByRole('button', {
name: 'Close the download modal',
Expand Down
40 changes: 29 additions & 11 deletions src/frontend/apps/impress/src/features/docs/doc-export/components/ModalExport.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@ import { DocumentProps, pdf } from '@react-pdf/renderer';
import jsonemoji from 'emoji-datasource-apple' with { type: 'json' };
import i18next from 'i18next';
import JSZip from 'jszip';
import { cloneElement, isValidElement, useState } from 'react';
import {
cloneElement,
isValidElement,
useEffect,
useRef,
useState,
} from 'react';
import { useTranslation } from 'react-i18next';
import { css } from 'styled-components';

Expand Down Expand Up @@ -59,6 +65,16 @@ export const ModalExport = ({ onClose, doc }: ModalExportProps) => {
);
const { untitledDocument } = useTrans();
const mediaUrl = useMediaUrl();
const formatSelectRef = useRef<HTMLDivElement>(null);

useEffect(() => {
const toggleButton = formatSelectRef.current?.querySelector<HTMLElement>(
'.c__select__inner__actions__open',
);

toggleButton?.setAttribute('aria-hidden', 'true');
toggleButton?.setAttribute('tabindex', '-1');
});
Comment on lines +70 to +77
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 29, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find export-modal tests and any assertions on the arrow/select attributes
fd -t f . | rg -i 'export' | rg -i 'spec|test|e2e'
rg -nP -C3 'c__select__inner__actions__open|aria-hidden|tabindex' \
  --glob '*.spec.*' --glob '*.test.*' --glob '*e2e*' --glob '*cypress*' --glob '*playwright*'

Repository: suitenumerique/docs

Length of output: 3831

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Check whether the internal Cunningham class is referenced anywhere in tests/code
rg -n --hidden --no-ignore-vcs 'c__select__inner__actions__open' . || true

# 2) Check for aria-hidden/tabindex assertions specifically in doc-export related e2e tests
rg -n --hidden --no-ignore-vcs \
  'doc-export|utils-export|app-impress/doc-export|ModalExport|Export' \
  src/frontend/apps/e2e/__tests__ || true

rg -n --hidden --no-ignore-vcs \
  'aria-hidden\s*=\s*["'\'']true["'\'']|aria-hidden|toHaveAttribute\(.+aria-hidden|tabindex\s*=\s*["'\'']-1["'\'']|toHaveAttribute\(.+tabindex' \
  src/frontend/apps/e2e/__tests__/app-impress/*.spec.* || true

# 3) Also check in app-impress tests/utils-export (if any)
rg -n --hidden --no-ignore-vcs \
  'aria-hidden\s*=\s*["'\'']true["'\'']|toHaveAttribute\(.+aria-hidden|tabindex\s*=\s*["'\'']-1["'\'']|toHaveAttribute\(.+tabindex' \
  src/frontend/apps/e2e/__tests__/app-impress/** || true

Repository: suitenumerique/docs

Length of output: 4640

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n --hidden --no-ignore-vcs \
  "toHaveAttribute\\(.+aria-hidden|aria-hidden\\s*=\\s*['\"]true['\"]|toHaveAttribute\\(.+tabindex\\s*,\\s*['\"]-1['\"]|tabindex\\s*=\\s*['\"]-1['\"]" \
  src/frontend/apps/e2e/__tests__/app-impress/**/*.spec.* \
  || true

rg -n --hidden --no-ignore-vcs \
  "aria-hidden|tabindex" \
  src/frontend/apps/e2e/__tests__/app-impress/**/*.spec.* \
  || true

Repository: suitenumerique/docs

Length of output: 255

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="src/frontend/apps/e2e/__tests__/app-impress"

rg -n --hidden --no-ignore-vcs \
  --glob '*.spec.*' --glob '*.test.*' --glob '*e2e*' --glob '*cypress*' --glob '*playwright*' \
  'toHaveAttribute\(.+aria-hidden|aria-hidden\s*=\s*['\'']true['\'']|toHaveAttribute\(.+tabindex\s*,\s*['\'']-1['\'']|tabindex\s*=\s*['\'']-1['\'']' \
  "$base" || true

rg -n --hidden --no-ignore-vcs \
  --glob '*.spec.*' --glob '*.test.*' --glob '*e2e*' --glob '*cypress*' --glob '*playwright*' \
  '(aria-hidden|tabindex)' \
  "$base" || true

Repository: suitenumerique/docs

Length of output: 603

Guard Cunningham internal Select class + add a11y regression test

ModalExport.tsx sets aria-hidden="true" and tabindex="-1" by querying Cunningham’s internal DOM node via .c__select__inner__actions__open. If Cunningham changes the markup/class, both setAttribute calls will silently no-op and the toggle can re-enter the accessibility/focus flow.

  • Add an e2e/RTL regression test that asserts the toggle element has aria-hidden="true" and tabindex="-1" in the export modal.
  • Prefer a supported Cunningham Select API/prop to hide the toggle from a11y (or add a guard/fallback), rather than relying on the internal class.

Keeping the effect without a dependency array is reasonable so the attributes are re-applied across Cunningham re-renders.


const formatOptions = [
{ label: t('PDF'), value: DocDownloadFormat.PDF },
Expand Down Expand Up @@ -271,16 +287,18 @@ export const ModalExport = ({ onClose, doc }: ModalExportProps) => {
'Export your document to print or download in .docx, .odt, .pdf or .html(zip) format.',
)}
</Text>
<Select
clearable={false}
fullWidth
label={t('Format')}
options={formatOptions}
value={format}
onChange={(options) =>
setFormat(options.target.value as DocDownloadFormat)
}
/>
<Box ref={formatSelectRef}>
<Select
clearable={false}
fullWidth
label={t('Format')}
options={formatOptions}
value={format}
onChange={(options) =>
setFormat(options.target.value as DocDownloadFormat)
}
/>
</Box>

{isExporting && (
<Box
Expand Down