-
Notifications
You must be signed in to change notification settings - Fork 0
[REFACTOR] 랜딩 페이지에 서비스 이용 가이드 추가 #289
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
WalkthroughA step-based service guide was introduced to the landing page. New components and types for guide slides, intro, and outro were added, along with styled components for layout and visual indicators. The main landing page was refactored to render these guide steps sequentially, replacing the previous static layout and carousel. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LandingPage
participant GuideIntro
participant GuideSlide
participant GuideOutro
User->>LandingPage: Visit page
LandingPage->>GuideIntro: Render intro section
User->>GuideIntro: View intro, scroll down
loop For each step in SERVICE_GUIDE_STEPS
LandingPage->>GuideSlide: Render GuideSlide(step)
User->>GuideSlide: View step, scroll to next
end
LandingPage->>GuideOutro: Render outro section
User->>GuideOutro: View outro, interact with login
Assessment against linked issues
Suggested reviewers
Poem
✨ Finishing Touches
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: 5
🧹 Nitpick comments (9)
frontend/src/pages/LandingPage/LandingPage.styled.ts (2)
8-10: Consider accessibility implications of scroll behavior.The mandatory scroll snapping and smooth scrolling enhance the user experience, but consider that some users may prefer reduced motion or find snap scrolling disorienting.
Consider respecting user motion preferences:
overflow-y: scroll; scroll-snap-type: y mandatory; - scroll-behavior: smooth; + scroll-behavior: smooth; + + @media (prefers-reduced-motion: reduce) { + scroll-behavior: auto; + scroll-snap-type: none; + }
12-16: Evaluate accessibility impact of hidden scrollbars.Hiding scrollbars completely may impact users who rely on visual scrollbar indicators for navigation context, especially on larger screens where scroll position might not be obvious.
Consider showing scrollbars on focus or hover for better accessibility:
scrollbar-width: none; -ms-overflow-style: none; &::-webkit-scrollbar { display: none; } + + &:focus-within::-webkit-scrollbar { + display: block; + }frontend/src/pages/LandingPage/LandingPage.types.ts (1)
10-14: Consider more specific typing for better type safety.The interface is well-structured, but could benefit from more constrained types for better validation and developer experience.
Consider adding constraints for better type safety:
export interface ServiceGuideStepType { - step: number; + step: 1 | 2 | 3 | 4 | 5; // or use a const assertion if steps are dynamic description: string; - image: string; + image: string; // Consider adding validation for image URLs/paths }Alternatively, if the step count is variable, consider adding a comment documenting the expected range or validation requirements.
frontend/src/pages/LandingPage/components/ServiceGuide/GuideOutro/GuideOutro.styled.ts (2)
14-18: Linear-gradient stop at 56 % creates an abrupt color cut-off
Unless you have a specific design spec, stopping the gradient at56.28%may produce a visible step on tall screens. Consider extending to100%or using additional color stops for a smoother transition.- ${({ theme }) => theme.colors.white} 56.28% + ${({ theme }) => theme.colors.white} 100%
28-31: DuplicateScrollDownTextstyle exists in GuideIntro
The exact same styled snippet lives inGuideIntro.styled.ts. Extract this to a shared helper (e.g.,common/ScrollDownText.ts) to keep styles DRY.frontend/src/pages/LandingPage/components/ServiceGuide/GuideIntro/GuideIntro.styled.ts (2)
27-30: DRY violation – sameScrollDownTextdefinition as Outro
See comment inGuideOutro.styled.ts; factor this out to a shared component to avoid style drift.
32-35: Hard-coded image dimensions may break responsive layout
Fixed4.4rem × 6.4remmeans the arrow can look blurry on high-DPI or oversized on small devices. Consider usingmax-width/height: autoor an SVG.frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/GuideSlide.styled.ts (2)
7-8: Left-over placeholder comment
/* justify-content: ; */looks like an unfinished line. Remove or complete it.- /* justify-content: ; */ + /* intentionally left blank – vertical spacing handled via gap */
52-55: ThirdButtonContainervariant found
Now we have three differently-styledButtonContainers (Intro, Outro, Slide). Preface with the component context (e.g.,SlideButtonContainer) or consolidate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
frontend/src/pages/LandingPage/LandingPage.constants.ts(2 hunks)frontend/src/pages/LandingPage/LandingPage.styled.ts(1 hunks)frontend/src/pages/LandingPage/LandingPage.types.ts(1 hunks)frontend/src/pages/LandingPage/components/CarouselList/CarouselList.styled.ts(1 hunks)frontend/src/pages/LandingPage/components/KakaoLoginButton/KakaoLoginButton.styled.ts(1 hunks)frontend/src/pages/LandingPage/components/KakaoLoginButton/index.tsx(1 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideIntro/GuideIntro.styled.ts(1 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideIntro/index.tsx(1 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideOutro/GuideOutro.styled.ts(1 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideOutro/index.tsx(1 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/GuideSlide.styled.ts(1 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/index.tsx(1 hunks)frontend/src/pages/LandingPage/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
frontend/src/pages/LandingPage/LandingPage.constants.ts (1)
frontend/src/pages/LandingPage/LandingPage.types.ts (1)
ServiceGuideStepType(10-14)
frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/index.tsx (1)
frontend/src/pages/LandingPage/LandingPage.types.ts (1)
ServiceGuideStepType(10-14)
frontend/src/pages/LandingPage/components/ServiceGuide/GuideIntro/GuideIntro.styled.ts (1)
frontend/src/pages/LandingPage/components/ServiceGuide/GuideOutro/GuideOutro.styled.ts (1)
ScrollDownText(28-31)
frontend/src/pages/LandingPage/components/ServiceGuide/GuideOutro/GuideOutro.styled.ts (2)
frontend/src/pages/LandingPage/components/ServiceGuide/GuideIntro/GuideIntro.styled.ts (1)
ScrollDownText(27-30)frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/GuideSlide.styled.ts (1)
ButtonContainer(52-55)
frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/GuideSlide.styled.ts (1)
frontend/src/pages/LandingPage/components/ServiceGuide/GuideOutro/GuideOutro.styled.ts (1)
ButtonContainer(33-38)
🔇 Additional comments (6)
frontend/src/pages/LandingPage/components/KakaoLoginButton/KakaoLoginButton.styled.ts (1)
3-9: LGTM! Well-structured styled component.The
ButtonContainerstyled component follows good practices with appropriate CSS properties for a flex container wrapper. The z-index positioning suggests it's designed for proper layering in the new guide layout.frontend/src/pages/LandingPage/components/CarouselList/CarouselList.styled.ts (1)
7-7: LGTM! Height adjustment aligns with new layout.The height reduction from 18rem to 15rem is appropriate for integrating the carousel into the new guided service flow layout.
frontend/src/pages/LandingPage/components/KakaoLoginButton/index.tsx (1)
12-19: LGTM! Clean wrapper addition without functional changes.The addition of
ButtonContainerwrapper maintains all existing functionality while providing the required container structure for the new layout. The onClick handler remains properly attached to the button element.frontend/src/pages/LandingPage/components/ServiceGuide/GuideOutro/index.tsx (1)
7-19: LGTM! Well-structured composition component.The
GuideOutrocomponent follows React best practices by cleanly composing existing components. The layout structure is logical and the Korean user prompt text is appropriate for the landing page context.frontend/src/pages/LandingPage/LandingPage.constants.ts (1)
1-1: LGTM! Proper type import added.The import statement correctly includes the
ServiceGuideStepTypeto support the new service guide steps constant.frontend/src/pages/LandingPage/components/ServiceGuide/GuideOutro/GuideOutro.styled.ts (1)
33-38: Re-use ofButtonContainername across modules can be error-prone
GuideSlide.styled.tsexports anotherButtonContainerwith different layout rules (position: absolute). Importing both in the same file increases the chance of a wrong import alias. Either rename one or colocate the shared style.
frontend/src/pages/LandingPage/components/ServiceGuide/GuideIntro/index.tsx
Outdated
Show resolved
Hide resolved
| {SERVICE_GUIDE_STEPS.map((step) => ( | ||
| <GuideSlide key={step.step} step={step} /> | ||
| ))} |
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.
💡 Verification agent
🧩 Analysis chain
Verify uniqueness of step keys for React reconciliation.
Using step.step as the key assumes that step numbers are unique across all guide steps. This should be validated to prevent React reconciliation issues.
Run the following script to verify that step numbers are unique in SERVICE_GUIDE_STEPS:
If duplicates are found, consider using a unique identifier or array index as the key:
- <GuideSlide key={step.step} step={step} />
+ <GuideSlide key={`step-${step.step}`} step={step} />🏁 Script executed:
#!/bin/bash
# Description: Check if step numbers in SERVICE_GUIDE_STEPS are unique
# Expected: No duplicate step numbers found
# Find the constants file and check for duplicate step numbers
fd "LandingPage.constants" --type f --exec cat {} \; | \
rg -A 20 "SERVICE_GUIDE_STEPS" | \
rg "step:" | \
sort | \
uniq -dLength of output: 114
Let's pinpoint the actual SERVICE_GUIDE_STEPS declaration and then extract its step values to check for duplicates:
This will clearly surface any repeated keys (or confirm uniqueness).
🏁 Script executed:
#!/bin/bash
# 1. Locate the file defining SERVICE_GUIDE_STEPS
file=$(rg -l "SERVICE_GUIDE_STEPS")
echo "Inspecting: $file"
# 2. Extract all `step:` values (numbers or strings) and list any duplicates
grep -Po "step\s*:\s*['\"]?\K[0-9A-Za-z_-]+(?=['\"]?)" "$file" \
| sort \
| uniq -d \
|| echo "No duplicate step values found."Length of output: 668
Let’s isolate the constants file and re-run the duplicate check just on that:
🏁 Script executed:
#!/bin/bash
# 1. Locate only the constants file defining SERVICE_GUIDE_STEPS
file=$(rg -l "SERVICE_GUIDE_STEPS" --glob "*LandingPage.constants.ts")
echo "Inspecting: $file"
# 2. Extract all `step:` values and report any repeats
grep -Po "step\s*:\s*['\"]?\K[0-9A-Za-z_-]+(?=['\"]?)" "$file" \
| sort \
| uniq -c \
| awk '$1 > 1 { print "Duplicate:", $2, "(count:", $1, ")" }' \
|| echo "✔ No duplicate step values found."Length of output: 467
Let’s inspect the actual SERVICE_GUIDE_STEPS declaration to confirm the property name and its values:
This will reveal how step is defined (and named) and allow us to accurately extract and check for duplicate values.
🏁 Script executed:
#!/bin/bash
# Show lines around the SERVICE_GUIDE_STEPS array in the constants file
rg -n "SERVICE_GUIDE_STEPS" -A 20 frontend/src/pages/LandingPage/LandingPage.constants.tsLength of output: 741
React key uniqueness verified – no duplicates found
I ran a scan against frontend/src/pages/LandingPage/LandingPage.constants.ts and confirmed that all step values (1 through 4) are unique, so using step.step as the React key is safe.
However, it’s worth adding a quick guard for an empty SERVICE_GUIDE_STEPS array to ensure the component handles that edge case gracefully. For example, in frontend/src/pages/LandingPage/index.tsx:
// Before rendering GuideSlide list
if (SERVICE_GUIDE_STEPS.length === 0) {
return <div>No guide steps available.</div>;
}
return (
<>
{SERVICE_GUIDE_STEPS.map((step) => (
<GuideSlide key={step.step} step={step} />
))}
</>
);This prevents rendering an empty fragment and makes the UI more robust if the steps ever come back empty.
🤖 Prompt for AI Agents
In frontend/src/pages/LandingPage/index.tsx around lines 11 to 13, add a guard
clause before rendering the SERVICE_GUIDE_STEPS list to check if the array is
empty. If it is empty, return a fallback UI element such as a div with a message
like "No guide steps available." This prevents rendering an empty fragment and
improves robustness when SERVICE_GUIDE_STEPS has no items.
| {Array.from({ length: 5 }).map((_, index) => ( | ||
| <S.IndexCircle key={index} $isActive={index + 1 === step.step} /> | ||
| ))} |
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.
🛠️ Refactor suggestion
Replace hardcoded step count with dynamic calculation.
The hardcoded array length of 5 creates a tight coupling between this component and the number of guide steps. If the number of steps changes, the indicators will be incorrect.
Make the step count dynamic by receiving the total count as a prop:
interface ServiceGuideProps {
step: ServiceGuideStepType;
+ totalSteps: number;
}
- const GuideSlide = ({ step }: ServiceGuideProps) => {
+ const GuideSlide = ({ step, totalSteps }: ServiceGuideProps) => {
return (
<S.GuideSlideLayout>
<LandingHeader />
<S.Description>{step.description}</S.Description>
<S.IndexContainer>
- {Array.from({ length: 5 }).map((_, index) => (
+ {Array.from({ length: totalSteps }).map((_, index) => (
<S.IndexCircle key={index} $isActive={index + 1 === step.step} />
))}
</S.IndexContainer>Then update the usage in LandingPage/index.tsx:
- <GuideSlide key={step.step} step={step} />
+ <GuideSlide key={step.step} step={step} totalSteps={SERVICE_GUIDE_STEPS.length} />🤖 Prompt for AI Agents
In frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/index.tsx
around lines 17 to 19, replace the hardcoded array length of 5 with a dynamic
value passed as a prop representing the total number of steps. Modify the
component to accept a prop like totalSteps and use Array.from({ length:
totalSteps }) for rendering the indicators. Then update the parent component
LandingPage/index.tsx to pass the correct totalSteps value to this component.
| export const Image = styled.img` | ||
| max-width: 260px; | ||
| z-index: 1; | ||
| position: relative; | ||
| bottom: 0; | ||
| `; |
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.
bottom: 0 on a position: relative element is a no-op
Offset properties have no effect with position: relative unless you intend to nudge the element. Use position: absolute inside a relative parent or drop the offset.
-export const Image = styled.img`
- max-width: 260px;
- z-index: 1;
- position: relative;
- bottom: 0;
-`;
+export const Image = styled.img`
+ max-width: 260px;
+ z-index: 1;
+ /* Removed ineffective positioning; parent handles layout */
+`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const Image = styled.img` | |
| max-width: 260px; | |
| z-index: 1; | |
| position: relative; | |
| bottom: 0; | |
| `; | |
| export const Image = styled.img` | |
| max-width: 260px; | |
| z-index: 1; | |
| /* Removed ineffective positioning; parent handles layout */ | |
| `; |
🤖 Prompt for AI Agents
In
frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/GuideSlide.styled.ts
between lines 26 and 31, the CSS rule 'bottom: 0' on a 'position: relative'
element has no effect. To fix this, either remove the 'bottom: 0' property if no
offset is needed, or change the position to 'absolute' if you intend to position
the element relative to its nearest positioned ancestor.
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 (2)
frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/index.tsx (2)
17-21: Previous feedback addressed, but consider making step calculation more robust.The dynamic step count implementation correctly addresses the previous review feedback. However, the step calculation
index + 1 === step.stepassumes 1-based step numbering, which could be fragile if the step numbering system changes.Consider adding a comment to clarify the 1-based assumption or making it more explicit:
<S.IndexContainer> {Array.from({ length: totalSteps }).map((_, index) => ( - <S.IndexCircle key={index} $isActive={index + 1 === step.step} /> + <S.IndexCircle key={index} $isActive={index + 1 === step.step} /> {/* Assumes 1-based step numbering */} ))} </S.IndexContainer>
22-22: Improve image alt text for better accessibility.The hardcoded alt text "service guide" is too generic and doesn't provide meaningful information about the specific step's content.
Consider making the alt text more descriptive:
- <S.Image src={step.image} alt="service guide" /> + <S.Image src={step.image} alt={`Service guide step ${step.step}: ${step.description}`} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/pages/LandingPage/LandingPage.constants.ts(2 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideIntro/index.tsx(1 hunks)frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/index.tsx(1 hunks)frontend/src/pages/LandingPage/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/pages/LandingPage/LandingPage.constants.ts
- frontend/src/pages/LandingPage/components/ServiceGuide/GuideIntro/index.tsx
- frontend/src/pages/LandingPage/index.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/index.tsx (1)
frontend/src/pages/LandingPage/LandingPage.types.ts (1)
ServiceGuideStepType(10-14)
🔇 Additional comments (3)
frontend/src/pages/LandingPage/components/ServiceGuide/GuideSlide/index.tsx (3)
1-9: LGTM! Interface properly implements previous feedback.The interface correctly includes the
totalStepsprop as suggested in previous reviews, making the component more flexible and maintainable.
24-26: LGTM! Clean button implementation.The button container wrapper provides good separation of concerns for styling.
11-31: Excellent implementation that addresses previous feedback.The component structure is clean and well-organized. The implementation successfully incorporates the dynamic step counting that was suggested in previous reviews, making the component more maintainable and flexible.
Issue Number
close #288
As-Is
To-Be
Check List
Test Screenshot
2025-07-15.13.49.33.mov
(Optional) Additional Description
Summary by CodeRabbit
New Features
Style
Refactor