-
Notifications
You must be signed in to change notification settings - Fork 447
Feat: Add hover animation effect for avatar selection in onboarding #862
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 hover animation effect for avatar selection in onboarding #862
Conversation
|
|
📝 WalkthroughWalkthroughThe change adds hover feedback to avatar selection buttons in the onboarding screen by applying Tailwind utility classes ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
landing-page/src/App.tsx (1)
18-27: Out of scope: Landing page changes unrelated to PR objectives.This change adds an anchor wrapper for smooth scrolling to the home section, but the PR description states it "adds a hover highlight animation to the avatar selection grid in the onboarding flow" with "no functional or logic changes — UI-only." The landing page navigation changes in this file (and Navbar.tsx) are:
- Functional changes, not UI-only
- In the landing-page directory, not onboarding
- Not mentioned in the PR description or objectives
Consider moving landing page navigation improvements to a separate PR to maintain clear scope and easier review/revert capability.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsxlanding-page/src/App.tsxlanding-page/src/Pages/Landing page/Navbar.tsx
🔇 Additional comments (2)
frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx (1)
111-111: LGTM! Clean hover effect implementation.The
hover:scale-105andhover:shadow-mdutilities provide subtle, smooth visual feedback that complements the existingtransition-all duration-300. The implementation works correctly with both light and dark modes and doesn't conflict with the selected state styling.landing-page/src/Pages/Landing page/Navbar.tsx (1)
1-1: Not a concern: landing-page and frontend are separate applications with independent builds.The different React versions (landing-page uses ^18.3.1, frontend uses 19.1.0) are not an issue. These are completely independent projects in the monorepo: landing-page is a standalone web application and frontend is the main Tauri desktop application. They have no shared dependencies, no cross-references, and build separately to their own bundles. Each application bundles its own React version, so there are no bundle size, compatibility, or behavior inconsistencies.
Likely an incorrect or invalid review comment.
| // Scroll with offset to account for sticky navbar | ||
| const offset = 90; | ||
| const elementTop = element.getBoundingClientRect().top + window.scrollY; | ||
| const targetScroll = elementTop - offset; | ||
|
|
||
| window.scrollTo({ | ||
| top: targetScroll, | ||
| behavior: "smooth", | ||
| block: "start", | ||
| }); |
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.
Out of scope: Smooth scrolling implementation unrelated to PR objectives.
This offset-based smooth scrolling implementation is functionally correct but completely unrelated to the stated PR objective of adding "hover highlight animation to the avatar selection grid in the onboarding flow." This is a landing page navigation feature, not an onboarding UI enhancement.
The 90px offset is hardcoded; consider extracting it as a constant if this change is retained.
🤖 Prompt for AI Agents
In landing-page/src/Pages/Landing page/Navbar.tsx around lines 30 to 38, this
smooth-scroll block is unrelated to the onboarding avatar PR; either
remove/revert these changes from this PR to keep scope focused, or if you need
to keep the behavior extract the hardcoded 90px into a named constant (e.g.
NAVBAR_OFFSET) near the top of the module and replace the literal with that
constant, keeping the rest of the scrolling logic intact.
cc9458c to
8a363f8
Compare
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx (1)
111-124: Address compound scaling and refine hover timing.The hover effect creates a compound scaling issue: when an avatar is selected, the
<img>already appliesscale-105(line 121), and hovering the button adds anotherscale-105(line 111), resulting in ~1.1025× total scale that may feel too aggressive.Additionally:
- The transition uses
duration-300(300ms) but the linked issue requests ~150–200ms.- The issue explicitly asks for
cursor-pointer, which is missing.shadow-mdprovides a shadow rather than a "soft glow or ring" as described in the issue (though this is a reasonable interpretation).🔎 Proposed refinements
Option 1 (Recommended): Apply hover scale only to the button, remove scale from selected image
className={`bg-background relative inline-flex h-20 w-20 items-center justify-center rounded-full transition-all duration-300 hover:scale-105 hover:shadow-md ${ isSelected ? 'border-primary ring-primary ring-offset-background ring-2 ring-offset-2' : 'border-muted' }`} > <img src={avatar} alt="Avatar" - className={`h-20 w-20 rounded-full object-cover transition-all duration-300 ${ - isSelected ? 'scale-105' : '' - }`} + className="h-20 w-20 rounded-full object-cover" />Option 2: Conditionally disable button hover when selected
- className={`bg-background relative inline-flex h-20 w-20 items-center justify-center rounded-full transition-all duration-300 hover:scale-105 hover:shadow-md ${ + className={`bg-background relative inline-flex h-20 w-20 items-center justify-center rounded-full cursor-pointer transition-all duration-200 hover:shadow-md ${ + isSelected ? '' : 'hover:scale-105' + } ${ isSelected ? 'border-primary ring-primary ring-offset-background ring-2 ring-offset-2' : 'border-muted' }`}Both options add
cursor-pointerand adjustduration-300→duration-200to better align with the 150–200ms spec.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
frontend/src/components/OnboardingSteps/AvatarSelectionStep.tsx
Summary
This PR improves the onboarding UX by adding a hover highlight animation
to the avatar selection grid.
Previously, avatars only changed state when clicked — there was no visual
feedback while hovering, which made the selection feel static.
Now, hovering over an avatar slightly scales and highlights it, giving users
a clearer sense that it is interactive.
Changes
Why this helps
Hover feedback improves discoverability and makes the onboarding screen
feel more responsive and modern. This aligns with other interactive
elements already present across the UI.
Manual Testing
Fixes #861
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.