fixed the moon icon & added animation#670
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
More reviews will be available in 42 minutes and 9 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughThis PR fixes the dark mode button visibility on mobile screens and improves menu interactions with smooth animations. A fade-in animation is added to the CSS utilities layer, then applied to mobile navbar components: the theme toggle button, menu toggle button, and menu container. The Moon icon in light mode now explicitly uses black text color for contrast. ChangesMobile Dark Mode Button & Animation Enhancements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/index.css (1)
93-109: ⚡ Quick winRespect reduced-motion preferences for the new fade animation.
Please gate this utility behind
prefers-reduced-motionso motion-sensitive users can opt out cleanly.Proposed patch
`@layer` utilities{ `@keyframes` fade-in{ @@ .ani-fade-in{ animation: fade-in 0.8s ease-out both; } + + `@media` (prefers-reduced-motion: reduce) { + .ani-fade-in { + animation: none; + } + } }🤖 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/index.css` around lines 93 - 109, The new keyframes fade-in and utility class .ani-fade-in must respect the user's reduced-motion preference: wrap a rule in a prefers-reduced-motion: reduce media query that disables the animation for .ani-fade-in (e.g., set animation: none or animation-duration: 0s and remove transforms/filters) so motion-sensitive users don't get the fade-in; update the stylesheet to include the prefers-reduced-motion block alongside the existing `@keyframes` and .ani-fade-in definitions.
🤖 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/Navbar.tsx`:
- Around line 106-107: The menu is being unmounted immediately by the {isOpen &&
...} conditional so exit animations never run; change the logic in Navbar to
keep the menu element mounted while animating out (e.g., render the div
unconditionally or via a local state like isMenuMounted) and toggle CSS classes
(ani-fade-in / ani-fade-out or similar) based on isOpen, then remove/unmount
only after the animation end (listen for transitionend/animationend or use a
timeout) so both entry and exit animations run; update references to isOpen and
the menu div's class handling to implement this
mount-then-unmount-after-animation pattern.
- Around line 92-93: The mobile menu toggle handler in Navbar.tsx uses
setIsOpen(!isOpen) which can suffer from stale closure values; change the
onClick handler to use a functional state update like setIsOpen(prev => !prev)
so toggling uses the latest state. Locate the onClick on the toggle element that
references setIsOpen and replace the direct negation with the functional updater
to ensure safe concurrent/rapid updates.
---
Nitpick comments:
In `@src/index.css`:
- Around line 93-109: The new keyframes fade-in and utility class .ani-fade-in
must respect the user's reduced-motion preference: wrap a rule in a
prefers-reduced-motion: reduce media query that disables the animation for
.ani-fade-in (e.g., set animation: none or animation-duration: 0s and remove
transforms/filters) so motion-sensitive users don't get the fade-in; update the
stylesheet to include the prefers-reduced-motion block alongside the existing
`@keyframes` and .ani-fade-in definitions.
🪄 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
Run ID: 4b77e94d-d964-4c84-be2f-103db35f886e
📒 Files selected for processing (2)
src/components/Navbar.tsxsrc/index.css
|
Hello @itsdakshjain , |
itsdakshjain
left a comment
There was a problem hiding this comment.
overall, looks clean and all the pre-merge checks are green.
-
state updater fixed: good call updating the toggle handler to setIsOpen((prev) => !prev) in f7259d3. that completely resolves the first issue the bot pointed out regarding stale state during fast clicks.
-
animation scope: saw your comment to the bot about the exit animation. it is out of scope for the assigned issue fix, but i will leave it up to the maintainers to decide if they want it added here or tracked as a separate issue
looks good to me.
could a maintainer please manually add the mentor:itsdakshjain label to this pr for gssoc tracking? thanks!
Related Issue
Description
How Has This Been Tested?
Using Dev tools on dimensions iPhone SE
Screenshots (if applicable)
Type of Change
Summary by CodeRabbit