-
Notifications
You must be signed in to change notification settings - Fork 62
chore: clean Hero/Signup/Router imports; remove ArrowRight; deps reset; a11y on CTA #182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: clean Hero/Signup/Router imports; remove ArrowRight; deps reset; a11y on CTA #182
Conversation
WalkthroughThis update focuses on dependency management and minor code cleanup. Several package versions are upgraded and new dependencies are added in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 Thank you @ASR1015 for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/components/Hero.tsx (1)
18-21: Fix CTA semantics and enhance accessibilityThe current implementation nests a React Router Link inside a
<button>, creating interactive-in-interactive elements which hinders keyboard navigation and screen-reader clarity. Replace the<button>with a styled<Link>and add appropriate a11y attributes.• File:
src/components/Hero.tsx
• Lines: 18–21- <button className="bg-blue-600 text-white px-8 py-4 rounded-lg font-semibold hover:bg-blue-700 transition-all transform hover:scale-105 shadow-lg flex items-center space-x-2"> - <Search className="h-5 w-5" /> - <Link to='/track'>Start Tracking</Link> - </button> + <Link + to="/track" + className="bg-blue-600 text-white px-8 py-4 rounded-lg font-semibold hover:bg-blue-700 transition-all transform hover:scale-105 shadow-lg inline-flex items-center space-x-2 focus:outline-none focus:ring-2 focus:ring-blue-400" + aria-label="Start tracking GitHub activity" + title="Start tracking GitHub activity" + > + <Search className="h-5 w-5" aria-hidden="true" /> + <span>Start Tracking</span> + </Link>
- Converts the CTA to a single interactive element (
<Link>).- Adds
aria-labelandtitlefor screen readers.- Marks the
<Search>icon as decorative witharia-hidden="true".
🧹 Nitpick comments (1)
src/Routes/Router.tsx (1)
11-22: Optional: add a catch-all 404 route.Consider a fallback
<Route path="*">for unknown paths to improve UX.Example:
<Route path="*" element={<div>Not Found</div>} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
package.json(2 hunks)src/Routes/Router.tsx(1 hunks)src/components/Hero.tsx(1 hunks)src/pages/Signup/Signup.tsx(1 hunks)
🔇 Additional comments (4)
package.json (1)
19-19: No client-side usages of @octokit/graphql found – no PAT exposure risk
Ripgrep across the entire repo only returned thepackage.jsonentry and no imports or requires in code.
No further action needed regarding bundling tokens in frontend.src/Routes/Router.tsx (1)
1-1: LGTM: cleaned import aligns with RRD v6.Removing
Navigateis fine since it's unused. No runtime changes.src/components/Hero.tsx (1)
1-1: LGTM: removed unused icon import.Import now reflects actual usage.
src/pages/Signup/Signup.tsx (1)
1-1: LGTM: pruned unused type imports.Keeping only
useStateis cleaner; fully-qualifiedReact.ChangeEvent/React.FormEventusage is still valid.
✅ All checks passing. |
|
All checks have passed. The branch is ready for review and merge. Please approve and merge when convenient. |
|
🎉🎉 Thank you for your contribution! Your PR #182 has been merged! 🎉🎉 |
Related Issue
• Closes #
Description
This PR cleans up unused imports in Hero.tsx, Signup.tsx, and Router.tsx to improve code maintainability.
• Removed unused ArrowRight, ChangeEvent, FormEvent, and Navigate imports.
• Reset node_modules and reinstalled dependencies to fix missing type declarations (lucide-react, react-router-dom).
• Added aria-label and title to CTA button for accessibility compliance.
How Has This Been Tested?
• Ran npm run dev and verified the app compiles without TypeScript errors.
• Navigated to /track from the Hero section to ensure routing still works.
Type of Change
• Code cleanup
• Accessibility improvement
• Bug fix
• New feature
• Documentation update
Summary by CodeRabbit