Skip to content

♿️(frontend) use anchor links for table of contents entries#2390

Merged
Ovgodd merged 1 commit into
mainfrom
fix/a11y-2345-toc-links
Jun 10, 2026
Merged

♿️(frontend) use anchor links for table of contents entries#2390
Ovgodd merged 1 commit into
mainfrom
fix/a11y-2345-toc-links

Conversation

@Ovgodd

@Ovgodd Ovgodd commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Purpose

Fix table of contents accessibility: TOC entries were <button> elements, so they were invisible in screen reader link navigation (RGAA criterion 6.1).

Proposal

  • Replace TOC entries with <a href="#heading-{blockId}"> links
  • Wrap the TOC list in a <nav> landmark with aria-labelledby referencing the existing h2
  • Use aria-current on the active TOC link (instead of invalid aria-selected on links)
  • Keep scroll via BlockNote [data-id] + setTextCursorPosition (same behavior as before)
  • Update doc-table-content e2e tests for link role and href / aria-current

Note

Focus on the target heading is not yet implemented: BlockNote does not expose id attributes on heading DOM elements, and manual DOM manipulation is wiped by ProseMirror re-renders. An upstream feature request has been opened on BlockNote to support native heading ids.

@Ovgodd Ovgodd requested a review from AntoLC June 3, 2026 11:45
@Ovgodd Ovgodd self-assigned this Jun 3, 2026
@Ovgodd Ovgodd added bug Something isn't working accessibility triage labels Jun 3, 2026
@Ovgodd Ovgodd force-pushed the fix/a11y-2345-toc-links branch from 2f43dd1 to f1fe46a Compare June 3, 2026 11:46
@Ovgodd Ovgodd marked this pull request as ready for review June 3, 2026 11:47
@Ovgodd Ovgodd linked an issue Jun 3, 2026 that may be closed by this pull request
@Ovgodd Ovgodd moved this from Backlog to In review in LaSuite Docs A11y Jun 3, 2026
@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

This PR converts TOC entries to semantic anchor links, wraps the headings list in a nav labeled by a stable toc-heading id, moves scrolling to the nav, replaces BoxButton with a Box rendered as an anchor (href="#heading-{id}") that prevents default and performs editor focus/selection and scrollIntoView, adjusts styling and ARIA to use aria-current, removes an inline comment in useHeadings, updates Playwright e2e tests to assert link hrefs and aria-current, and adds a changelog entry.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • AntoLC
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: replacing TOC entries with anchor links for improved accessibility.
Description check ✅ Passed The description is directly related to the changeset, clearly explaining the accessibility fix, proposed changes, and implementation details.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/a11y-2345-toc-links

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/frontend/apps/e2e/__tests__/app-impress/doc-table-content.spec.ts`:
- Around line 64-74: The test clicks on TOC entries but doesn't assert the new
focus contract; after clicking link1 and link2 add explicit focus assertions
using toBeFocused() on the corresponding heading anchors (i.e., assert
link1.toBeFocused() after the first click and link2.toBeFocused() after the
second click) in the block around the existing viewport/aria-current checks so
the test verifies both visual visibility and keyboard focus transfer for
editorLevel1/editorLevel2 navigation.

In `@src/frontend/apps/impress/src/features/docs/doc-editor/hook/useHeadings.tsx`:
- Around line 37-39: In useHeadings (useHeadings.tsx) the logic only checks for
existence of the tabindex attribute; update it to enforce tabindex="-1" whenever
the current value !== "-1" — i.e., read el.getAttribute('tabindex') and call
el.setAttribute('tabindex', '-1') if the value is absent or not equal to '-1' so
TOC anchor focus behavior is normalized; ensure you still use the same el
identifier and setAttribute method used in the diff.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 92b7510b-ac11-4bc9-aabc-ee9a438ac8b1

📥 Commits

Reviewing files that changed from the base of the PR and between a78269a and f1fe46a.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • src/frontend/apps/e2e/__tests__/app-impress/doc-table-content.spec.ts
  • src/frontend/apps/impress/src/features/docs/doc-editor/hook/useHeadings.tsx
  • src/frontend/apps/impress/src/features/docs/doc-table-content/components/Heading.tsx
  • src/frontend/apps/impress/src/features/docs/doc-table-content/components/TableContentSideBar.tsx

Comment on lines +64 to 74
await link1.click();
await expect(editorLevel1).toBeInViewport();
await expect(level1).toHaveAttribute('aria-selected', 'true');
await expect(level2).toHaveAttribute('aria-selected', 'false');
await expect(link1).toHaveAttribute('aria-current', 'true');
await expect(link2).not.toHaveAttribute('aria-current');

await level2.click();
await link2.click();
await expect(editorLevel1).not.toBeInViewport();
await expect(editorLevel2).toBeInViewport();
await expect(level2).toHaveAttribute('aria-selected', 'true');
await expect(level1).toHaveAttribute('aria-selected', 'false');
await expect(link2).toHaveAttribute('aria-current', 'true');
await expect(link1).not.toHaveAttribute('aria-current');
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Assert focus transfer after TOC activation.

On Line 64 and Line 69, clicks currently validate viewport and aria-current, but not the new focus-move contract. Add explicit toBeFocused() assertions on the heading anchors to lock in the accessibility fix.

Suggested test diff
@@
     await expect(link1).toHaveAttribute('href', /^`#heading-/`);
     await expect(link2).toHaveAttribute('href', /^`#heading-/`);
     await expect(link3).toHaveAttribute('href', /^`#heading-/`);
