-
Notifications
You must be signed in to change notification settings - Fork 24
🎨 Palette: Add loading state to Button component #217
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
base: main
Are you sure you want to change the base?
🎨 Palette: Add loading state to Button component #217
Conversation
- Added `Spinner` component using Lucide `Loader2`. - Updated `Button` component to accept `isLoading` prop. - Integrated loading state into Auth page login button for better UX.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. New to Jules? Learn more at jules.google/docs. |
WalkthroughAdds a new Changes
Suggested reviewers
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)web/pages/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-11-27T18:33:22.596ZApplied to files:
🧬 Code graph analysis (1)web/pages/Auth.tsx (2)
🔇 Additional comments (4)
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 |
✅ Deploy Preview for split-but-wiser ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #217 +/- ##
=======================================
Coverage 74.42% 74.42%
=======================================
Files 17 17
Lines 1693 1693
Branches 158 158
=======================================
Hits 1260 1260
Misses 381 381
Partials 52 52
🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/pages/Auth.tsx (1)
163-200: Consider refactoring the Google sign-in button for consistency.The Google sign-in button (lines 163-200) implements its own custom loading spinner, which creates code duplication with the new Spinner component. While the current implementation works, consider whether the reusable Button component could be adapted to support this use case, which would improve maintainability and consistency.
If the custom styling is intentional to match Google's branding, this is fine as-is.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
web/components/ui/Button.tsxweb/components/ui/Spinner.tsxweb/pages/Auth.tsx
🧰 Additional context used
📓 Path-based instructions (1)
web/pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Web pages should be created as components in
web/pages/and added to routing configuration
Files:
web/pages/Auth.tsx
🧬 Code graph analysis (2)
web/pages/Auth.tsx (1)
web/components/ui/Button.tsx (1)
Button(12-60)
web/components/ui/Button.tsx (1)
web/components/ui/Spinner.tsx (1)
Spinner(9-17)
🔇 Additional comments (4)
web/components/ui/Button.tsx (3)
4-4: LGTM! Clean integration of the Spinner component.The import and prop definition are well-structured and follow the existing patterns in the component.
Also applies to: 9-9
53-53: LGTM! Proper handling of disabled state.The button correctly disables itself during loading state while respecting any explicitly passed
disabledprop. This prevents double submissions and provides good accessibility.
56-56: Spinner placement looks good.The Spinner is correctly positioned before the button content with appropriate spacing. The
mr-1margin works well with thegap-2flexbox spacing defined inbaseStyles(line 22).web/pages/Auth.tsx (1)
254-255: LGTM! Clean migration to the new loading state pattern.The changes correctly use the new
isLoadingprop and conditionally render the ArrowRight icon only when not loading. This provides clear visual feedback while maintaining consistent button text, which is better UX than the previous text-changing approach.
- Updated `Spinner` component to accept an optional `ariaLabel` prop. - Sets default `aria-label` to "Loading". - Ensures accessibility for screen readers by allowing context-specific labels.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@jules 163-200: Consider refactoring the Google sign-in button for consistency. The Google sign-in button (lines 163-200) implements its own custom loading spinner, which creates code duplication with the new Spinner component. While the current implementation works, consider whether the reusable Button component could be adapted to support this use case, which would improve maintainability and consistency. If the custom styling is intentional to match Google's branding, this is fine as-is. |
Good catch on the code duplication! I've refactored the Google sign-in button to use the new |
- Replaced custom inline spinner with `Spinner` component in Google sign-in button. - Added `role="status"` to `Spinner` component for better accessibility. - Removed unnecessary `pnpm-lock.yaml` file.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
💡 What: Added a visual loading state to the
Buttoncomponent and implemented it on the Auth page.🎯 Why: To provide immediate visual feedback to users when an asynchronous action (like logging in) is in progress, preventing confusion and double-submissions.
📸 Changes:
web/components/ui/Spinner.tsxweb/components/ui/Button.tsxto supportisLoadingweb/pages/Auth.tsx♿ Accessibility: The button is disabled (
disabledattribute) when loading, preventing keyboard interaction and announcing the state to screen readers via the disabled attribute (and implicit visual change).PR created automatically by Jules for task 14124150984758611977 started by @Devasy23
Summary by CodeRabbit
New Features
Bug Fixes / UX
✏️ Tip: You can customize this high-level summary in your review settings.