feat: Add aria-labels to buttons for accessibility - #57
Open
rohitkumarnaidu wants to merge 5 commits into
Open
Conversation
|
@rohitkumarnaidu is attempting to deploy a commit to the itzzavdhesh's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
✅ Thanks! This PR is linked to #54 and will close it automatically when merged. |
📝 WalkthroughWalkthroughAdds persisted dark-mode support with a toolbar toggle, applies dark styling across the canvas and sidebars, and improves accessibility labels for icon-only controls. ChangesDark mode and accessibility
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/LeftSidebar.tsx (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winConflicting and duplicate Tailwind dark-mode classes.
Several elements have multiple conflicting utility classes applied to the same CSS property (e.g., both
dark:text-gray-300anddark:text-gray-600). In Tailwind, the class generated last in the stylesheet will win, leading to unpredictable visual bugs and broken focus/hover states where state modifiers were accidentally omitted.
src/components/LeftSidebar.tsx#L172-172: Removedark:text-gray-600to resolve conflict withdark:text-gray-300.src/components/LeftSidebar.tsx#L179-179: Replacedark:bg-gray-800/20anddark:bg-gray-900with appropriate state modifiers (e.g.,dark:focus:bg-gray-900) to avoid overridingdark:bg-gray-800/50.src/components/LeftSidebar.tsx#L186-186: Remove conflictingdark:text-gray-500to leavedark:text-gray-400.src/components/LeftSidebar.tsx#L193-193: Replacedark:bg-gray-900withdark:focus:bg-gray-900to prevent overriding the base dark background.src/components/LeftSidebar.tsx#L197-197: Remove conflictingdark:text-gray-500to leavedark:text-gray-400.src/components/LeftSidebar.tsx#L204-204: Replacedark:bg-gray-900withdark:focus:bg-gray-900to prevent overriding the base dark background.src/components/LeftSidebar.tsx#L239-239: Removedark:bg-gray-800/50which conflicts with the basedark:bg-gray-900.src/components/RightSidebar.tsx#L37-37: Removedark:text-gray-600to resolve conflict withdark:text-gray-300.src/components/RightSidebar.tsx#L106-106: Removedark:text-gray-600to resolve conflict withdark:text-gray-300.src/components/RightSidebar.tsx#L112-112: Replacedark:bg-gray-800/20anddark:bg-gray-900with state modifiers (e.g.,dark:focus:bg-gray-900) to avoid conflict withdark:bg-gray-800/50.src/components/RightSidebar.tsx#L128-128: Removedark:text-gray-600to resolve conflict withdark:text-gray-300.src/components/RightSidebar.tsx#L148-148: Removedark:text-gray-600to resolve conflict withdark:text-gray-300.src/components/RightSidebar.tsx#L170-170: Removedark:text-gray-600to resolve conflict withdark:text-gray-300.src/components/Toast.tsx#L68-69: Clean up duplicatedark:text-gray-400anddark:text-gray-500to leave a single intended base text color.🤖 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/components/LeftSidebar.tsx` at line 1, The sidebar and toast components contain conflicting duplicate Tailwind dark-mode utilities. Update the affected className strings in LeftSidebar, RightSidebar, and Toast by removing the specified conflicting text/background classes, converting focus-only dark backgrounds to dark:focus variants, and leaving each element with one intended base dark color/background.src/components/CenterCanvas.tsx (1)
440-447: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply dark styling to the canvas grid background.
The main canvas area has a hardcoded light background (
bg-[#f8f9fa]) and light grid dots (#e2e8f0). Consequently, even when dark mode is toggled, the main drawing area remains bright, which visually breaks the dark mode integration.Add a
dark:bg-gray-900class to the container and dynamically switch the dot color based on theisDarkModestate.🎨 Proposed fix for the canvas background
- className={`flex-grow relative overflow-hidden bg-[`#f8f9fa`] select-none ${ + className={`flex-grow relative overflow-hidden bg-[`#f8f9fa`] dark:bg-gray-900 select-none ${ isPanning ? 'cursor-grabbing' : 'cursor-grab' }`} style={{ - backgroundImage: 'radial-gradient(`#e2e8f0` 1.5px, transparent 1.5px)', + backgroundImage: `radial-gradient(${isDarkMode ? '`#334155`' : '`#e2e8f0`'} 1.5px, transparent 1.5px)`, backgroundSize: `${20 * scale}px ${20 * scale}px`, backgroundPosition: `${pan.x}px ${pan.y}px`, }}🤖 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/components/CenterCanvas.tsx` around lines 440 - 447, Update the main canvas container’s classes in CenterCanvas to include dark:bg-gray-900 alongside the existing light background, and make its radial-gradient dot color depend on isDarkMode so dark mode uses a darker grid color while light mode preserves `#e2e8f0`. Keep the existing scale and pan positioning behavior unchanged.
🤖 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/components/CenterCanvas.tsx`:
- Line 365: Clean up the duplicated and conflicting Tailwind classes in
src/components/CenterCanvas.tsx at lines 365, 375, 395, 405, 414, 778, and 785:
align lines 365, 375, 405, and 414 with the Save button’s corrected class set;
remove redundant dark:border-gray-600, dark:text-gray-500, and
dark:hover:bg-gray-800 variants; fix the invalid :text-indigo-400 and
:bg-gray-800 tokens at line 395; remove dark:text-gray-500 at line 778; and
resolve conflicting border, text, and background classes at line 785 while
preserving the intended dark-mode styling.
In `@src/utils/useDarkMode.ts`:
- Around line 17-28: Update the useDarkMode synchronization effect to continue
applying or removing the dark class without writing to localStorage. Move
persistence into the explicit toggleDarkMode handler so localStorage is updated
only when the user toggles the theme, preserving the existing isDarkMode state
behavior.
---
Outside diff comments:
In `@src/components/CenterCanvas.tsx`:
- Around line 440-447: Update the main canvas container’s classes in
CenterCanvas to include dark:bg-gray-900 alongside the existing light
background, and make its radial-gradient dot color depend on isDarkMode so dark
mode uses a darker grid color while light mode preserves `#e2e8f0`. Keep the
existing scale and pan positioning behavior unchanged.
In `@src/components/LeftSidebar.tsx`:
- Line 1: The sidebar and toast components contain conflicting duplicate
Tailwind dark-mode utilities. Update the affected className strings in
LeftSidebar, RightSidebar, and Toast by removing the specified conflicting
text/background classes, converting focus-only dark backgrounds to dark:focus
variants, and leaving each element with one intended base dark color/background.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 57f483f4-a6af-4006-b934-ee95c699ba2f
📒 Files selected for processing (7)
src/App.tsxsrc/components/CenterCanvas.tsxsrc/components/LeftSidebar.tsxsrc/components/RightSidebar.tsxsrc/components/Toast.tsxsrc/index.csssrc/utils/useDarkMode.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
Author
|
Hi! Could you please add the ELUSOC label to this PR so it gets tracked properly by the dashboard? Thanks! |
Collaborator
|
@rohitkumarnaidu Please resolve the branch conflicts |
Contributor
Author
|
Hi @itzzavdhesh! Just wanted to give a quick heads up on this PR. It looks like the only failing CI check is Vercel, which says 'Authorization required to deploy'. Could you please authorize the Vercel deployment so the checks can pass? Thank you! 🚀 |
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
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.
Closes #54
🚀 Program
ELUSOC
📝 Description
aria-labelattributes to icon-only buttons acrossCenterCanvas,LeftSidebar, andToastcomponents to improve accessibility for screen readers.🌱 Contributor Checklist
Summary by cubic
Adds aria-labels to icon-only buttons and introduces a reusable
useDarkModehook with a toolbar toggle and saved preference. Improves accessibility and theme consistency, with system preference support.New Features
CenterCanvas(Zoom In/Out),LeftSidebar(cancel chain, edit, delete), andToast(close).useDarkModehook with Sun/Moon toggle, dark styles across components andindex.css, preference stored inlocalStorage(flowforge_theme), and auto-sync with system setting when no saved choice.Bug Fixes
App.tsx, unifying the storage key, and ensuring rootdarkclass toggles correctly after resolving merge conflicts.Written for commit 562ef4e. Summary will update on new commits.
Summary by CodeRabbit