+    const heading1Anchor = page.locator((await link1.getAttribute('href'))!);
+    const heading2Anchor = page.locator((await link2.getAttribute('href'))!);
@@
     await link1.click();
     await expect(editorLevel1).toBeInViewport();
+    await expect(heading1Anchor).toBeFocused();
     await expect(link1).toHaveAttribute('aria-current', 'true');
     await expect(link2).not.toHaveAttribute('aria-current');
@@
     await link2.click();
     await expect(editorLevel1).not.toBeInViewport();
     await expect(editorLevel2).toBeInViewport();
+    await expect(heading2Anchor).toBeFocused();
     await expect(link2).toHaveAttribute('aria-current', 'true');
     await expect(link1).not.toHaveAttribute('aria-current');
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/apps/e2e/__tests__/app-impress/doc-table-content.spec.ts` around
lines 64 - 74, The test clicks on TOC entries but doesn't assert the new focus
contract; after clicking link1 and link2 add explicit focus assertions using
toBeFocused() on the corresponding heading anchors (i.e., assert
link1.toBeFocused() after the first click and link2.toBeFocused() after the
second click) in the block around the existing viewport/aria-current checks so
the test verifies both visual visibility and keyboard focus transfer for
editorLevel1/editorLevel2 navigation.

Comment on lines +37 to +39
if (!el.hasAttribute('tabindex')) {
el.setAttribute('tabindex', '-1');
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Enforce tabindex="-1" value, not only attribute presence.

Line 37 only checks whether tabindex exists. If it exists with a different value, this won’t normalize focus behavior for TOC anchors. Set it whenever the value is not -1.

Suggested patch
-    if (!el.hasAttribute('tabindex')) {
+    if (el.getAttribute('tabindex') !== '-1') {
       el.setAttribute('tabindex', '-1');
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!el.hasAttribute('tabindex')) {
el.setAttribute('tabindex', '-1');
}
if (el.getAttribute('tabindex') !== '-1') {
el.setAttribute('tabindex', '-1');
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/apps/impress/src/features/docs/doc-editor/hook/useHeadings.tsx`
around lines 37 - 39, In useHeadings (useHeadings.tsx) the logic only checks for
existence of the tabindex attribute; update it to enforce tabindex="-1" whenever
the current value !== "-1" — i.e., read el.getAttribute('tabindex') and call
el.setAttribute('tabindex', '-1') if the value is absent or not equal to '-1' so
TOC anchor focus behavior is normalized; ensure you still use the same el
identifier and setAttribute method used in the diff.

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Size Change: 0 B

Total Size: 4.33 MB

📦 View Changed
Filename Size Change
apps/impress/out/_next/static/2691ee27/_buildManifest.js 0 B -672 B (removed) 🏆
apps/impress/out/_next/static/e015b4b1/_buildManifest.js 672 B +672 B (new file) 🆕

compressed-size-action

Comment thread src/frontend/apps/impress/src/features/docs/doc-editor/hook/useHeadings.tsx Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/frontend/apps/impress/src/features/docs/doc-editor/hook/useHeadings.tsx (1)

14-32: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Restore heading anchor synchronization before updating TOC state.

setHeadings(editor) runs, but nothing now guarantees heading DOM nodes have id="heading-{blockId}" and tabindex="-1". Since TOC links are #heading-*, this breaks native fragment targeting/focus behavior and undermines the accessibility objective.

