-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: cleaner alert notifs #927
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: develop
Are you sure you want to change the base?
Conversation
Comment out notification for successful logout.
WalkthroughThe PR modifies two UI components: removes the logout success notification from NavBar's logout handler while preserving the logout logic, and updates AlertSnackbar's Alert component with background color and outlined variant styling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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: 0
🧹 Nitpick comments (1)
src/components/ui/AlertSnackbar.tsx (1)
6-11: Consider refactoring the Alert wrapper to avoid variant override confusion.The Alert wrapper on line 10 defines
variant="filled"as a default, but line 35 overrides it withvariant="outlined". Additionally, theelevation={6}prop on line 10 has no visual effect when using the outlined variant.🔎 Proposed refactor to eliminate the override pattern
Update the Alert wrapper to remove the hardcoded variant:
const Alert = React.forwardRef<HTMLDivElement, AlertProps>(function Alert( props, ref, ) { - return <MuiAlert elevation={6} ref={ref} variant="filled" {...props} />; + return <MuiAlert elevation={6} ref={ref} {...props} />; });This makes the variant explicitly controlled at the usage site rather than relying on override behavior.
Also applies to: 35-35
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/dashboard/NavBar.tsx(1 hunks)src/components/ui/AlertSnackbar.tsx(1 hunks)
🔇 Additional comments (1)
src/components/dashboard/NavBar.tsx (1)
110-144: LGTM! Cleaner logout UX.Removing the success notification on logout aligns well with the PR objective of cleaner notifications. The logout flow remains intact—dispatching LOGOUT, clearing localStorage, and navigating to the login page all function correctly. The navigation itself provides sufficient user feedback that the logout succeeded.
Summary by CodeRabbit
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.