Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough개별 랜딩 섹션들을 하나의 새 Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 배포 확인: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/landing/src/App.tsx`:
- Around line 61-67: The Hero CTA stopped working because the target id
"#bookmark-section" was removed from the new sections; restore the scroll target
by either adding id="bookmark-section" to the intended section element (e.g., on
the container wrapping ShareBookmarkSection or FeatureBookmarkSection) or update
the selector used in HeroSection.tsx (the call to
document.querySelector('#bookmark-section')?.scrollIntoView(...)) to match the
new structure (e.g., target the appropriate class or a new id); ensure the
selected element exists and retains the same scrolling behavior.
- Around line 14-53: The global keydown handler inside the useEffect
(handleKeyDown) is preventing default keyboard behavior for all focused
elements; update handleKeyDown to early-return unless the scroll container
(scrollRef.current) itself is the active/focused element or the event target is
not a focusable control — i.e., if document.activeElement is not the
scrollRef.current or the event.target is a button, a, input, textarea, select,
or [contenteditable] then do nothing; only call e.preventDefault() and
container.scrollBy when the scroll container is focused (or the event target is
inside the scroll container and not a form control). Ensure the cleanup logic
and tabIndex on the container remain unchanged.
In `@apps/landing/src/components/ShareBookmarkSection.tsx`:
- Line 9: In ShareBookmarkSection (the <img src={JobBookmark} ... /> JSX)
replace the current alt="JobBookmark" with an appropriate value: if the image is
purely decorative set alt="" to hide it from screen readers, otherwise provide a
concise Korean description of the image’s purpose/content that matches the
section (e.g., a short sentence describing the bookmark visual or its function);
apply the same change to any other section images in this PR that use file-name
alt text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f0d8565d-c746-40d5-9450-8115355f8863
⛔ Files ignored due to path filters (5)
apps/landing/src/assets/�Bookmark.pngis excluded by!**/*.pngapps/landing/src/assets/Bookmark.svgis excluded by!**/*.svgapps/landing/src/assets/Dotori.svgis excluded by!**/*.svgapps/landing/src/assets/JobBookmark.svgis excluded by!**/*.svgapps/landing/src/assets/Remind.svgis excluded by!**/*.svg
📒 Files selected for processing (11)
apps/landing/src/App.tsxapps/landing/src/assets/1_landingmain.jsonapps/landing/src/assets/2_bookmark.jsonapps/landing/src/assets/3_bell.jsonapps/landing/src/assets/4_up.jsonapps/landing/src/assets/5_chippiface.jsonapps/landing/src/components/.gitkeepapps/landing/src/components/FeatureBookmarkSection.tsxapps/landing/src/components/FeatureReminderSection.tsxapps/landing/src/components/FeatureRewardSection.tsxapps/landing/src/components/ShareBookmarkSection.tsx
💤 Files with no reviewable changes (3)
- apps/landing/src/assets/2_bookmark.json
- apps/landing/src/assets/3_bell.json
- apps/landing/src/assets/4_up.json
apps/landing/src/App.tsx
Outdated
| <section className="h-dvh snap-start"> | ||
| <ShareBookmarkSection /> | ||
| </section> | ||
|
|
||
| <section className="h-dvh snap-start"> | ||
| <FeatureBookmarkSection /> | ||
| </section> |
There was a problem hiding this comment.
#bookmark-section 타깃을 지워서 Hero CTA가 더 이상 동작하지 않습니다.
apps/landing/src/components/HeroSection.tsx:49-60는 여전히 document.querySelector('#bookmark-section')?.scrollIntoView(...)를 호출합니다. 이 PR에서 그 id가 사라져서 버튼을 눌러도 아무 일도 일어나지 않습니다. 의도한 대상 섹션에 id를 유지하거나, Hero 쪽 selector를 새 섹션 구조에 맞게 같이 갱신해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/landing/src/App.tsx` around lines 61 - 67, The Hero CTA stopped working
because the target id "#bookmark-section" was removed from the new sections;
restore the scroll target by either adding id="bookmark-section" to the intended
section element (e.g., on the container wrapping ShareBookmarkSection or
FeatureBookmarkSection) or update the selector used in HeroSection.tsx (the call
to document.querySelector('#bookmark-section')?.scrollIntoView(...)) to match
the new structure (e.g., target the appropriate class or a new id); ensure the
selected element exists and retains the same scrolling behavior.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/landing/src/components/contents/FeatureBookmarkSection.tsx`:
- Line 18: In FeatureBookmarkSection, the img element uses an uninformative
alt="Bookmark"; either mark the image as purely decorative by setting alt="" and
adding aria-hidden="true" on the <img>, or replace the alt text with a concise
Korean description that conveys the image's informational role in this section
(e.g., describe what the bookmark graphic represents in context); update the
<img> in FeatureBookmarkSection accordingly depending on whether it is
decorative or content-bearing.
In `@apps/landing/src/components/contents/FeatureRewardSection.tsx`:
- Line 19: FeatureRewardSection의 이미지 태그 (img src={Dotori})의 alt 텍스트가 의미 전달이
부족하므로, 의도에 따라 빈 alt로 처리하거나 화면 카피에 맞는 설명형 대체텍스트로 교체하세요; 예를 들어 장식용이면 alt=""로 설정하고,
의미가 있다면 "도토리 보상 루프 일러스트" 같은 짧고 명확한 설명으로 alt 값을 바꾸어 접근성을 개선하세요.
In `@apps/landing/src/hooks/useKeyboardScroll.ts`:
- Around line 20-32: 현재 핸들러는 Space를 항상 아래로만 처리하고 키 반복을 막지 않아 Shift+Space의 기본 위로
이동을 깨고 길게 누르면 여러 섹션이 연속 전환됩니다; 수정 방법은 useKeyboardScroll 내에서 key 이벤트 처리부(e.key
cases)를 변경해 Space(' ')일 때 e.shiftKey가 true이면 위로(scrollBy top: -pageHeight) 아니면
아래로(scrollBy top: pageHeight) 이동하도록 분기하고, 반복 키 입력을 막기 위해 핸들러 초기에 e.repeat를 검사해
true면 바로 반환하거나 반복 시 동작을 무시하도록 하며, preventDefault는 실제로 스크롤 동작을 수행할 때만 호출하게 하십시오
(참조: e.key, e.shiftKey, e.repeat, container.scrollBy, pageHeight).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bb76707b-fa17-482b-b1fa-3c8125a74a40
📒 Files selected for processing (10)
apps/landing/src/App.tsxapps/landing/src/components/contents/Contents.tsxapps/landing/src/components/contents/FeatureBookmarkSection.tsxapps/landing/src/components/contents/FeatureReminderSection.tsxapps/landing/src/components/contents/FeatureRewardSection.tsxapps/landing/src/components/contents/FinalCTASection.tsxapps/landing/src/components/contents/HeroSection.tsxapps/landing/src/components/contents/ShareBookmarkSection.tsxapps/landing/src/hooks/.gitkeepapps/landing/src/hooks/useKeyboardScroll.ts
✅ Files skipped from review due to trivial changes (5)
- apps/landing/src/components/contents/HeroSection.tsx
- apps/landing/src/components/contents/FeatureReminderSection.tsx
- apps/landing/src/components/contents/FinalCTASection.tsx
- apps/landing/src/components/contents/Contents.tsx
- apps/landing/src/components/contents/ShareBookmarkSection.tsx
apps/landing/src/components/contents/FeatureBookmarkSection.tsx
Outdated
Show resolved
Hide resolved
| const pageHeight = window.innerHeight; | ||
|
|
||
| switch (e.key) { | ||
| case 'ArrowDown': | ||
| case 'PageDown': | ||
| case ' ': | ||
| e.preventDefault(); | ||
| container.scrollBy({ top: pageHeight, behavior: 'smooth' }); | ||
| break; | ||
| case 'ArrowUp': | ||
| case 'PageUp': | ||
| e.preventDefault(); | ||
| container.scrollBy({ top: -pageHeight, behavior: 'smooth' }); |
There was a problem hiding this comment.
Shift+Space와 반복 입력을 처리하지 않아 페이지 전환이 과하게 발생할 수 있습니다.
지금 구현은 Space를 항상 아래로만 보내서 기본 브라우저 동작인 Shift+Space 위로 이동을 깨고, keydown 반복도 그대로 받아서 키를 길게 누르면 여러 섹션이 연속으로 넘어갑니다. PR 목표가 “브라우저 표준 동작 + 한 페이지씩 전환”이라면 여기서 방향과 반복을 같이 제어해야 합니다.
🔧 제안 수정안
- const pageHeight = window.innerHeight;
+ const pageHeight = container.clientHeight;
+ if (e.repeat) return;
switch (e.key) {
case 'ArrowDown':
case 'PageDown':
case ' ':
e.preventDefault();
- container.scrollBy({ top: pageHeight, behavior: 'smooth' });
+ container.scrollBy({
+ top: e.key === ' ' && e.shiftKey ? -pageHeight : pageHeight,
+ behavior: 'smooth',
+ });
break;
case 'ArrowUp':
case 'PageUp':
e.preventDefault();
container.scrollBy({ top: -pageHeight, behavior: 'smooth' });
break;📝 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.
| const pageHeight = window.innerHeight; | |
| switch (e.key) { | |
| case 'ArrowDown': | |
| case 'PageDown': | |
| case ' ': | |
| e.preventDefault(); | |
| container.scrollBy({ top: pageHeight, behavior: 'smooth' }); | |
| break; | |
| case 'ArrowUp': | |
| case 'PageUp': | |
| e.preventDefault(); | |
| container.scrollBy({ top: -pageHeight, behavior: 'smooth' }); | |
| const pageHeight = container.clientHeight; | |
| if (e.repeat) return; | |
| switch (e.key) { | |
| case 'ArrowDown': | |
| case 'PageDown': | |
| case ' ': | |
| e.preventDefault(); | |
| container.scrollBy({ | |
| top: e.key === ' ' && e.shiftKey ? -pageHeight : pageHeight, | |
| behavior: 'smooth', | |
| }); | |
| break; | |
| case 'ArrowUp': | |
| case 'PageUp': | |
| e.preventDefault(); | |
| container.scrollBy({ top: -pageHeight, behavior: 'smooth' }); | |
| break; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/landing/src/hooks/useKeyboardScroll.ts` around lines 20 - 32, 현재 핸들러는
Space를 항상 아래로만 처리하고 키 반복을 막지 않아 Shift+Space의 기본 위로 이동을 깨고 길게 누르면 여러 섹션이 연속
전환됩니다; 수정 방법은 useKeyboardScroll 내에서 key 이벤트 처리부(e.key cases)를 변경해 Space(' ')일 때
e.shiftKey가 true이면 위로(scrollBy top: -pageHeight) 아니면 아래로(scrollBy top:
pageHeight) 이동하도록 분기하고, 반복 키 입력을 막기 위해 핸들러 초기에 e.repeat를 검사해 true면 바로 반환하거나 반복 시
동작을 무시하도록 하며, preventDefault는 실제로 스크롤 동작을 수행할 때만 호출하게 하십시오 (참조: e.key,
e.shiftKey, e.repeat, container.scrollBy, pageHeight).
constantly-dev
left a comment
constantly-dev
left a comment
There was a problem hiding this comment.
수고하셨어요! 간단 코멘트만 확인 부탁드려요 👍
| export const useKeyboardScroll = ( | ||
| scrollRef: RefObject<HTMLDivElement | null> | ||
| ) => { |
There was a problem hiding this comment.
useKeyboardScroll는 이름만으로 어느정도 예측이 가능하긴해요. 그리고 return이 되는 요소도 없기 때문에 사이드 이펙트 전용이라는 것이 명확하게 보이기도 합니다. 👍
다만 여기서 scrollRef를 받는데 ref라는 이름만 보고는 사실 요소 대부분이 사용이 가능한 것처럼 보여요. (=예측이 돼요)
하지만 실제로는 HTMLDivElement라는 구체 타입으로 좁혀져 잇는 것을 볼 수 있어요. 사실상 .scrollBy()이 되는 요소라면 모두 사용이 가능하고, scrollRef라는 이름을 보고 예측이 쉽게하려면 HTMLElement으로 타입을 지정하는 것은 어떨지 생각이 들기도 합니다.
정민님은 어떻게 생각하시나요??
There was a problem hiding this comment.
확실히 현재 코드에서만 사용한다 생각해서 단순히 HTMLDivElement 로 했지만 확장이나 유지보수성을 생각하면 HTMLElement타입을 지정하면 훨씬 직관적으로 Ref를 넘겨줄 수 있을 것 같습니다.
"이 요소가 무엇인지(div)" 보다 "이 요소가 무엇을 할 수 있는지(HTMLElement - scrollBy)" 에 집중하자는게 더 좋은 방향성이라고 생각이드네요-!
좋은 의견 감사합니다
There was a problem hiding this comment.
리액트의 <div> 태그는 내부적으로 HTMLDivElement 타입을 엄격하게 요구하기 때문에, 훅에서 단순히 HTMLElement로 타입을 넓혀버리면 사용하는 쪽에서 타입 불일치 에러가 발생하게 됩니다.
이 문제를 해결하기 위해 사용처의 타입을 상위 타입인 HTMLElement로 맞추는 방법도 있겠지만, 저는 훅 자체를 제네릭 타입()으로 설계하는 것이 가장 확장성 있다고 생각합니다.
혹시 진혁님은 어떻게 생각하시나요?
| <section className="h-dvh snap-start"> | ||
| <HeroSection /> | ||
| </section> |
There was a problem hiding this comment.
궁금한 부분이 section가 HeroSection이 아닌 외부에서 정의하신 이유가 있으신가요?
해당 태그가 어떤 관심사로 의도하신 것인지 궁금해요!
There was a problem hiding this comment.
section이 외부에서 정의된 이유는 스크롤 스냅의 단위 설정을 하기 위함이었어요.
snap-start와 h-dvh를 가지고 있어, 브라우저가 "여기가 한 페이지의 시작점이다"라고 인식하게 만드는 프레임 역할을 하도록 만들었습니다.
다만 HeroSection 안에서도 section이 중복되고 있어 이 부분은 section을 안쪽으로 다 옮기거나 바깥으로 빼는 방식으로 수정해야할 것 같네요
2 participants
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
키보드 이벤트 추가
화면 전환에 있어 키보드로 화면전환하게 할 수 있으면 ux에 좋겠다고 생각했어요.
그래서 DownArrow, Space, UpArrow 등의 키를 지원하여, 브라우저 표준 동작을 유지하면서도 한 페이지씩 명확하게 전환되는 UX를 제공하고자 했습니다.
useEffect 내에서 keydown 이벤트를 등록하고, 컴포넌트 언마운트 시 removeEventListener를 통해 메모리 누수를 방지하고 자원을 효율적으로 관리하도록 구현했습니다.
app.tsx 리팩토링
기존 App.tsx에 나열되어 있던 수많은 섹션 컴포넌트들을 Contents.tsx로 분리하여 메인 레이아웃과 콘텐츠 구성했습니다.
또한 커스텀 훅을 통한 로직 추상화로 useKeyboardScroll을 만들어 키보드 핸들링 로직을 분리했습니다.
App.tsx가 "어떻게 스크롤을 계산하는지"에 대한 상세 구현까지 알 필요가 없다고 판단했습니다. 훅을 통해 "키보드 스크롤 기능을 사용한다"는 의도만 선언적으로 드러내어 유지보수성을 향상시켰습니다.
📷 Screenshot
Summary by CodeRabbit
새로운 기능
스타일
잡일(Chores)