[FEAT] Header 컴포넌트 제작 #92
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
개요Header 컴포넌트 시스템이 추가되었습니다. Header, NavigationItem, MenuContainer, Menu, MenuTrigger, MenuItem 컴포넌트와 이들을 지원하는 컨텍스트, 커스텀 훅, Storybook 스토리가 포함됩니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant User
participant MenuTrigger
participant MenuContext as MenuContainer<br/>(Context)
participant Menu
participant MenuItem
participant useClickOutside
User->>MenuTrigger: 클릭
MenuTrigger->>MenuContext: setOpen(true)
MenuContext->>Menu: open = true
Menu->>Menu: 렌더링 시작
Menu->>useClickOutside: 외부 클릭 감지 등록
User->>MenuItem: 클릭
MenuItem->>MenuContext: setOpen(false)
MenuContext->>Menu: open = false
Menu->>Menu: 렌더링 중지
User->>useClickOutside: 메뉴 외부 영역 클릭
useClickOutside->>MenuContext: setOpen(false)
MenuContext->>Menu: open = false
Menu->>Menu: 렌더링 중지
예상 코드 리뷰 소요 시간🎯 3 (보통) | ⏱️ ~25분 추가 검토 필요 영역:
관련 이슈
추천 검토자
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 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 |
|
Update: 2025년 10월 30일 19시 38분 02초 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
src/shared/index.ts(1 hunks)src/shared/ui/Header/Header.stories.tsx(1 hunks)src/shared/ui/Header/Header.tsx(1 hunks)src/shared/ui/Header/Menu.tsx(1 hunks)src/shared/ui/Header/MenuContext.tsx(1 hunks)src/shared/ui/Header/MenuItem.tsx(1 hunks)src/shared/ui/Header/MenuTrigger.tsx(1 hunks)src/shared/ui/Header/NavigationItem.tsx(1 hunks)src/shared/ui/Header/index.ts(1 hunks)src/shared/ui/Header/menu-context.ts(1 hunks)src/shared/ui/Header/useClickOutside.ts(1 hunks)src/shared/ui/Header/useMenuCtx.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
src/shared/ui/Header/NavigationItem.tsx (3)
src/shared/ui/Header/MenuItem.tsx (1)
Props(13-22)src/shared/index.ts (3)
NavigationItem(27-27)cn(32-32)Body3(26-26)src/shared/ui/Header/index.ts (1)
NavigationItem(2-2)
src/shared/ui/Header/MenuContext.tsx (3)
src/shared/index.ts (2)
MenuContainer(27-27)cn(32-32)src/shared/ui/Header/index.ts (1)
MenuContainer(3-3)src/shared/ui/Header/menu-context.ts (1)
MenuContext(10-10)
src/shared/ui/Header/useMenuCtx.ts (1)
src/shared/ui/Header/menu-context.ts (1)
MenuContext(10-10)
src/shared/ui/Header/MenuItem.tsx (1)
src/shared/ui/Header/useMenuCtx.ts (1)
useMenuCtx(7-11)
src/shared/ui/Header/MenuTrigger.tsx (1)
src/shared/ui/Header/useMenuCtx.ts (1)
useMenuCtx(7-11)
src/shared/ui/Header/Menu.tsx (4)
src/shared/index.ts (2)
Menu(27-27)cn(32-32)src/shared/ui/Header/index.ts (1)
Menu(4-4)src/shared/ui/Header/useMenuCtx.ts (1)
useMenuCtx(7-11)src/shared/ui/Header/useClickOutside.ts (1)
useClickOutside(9-19)
src/shared/ui/Header/Header.tsx (2)
src/shared/index.ts (3)
Header(27-27)cn(32-32)Flex(10-10)src/shared/ui/Header/index.ts (1)
Header(1-1)
src/shared/ui/Header/Header.stories.tsx (6)
src/shared/ui/Header/Header.tsx (1)
Header(12-31)src/shared/ui/Header/NavigationItem.tsx (1)
NavigationItem(16-26)src/shared/ui/Header/MenuContext.tsx (1)
MenuContainer(16-28)src/shared/ui/Header/MenuTrigger.tsx (1)
MenuTrigger(13-35)src/shared/ui/Header/Menu.tsx (1)
Menu(15-40)src/shared/ui/Header/MenuItem.tsx (1)
MenuItem(24-67)
🔇 Additional comments (12)
src/shared/ui/Header/useMenuCtx.ts (1)
7-11: LGTM!컨텍스트 가드 패턴이 올바르게 구현되었습니다. 에러 메시지도 명확합니다.
src/shared/index.ts (1)
27-27: LGTM!Header 관련 컴포넌트들이 올바르게 export되었습니다.
src/shared/ui/Header/MenuContext.tsx (1)
16-28: LGTM!접근성을 위한
useId사용과 ref 포워딩이 잘 구현되었습니다. 메뉴 상태 관리 로직도 명확합니다.src/shared/ui/Header/Header.tsx (1)
14-19: Fixed 포지셔닝 사용 시 고려사항을 확인하세요.Header가
fixed포지셔닝을 사용하므로 페이지 컨텐츠가 헤더 아래에 가려질 수 있습니다. 레이아웃에서 적절한padding-top이나margin-top이 적용되는지 확인하세요.src/shared/ui/Header/index.ts (1)
1-6: LGTM!배럴 export가 깔끔하게 구성되었습니다.
src/shared/ui/Header/menu-context.ts (1)
3-10: LGTM!MenuContext 타입과 컨텍스트 정의가 명확하고 올바릅니다.
src/shared/ui/Header/Menu.tsx (1)
1-13: 타입 정의 및 임포트가 적절합니다.클라이언트 컴포넌트 지시어와 필요한 의존성들이 올바르게 설정되어 있습니다.
src/shared/ui/Header/Header.stories.tsx (2)
1-48: 스토리북 설정이 잘 구성되어 있습니다.메타데이터, 데코레이터, 그리고 네비게이션 데이터 구조가 적절하게 설정되어 있습니다.
49-61: 기본 스토리가 올바르게 구현되었습니다.로고 이미지와 링크가 적절하게 설정되어 있으며 접근성 속성(alt)도 포함되어 있습니다.
src/shared/ui/Header/MenuTrigger.tsx (2)
1-11: 타입 정의가 적절합니다.
aria-label속성을 포함한 Props 타입이 올바르게 정의되어 있습니다.
13-35: 접근성 속성이 올바르게 구현되었습니다.
MenuTrigger컴포넌트가 ARIA 속성(aria-haspopup,aria-expanded,aria-controls,aria-label)을 모두 적절하게 설정하고 있으며, 키보드 접근성을 위한type="button"속성도 포함되어 있습니다.src/shared/ui/Header/MenuItem.tsx (1)
24-67:MenuItem구현이 올바릅니다.링크(
href)와 버튼(onClick) 두 가지 경우를 모두 처리하고 있으며, 각 경우에 메뉴를 닫는 동작이 적절하게 구현되어 있습니다. 접근성을 위한role="menuitem"속성도 포함되어 있습니다.
Comment on lines
+89
to
+101
| {items.map((item) => { | ||
| const isExternal = /^https?:\/\//.test(item.href); | ||
| return ( | ||
| <MenuItem | ||
| key={item.id} | ||
| href={item.href} | ||
| target={isExternal ? '_blank' : '_self'} | ||
| rel="noopener noreferrer" | ||
| > | ||
| {item.content} | ||
| </MenuItem> | ||
| ); | ||
| })} |
There was a problem hiding this comment.
rel 속성이 모든 링크에 적용되고 있습니다.
Line 96에서 rel="noopener noreferrer"가 모든 MenuItem에 적용되고 있지만, 이는 외부 링크(target="_blank")에만 필요한 보안 속성입니다. 내부 링크에는 불필요합니다.
다음과 같이 외부 링크에만 rel 속성을 적용하도록 수정하세요:
{items.map((item) => {
const isExternal = /^https?:\/\//.test(item.href);
return (
<MenuItem
key={item.id}
href={item.href}
target={isExternal ? '_blank' : '_self'}
- rel="noopener noreferrer"
+ rel={isExternal ? 'noopener noreferrer' : undefined}
>
{item.content}
</MenuItem>
);
})}🤖 Prompt for AI Agents
In src/shared/ui/Header/Header.stories.tsx around lines 89 to 101, the
rel="noopener noreferrer" prop is being applied to every MenuItem but should
only be present for external links; change the JSX so that rel is set
conditionally (e.g., rel={isExternal ? 'noopener noreferrer' : undefined}) and
keep target as before so only links with target="_blank" receive the rel
attribute.
| return ( | ||
| <header | ||
| className={cn( | ||
| 'fixed top-0 left-0 z-10 w-full border-b border-gray-200 bg-white px-6 md:z-20', |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
z-index 값을 상수로 관리하는 것을 고려하세요.
하드코딩된 z-index 값(10, 20)이 여러 컴포넌트에서 일관되지 않게 사용될 수 있습니다. z-index 값을 중앙에서 관리하면 레이어 순서를 더 쉽게 제어할 수 있습니다.
예시:
// src/shared/lib/z-index.ts
export const Z_INDEX = {
header: 20,
modal: 50,
tooltip: 100,
} as const;- 'fixed top-0 left-0 z-10 w-full border-b border-gray-200 bg-white px-6 md:z-20',
+ `fixed top-0 left-0 w-full border-b border-gray-200 bg-white px-6`,
className
)}
+ style={{ zIndex: Z_INDEX.header }}🤖 Prompt for AI Agents
In src/shared/ui/Header/Header.tsx around line 16, the z-index values are
hardcoded ('z-10' and 'md:z-20'); extract z-index values to a single constants
file (e.g., src/shared/lib/z-index.ts) and replace the hardcoded classes with
references that map to those constants so all components use the same
centralized Z_INDEX values; update the Header to import the constant and use the
header z-index value for default and responsive variants, and refactor other
components that use 10/20 to reference the same constants for consistency.
Comment on lines
+19
to
+24
| useClickOutside({ | ||
| ref: menuRef, | ||
| handler: () => setOpen(false), | ||
| }); | ||
|
|
||
| if (!open) return null; |
There was a problem hiding this comment.
메뉴가 닫혀있을 때도 이벤트 리스너가 등록됩니다.
useClickOutside 훅이 메뉴의 open 상태와 무관하게 항상 호출되어 이벤트 리스너가 등록됩니다. 메뉴가 렌더링되지 않을 때(Line 24에서 null 반환)도 매 클릭마다 불필요한 체크가 발생합니다.
다음과 같이 메뉴가 열려있을 때만 훅을 실행하도록 개선할 수 있습니다:
export function Menu({ children, className }: Props) {
const { open, contentId, triggerId, setOpen } = useMenuCtx();
const menuRef = useRef<HTMLDivElement>(null);
- useClickOutside({
- ref: menuRef,
- handler: () => setOpen(false),
- });
-
if (!open) return null;
+
+ useClickOutside({
+ ref: menuRef,
+ handler: () => setOpen(false),
+ });
return (또는 훅 내부에서 조건부로 리스너를 등록하도록 수정할 수도 있습니다.
📝 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.
Suggested change
| useClickOutside({ | |
| ref: menuRef, | |
| handler: () => setOpen(false), | |
| }); | |
| if (!open) return null; | |
| if (!open) return null; | |
| useClickOutside({ | |
| ref: menuRef, | |
| handler: () => setOpen(false), | |
| }); |
🤖 Prompt for AI Agents
In src/shared/ui/Header/Menu.tsx around lines 19 to 24, the useClickOutside hook
is always invoked even when the menu is closed which registers unnecessary event
listeners; update the component so the hook is only executed when open is true
(e.g. call useClickOutside(...) inside a conditional if (open) block) OR modify
the hook signature to accept an enabled flag and only attach listeners when
enabled is true, ensuring listeners are removed/never added when the menu is
closed.
Comment on lines
+13
to
+22
| export type Props = { | ||
| children: ReactNode; | ||
| onClick?: () => void; | ||
| href?: string; | ||
| rel?: string; | ||
| target?: React.AnchorHTMLAttributes<HTMLAnchorElement>['target']; | ||
| icon?: IconName; | ||
| className?: string; | ||
| disabled?: boolean; | ||
| }; |
There was a problem hiding this comment.
사용되지 않는 disabled 속성이 정의되어 있습니다.
Line 21에서 disabled?: boolean 속성이 Props 타입에 정의되어 있지만, 컴포넌트 구현부(Lines 24-67)에서 전혀 사용되지 않습니다. 이 속성을 구현할 계획이 없다면 제거하는 것이 좋습니다.
disabled 기능을 구현하거나 타입에서 제거하는 코드를 생성해드릴까요?
🤖 Prompt for AI Agents
In src/shared/ui/Header/MenuItem.tsx around lines 13–22, the Props type defines
a disabled?: boolean that the component (lines 24–67) never uses; either remove
the disabled property from the Props type or implement it: if implementing,
accept disabled in Props, use it to short-circuit onClick handlers (no-op when
disabled), add appropriate accessibility attributes (aria-disabled=true), apply
disabled styling/className, and for anchor elements prevent navigation (e.g.,
preventDefault or remove href) and set tabIndex={-1} so the item is not
focusable; update any tests/types that reference the prop accordingly.
Comment on lines
+16
to
+26
| export function NavigationItem({ href, onClick, active, className, children }: Props) { | ||
| const base = | ||
| 'inline-flex items-center gap-2 rounded-md px-2 py-2 whitespace-nowrap transition-colors duration-200'; | ||
| const color = active ? 'text-primary-500' : 'text-neutral-500 hover:text-primary-300'; | ||
|
|
||
| return ( | ||
| <a href={href} onClick={onClick} className={cn(base, color, className)}> | ||
| <Body3 weight="semibold">{children}</Body3> | ||
| </a> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Next.js Link 컴포넌트 사용을 권장합니다.
현재 네이티브 <a> 태그를 사용하고 있어 클라이언트 사이드 네비게이션의 이점을 활용하지 못합니다. 또한 href와 onClick을 함께 사용할 때 event.preventDefault()가 없어 의도하지 않은 동작이 발생할 수 있습니다.
다음과 같이 리팩토링하는 것을 권장합니다:
+'use client';
+
+import { ReactNode } from 'react';
+import Link from 'next/link';
+
+import { cn } from '../../lib/core';
+import { Body3 } from '../Typography';
+
+type Props = {
+ href: string;
+ onClick?: () => void;
+ active?: boolean;
+ className?: string;
+ children: ReactNode;
+};
+
+export function NavigationItem({ href, onClick, active, className, children }: Props) {
const base =
'inline-flex items-center gap-2 rounded-md px-2 py-2 whitespace-nowrap transition-colors duration-200';
const color = active ? 'text-primary-500' : 'text-neutral-500 hover:text-primary-300';
+ const handleClick = (e: React.MouseEvent<HTMLAnchorElement>) => {
+ if (onClick) {
+ onClick();
+ }
+ };
+
return (
- <a href={href} onClick={onClick} className={cn(base, color, className)}>
+ <Link href={href} onClick={handleClick} className={cn(base, color, className)}>
<Body3 weight="semibold">{children}</Body3>
- </a>
+ </Link>
);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/shared/ui/Header/NavigationItem.tsx around lines 16 to 26, the component
uses a plain <a> which loses Next.js client-side navigation benefits and can
misbehave when onClick is provided; refactor to use Next.js Link (import Link
from 'next/link') and move className and children onto the inner <a> or use
Link's passHref behavior, ensure you still compute base and color classes and
apply them to the rendered anchor, and if an onClick is passed wrap it to call
event.preventDefault() before running the handler when you want to avoid default
navigation (or allow navigation by not preventing) so clicks behave predictably
while preserving client-side routing.
Comment on lines
+9
to
+18
| export function useClickOutside<T extends HTMLElement>({ ref, handler }: UseClickOutsideProps<T>) { | ||
| useEffect(() => { | ||
| const listener = (event: MouseEvent) => { | ||
| if (ref.current && !ref.current.contains(event.target as Node)) { | ||
| handler(); | ||
| } | ||
| }; | ||
| document.addEventListener('mousedown', listener); | ||
| return () => document.removeEventListener('mousedown', listener); | ||
| }, [ref, handler]); |
There was a problem hiding this comment.
enabled 속성이 구현에서 사용되지 않습니다.
UseClickOutsideProps에 enabled 속성이 정의되어 있지만 실제 훅 로직에서 사용되지 않고 있습니다. 이 속성을 사용하여 조건부로 이벤트 리스너를 등록/해제하거나, 불필요하다면 타입 정의에서 제거해야 합니다.
다음과 같이 수정하는 것을 권장합니다:
-export function useClickOutside<T extends HTMLElement>({ ref, handler }: UseClickOutsideProps<T>) {
+export function useClickOutside<T extends HTMLElement>({ ref, handler, enabled = true }: UseClickOutsideProps<T>) {
useEffect(() => {
+ if (!enabled) return;
+
const listener = (event: MouseEvent) => {
if (ref.current && !ref.current.contains(event.target as Node)) {
handler();
}
};
document.addEventListener('mousedown', listener);
return () => document.removeEventListener('mousedown', listener);
- }, [ref, handler]);
+ }, [ref, handler, enabled]);
}📝 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.
Suggested change
| export function useClickOutside<T extends HTMLElement>({ ref, handler }: UseClickOutsideProps<T>) { | |
| useEffect(() => { | |
| const listener = (event: MouseEvent) => { | |
| if (ref.current && !ref.current.contains(event.target as Node)) { | |
| handler(); | |
| } | |
| }; | |
| document.addEventListener('mousedown', listener); | |
| return () => document.removeEventListener('mousedown', listener); | |
| }, [ref, handler]); | |
| export function useClickOutside<T extends HTMLElement>({ ref, handler, enabled = true }: UseClickOutsideProps<T>) { | |
| useEffect(() => { | |
| if (!enabled) return; | |
| const listener = (event: MouseEvent) => { | |
| if (ref.current && !ref.current.contains(event.target as Node)) { | |
| handler(); | |
| } | |
| }; | |
| document.addEventListener('mousedown', listener); | |
| return () => document.removeEventListener('mousedown', listener); | |
| }, [ref, handler, enabled]); | |
| } |
🤖 Prompt for AI Agents
In src/shared/ui/Header/useClickOutside.ts around lines 9 to 18, the
UseClickOutsideProps includes an enabled boolean that is not used; update the
hook to respect enabled by only adding the mousedown listener when enabled is
true (and removing it when enabled becomes false or on cleanup), and include
enabled in the effect dependency array, or if enabled is unnecessary remove it
from the prop/type definition and all call sites; ensure the behavior is tested
so the listener is not attached when enabled is false.
Comment on lines
+10
to
+18
| useEffect(() => { | ||
| const listener = (event: MouseEvent) => { | ||
| if (ref.current && !ref.current.contains(event.target as Node)) { | ||
| handler(); | ||
| } | ||
| }; | ||
| document.addEventListener('mousedown', listener); | ||
| return () => document.removeEventListener('mousedown', listener); | ||
| }, [ref, handler]); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
handler 함수를 useCallback으로 메모이제이션하도록 권장하세요.
handler가 의존성 배열에 포함되어 있어, 부모 컴포넌트에서 매 렌더링마다 새로운 함수를 전달하면 불필요하게 이벤트 리스너가 재등록됩니다. 이는 성능 문제를 야기할 수 있습니다.
사용하는 쪽에서 handler를 useCallback으로 감싸도록 문서화하거나, 다음과 같이 useRef를 사용하여 최신 handler를 참조하도록 개선할 수 있습니다:
+import { useEffect, useRef, RefObject } from 'react';
type UseClickOutsideProps<T extends HTMLElement = HTMLElement> = {
ref: RefObject<T | null>;
handler: () => void;
enabled?: boolean;
};
export function useClickOutside<T extends HTMLElement>({ ref, handler, enabled = true }: UseClickOutsideProps<T>) {
+ const handlerRef = useRef(handler);
+
+ useEffect(() => {
+ handlerRef.current = handler;
+ }, [handler]);
+
useEffect(() => {
if (!enabled) return;
const listener = (event: MouseEvent) => {
if (ref.current && !ref.current.contains(event.target as Node)) {
- handler();
+ handlerRef.current();
}
};
document.addEventListener('mousedown', listener);
return () => document.removeEventListener('mousedown', listener);
- }, [ref, handler, enabled]);
+ }, [ref, enabled]);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/shared/ui/Header/useClickOutside.ts around lines 10 to 18, the effect
depends on the handler function which may change every render causing
unnecessary removal/re-registration of the mousedown listener; update the hook
to avoid this by storing the latest handler in a ref (e.g., create a handlerRef,
update handlerRef.current inside an effect when handler changes, and have the
event listener call handlerRef.current()) or alternatively document that callers
must wrap handler in useCallback—implement the handlerRef approach so the event
listener is stable and always invokes the latest handler without reattaching.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔥 연관 이슈
🚀 작업 내용
메인 페이지에 Header에 사용될 컴포넌트 제작을 하였습니다
🤔 고민했던 내용
헤더에 사용하는
useClickOutside를 헤더 컴포넌트 내부에 위치하게 해두었습니다해당 훅 또한 사용되는 곳이 많을 것으로 예상되어서, 따로 컴포넌트로 분리하면 어떨지 의견 부탁드립니다!
💬 리뷰 중점사항
Summary by CodeRabbit
릴리스 노트