Suggested direction
+const syncHeadingAnchors = (editor: DocsBlockNoteEditor) => {
+  const headingBlocks = editor.document.filter(
+    (block) =>
+      block.type === 'heading' &&
+      block.props.level >= 1 &&
+      block.props.level <= 3,
+  );
+
+  headingBlocks.forEach((block) => {
+    const el = document.body.querySelector<HTMLElement>(
+      `.bn-block-outer[data-id="${block.id}"] [data-content-type="heading"]:first-child`,
+    );
+    if (!el) return;
+    const expectedId = `heading-${block.id}`;
+    if (el.id !== expectedId) el.id = expectedId;
+    if (el.getAttribute('tabindex') !== '-1') el.setAttribute('tabindex', '-1');
+  });
+};
+
+const updateHeadings = (editor: DocsBlockNoteEditor) => {
+  setHeadings(editor);
+  requestAnimationFrame(() => syncHeadingAnchors(editor));
+};
-
-    setHeadings(editor);
+    updateHeadings(editor);
...
-        setHeadings(editor);
+        updateHeadings(editor);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/apps/impress/src/features/docs/doc-editor/hook/useHeadings.tsx`
around lines 14 - 32, The TOC update happens before heading DOM anchors are
guaranteed to exist; modify the onChange debounce handler so that before calling
setHeadings(editor) you first ensure heading DOM nodes have the expected
id="heading-{blockId}" and tabindex="-1" (e.g. call or implement a helper like
syncHeadingAnchors(editor) right before setHeadings); locate the debounce block
around editor.onChange and context.getChanges and insert the anchor
synchronization step to run after changes are detected and before
setHeadings(editor) so fragment links and focus behavior remain consistent.
src/frontend/apps/impress/src/features/docs/doc-table-content/components/Heading.tsx (1)

49-65: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not block native anchor navigation/focus for TOC links.

e.preventDefault() plus editor.focus() prevents the fragment link from performing native hash navigation and moving focus to the heading target, which is the behavior this migration is meant to restore.

Suggested fix
       onClick={(e: React.MouseEvent<HTMLAnchorElement>) => {
-        e.preventDefault();
-
-        if (!isMobile) {
-          editor.focus();
-        }
-
-        editor.setTextCursorPosition(headingId, 'end');
-
-        document
-          .querySelector<HTMLElement>(`[data-id="${headingId}"]`)
-          ?.scrollIntoView({
-            behavior: 'smooth',
-            inline: 'start',
-            block: 'start',
-          });
+        if (isMobile) {
+          // keep optional mobile-specific behavior only if needed
+          return;
+        }
       }}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/frontend/apps/impress/src/features/docs/doc-table-content/components/Heading.tsx`
