fix: resolve icon shifting issue while scrolling on Apply page#682
Conversation
📝 WalkthroughWalkthroughTimelineElement's animation structure was refactored to separate icon and content animations; ApplyHeader gained scroll-linked horizontal translation using framer-motion hooks to drive a smooth, spring-based parallax effect. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can customize the high-level summary generated by CodeRabbit.Configure the |
|
I’ve opened a PR fixing the icon shifting issue on the Apply page and improving the alignment of the five step icons. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/apply/ApplyHeader.jsx (1)
14-20: Consider extracting steps array to module scope.The
stepsarray is recreated on every render. While the impact is negligible for this component, moving it outside the component as a constant is a minor optimization and cleaner pattern.♻️ Suggested refactor
+const STEPS = [ + { icon: faDiscord, label: "Join Discord" }, + { icon: faGithub, label: "Start Contributing" }, + { icon: faLightbulb, label: "Choose Idea" }, + { icon: faComments, label: "Discuss" }, + { icon: faPaperPlane, label: "Submit App" }, +]; + export function ApplyHeader({ children }) { const { scrollY } = useScroll(); const smoothScroll = useSpring(scrollY, { stiffness: 100, damping: 30, restDelta: 0.001 }); const translateX = useTransform(smoothScroll, [0, 800], [0, -30]); - const steps = [ - { icon: faDiscord, label: "Join Discord" }, - { icon: faGithub, label: "Start Contributing" }, - { icon: faLightbulb, label: "Choose Idea" }, - { icon: faComments, label: "Discuss" }, - { icon: faPaperPlane, label: "Submit App" }, - ];Then update line 65 to use
STEPS.map(...).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/apply/ApplyHeader.jsx` around lines 14 - 20, Move the steps array out of the ApplyHeader component into module scope as a constant (e.g., export or const STEPS = [...]) so it isn't recreated on every render; update the component to reference STEPS instead of the local steps variable and change the mapping call where steps was iterated (currently using steps.map in ApplyHeader) to STEPS.map. Ensure the new constant uses the same objects (icon and label) as the original steps and import any icon symbols (faDiscord, faGithub, faLightbulb, faComments, faPaperPlane) at top-level if needed.src/components/about/TimelineElement.jsx (1)
13-13: Removedurationfrom spring transitions with explicitstiffness/damping.In framer-motion,
type: "spring"supports two distinct configuration modes:
- Physics-based: Use
stiffness,damping, andmass(duration is an outcome of these values)- Duration-based: Use
durationandbounceinsteadCombining physics parameters (
stiffness: 50, damping: 20) withduration: 0.8mixes these approaches, which can lead to thedurationbeing ignored or producing inconsistent behavior.♻️ Suggested simplification
- transition={{ type: "spring", stiffness: 50, damping: 20, duration: 0.8 }} + transition={{ type: "spring", stiffness: 50, damping: 20 }}- transition={{ type: "spring", stiffness: 50, damping: 20, duration: 0.8, delay: 0.1 }} + transition={{ type: "spring", stiffness: 50, damping: 20, delay: 0.1 }}Also applies to: 23-23
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/about/TimelineElement.jsx` at line 13, The transition for the motion element in TimelineElement.jsx currently mixes physics and duration-based config (transition prop with type: "spring", stiffness: 50, damping: 20, duration: 0.8); remove the duration when using physics parameters (stiffness/damping) or convert the transition to a duration-based spring (use duration and bounce instead). Update the transition object used in the component (the transition prop passed to the motion element) to either drop duration or replace stiffness/damping with duration/bounce consistently, and apply the same change to the other occurrence noted in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/about/TimelineElement.jsx`:
- Line 6: Remove unused props and unused import from the TimelineElement
component: delete button, link, and classCondition from the TimelineElement({
... }) signature and remove the unused clsx import so the component only accepts
title, description, and time. Also fix the animation configs used in the motion
variants inside TimelineElement (the spring configs referenced in the current
animate/initial variants): either remove the duration property from the spring
configs (and rely on stiffness/damping/mass) or change the animation type to
"tween" and supply duration in that tween config (e.g., replace type: "spring" +
duration with type: "tween" + duration), updating both spring occurrences so
they are consistent.
---
Nitpick comments:
In `@src/components/about/TimelineElement.jsx`:
- Line 13: The transition for the motion element in TimelineElement.jsx
currently mixes physics and duration-based config (transition prop with type:
"spring", stiffness: 50, damping: 20, duration: 0.8); remove the duration when
using physics parameters (stiffness/damping) or convert the transition to a
duration-based spring (use duration and bounce instead). Update the transition
object used in the component (the transition prop passed to the motion element)
to either drop duration or replace stiffness/damping with duration/bounce
consistently, and apply the same change to the other occurrence noted in the
file.
In `@src/components/apply/ApplyHeader.jsx`:
- Around line 14-20: Move the steps array out of the ApplyHeader component into
module scope as a constant (e.g., export or const STEPS = [...]) so it isn't
recreated on every render; update the component to reference STEPS instead of
the local steps variable and change the mapping call where steps was iterated
(currently using steps.map in ApplyHeader) to STEPS.map. Ensure the new constant
uses the same objects (icon and label) as the original steps and import any icon
symbols (faDiscord, faGithub, faLightbulb, faComments, faPaperPlane) at
top-level if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ef3a6db4-45e8-4953-b950-4a492679d99c
📒 Files selected for processing (2)
src/components/about/TimelineElement.jsxsrc/components/apply/ApplyHeader.jsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/about/TimelineElement.jsx (1)
39-41: Render<time>only whentimeexists.
TimelineElementis used withouttimeinsrc/app/apply/page.jsx(Lines 22-41), so this currently renders an empty<time>block. Guarding it avoids unnecessary spacing and empty semantic markup.♻️ Suggested change
- <time className="block mb-2 font-mono text-sm font-normal leading-none text-gray-400 dark:text-gray-500"> - {time} - </time> + {time ? ( + <time className="block mb-2 font-mono text-sm font-normal leading-none text-gray-400 dark:text-gray-500"> + {time} + </time> + ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/about/TimelineElement.jsx` around lines 39 - 41, The <time> element in the TimelineElement component is always rendered even when the time prop is falsy, producing empty semantic markup and extra spacing; update the TimelineElement render to conditionally include the <time> block only when the time prop is present (e.g., wrap the existing <time className="...">{time}</time> in a conditional that checks the time prop) so that TimelineElement omits the element when time is undefined/null/empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/about/TimelineElement.jsx`:
- Around line 39-41: The <time> element in the TimelineElement component is
always rendered even when the time prop is falsy, producing empty semantic
markup and extra spacing; update the TimelineElement render to conditionally
include the <time> block only when the time prop is present (e.g., wrap the
existing <time className="...">{time}</time> in a conditional that checks the
time prop) so that TimelineElement omits the element when time is
undefined/null/empty.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 787a56d6-3f33-464e-a3a3-838db951916d
📒 Files selected for processing (1)
src/components/about/TimelineElement.jsx
Addressed Issues:
Fixes #681
Screenshots/Recordings:
Before Fix (icons shift incorrectly while scrolling):
Screen.Recording.2026-03-16.at.9.08.24.PM-2.mov
After Fix (smooth animation and improved alignment):
Screen.Recording.2026-03-16.at.11.15.36.PM.mov
Additional Notes:
This PR fixes a UI/UX issue on the Apply page where the icons in the steps section:
were shifting incorrectly to the left during scrolling.
The fix ensures that the icons now move smoothly and consistently from right to left without abrupt horizontal shifts.
Additionally, small UI/UX improvements were made to the top row of the five icons to improve alignment and visual consistency while keeping the original design intact.
AI Usage Disclosure:
I have used the following AI models and tools: None
Checklist
Summary by CodeRabbit
New Features
Improvements