[refactor] Header 컴포넌트 리팩터링 - View와 service 로직 분리 및 선언 순서 개선#340
Conversation
- navigate, setKeyword, setInputValue, trackEvent 기능을 하나의 서비스 훅으로 통합 - device에 따라 다른 이벤트명을 전송할 수 있도록 trackEventNames 상수를 추가
|
""" Walkthrough이 변경사항은 Header 컴포넌트의 리팩터링을 중심으로, 모바일과 데스크톱 뷰를 각각 별도의 컴포넌트로 분리하고, 네비게이션 및 트래킹 로직을 커스텀 훅( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Header
participant useIsMobile
participant useHeaderService
participant DesktopHeader
participant MobileHeader
User->>Header: 페이지 진입
Header->>useIsMobile: 현재 뷰포트 확인
useIsMobile-->>Header: isMobile 여부 반환
Header->>useHeaderService: 네비/트래킹 핸들러 준비
alt isMobile
Header->>MobileHeader: 모바일 헤더 렌더링 (핸들러 전달)
else
Header->>DesktopHeader: 데스크톱 헤더 렌더링 (핸들러 전달)
end
Assessment against linked issues
Possibly related issues
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ 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:
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 (
|
✅ Deploy Preview for moadong ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
frontend/src/hooks/useIsMobile.ts (3)
4-4: 매직 넘버 상수화 고려해보세요.모바일 기준점으로 사용하는
500px값은 매직 넘버입니다. 나중에 이 값을 변경해야 할 경우 여러 파일에서 수정해야 할 수 있습니다. 스타일 상수 파일에 정의하여 재사용하는 것이 좋습니다.- const [isMobile, setIsMobile] = useState(window.innerWidth <= 500); + const MOBILE_BREAKPOINT = 500; // 또는 상수 파일에서 import + const [isMobile, setIsMobile] = useState(window.innerWidth <= MOBILE_BREAKPOINT);
7-8: 리사이즈 핸들러도 상수를 사용하세요.이벤트 핸들러에서도 동일한 매직 넘버를 사용하고 있습니다. 상수화된 값을 여기서도 사용해야 합니다.
+ const MOBILE_BREAKPOINT = 500; // 또는 상수 파일에서 import const handleResize = () => { - setIsMobile(window.innerWidth <= 500); + setIsMobile(window.innerWidth <= MOBILE_BREAKPOINT); };
3-16: SSR 대응 필요이 훅은
window객체에 직접 접근하므로 서버 사이드 렌더링 환경에서는 오류가 발생할 수 있습니다. SSR 환경을 고려하여 초기 상태 설정과 이벤트 리스너 등록을 수정할 필요가 있습니다.const useIsMobile = () => { - const [isMobile, setIsMobile] = useState(window.innerWidth <= 500); + const [isMobile, setIsMobile] = useState(false); useEffect(() => { + // SSR 환경에서 안전하게 초기값 설정 + setIsMobile(window?.innerWidth <= 500); + const handleResize = () => { setIsMobile(window.innerWidth <= 500); }; window.addEventListener('resize', handleResize); return () => window.removeEventListener('resize', handleResize); }, []); return isMobile; };frontend/src/services/header/useHeaderService.ts (3)
5-8: 상수명 개선
trackEventNames는 용도를 더 명확히 드러내는 이름이 좋습니다. 헤더의 홈 버튼 클릭에 관한 이벤트 이름이므로homeButtonTrackEventNames와 같이 더 구체적인 이름을 사용하는 것이 좋습니다.-const trackEventNames = { +const homeButtonTrackEventNames = { desktop: 'Home Button Clicked', mobile: 'Mobile Home Button Clicked', } as const;
15-20: 함수 매개변수 타입 개선
navigateToHome함수에서device매개변수의 타입을trackEventNames에서 유도하고 있는데, 이는 상수명이 변경될 경우 타입도 함께 변경해야 합니다. 타입을 직접 정의하거나 유니온 타입을 사용하는 것이 더 명확할 수 있습니다.- const navigateToHome = (device: keyof typeof trackEventNames) => { + const navigateToHome = (device: 'desktop' | 'mobile') => { navigate('/'); setKeyword(''); setInputValue(''); - trackEvent(trackEventNames[device]); + trackEvent(homeButtonTrackEventNames[device]); };
22-26: 새 탭에서 열기 고려소개 페이지를 현재 탭에서 여는 것은 사용자가 현재 작업 중인 내용을 잃게 할 수 있습니다. 외부 링크는 일반적으로 새 탭에서 여는 것이 더 나은 사용자 경험을 제공합니다.
const goIntroducePage = () => { - window.location.href = - 'https://valiant-schooner-12c.notion.site/1a64ac84bab3805287e0cef50b563370'; + window.open( + 'https://valiant-schooner-12c.notion.site/1a64ac84bab3805287e0cef50b563370', + '_blank', + 'noopener,noreferrer' + ); trackEvent('Introduce Button Clicked'); };frontend/src/components/common/Header/Header.tsx (2)
36-38: 접근성 속성 중복
MobileMenu버튼에aria-label과alt속성이 모두 동일한 값으로 설정되어 있습니다. 이미지에는alt속성만, 버튼에는aria-label속성만 있으면 충분합니다.- <Styled.MobileMenu aria-label='메뉴 버튼'> + <Styled.MobileMenu> <img src={MenuBar} alt='메뉴 버튼' onClick={handleMenuClick} /> </Styled.MobileMenu>또는
alt속성을 더 구체적으로 수정:<Styled.MobileMenu aria-label='메뉴 버튼'> - <img src={MenuBar} alt='메뉴 버튼' onClick={handleMenuClick} /> + <img src={MenuBar} alt='' onClick={handleMenuClick} /> </Styled.MobileMenu>
31-33: 이미지 클릭 핸들러 위치 변경 고려이미지에 클릭 핸들러를 직접 부여하는 대신, 상위 버튼 컴포넌트에 핸들러를 부여하는 것이 더 접근성 측면에서 좋습니다. 이렇게 하면 키보드 탐색도 자연스럽게 지원됩니다.
데스크톱 헤더:
- <Styled.LogoButtonStyles> + <Styled.LogoButtonStyles onClick={() => handleHomeClick('desktop')}> <img src={DesktopMainIcon} alt='홈 버튼' - onClick={() => handleHomeClick('desktop')} /> </Styled.LogoButtonStyles>모바일 헤더도 비슷하게 수정할 수 있습니다.
Also applies to: 55-56
frontend/src/components/common/Header/Header.styles.ts (2)
109-121: 중복된 버튼 스타일링
MobileMainIcon과MobileMenu컴포넌트는 스타일링이 매우 유사합니다. 재사용 가능한 base 버튼 스타일을 만들고 확장하는 방식으로 리팩토링하면 코드 중복을 줄일 수 있습니다.+export const MobileIconButton = styled.button` + background-color: transparent; + border: none; + cursor: pointer; + + img { + width: 100%; + height: auto; + object-fit: cover; + } +`; + -export const MobileMainIcon = styled.button` +export const MobileMainIcon = styled(MobileIconButton)` width: 26px; height: 22px; - background-color: transparent; - border: none; - cursor: pointer; - - img { - width: 26px; - height: auto; - object-fit: cover; - } `; ... (비슷한 방식으로 다른 버튼 컴포넌트도 수정)Also applies to: 145-157
84-84: 매직 넘버 상수화 고려미디어 쿼리에서 사용하는
500px값은useIsMobile.ts에서도 사용되는 매직 넘버입니다. 모바일 구분 기준이 변경되면 모든 파일에서 수정해야 하므로, 이 값을 상수로 추출하여 공유하는 것이 좋습니다.+// 별도의 상수 파일에 정의하고 import하는 것이 이상적입니다 +const MOBILE_BREAKPOINT = '500px'; export const MobileHeaderContainer = styled.header` display: none; - @media (max-width: 500px) { + @media (max-width: ${MOBILE_BREAKPOINT}) { position: fixed; display: flex; /* ... */ } `;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/src/components/common/Header/Header.styles.ts(1 hunks)frontend/src/components/common/Header/Header.tsx(1 hunks)frontend/src/hooks/useIsMobile.ts(1 hunks)frontend/src/pages/MainPage/MainPage.tsx(0 hunks)frontend/src/pages/MainPage/components/MobileHeader/MobileHeader.styles.ts(0 hunks)frontend/src/pages/MainPage/components/MobileHeader/MobileHeader.tsx(0 hunks)frontend/src/services/header/useHeaderService.ts(1 hunks)
💤 Files with no reviewable changes (3)
- frontend/src/pages/MainPage/MainPage.tsx
- frontend/src/pages/MainPage/components/MobileHeader/MobileHeader.tsx
- frontend/src/pages/MainPage/components/MobileHeader/MobileHeader.styles.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/services/header/useHeaderService.ts (1)
frontend/src/context/SearchContext.tsx (1)
useSearch(33-39)
🔇 Additional comments (8)
frontend/src/hooks/useIsMobile.ts (1)
1-16: 좋은 커스텀 훅 구현입니다!이 훅은 모바일 디바이스 감지에 필요한 로직을 잘 캡슐화했습니다. 창 크기 변경에 대한 이벤트 리스너 등록과 정리가 적절히 처리되었습니다.
frontend/src/services/header/useHeaderService.ts (1)
1-37: 서비스 로직 분리가 잘 되었습니다!헤더의 서비스 로직을 별도의 훅으로 분리한 것은 매우 좋은 구조입니다. 이벤트 트래킹 및 네비게이션 로직이 깔끔하게 정리되었습니다.
frontend/src/components/common/Header/Header.tsx (5)
1-10: 헤더 컴포넌트 리팩토링 잘 되었습니다!모바일과 데스크톱 헤더를 통합하고 관련 로직을 분리한 구조가 매우 깔끔합니다. 각 기능별로 책임이 명확히 분리되어 있어 유지보수하기 좋은 코드가 되었습니다.
11-20: 인터페이스 명확히 정의되었습니다.모바일 헤더와 데스크톱 헤더의 props 인터페이스가 잘 정의되어 있어 타입 안전성이 보장됩니다.
22-41: 모바일 헤더 컴포넌트가 깔끔합니다.함수형 컴포넌트와 화살표 함수를 사용해 간결하게 정의되었습니다. 이미지에 대한 접근성 속성(alt)도 적절히 추가되어 있습니다.
43-67: 데스크톱 헤더 컴포넌트가 깔끔합니다.함수형 컴포넌트와 화살표 함수를 사용해 간결하게 정의되었습니다. 관리자 페이지에 따른 조건부 렌더링도 잘 구현되어 있습니다.
69-88: 메인 헤더 컴포넌트가 깔끔합니다.
useIsMobile훅을 사용한 조건부 렌더링과 필요한 로직만 전달하는 방식이 좋습니다. 코드가 간결하고 이해하기 쉽습니다.frontend/src/components/common/Header/Header.styles.ts (1)
81-157: 모바일 스타일 컴포넌트가 잘 구현되었습니다.모바일 헤더를 위한 스타일 컴포넌트들이 디자인 요구사항에 맞게 잘 구현되었습니다. 특히 500px 기준점을
useIsMobile훅과 일관되게 사용한 점이 좋습니다.
oesnuj
left a comment
oesnuj
left a comment
There was a problem hiding this comment.
Header 코드가 복잡했는데 훨씬 간결하고 가독성도 좋네요!
분리 좋습니다
seongwon030
deleted the
refactor/#339-header-view-logic-separation-FE-92
branch
2 participants
#️⃣연관된 이슈
📝작업 내용
Header 컴포넌트 통합
src/components/common/Header.tsxsrc/pages/MainPage/MobileHeader/MobileHeader.tsx해당 구조의 단점
모바일 헤더를 Header로 가져오기
중점적으로 리뷰받고 싶은 부분(선택)
처음으로 서비스 로직을 분리해봤습니다. 구조에 대해서 피드백 부탁드립니다!
논의하고 싶은 부분(선택)
handleClick같은 함수명을 어떤 식으로 바꾸면 좋을까요?🫡 참고사항
Summary by CodeRabbit
Summary by CodeRabbit