Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough랜딩 앱에 고정 상단 헤더 컴포넌트를 신설하고 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as App (Landing)
participant Header as Header
participant Hero as HeroSection
User->>App: 페이지 진입
App->>Header: 고정 상단 바 렌더
App->>Hero: 히어로 섹션 렌더
note over Header,Hero: 독립적으로 UI 구성 (상단 헤더 + 본문 히어로)
User->>Header: "다운로드" 버튼 클릭
note right of Header: onClick TODO (미연결)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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. Comment |
|
✅ Storybook chromatic 배포 확인: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
apps/landing/src/components/HeroSection.tsx (3)
49-57: 스크롤 타겟 접근성 힌트 추가 권장버튼에 대상 섹션을 가리키는 힌트가 있으면 스크린리더 사용성이 개선됩니다.
- <Button - variant="secondary" - onClick={() => { + <Button + type="button" + aria-controls="bookmark-section" + aria-label="북마크 섹션으로 스크롤" + variant="secondary" + onClick={() => {
32-40: 이미지 대체 텍스트 정교화두 이미지가 모두 "Landing Icon"으로 동일합니다. 장식용이면 빈 alt로, 의미가 있다면 구체적으로 작성하세요.
- <img src={landing_icon} alt="Landing Icon" /> + <img src={landing_icon} alt="" aria-hidden="true" /> ... - <img src={landing_bell} alt="Landing Icon" /> + <img src={landing_bell} alt="알림 벨 아이콘" />
20-26: 스타일 태그 중복 주입 방지HMR/중복 마운트 시 스타일 태그가 누적될 수 있습니다. id로 식별해 1회만 추가하는 편이 안전합니다.
useEffect(() => { - const style = document.createElement('style'); - style.innerHTML = floatAnimationStyle; - document.head.appendChild(style); + const id = 'hero-float-keyframes'; + let style = document.getElementById(id) as HTMLStyleElement | null; + if (!style) { + style = document.createElement('style'); + style.id = id; + style.innerHTML = floatAnimationStyle; + document.head.appendChild(style); + } return () => { - document.head.removeChild(style); + const el = document.getElementById(id); + if (el) document.head.removeChild(el); }; }, []);apps/landing/src/components/Header.tsx (3)
6-6: 헤더 배경/분리감 추가로 가독성 개선배경이 없으면 콘텐츠 위에 텍스트가 겹쳐 보일 수 있습니다. 얇은 배경/블러/보더 추천합니다.
- <header className="fixed left-0 right-0 top-0 z-50 flex h-[7.4rem] w-full items-center justify-between px-4 md:px-8"> + <header className="fixed left-0 right-0 top-0 z-50 flex h-[7.4rem] w-full items-center justify-between px-4 md:px-8 bg-white/80 backdrop-blur-sm supports-[backdrop-filter]:bg-white/60 border-b border-gray200">
7-11: 로고/버튼 접근성 보강
- 로고는 홈 링크이거나 대체 텍스트가 있어야 합니다.
- 버튼은 명확한 타입/라벨이 있으면 좋습니다.
- <Icon name={'logo'} width={87} height={24} /> + <a href="/" aria-label="홈"> + <Icon name={'logo'} width={87} height={24} aria-hidden="true" /> + </a> ... - {/*TODO: onClick 추가 */} - <Button>다운로드</Button> + {/*TODO: onClick 추가 */} + <Button type="button" aria-label="앱 다운로드">다운로드</Button>
10-10: 다운로드 동작 미구현(TODO)스토어 링크(or 파일) 연결 필요 시 알려주세요. 구현 PR 보조 가능합니다.
apps/landing/src/App.tsx (2)
20-22: 고정 헤더에 의해 앵커가 가려지는 현상 예방북마크 섹션에
scroll-mt를 주어 헤더 높이만큼 오프셋을 두세요. 버튼의scrollIntoView와도 잘 동작합니다.- <section className="h-dvh snap-start" id="bookmark-section"> + <section className="h-dvh snap-start scroll-mt-[7.4rem]" id="bookmark-section">
12-14: 헤더를 스크롤 컨테이너 밖으로 이동 고려지금도 fixed라 동작엔 문제 없으나, 스크롤-스냅/포커스 관리 분리를 위해 컨테이너 밖에 두면 더 안전합니다.
이 변경이 디자인 의도와 맞는지 확인 부탁드립니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/landing/src/App.tsx(1 hunks)apps/landing/src/components/Header.tsx(1 hunks)apps/landing/src/components/HeroSection.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
🔇 Additional comments (2)
apps/landing/src/components/HeroSection.tsx (1)
58-58: 오타 수정 LGTM버튼 라벨 “스크롤하기” 수정 좋습니다.
apps/landing/src/App.tsx (1)
8-8: 헤더 연결 LGTM컴포넌트 분리 및 import 적절합니다.
|
|
||
| const Header = () => { | ||
| return ( | ||
| <header className="fixed left-0 right-0 top-0 z-50 flex h-[7.4rem] w-full items-center justify-between px-[8rem]"> |
There was a problem hiding this comment.
모바일에서 좌우 패딩 과다로 레이아웃 깨짐 가능
px-[8rem](128px)은 작은 뷰포트에서 요소가 겹치거나 넘칠 수 있습니다. 반응형 패딩으로 조정하세요.
- <header className="fixed left-0 right-0 top-0 z-50 flex h-[7.4rem] w-full items-center justify-between px-[8rem]">
+ <header className="fixed left-0 right-0 top-0 z-50 flex h-[7.4rem] w-full items-center justify-between px-4 md:px-8">📝 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.
| <header className="fixed left-0 right-0 top-0 z-50 flex h-[7.4rem] w-full items-center justify-between px-[8rem]"> | |
| <header className="fixed left-0 right-0 top-0 z-50 flex h-[7.4rem] w-full items-center justify-between px-4 md:px-8"> |
🤖 Prompt for AI Agents
In apps/landing/src/components/Header.tsx around line 6 the header uses a fixed
large horizontal padding `px-[8rem]` which can break layout on small viewports;
replace the hardcoded padding with responsive utility classes (e.g., small fixed
padding on mobile and progressively larger paddings on wider breakpoints) or use
a centered container with responsive horizontal padding (container/mx-auto +
responsive px) so the header no longer overflows on mobile.
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
📷 Screenshot
Summary by CodeRabbit