refactor: simplify translation loaders and improve accessibility#23
refactor: simplify translation loaders and improve accessibility#23adipascu wants to merge 1 commit into
Conversation
WalkthroughCountdown interactions now support keyboard and screen-reader accessibility. Translation loading now selects dynamic imports through a typed map keyed by the matched language. ChangesCountdown accessibility
Translation loading
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 3
🧹 Nitpick comments (1)
src/Countdown.tsx (1)
89-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared copy control to eliminate duplication.
The copy control
divwith its accessibility attributes and event handlers is duplicated at lines 89-97 and 132-138. Extracting a shared component would ensure both instances stay in sync — especially important once Space key support is added to both.Also applies to: 129-138
🤖 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/Countdown.tsx` around lines 89 - 97, Extract the duplicated copy control markup into a shared component or render helper near the relevant Countdown code, preserving the role, tabIndex, aria-label, onClick, and keyboard handlers. Replace both instances around the AGE display with this shared control, ensuring future accessibility changes such as Space-key support are implemented in one place.
🤖 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/Countdown.tsx`:
- Around line 89-92: Clean up src/Countdown.tsx by removing trailing whitespace
from the affected JSX lines and replacing single-quoted "Enter" comparisons with
double-quoted strings in the keyboard handlers around copyAgeToClipboard and
related handlers, matching the project lint configuration.
- Around line 89-97: Update the onKeyDown handlers for copyAgeToClipboard and
the settings and second copy controls to support both Enter and Space keys. Call
preventDefault() for handled keys, then invoke the corresponding action so Space
does not scroll the page.
In `@src/translation/index.ts`:
- Line 38: Update the loaders declaration’s Record key type to wrap typeof
SUPPORTED_LANGUAGES in parentheses and apply the formatter-preferred line
breaks, preserving the existing loaders mapping and return type.
---
Nitpick comments:
In `@src/Countdown.tsx`:
- Around line 89-97: Extract the duplicated copy control markup into a shared
component or render helper near the relevant Countdown code, preserving the
role, tabIndex, aria-label, onClick, and keyboard handlers. Replace both
instances around the AGE display with this shared control, ensuring future
accessibility changes such as Space-key support are implemented in one place.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ba059fb-59bc-48f4-a007-55394342d06a
📒 Files selected for processing (2)
src/Countdown.tsxsrc/translation/index.ts
| <div | ||
| onClick={copyAgeToClipboard} | ||
| role="button" | ||
| tabIndex={0} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix trailing whitespace and quote consistency flagged by linter.
Static analysis reports trailing whitespace on lines 89-92 and 132-133, and single quotes on 'Enter' that should be double quotes (lines 94, 112, 137) per the project's lint configuration.
🧹 Proposed cleanup
- <div
- onClick={copyAgeToClipboard}
- role="button"
- tabIndex={0}
+ <div
+ onClick={copyAgeToClipboard}
+ role="button"
+ tabIndex={0}
aria-label={`Copy ${AGE} to clipboard`}
- onKeyDown={(e) => e.key === 'Enter' && copyAgeToClipboard()}
+ onKeyDown={(e) => e.key === "Enter" && copyAgeToClipboard()}
>- <div
- onClick={copyAgeToClipboard}
+ <div
+ onClick={copyAgeToClipboard}
title={COPY_LABEL}
role="button"
tabIndex={0}
- onKeyDown={(e) => e.key === 'Enter' && copyAgeToClipboard()}
+ onKeyDown={(e) => e.key === "Enter" && copyAgeToClipboard()}- onKeyDown={(e) => e.key === 'Enter' && openSettings()}
+ onKeyDown={(e) => e.key === "Enter" && openSettings()}Also applies to: 94-94, 112-112, 132-133, 137-137
🧰 Tools
🪛 GitHub Actions: CI / 3_lint.txt
[warning] 89-89: ESLint (prettier/prettier): Delete ·
[warning] 90-90: ESLint (prettier/prettier): Delete ·
[warning] 91-91: ESLint (prettier/prettier): Delete ·
[warning] 92-92: ESLint (prettier/prettier): Delete ·
🪛 GitHub Actions: CI / lint
[warning] 89-89: ESLint (prettier/prettier): Delete ·.
[warning] 90-90: ESLint (prettier/prettier): Delete ·.
[warning] 91-91: ESLint (prettier/prettier): Delete ·.
[warning] 92-92: ESLint (prettier/prettier): Delete ·.
🪛 GitHub Check: lint
[warning] 92-92:
Delete ·
[warning] 91-91:
Delete ·
[warning] 90-90:
Delete ·
[warning] 89-89:
Delete ·
🤖 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/Countdown.tsx` around lines 89 - 92, Clean up src/Countdown.tsx by
removing trailing whitespace from the affected JSX lines and replacing
single-quoted "Enter" comparisons with double-quoted strings in the keyboard
handlers around copyAgeToClipboard and related handlers, matching the project
lint configuration.
Source: Linters/SAST tools
| <div | ||
| onClick={copyAgeToClipboard} | ||
| role="button" | ||
| tabIndex={0} | ||
| aria-label={`Copy ${AGE} to clipboard`} | ||
| onKeyDown={(e) => e.key === 'Enter' && copyAgeToClipboard()} | ||
| > | ||
| {AGE} | ||
| </div> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add Space key support and preventDefault to keyboard handlers.
Elements with role="button" must activate on both Enter and Space per WAI-ARIA. Currently only Enter is handled, so keyboard users who press Space will scroll the page instead of triggering the action. Additionally, e.preventDefault() should be called to suppress the Space key's default scroll behavior.
♿ Proposed fix for keyboard activation
- onKeyDown={(e) => e.key === 'Enter' && copyAgeToClipboard()}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ copyAgeToClipboard();
+ }
+ }}Apply the same pattern to the settings control (line 112) and the second copy control (line 137):
- onKeyDown={(e) => e.key === 'Enter' && openSettings()}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ openSettings();
+ }
+ }}Also applies to: 109-112, 129-138
🧰 Tools
🪛 GitHub Actions: CI / 3_lint.txt
[warning] 89-89: ESLint (prettier/prettier): Delete ·
[warning] 90-90: ESLint (prettier/prettier): Delete ·
[warning] 91-91: ESLint (prettier/prettier): Delete ·
[warning] 92-92: ESLint (prettier/prettier): Delete ·
[warning] 94-94: ESLint (prettier/prettier): Replace 'Enter' with "Enter"
🪛 GitHub Actions: CI / lint
[warning] 89-89: ESLint (prettier/prettier): Delete ·.
[warning] 90-90: ESLint (prettier/prettier): Delete ·.
[warning] 91-91: ESLint (prettier/prettier): Delete ·.
[warning] 92-92: ESLint (prettier/prettier): Delete ·.
[warning] 94-94: ESLint (prettier/prettier): Replace 'Enter' with "Enter".
🪛 GitHub Check: lint
[warning] 94-94:
Replace 'Enter' with "Enter"
[warning] 92-92:
Delete ·
[warning] 91-91:
Delete ·
[warning] 90-90:
Delete ·
[warning] 89-89:
Delete ·
🤖 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/Countdown.tsx` around lines 89 - 97, Update the onKeyDown handlers for
copyAgeToClipboard and the settings and second copy controls to support both
Enter and Space keys. Call preventDefault() for handled keys, then invoke the
corresponding action so Space does not scroll the page.
| assertUnreachable(path); | ||
| } | ||
|
|
||
| const loaders: Record<typeof SUPPORTED_LANGUAGES[number], () => Promise<any>> = { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix Prettier formatting on the Record type annotation.
The lint check flags this line for missing parentheses around typeof SUPPORTED_LANGUAGES and preferred line breaks. This will fail the format/lint CI check.
🎨 Proposed formatting fix
-const loaders: Record<typeof SUPPORTED_LANGUAGES[number], () => Promise<any>> = {
+const loaders: Record<
+ (typeof SUPPORTED_LANGUAGES)[number],
+ () => Promise<any>
+> = {
en: () => import("./en"),📝 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.
| const loaders: Record<typeof SUPPORTED_LANGUAGES[number], () => Promise<any>> = { | |
| const loaders: Record< | |
| (typeof SUPPORTED_LANGUAGES)[number], | |
| () => Promise<any> | |
| > = { |
🧰 Tools
🪛 GitHub Check: lint
[warning] 38-38:
Replace typeof·SUPPORTED_LANGUAGES[number],·()·=>·Promise<any> with ⏎··(typeof·SUPPORTED_LANGUAGES)[number],⏎··()·=>·Promise<any>⏎
🤖 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/translation/index.ts` at line 38, Update the loaders declaration’s Record
key type to wrap typeof SUPPORTED_LANGUAGES in parentheses and apply the
formatter-preferred line breaks, preserving the existing loaders mapping and
return type.
Source: Linters/SAST tools
1 participant
This MR focuses on two main areas: cleaning up technical debt in the translation system and improving the accessibility (a11y) of the user interface.
🛠️ Changes Made
Summary by CodeRabbit
Accessibility
Bug Fixes