♿️(frontend) fix sidebar resize handle for screen readers#2122
Open
♿️(frontend) fix sidebar resize handle for screen readers#2122
Conversation
Ovgodd
force-pushed
the
fix/separator-keyboard-interact
branch
from
aa9cacf to
c22927b
Compare
Expose the handle as a slider so arrow keys work with NVDA
Ovgodd
force-pushed
the
fix/separator-keyboard-interact
branch
from
c22927b to
b7d0ba7
Compare
|
Size Change: +214 B (+0.01%) Total Size: 4.23 MB
|
Collaborator
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
WalkthroughAdds localized accessibility for the Impress left-panel resize handle. Introduces Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 11-12: The second changelog line ("♿️(frontend) fix sidebar resize
handle for screen readers `#2122`") is missing a list marker and is treated as a
continuation of the previous bullet; add a list marker (e.g., a leading "-" or
"•") before that line so it becomes its own list item under the "Changed"
section, ensuring the two entries ("💄(frontend) improve comments highlights
`#1961`" and "♿️(frontend) fix sidebar resize handle for screen readers `#2122`")
are separate bullets.
In
`@src/frontend/apps/impress/src/features/left-panel/components/ResizableLeftPanel.tsx`:
- Around line 120-131: The current updateValueText function hardcodes the unit
"pixels" inside the translation key passed to t(...) (see updateValueText,
handle.setAttribute('aria-valuetext')), which prevents translators from
controlling unit formatting or order; change the call to t to pass the numeric
width as a variable only (e.g., t('Sidebar width', { widthPx })) and move the
unit text into the translation string itself (or use ICU/pluralization patterns)
so translators can provide the full localized phrase (including "px" or
localized unit) in the translation files.
- Line 154: Remove the unsupported keyboardResizeBy prop from the PanelGroup
usage in ResizableLeftPanel (the JSX element PanelGroup in
ResizableLeftPanel.tsx); update the JSX to omit keyboardResizeBy entirely and
rely on the library's Separator components for keyboard accessibility per
react-resizable-panels API, ensuring there are no leftover references to
keyboardResizeBy elsewhere in this component.
🪄 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: 0326f0b8-0ef0-4330-9795-3d271f0727c6
📒 Files selected for processing (2)
CHANGELOG.mdsrc/frontend/apps/impress/src/features/left-panel/components/ResizableLeftPanel.tsx
AntoLC
reviewed
Apr 21, 2026
Ovgodd
force-pushed
the
fix/separator-keyboard-interact
branch
from
a257fa6 to
bfeeba5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/frontend/apps/impress/src/features/left-panel/components/ResizableLeftPanel.tsx`:
- Around line 172-186: The aria-orientation on the slider is incorrect
(currently "vertical") and must be changed to "horizontal" to match the actual
keyboard interaction and role="slider" behavior; locate the element using
RESIZE_HANDLE_ID in ResizableLeftPanel (the same block that sets
aria-valuemin/aria-valuemax/aria-valuenow/aria-valuetext and references
getSidebarWidthLabel) and update aria-orientation="vertical" to
aria-orientation="horizontal" so it aligns with keyboardResizeBy={1} on
PanelGroup and W3C APG expectations for a horizontal width slider.
🪄 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: 41a277df-55a9-41f6-85c2-713da769dd62
📒 Files selected for processing (2)
CHANGELOG.mdsrc/frontend/apps/impress/src/features/left-panel/components/ResizableLeftPanel.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/frontend/apps/impress/src/features/left-panel/components/ResizableLeftPanel.tsx`:
- Around line 178-185: The aria-valuetext currently nests t() around
getSidebarWidthLabel which hides full sentences from extraction; replace the
nested translation with a single static translation key per full sentence: map
the result of getSidebarWidthLabel(panelSizePercent, minPanelSizePercent,
maxPanelSizePercent) to a static key (e.g. 'leftPanel.sidebarWidth.narrow' /
'.medium' / '.wide') and call t(key) directly in the aria-valuetext (update
ResizableLeftPanel.tsx where aria-valuetext is set) so each full sentence is a
separate, extractable translation string.
🪄 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: 7781cf7e-98cf-4285-bcc1-427de244d7d3
📒 Files selected for processing (1)
src/frontend/apps/impress/src/features/left-panel/components/ResizableLeftPanel.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/frontend/apps/impress/src/features/left-panel/components/ResizableLeftPanel.tsx (1)
178-191: 🧹 Nitpick | 🔵 TrivialSimplify the
aria-valuetextIIFE with a lookup map.The inline IIFE with chained
ifs is harder to scan than a plain object lookup keyed by the label. Functionally equivalent and keeps all three translation keys statically analyzable for extraction.♻️ Proposed refactor
- aria-valuetext={(() => { - const label = getSidebarWidthLabel( - panelSizePercent, - minPanelSizePercent, - maxPanelSizePercent, - ); - if (label === 'narrow') { - return t('Sidebar width: narrow'); - } - if (label === 'medium') { - return t('Sidebar width: medium'); - } - return t('Sidebar width: wide'); - })()} + aria-valuetext={ + { + narrow: t('Sidebar width: narrow'), + medium: t('Sidebar width: medium'), + wide: t('Sidebar width: wide'), + }[ + getSidebarWidthLabel( + panelSizePercent, + minPanelSizePercent, + maxPanelSizePercent, + ) + ] + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/apps/impress/src/features/left-panel/components/ResizableLeftPanel.tsx` around lines 178 - 191, Replace the IIFE used for aria-valuetext with a simple lookup map keyed by the result of getSidebarWidthLabel(panelSizePercent, minPanelSizePercent, maxPanelSizePercent); compute const label = getSidebarWidthLabel(...) then map { narrow: t('Sidebar width: narrow'), medium: t('Sidebar width: medium'), wide: t('Sidebar width: wide') }[label] || t('Sidebar width: wide') and use that value for aria-valuetext — this keeps the translation keys statically visible, preserves current behavior, and removes the chained ifs in ResizableLeftPanel.tsx.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/frontend/apps/impress/src/features/left-panel/components/ResizableLeftPanel.tsx`:
- Around line 21-34: getSidebarWidthLabel can produce NaN when max === min
(division by zero) and then incorrectly falls through to 'wide'; update the
function (getSidebarWidthLabel) to guard the degenerate case by checking if (max
=== min || max - min === 0) and return a sensible label (e.g., return 'narrow'
or return current <= min ? 'narrow' : 'wide') before computing ratio, so no
division by zero occurs and screen-reader labels remain correct.
---
Duplicate comments:
In
`@src/frontend/apps/impress/src/features/left-panel/components/ResizableLeftPanel.tsx`:
- Around line 178-191: Replace the IIFE used for aria-valuetext with a simple
lookup map keyed by the result of getSidebarWidthLabel(panelSizePercent,
minPanelSizePercent, maxPanelSizePercent); compute const label =
getSidebarWidthLabel(...) then map { narrow: t('Sidebar width: narrow'), medium:
t('Sidebar width: medium'), wide: t('Sidebar width: wide') }[label] ||
t('Sidebar width: wide') and use that value for aria-valuetext — this keeps the
translation keys statically visible, preserves current behavior, and removes the
chained ifs in ResizableLeftPanel.tsx.
🪄 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: ff32f95c-d995-4f64-a5c3-904caba2abc9
📒 Files selected for processing (1)
src/frontend/apps/impress/src/features/left-panel/components/ResizableLeftPanel.tsx
Comment on lines
+21
to
+34
| const getSidebarWidthLabel = ( | ||
| current: number, | ||
| min: number, | ||
| max: number, | ||
| ): 'narrow' | 'medium' | 'wide' => { | ||
| const ratio = (current - min) / (max - min); | ||
| if (ratio < 1 / 3) { | ||
| return 'narrow'; | ||
| } | ||
| if (ratio < 2 / 3) { | ||
| return 'medium'; | ||
| } | ||
| return 'wide'; | ||
| }; |
There was a problem hiding this comment.
Guard against max === min to avoid NaN in the width ratio.
When the viewport is narrow enough that Math.min(pxToPercent(maxPanelSizePx), 40) collapses to minPanelSizePercent (e.g., maxPanelSizePx=450, minPanelSizePx=300 at viewport width ≈ 750px: 40% = 300px = min), (current - min) / (max - min) becomes 0 / 0 = NaN. All ordering comparisons against NaN are false, so the function silently falls through to 'wide', which is misleading for SR users. Consider handling the degenerate range explicitly.
🛡️ Proposed guard
const getSidebarWidthLabel = (
current: number,
min: number,
max: number,
): 'narrow' | 'medium' | 'wide' => {
+ if (max <= min) {
+ return 'medium';
+ }
const ratio = (current - min) / (max - min);
if (ratio < 1 / 3) {
return 'narrow';
}
if (ratio < 2 / 3) {
return 'medium';
}
return 'wide';
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/frontend/apps/impress/src/features/left-panel/components/ResizableLeftPanel.tsx`
around lines 21 - 34, getSidebarWidthLabel can produce NaN when max === min
(division by zero) and then incorrectly falls through to 'wide'; update the
function (getSidebarWidthLabel) to guard the degenerate case by checking if (max
=== min || max - min === 0) and return a sensible label (e.g., return 'narrow'
or return current <= min ? 'narrow' : 'wide') before computing ratio, so no
division by zero occurs and screen-reader labels remain correct.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Fix left sidebar keyboard resize with a screen reader on, and improve SR output plus step size.
Proposal
aria-valuenowintoaria-valuetext(width in px).keyboardResizeBy={1}for smaller steps.