around lines 49 - 65, The click handler currently calls e.preventDefault() and
editor.focus(), which blocks native hash navigation and focus; remove the
e.preventDefault() call and the editor.focus() invocation so the browser can
perform native fragment navigation and move focus to the heading, and instead
(if needed) set the editor cursor after navigation by calling
editor.setTextCursorPosition(headingId, 'end') without stealing focus (or defer
that call via setTimeout/nextTick if you must run it after the browser scrolls);
update the onClick handler around the headingId usage accordingly so the anchor
behaves like a normal TOC link.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/frontend/apps/impress/src/features/docs/doc-editor/hook/useHeadings.tsx`:
- Around line 14-32: The TOC update happens before heading DOM anchors are
guaranteed to exist; modify the onChange debounce handler so that before calling
setHeadings(editor) you first ensure heading DOM nodes have the expected
id="heading-{blockId}" and tabindex="-1" (e.g. call or implement a helper like
syncHeadingAnchors(editor) right before setHeadings); locate the debounce block
around editor.onChange and context.getChanges and insert the anchor
synchronization step to run after changes are detected and before
setHeadings(editor) so fragment links and focus behavior remain consistent.

In
`@src/frontend/apps/impress/src/features/docs/doc-table-content/components/Heading.tsx`:
- Around line 49-65: The click handler currently calls e.preventDefault() and
editor.focus(), which blocks native hash navigation and focus; remove the
e.preventDefault() call and the editor.focus() invocation so the browser can
perform native fragment navigation and move focus to the heading, and instead
(if needed) set the editor cursor after navigation by calling
editor.setTextCursorPosition(headingId, 'end') without stealing focus (or defer
that call via setTimeout/nextTick if you must run it after the browser scrolls);
update the onClick handler around the headingId usage accordingly so the anchor
behaves like a normal TOC link.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 789f5427-228c-48a7-b409-280abccb37c2

📥 Commits

Reviewing files that changed from the base of the PR and between f1fe46a and 6fe11d8.

📒 Files selected for processing (3)
  • src/frontend/apps/impress/src/features/docs/doc-editor/hook/useHeadings.tsx
  • src/frontend/apps/impress/src/features/docs/doc-table-content/components/Heading.tsx
  • src/frontend/apps/impress/src/features/docs/doc-table-content/components/TableContentSideBar.tsx

@Ovgodd Ovgodd requested a review from AntoLC June 9, 2026 08:40
Comment thread CHANGELOG.md Outdated
@Ovgodd Ovgodd force-pushed the fix/a11y-2345-toc-links branch from 8a1192c to b7b1750 Compare June 10, 2026 12:04
Replace TOC buttons with anchor links and heading ids for a11y navigation.
@Ovgodd Ovgodd force-pushed the fix/a11y-2345-toc-links branch from b7b1750 to ad33e9e Compare June 10, 2026 12:05

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/frontend/apps/impress/src/features/docs/doc-table-content/components/Heading.tsx (1)

49-66: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

preventDefault() breaks URL hash updates and deep linking.

Calling preventDefault() on the anchor click blocks the browser from updating the URL hash, which defeats core anchor link functionality:

  • Users cannot share links to specific TOC sections
  • Browser back/forward navigation won't work with TOC clicks
  • Bookmarking a specific section is impossible

If custom scroll behavior is needed (to avoid keyboard issues on mobile), consider removing preventDefault() and letting the browser update the URL, or use history.pushState() / history.replaceState() to manually update the hash without triggering navigation.

🔗 Proposed fix to preserve URL hash updates

Option 1: Remove preventDefault and let browser handle navigation (simplest):

-      onClick={(e: React.MouseEvent<HTMLAnchorElement>) => {
-        // With mobile the focus open the keyboard and the scroll is not working
-        e.preventDefault();
-
+      onClick={(e: React.MouseEvent<HTMLAnchorElement>) => {
         if (!isMobile) {
           editor.focus();
         }

Option 2: Manually update URL hash if you must preventDefault:

       onClick={(e: React.MouseEvent<HTMLAnchorElement>) => {
         // With mobile the focus open the keyboard and the scroll is not working
         e.preventDefault();
+        window.history.pushState(null, '', `#heading-${headingId}`);
 
         if (!isMobile) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/frontend/apps/impress/src/features/docs/doc-table-content/components/Heading.tsx`
around lines 49 - 66, The current onClick handler calls e.preventDefault(),
which blocks the browser from updating the URL hash and breaks deep links;
update the handler so it no longer prevents the default for non-mobile
interactions (remove e.preventDefault() or only call it when isMobile is true),
and if you must preventDefault on mobile keep URL semantics by calling
history.pushState(null, '', `#${headingId}`) or history.replaceState(null, '',
`#${headingId}`) after editor.setTextCursorPosition(headingId, 'end'); keep the
existing editor.focus() conditional and the scrollIntoView call intact so
behavior remains consistent across devices.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/frontend/apps/e2e/__tests__/app-impress/doc-table-content.spec.ts`:
- Around line 63-72: The test in doc-table-content.spec.ts should assert that
clicking TOC links updates the URL hash: after await link1.click() add
expect(page.url()).toContain('`#heading-`') (or the exact slug for link1) and
after await link2.click() add expect(page.url()).toContain('`#heading-`') for
link2; if the app prevents default hash navigation due to preventDefault() in
Heading.tsx, remove or adjust that preventDefault() in the Heading component so
clicks allow the browser to update location.hash (or explicitly call
history.pushState/location.hash update in the Heading click handler) so the test
can pass.

In
`@src/frontend/apps/impress/src/features/docs/doc-table-content/components/Heading.tsx`:
- Line 90: The aria-current usage in Heading.tsx sets aria-current to "true" for
highlighted TOC items; change it to the more specific token "location" instead.
Update the JSX where aria-current is set (the attribute in the Heading component
rendering, controlled by isHighlight) to use 'location' when isHighlight is true
and undefined otherwise, so aria-current reads aria-current={isHighlight ?
'location' : undefined}.

---

Outside diff comments:
In
`@src/frontend/apps/impress/src/features/docs/doc-table-content/components/Heading.tsx`:
- Around line 49-66: The current onClick handler calls e.preventDefault(), which
blocks the browser from updating the URL hash and breaks deep links; update the
handler so it no longer prevents the default for non-mobile interactions (remove
e.preventDefault() or only call it when isMobile is true), and if you must
preventDefault on mobile keep URL semantics by calling history.pushState(null,
'', `#${headingId}`) or history.replaceState(null, '', `#${headingId}`) after
editor.setTextCursorPosition(headingId, 'end'); keep the existing editor.focus()
conditional and the scrollIntoView call intact so behavior remains consistent
across devices.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b83c6086-b413-4bc3-b33f-0ab4ee767245

