-
Notifications
You must be signed in to change notification settings - Fork 0
[REFACTOR] 랜딩페이지 로그인 버튼 통합 및 스크롤 기반 위치 조정 #321
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
Conversation
WalkthroughThe changes consolidate multiple instances of the login button across various landing page guide components into a single, fixed-position login button on the main landing page. The button's vertical position dynamically adjusts based on scroll position using a new state. Associated style and layout adjustments are made, and redundant button code is removed from subcomponents. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LandingPage
participant LoginButtonWrapper
User->>LandingPage: Scrolls page
LandingPage->>LandingPage: Updates isIntro state based on scroll position
LandingPage->>LoginButtonWrapper: Passes isIntro prop
LoginButtonWrapper->>User: Renders LoginButton at dynamic position
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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)
frontend/src/pages/LandingPage/index.tsx (1)
15-32: Well-implemented scroll tracking with proper cleanup.The scroll event handling is correctly implemented with proper event listener cleanup. The logic appropriately tracks whether the user is in the intro section.
Consider throttling the scroll events for better performance, especially on lower-end devices:
+ import { useCallback } from 'react'; + + // Add throttle utility or use a custom hook + const useThrottledCallback = (callback: Function, delay: number) => { + const [throttledCallback, setThrottledCallback] = useState(null); + + useEffect(() => { + const handler = setTimeout(() => { + setThrottledCallback(null); + }, delay); + + if (!throttledCallback) { + setThrottledCallback(callback); + callback(); + } + + return () => clearTimeout(handler); + }, [callback, delay, throttledCallback]); + }; const handleScroll = () => { const scrollTop = scrollContainer.scrollTop; const viewportHeight = window.innerHeight; setIsIntro(scrollTop < viewportHeight * 0.5); }; + const throttledHandleScroll = useThrottledCallback(handleScroll, 16); // ~60fps - scrollContainer.addEventListener('scroll', handleScroll); + scrollContainer.addEventListener('scroll', throttledHandleScroll);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
frontend/src/pages/LandingPage/LandingPage.styled.ts(1 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideIntro/GuideIntro.styled.ts(1 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideIntro/index.tsx(0 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideOutro/GuideOutro.styled.ts(1 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideOutro/index.tsx(0 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/GuideSlide.styled.ts(1 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/index.tsx(0 hunks)frontend/src/pages/LandingPage/index.tsx(1 hunks)
💤 Files with no reviewable changes (3)
- frontend/src/pages/LandingPage/components/ServiceGuide/GuideOutro/index.tsx
- frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/index.tsx
- frontend/src/pages/LandingPage/components/ServiceGuide/GuideIntro/index.tsx
🔇 Additional comments (8)
frontend/src/pages/LandingPage/components/ServiceGuide/GuideOutro/GuideOutro.styled.ts (1)
19-19: LGTM! Proper spacing adjustment for fixed login button.The added bottom padding appropriately accommodates the new fixed-position login button, ensuring the outro content isn't obscured by the overlay.
frontend/src/pages/LandingPage/components/ServiceGuide/GuideIntro/GuideIntro.styled.ts (1)
25-25: LGTM! Appropriate spacing adjustment.The added top margin properly positions the scroll down container to work with the new unified login button layout.
frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/GuideSlide.styled.ts (1)
15-15: Good responsive design improvement.Changing from fixed
5remto relative10%gap makes the layout more responsive and adaptive to different screen sizes, which aligns well with the centralized login button approach.Consider testing this on very large and very small screens to ensure the 10% gap maintains appropriate visual balance across all viewport sizes.
frontend/src/pages/LandingPage/LandingPage.styled.ts (1)
19-29: Floating login button styling approved with no z-index conflictsI’ve searched the codebase and found no other usages of z-index above 10, so the
z-index: 100onLoginButtonWrapperwon’t collide with existing layers. Everything else looks solid.• Continue with your cross-device and orientation tests to ensure the 28%/6% bottom offsets feel natural on all viewports.
• No further changes needed for z-index.frontend/src/pages/LandingPage/index.tsx (4)
1-2: LGTM! Proper imports for the new functionality.The added React hooks (useEffect, useState, useRef) are correctly imported for implementing the scroll tracking feature.
9-9: LGTM! LoginButton import added for centralized rendering.The LoginButton import supports the new unified approach where the button is rendered at the landing page level instead of in individual guide components.
12-13: Good state management for scroll-based positioning.The
isIntrostate andscrollContainerRefare properly declared to support the dynamic login button positioning based on scroll position.
35-46: Confirm scroll threshold aligns with intro heightThe scroll handler in
frontend/src/pages/LandingPage/index.tsxusessetIsIntro(scrollTop < window.innerHeight * 0.5);This assumes
<GuideIntro>occupies exactly half the viewport. Please verify that this 0.5 multiplier corresponds to the rendered height of the intro section. You can:
- Check that
<GuideIntro>’s CSS or styled-component sets its height to50vh(or otherwise equals half ofwindow.innerHeight).- If its height differs or is dynamic, compute the threshold from the actual element size, for example:
const introEl = scrollContainer.querySelector('#guide-intro') as HTMLElement; const introHeight = introEl.offsetHeight; setIsIntro(scrollTop < introHeight);Adjust the multiplier or implementation so that
isIntroflips exactly when users finish viewing the intro.
Issue Number
close #320
As-Is
To-Be
Check List
Test Screenshot
2025-07-25.19.03.09.mov
2025-07-25.19.08.15.mov
(Optional) Additional Description
Summary by CodeRabbit
New Features
Style
Refactor