♿️(frontend) use anchor links for interlinking sub-documents#2391
♿️(frontend) use anchor links for interlinking sub-documents#2391Ovgodd wants to merge 2 commits into
Conversation
Render sub-doc interlinks as a tag> instead of span typed in button
Ovgodd
force-pushed
the
fix/a11y-2344-subdocs-links-semantic
branch
from
57b7842 to
0b62029
Compare
|
Size Change: 0 B Total Size: 4.33 MB 📦 View Changed
|
WalkthroughThis PR updates LinkSelected to render the interlinking control as a semantic anchor (Box as="a") instead of a button/span, removes the unused BoxButton import, retargets the click handlers to use anchor mouse events (preventDefault, open in new tab on modifier keys, otherwise router.push), and adds cursor and focus-outline styles. CHANGELOG.md gets a corresponding Unreleased → Added entry. Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/LinkSelected.tsx`:
- Around line 88-92: The CSS rule uses
".--docs--interlinking-link-inline-content:focus-visible" incorrectly as a
descendant selector, so update the stylesheet/StyledComponent for the
LinkSelected component (the selector targeting
--docs--interlinking-link-inline-content) to use the parent-focused selector
form (&:focus-visible) so the root link element receives the focus-visible
outline; ensure the outline and outline-offset declarations remain unchanged.
- Around line 64-67: The LinkSelected component currently renders a native
anchor via <Box as="a" href={href} ...>, which reintroduces native anchor
behavior; change it to render a non-anchor element (e.g., <Box as="span"> or
plain <span>) while preserving the existing onClick and onAuxClick handlers and
any accessibility attributes. Update the JSX in LinkSelected to remove the href
prop (or move it to a data-href attribute if the value must be preserved) and
ensure the className "--docs--interlinking-link-inline-content" and event
handlers remain on the new element so BlockNote/tiptap interlink behavior is not
overridden. Verify focus/keyboard interactions still work and adjust
role/tabIndex if needed to maintain accessibility.
🪄 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: 0424edd9-68fe-4ab7-b43e-2ef50ea17a8e
📒 Files selected for processing (2)
CHANGELOG.mdsrc/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/LinkSelected.tsx
| <Box | ||
| as="a" | ||
| href={href} | ||
| className="--docs--interlinking-link-inline-content" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/LinkSelected.tsx"
# Show around the cited lines
nl -ba "$FILE" | sed -n '40,90p'Repository: suitenumerique/docs
Length of output: 106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/LinkSelected.tsx"
nl -ba "$FILE" | sed -n '40,90p'Repository: suitenumerique/docs
Length of output: 106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/LinkSelected.tsx"
# Show with line numbers using cat -n
cat -n "$FILE" | sed -n '40,90p'Repository: suitenumerique/docs
Length of output: 1769
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "BoxButton|--docs--interlinking-link-inline-content|LinkSelected" src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking src/frontend/apps/impress/src/features/docs/doc-editor -SRepository: suitenumerique/docs
Length of output: 2799
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect LinkSelected usage and surrounding code
sed -n '1,180p' src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/InterlinkingLinkInlineContent.tsx
# Inspect CalloutBlock BoxButton usage (different component but may show intended patterns)
sed -n '80,170p' src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-blocks/CalloutBlock.tsx
# Find BoxButton implementation
rg -n "export (type|interface|const) BoxButton|function BoxButton|BoxButton\s*=" src/frontend -SRepository: suitenumerique/docs
Length of output: 7467
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/frontend/apps/impress/src/components/BoxButton.tsx"
cat -n "$FILE" | sed -n '1,200p'Repository: suitenumerique/docs
Length of output: 2655
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect Box implementation to understand how `as` is handled
FILE="src/frontend/apps/impress/src/components/Box.tsx"
if [ -f "$FILE" ]; then
cat -n "$FILE" | sed -n '1,220p'
else
echo "Missing $FILE; searching for Box.tsx..."
fd -a "Box.tsx" src/frontend/apps/impress/src/components | head -n 20
fi
# Also check how Box is imported in LinkSelected
rg -n "import \\{ Box" src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/LinkSelected.tsxRepository: suitenumerique/docs
Length of output: 7553
Avoid rendering a native <a> for this inline interlink control.
LinkSelected currently renders <Box as="a" href={href} ... onClick/onAuxClick> (lines 64-71), which reintroduces native anchor semantics and can conflict with the BlockNote/tiptap interlink behavior this component relies on. Render it as a non-anchor element (e.g., span) with the existing handlers instead.
🤖 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/components/custom-inline-content/Interlinking/LinkSelected.tsx`
around lines 64 - 67, The LinkSelected component currently renders a native
anchor via <Box as="a" href={href} ...>, which reintroduces native anchor
behavior; change it to render a non-anchor element (e.g., <Box as="span"> or
plain <span>) while preserving the existing onClick and onAuxClick handlers and
any accessibility attributes. Update the JSX in LinkSelected to remove the href
prop (or move it to a data-href attribute if the value must be preserved) and
ensure the className "--docs--interlinking-link-inline-content" and event
handlers remain on the new element so BlockNote/tiptap interlink behavior is not
overridden. Verify focus/keyboard interactions still work and adjust
role/tabIndex if needed to maintain accessibility.
There was a problem hiding this comment.
The point of this PR is to use a native <a href> for RGAA 6.1 compliance, a <span> with role="link" and tabindex is explicitly listed as insufficient in the accessibility audit. Native anchor behavior is handled by preventDefault in the click handler.
There was a problem hiding this comment.
@Ovgodd, that's a fair and clear correction — thank you. RGAA 6.1 (and the underlying WCAG success criterion 1.3.1) explicitly requires that links be marked up as native <a href> elements; a <span role="link" tabindex="0"> does not satisfy the criterion. Using <a href> with preventDefault in the click handler is precisely the right pattern here, and my previous suggestion to revert to a non-anchor element was incorrect in this context. I'll retract that concern.
One small thing worth verifying while you're here: in the $css block the focus-visible rule is written as a descendant selector:
.--docs--interlinking-link-inline-content:focus-visible { … }Since the Box component is that element (not a parent of it), styled-components will look for a child with that class, so the rule may never fire. You likely want:
&:focus-visible { … }
/* or */
&.--docs--interlinking-link-inline-content:focus-visible { … }✏️ Learnings added
Learnt from: Ovgodd
Repo: suitenumerique/docs PR: 2391
File: src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/LinkSelected.tsx:64-67
Timestamp: 2026-06-03T12:43:29.596Z
Learning: In the `LinkSelected` component (`src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/LinkSelected.tsx`), rendering as `<Box as="a" href={href}>` with `preventDefault` in the click handler is intentional and required for RGAA 6.1 / WCAG 1.3.1 compliance. A `<span role="link" tabindex="0">` is explicitly listed as insufficient in the project's accessibility audit. Do not suggest reverting to a non-anchor element.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/LinkSelected.tsx (1)
88-90:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse keyboard-targeted focus styling on Line 88.
&:focus-withinalso matches non-keyboard focus/descendants. For this anchor control,&:focus-visiblebetter matches the accessibility intent for keyboard users.Suggested fix
- &:focus-within { + &:focus-visible { outline: 2px solid var(--c--globals--colors--brand-400); outline-offset: 2px; }🤖 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/components/custom-inline-content/Interlinking/LinkSelected.tsx` around lines 88 - 90, Replace the keyboard-focus styling selector on the LinkSelected anchor control: change the CSS rule using "&:focus-within" to "&:focus-visible" in the styled component (LinkSelected) so the visible outline only appears for keyboard focus; update any related focus-offset/outline declarations accordingly and consider keeping a graceful fallback for browsers without :focus-visible support if needed.
🤖 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.
Duplicate comments:
In
`@src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/LinkSelected.tsx`:
- Around line 88-90: Replace the keyboard-focus styling selector on the
LinkSelected anchor control: change the CSS rule using "&:focus-within" to
"&:focus-visible" in the styled component (LinkSelected) so the visible outline
only appears for keyboard focus; update any related focus-offset/outline
declarations accordingly and consider keeping a graceful fallback for browsers
without :focus-visible support if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e2284aba-767c-4900-9d19-a17959182cad
📒 Files selected for processing (1)
src/frontend/apps/impress/src/features/docs/doc-editor/components/custom-inline-content/Interlinking/LinkSelected.tsx
1 participant
Purpose
Make sub-doc interlinks accessible by using real
<a href>links instead of invalid<span type="button">markup.Proposal
<a href>inLinkSelected.tsx:focus-visibleoutline for keyboard users