📥 Commits

Reviewing files that changed from the base of the PR and between 6fe11d8 and ad33e9e.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • src/frontend/apps/e2e/__tests__/app-impress/doc-table-content.spec.ts
  • src/frontend/apps/impress/src/features/docs/doc-editor/hook/useHeadings.tsx
  • src/frontend/apps/impress/src/features/docs/doc-table-content/components/Heading.tsx
  • src/frontend/apps/impress/src/features/docs/doc-table-content/components/TableContentSideBar.tsx
💤 Files with no reviewable changes (1)
  • src/frontend/apps/impress/src/features/docs/doc-editor/hook/useHeadings.tsx

Comment on lines +63 to +72
await link1.click();
await expect(editorLevel1).toBeInViewport();
await expect(level1).toHaveAttribute('aria-selected', 'true');
await expect(level2).toHaveAttribute('aria-selected', 'false');
await expect(link1).toHaveAttribute('aria-current', 'true');
await expect(link2).not.toHaveAttribute('aria-current');

await level2.click();
await link2.click();
await expect(editorLevel1).not.toBeInViewport();
await expect(editorLevel2).toBeInViewport();
await expect(level2).toHaveAttribute('aria-selected', 'true');
await expect(level1).toHaveAttribute('aria-selected', 'false');
await expect(link2).toHaveAttribute('aria-current', 'true');
await expect(link1).not.toHaveAttribute('aria-current');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add URL hash verification to anchor link tests.

The test verifies viewport scrolling and aria-current state, but doesn't verify that clicking TOC links updates the URL hash (e.g., expect(page.url()).toContain('#heading-')). This is a core anchor link behavior and should be tested.

🧪 Proposed test enhancement
     await link1.click();
     await expect(editorLevel1).toBeInViewport();
+    await expect(page).toHaveURL(/`#heading-`/);
     await expect(link1).toHaveAttribute('aria-current', 'location');
     await expect(link2).not.toHaveAttribute('aria-current');
 
     await link2.click();
     await expect(editorLevel1).not.toBeInViewport();
     await expect(editorLevel2).toBeInViewport();
+    await expect(page).toHaveURL(/`#heading-`/);
     await expect(link2).toHaveAttribute('aria-current', 'location');
     await expect(link1).not.toHaveAttribute('aria-current');

Note: This test would currently fail due to the preventDefault() call in Heading.tsx that blocks URL hash updates.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/apps/e2e/__tests__/app-impress/doc-table-content.spec.ts` around
lines 63 - 72, The test in doc-table-content.spec.ts should assert that clicking
TOC links updates the URL hash: after await link1.click() add
expect(page.url()).toContain('`#heading-`') (or the exact slug for link1) and
after await link2.click() add expect(page.url()).toContain('`#heading-`') for
link2; if the app prevents default hash navigation due to preventDefault() in
Heading.tsx, remove or adjust that preventDefault() in the Heading component so
clicks allow the browser to update location.hash (or explicitly call
history.pushState/location.hash update in the Heading click handler) so the test
can pass.

`}
aria-label={text}
aria-selected={isHighlight}
aria-current={isHighlight ? 'true' : undefined}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Prefer aria-current="location" over "true" for navigation links.

Per the ARIA specification, aria-current accepts specific token values: page | step | location | date | time | true. For table-of-contents navigation, "location" is more semantically accurate than the generic "true".

♻️ Proposed fix
-      aria-current={isHighlight ? 'true' : undefined}
+      aria-current={isHighlight ? 'location' : undefined}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/frontend/apps/impress/src/features/docs/doc-table-content/components/Heading.tsx`
at line 90, The aria-current usage in Heading.tsx sets aria-current to "true"
for highlighted TOC items; change it to the more specific token "location"
instead. Update the JSX where aria-current is set (the attribute in the Heading
component rendering, controlled by isHighlight) to use 'location' when
isHighlight is true and undefined otherwise, so aria-current reads
aria-current={isHighlight ? 'location' : undefined}.

@Ovgodd Ovgodd merged commit ad33e9e into main Jun 10, 2026
41 checks passed
@Ovgodd Ovgodd deleted the fix/a11y-2345-toc-links branch June 10, 2026 12:26
@github-project-automation github-project-automation Bot moved this from In review to Done in LaSuite Docs A11y Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accessibility bug Something isn't working triage

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Table of contents: elements implemented as buttons instead of links

2 participants