-
Notifications
You must be signed in to change notification settings - Fork 173
feat: Add loading spinner to create election button #209
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?
feat: Add loading spinner to create election button #209
Conversation
WalkthroughAdded a loading state to the Create Election flow: the component tracks Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
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
🧹 Nitpick comments (2)
client/app/create/page.tsx (2)
20-20: State initialization looks good, minor formatting suggestion.The loading state is correctly initialized to
false. Consider adding a space after the comma for consistency with code style conventions.🔎 Optional formatting improvement
- const [isLoading,setIsLoading] = useState<boolean>(false); + const [isLoading, setIsLoading] = useState<boolean>(false);
238-238: Consider adding size props to the Loader for better visual alignment.The conditional rendering logic is correct, but the Loader component may benefit from explicit sizing to ensure it aligns well with the button's text height and provides clear visual feedback.
🔎 Optional improvements
- {isLoading ? <Loader/>: "Create Election"} + {isLoading ? <Loader size="sm" /> : "Create Election"}This ensures the loader is appropriately sized for the button context. You may also want to add descriptive text alongside the loader:
- {isLoading ? <Loader/>: "Create Election"} + {isLoading ? ( + <span className="flex items-center justify-center gap-2"> + <Loader size="sm" /> + Creating... + </span> + ) : "Create Election"}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/app/create/page.tsx(4 hunks)
🔇 Additional comments (1)
client/app/create/page.tsx (1)
16-16: LGTM!The import statement is correct and rsuite is already a project dependency.
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: 0
🧹 Nitpick comments (2)
client/app/create/page.tsx (2)
20-20: Minor: Add space after comma for consistency.For better readability and consistency with standard TypeScript formatting conventions, consider adding a space after the comma in the array destructuring.
🔎 Proposed formatting adjustment
- const [isLoading,setIsLoading] = useState<boolean>(false); + const [isLoading, setIsLoading] = useState<boolean>(false);
234-242: Previous critical issue resolved! Consider conditionally applying animations.Great work addressing the previous review feedback! The button now correctly:
- Disables during loading (line 234)
- Displays visual disabled styling (lines 235-239)
- Shows a Loader component during submission (line 242)
This successfully prevents double-submissions as intended by issue #208.
Optional refinement: The
whileHoverandwhileTapprops on lines 239-240 might still attempt to animate even when the button is disabled. For cleaner UX, consider conditionally applying these animations only when not loading.🔎 Optional refinement to conditionally apply animations
<motion.button type="submit" disabled={isLoading} className={`w-full py-3 px-4 border border-transparent rounded-md shadow-sm text-sm font-medium text-white ${isLoading ? "bg-indigo-400 cursor-not-allowed opacity-70" : "bg-gradient-to-r from-indigo-500 to-purple-600 hover:from-indigo-600 hover:to-purple-700" } focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-indigo-500`} - whileHover={{ scale: 1.02 }} - whileTap={{ scale: 0.98 }} + whileHover={!isLoading ? { scale: 1.02 } : {}} + whileTap={!isLoading ? { scale: 0.98 } : {}} > {isLoading ? <Loader/>: "Create Election"} </motion.button>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/app/create/page.tsx(4 hunks)
🔇 Additional comments (2)
client/app/create/page.tsx (2)
84-84: LGTM! Loading state management is correctly implemented.The loading state is set after all validation checks pass and is properly reset in the finally block, ensuring the UI recovers correctly whether the transaction succeeds or fails. This addresses the core requirement from issue #208.
Also applies to: 102-104
16-16: Addsizeprop to Loader component for consistent button appearance.Use the Loader's size prop to set appropriate dimensions when rendering within the button. For a button context, consider
size="sm"orsize="xs"to match button sizing conventions.
Description
Added a loading state to the "Create Election" button to improve UX during transaction processing. Currently, the button remains static when clicked, leading to user confusion and potential double-submissions.
Summary of the Change :
This PR introduces an
isLoadingstate that:Fixes #208
Type of change
Checklist:
Screencast.from.2025-12-20.04-16-29.mp4
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.