-
Notifications
You must be signed in to change notification settings - Fork 0
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
[FEAT] NavBack 컴포넌트 개발 , 스토리북 제작 #5
base: main
Are you sure you want to change the base?
Conversation
Walkthrough이번 PR은 UI 자산 관리와 내비게이션 기능 개선을 위한 변경사항을 포함합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant U as 사용자
participant N as NavBack 컴포넌트
participant R as Next.js Router
U->>N: 아이콘 또는 타이틀 클릭
N->>R: router.back() 호출
R-->>U: 이전 페이지로 이동
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
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: 1
🧹 Nitpick comments (3)
src/shared/Typography/Typography.tsx (1)
48-51
: Props 타입 정의를 개선할 수 있습니다.
className
타입을 명시적으로 정의하면 좋겠습니다.- Props에 대한 JSDoc 문서화가 필요합니다.
다음과 같이 수정을 제안합니다:
+/** + * Typography 컴포넌트의 공통 Props + * @property {boolean} regularWeight - 일반 굵기 사용 여부 + * @property {keyof typeof colors} color - 텍스트 색상 + * @property {string} className - 추가 스타일 클래스 + */ type Props = { regularWeight?: boolean; color?: keyof typeof colors; + className?: string; } & ComponentPropsWithoutRef<'p'>;src/shared/Typography/Typography.mdx (2)
27-27
: 샘플 텍스트 형식을 개선해주세요.샘플 텍스트에 불필요한 공백이 있습니다. "예시입니다." 형식으로 수정하는 것이 좋겠습니다.
29-126
: 타이포그래피 문서화가 잘 되어있습니다.각 타이포그래피 스타일이 체계적으로 정리되어 있어 좋습니다. 다만, 접근성 관점에서 각 텍스트 스타일의 권장 사용 사례를 추가하면 더 유용할 것 같습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.storybook/preview.ts
(2 hunks)src/shared/BackSubtitle/BackSubtitle.stories.tsx
(1 hunks)src/shared/BackSubtitle/BackSubtitle.tsx
(1 hunks)src/shared/Typography/Typography.mdx
(1 hunks)src/shared/Typography/Typography.tsx
(1 hunks)tailwind.config.js
(1 hunks)tailwind.config.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- tailwind.config.js
- src/shared/BackSubtitle/BackSubtitle.stories.tsx
🔇 Additional comments (3)
tailwind.config.ts (1)
8-8
: 스토리북 통합을 위한 적절한 설정 추가입니다!스토리북 컴포넌트에 Tailwind CSS 스타일이 올바르게 적용될 수 있도록 content 배열에 stories 디렉토리가 포함되었습니다. 이는 PR의 목적인 스토리북 제작과 잘 부합됩니다.
.storybook/preview.ts (1)
2-3
: Storybook 설정이 적절히 구성되었습니다.Next.js의 App Router와 전역 스타일이 올바르게 통합되었습니다. 라우터 데코레이터를 통해 컴포넌트에서 라우팅 기능을 사용할 수 있습니다.
Also applies to: 13-15, 17-17
src/shared/Typography/Typography.tsx (1)
73-74
: ButtonTitle 컴포넌트의 기본 요소 타입을 재검토해주세요.버튼 타이틀 스타일을
button
요소에 직접 적용하면 의도치 않은 스타일 충돌이 발생할 수 있습니다.span
이나p
요소를 사용하는 것이 더 적절할 수 있습니다.
f7a1bd3
to
e080320
Compare
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: 1
🧹 Nitpick comments (6)
src/shared/BackSubtitle/BackSubtitle.tsx (3)
19-33
: SVG 아이콘을 별도 컴포넌트로 분리하는 것이 좋습니다.인라인 SVG는 코드의 가독성을 떨어뜨리고 재사용이 어렵습니다. 또한 현재 stroke 색상이 중복 정의되어 있습니다.
shared/icons/BackArrow.tsx
와 같은 별도 파일로 분리하는 것을 추천드립니다:export const BackArrow = () => ( <svg width="13" height="21" viewBox="0 0 13 21" fill="none" xmlns="http://www.w3.org/2000/svg" className="stroke-gray-600" > <path d="M10.9609 1.50024L2.75643 9.70475C2.31709 10.1441 2.31709 10.8564 2.75643 11.2957L10.9609 19.5002" strokeWidth="3" strokeLinecap="round" /> </svg> );그리고 컴포넌트에서는 다음과 같이 사용:
- <svg ... /> + <BackArrow />
7-9
: 타입 정의에 JSDoc 문서화가 필요합니다.컴포넌트의 props 타입에 대한 설명이 누락되어 있습니다.
다음과 같이 JSDoc을 추가하는 것을 추천드립니다:
+ /** + * BackSubtitle 컴포넌트의 속성을 정의합니다. + * @property {string} title - 표시할 부제목 텍스트 + */ type BackSubtitleProps = { title: string; };
1-6
: import 문을 그룹화하고 정렬하는 것이 좋습니다.외부 라이브러리와 내부 컴포넌트 import를 구분하여 가독성을 높일 수 있습니다.
다음과 같이 수정하는 것을 추천드립니다:
'use client'; + // External imports import React from 'react'; import { useRouter } from 'next/navigation'; + // Internal imports import { Head3 } from '../Typography/Typography';src/shared/Navbar/Navbar.stories.tsx (2)
5-27
: 메타 설정이 잘 구성되어 있습니다.스토리북 설정이 체계적으로 잘 되어있으나, 몇 가지 개선사항을 제안드립니다:
- 컴포넌트 문서화를 위해 더 자세한 설명을 추가하면 좋을 것 같습니다
- 다양한 사용 사례를 보여주기 위한 controls 설정을 추가하면 좋을 것 같습니다
parameters: { nextjs: { appDirectory: true, }, docs: { description: { - story: '클릭시 페이지 뒤로가기 이벤트를 발생시키는 컴포넌트입니다.', + story: '클릭시 페이지 뒤로가기 이벤트를 발생시키는 네비게이션 컴포넌트입니다. 상단에 타이틀과 뒤로가기 아이콘을 표시합니다.', }, }, + controls: { + expanded: true, + }, },
33-37
: 기본 스토리 구현이 단순 명확합니다.다만, 다양한 사용 사례를 보여주기 위해 추가 스토리를 작성하면 좋을 것 같습니다:
- 긴 제목 케이스
- 특수 문자가 포함된 제목 케이스
export const LongTitle: Story = { args: { title: '매우 긴 제목입니다. 이런 경우 어떻게 표시되는지 확인해보세요.', }, }; export const SpecialCharacters: Story = { args: { title: '특수문자 테스트 (★☆#@)', }, };src/shared/Navbar/Navbar.tsx (1)
1-10
: 컴포넌트 구조가 깔끔합니다.타입 정의와 클라이언트 컴포넌트 지시자가 잘 설정되어 있습니다. 다만, 몇 가지 개선사항을 제안드립니다:
- 타입에 JSDoc 문서화 추가
- 선택적 className prop 추가로 스타일 커스터마이징 지원
+/** + * 뒤로가기 기능이 있는 네비게이션 바 컴포넌트 + * @param title - 네비게이션 바에 표시될 제목 + * @param className - 추가적인 스타일링을 위한 클래스명 + */ type NavbarProps = { title: string; + className?: string; };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.storybook/preview.ts
(1 hunks)src/shared/BackSubtitle/BackSubtitle.stories.tsx
(1 hunks)src/shared/BackSubtitle/BackSubtitle.tsx
(1 hunks)src/shared/Navbar/Navbar.stories.tsx
(1 hunks)src/shared/Navbar/Navbar.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .storybook/preview.ts
- src/shared/BackSubtitle/BackSubtitle.stories.tsx
🔇 Additional comments (1)
src/shared/BackSubtitle/BackSubtitle.tsx (1)
15-18
: 🛠️ Refactor suggestiononClick 핸들러를 최적화해야 합니다.
인라인 화살표 함수를 사용하면 불필요한 리렌더링이 발생할 수 있습니다. 또한 접근성을 위한
aria-label
이 누락되었습니다.다음과 같이 수정하는 것을 추천드립니다:
+ const handleBack = React.useCallback(() => { + router.back(); + }, [router]); <button className="flex cursor-pointer flex-row items-center gap-4" - onClick={() => router.back()} + onClick={handleBack} + aria-label="이전 페이지로 돌아가기" >Likely invalid or redundant comment.
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: 1
🧹 Nitpick comments (3)
src/shared/ui/Navbar/Navbar.stories.tsx (1)
8-14
: 데코레이터의 스타일 속성을 Tailwind로 변경하는 것이 좋습니다.현재 인라인 스타일을 사용하고 있는데, 프로젝트의 일관성을 위해 Tailwind 클래스로 변경하는 것이 좋을 것 같습니다.
(Story) => ( - <div style={{ width: 400, backgroundColor: 'white', padding: 20 }}> + <div className="w-[400px] bg-white p-5"> <Story /> </div> ),src/shared/ui/Navbar/Navbar.tsx (2)
19-33
: SVG 아이콘을 별도 컴포넌트로 분리하는 것이 좋습니다.현재 인라인으로 정의된 SVG를 별도의 컴포넌트로 분리하면 다음과 같은 이점이 있습니다:
- 코드 재사용성 향상
- 유지보수성 개선
- 일관된 스타일링 적용 용이
또한 stroke 색상이 className과 path 속성에서 중복 정의되어 있습니다.
- <svg - width="13" - height="21" - viewBox="0 0 13 21" - fill="none" - xmlns="http://www.w3.org/2000/svg" - className="stroke-gray-600" - > - <path - d="M10.9609 1.50024L2.75643 9.70475C2.31709 10.1441 2.31709 10.8564 2.75643 11.2957L10.9609 19.5002" - stroke="#4B5563" - strokeWidth="3" - strokeLinecap="round" - /> - </svg> + <BackIcon className="stroke-gray-600" />
11-18
: onClick 핸들러를 최적화해주세요.
useCallback
을 사용하여 핸들러를 메모이제이션하면 불필요한 리렌더링을 방지할 수 있습니다.export default function Navbar({ title }: NavbarProps) { const router = useRouter(); + const handleBack = useCallback(() => { + router.back(); + }, [router]); return ( <button className="flex cursor-pointer flex-row items-center gap-4" - onClick={() => router.back()} + onClick={handleBack} >React import에 useCallback을 추가해주세요:
- import React from 'react'; + import React, { useCallback } from 'react';
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/shared/ui/Navbar/Navbar.stories.tsx
(1 hunks)src/shared/ui/Navbar/Navbar.tsx
(1 hunks)src/shared/ui/Navbar/index.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/shared/ui/Navbar/index.tsx
🔇 Additional comments (1)
src/shared/ui/Navbar/Navbar.stories.tsx (1)
5-27
: 메타데이터 구성이 잘 되어있습니다!컴포넌트의 문서화와 스토리 구성이 명확하게 작성되어 있습니다.
src/shared/ui/Navbar/Navbar.tsx
Outdated
<button | ||
className="flex cursor-pointer flex-row items-center gap-4" | ||
onClick={() => router.back()} | ||
> |
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
접근성 개선이 필요합니다.
버튼에 다음 속성들을 추가하여 접근성을 개선해주세요:
aria-label
type="button"
<button
className="flex cursor-pointer flex-row items-center gap-4"
+ aria-label="이전 페이지로 이동"
+ type="button"
onClick={() => router.back()}
>
📝 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.
<button | |
className="flex cursor-pointer flex-row items-center gap-4" | |
onClick={() => router.back()} | |
> | |
<button | |
className="flex cursor-pointer flex-row items-center gap-4" | |
aria-label="이전 페이지로 이동" | |
type="button" | |
onClick={() => router.back()} | |
> |
Update: 2025년 02월 01일 16시 37분 38초 |
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 (1)
src/shared/ui/assets/index.tsx (1)
30-30
: 아이콘 그룹화 검토 필요
arrowLeft
속성이 일반 아이콘들과 소셜 미디어 아이콘들 사이에 위치해 있습니다. 유지보수성을 높이기 위해 일반 아이콘들과 함께 그룹화하는 것이 좋을 것 같습니다.다음과 같이 수정하는 것을 제안드립니다:
add: Add, arrowDown: ArrowDown, + arrowLeft: ArrowLeft, arrowUp: ArrowUp, check: Check, close: Close, download: DownLoad, etc: Etc, list: List, new: New, pin: Pin, trash: Trash, write: Write, - arrowLeft: ArrowLeft, instagram: Insta, youtube: Youtube,
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/shared/ui/assets/arrow-left.svg
is excluded by!**/*.svg
📒 Files selected for processing (3)
src/shared/ui/Navbar/Navbar.tsx
(1 hunks)src/shared/ui/assets/index.tsx
(2 hunks)tailwind.config.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tailwind.config.ts
- src/shared/ui/Navbar/Navbar.tsx
🔇 Additional comments (1)
src/shared/ui/assets/index.tsx (1)
3-3
: 아이콘 임포트 순서가 알파벳순으로 정렬되어 있습니다.새로 추가된
ArrowLeft
아이콘이 알파벳순으로 올바르게 배치되어 있습니다.
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: 1
🧹 Nitpick comments (2)
src/shared/ui/NavBack/NavBack.tsx (2)
7-9
: 타입 정의를 더 명확하게 개선해 주세요.
title
프롭의 의미와 용도를 더 잘 설명하기 위해 JSDoc 주석을 추가하는 것이 좋겠습니다.type NavBackProps = { + /** 네비게이션 바에 표시될 페이지 제목 */ title: string; };
14-22
: 사용자 경험을 개선해 주세요.
- 호버/포커스 상태에 대한 스타일이 없습니다.
- 클릭 시 시각적 피드백이 없습니다.
<button - className="flex cursor-pointer flex-row items-center gap-2 align-middle" + className="flex cursor-pointer flex-row items-center gap-2 align-middle + hover:opacity-80 focus:outline-none focus:ring-2 focus:ring-gray-300 + active:scale-95 transition-all" onClick={() => router.back()} aria-label={`${title} 페이지로 돌아가기`} >
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/shared/ui/NavBack/NavBack.stories.tsx
(1 hunks)src/shared/ui/NavBack/NavBack.tsx
(1 hunks)src/shared/ui/NavBack/index.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/shared/ui/NavBack/index.tsx
- src/shared/ui/NavBack/NavBack.stories.tsx
🔇 Additional comments (1)
src/shared/ui/NavBack/NavBack.tsx (1)
11-23
: 컴포넌트의 안정성을 개선해 주세요.라우터 동작 중 발생할 수 있는 오류 처리가 누락되어 있습니다.
다음 사항들을 확인해 주세요:
- 라우터 동작 중 로딩 상태 표시
- 오류 발생 시 처리 방안
- 브라우저 히스토리가 없는 경우의 처리
<button | ||
className="flex cursor-pointer flex-row items-center gap-2 align-middle" | ||
onClick={() => router.back()} | ||
> |
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
접근성 속성을 추가해 주세요.
버튼에 접근성 속성이 누락되어 있습니다. 스크린 리더 사용자를 위해 적절한 aria 속성을 추가해 주세요.
<button
className="flex cursor-pointer flex-row items-center gap-2 align-middle"
onClick={() => router.back()}
+ aria-label={`${title} 페이지로 돌아가기`}
>
Committable suggestion skipped: line range outside the PR's diff.
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.
수고하셨어요 👍
Navbar폴더가 살아있어서 삭제가 필요할 것 같아요!
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.
navback으로 변경된 사항이 적용되지 않은 것 같아요.
icon네이밍을 left-arrow로 설정하면 더 범용적으로 쓰일 수 있을 것 같다고 느껴지는데, 어떻게 생각하시나요?
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.
현재 페이지네이션에 arrow-left라는 아이콘이 존재하여
arrow-left2같은 네이밍은 어떠신가요?
저는 해당 icon이 디자인상 nav역할로 사용되여서 nav-arrow 로 지었습니다
src/shared/ui/NavBack/index.tsx
Outdated
@@ -0,0 +1 @@ | |||
export { NavBack }; |
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.
export { NavBack }; | |
export { NavBack } from './NavBack'; |
NavBack
파일에서도 default
keyword제거가 필요해요.
혹시, 유진이 파일에서는 이게 에러가 안잡히나요?
저는 PR올리기 전에
npm run build
를 한 번 실행해서 오류를 체크하고- file change도 확인해서 잘못 들어간 변경사항이 있는지 확인하는 편 이에요
유진이 PR에서도 컴파일에러가 종종 발견될때가 있어서 npm run build 실행해주면 리뷰시간이 더 짧아질 것 같아용 참고해주세요
tailwind.config.ts
Outdated
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.
tailwind가 변경사항에 반영 된 이유가 있나요? 기존에 적용되어져있는 color palatte가 기능하지않고 있어요
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.
파일이 잘못 삭제되어 확인 후 다시 올렸습니다 !
src/shared/ui/NavBack/NavBack.tsx
Outdated
onClick={() => router.back()} | ||
> | ||
<Icon name="navbarArrow" /> | ||
<p className="text-2xl font-bold text-gray-600">{title}</p> |
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.
- 요거 만들어주신 typography적용하지 않으신 이유가 있나요?
- 디자인상 아이콘과
<p>
에서 반응형은 따로 없는지 궁금해요
src/shared/ui/NavBack/NavBack.tsx
Outdated
|
||
import { Icon } from '../Icon'; | ||
|
||
type NavBackProps = { |
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.
type NavBackProps = { | |
type Props = { |
convention 으로 변경이 필요해 보입니다 !
🔥 연관 이슈
🚀 작업 내용
🤔 고민했던 내용
💬 리뷰 중점사항
Summary by CodeRabbit
NavBack
컴포넌트의 스토리북 구성 파일이 추가되어 문서화가 개선되었습니다.NavBack
컴포넌트의 메타데이터가 추가되어 사용자가 기능을 이해하기 쉬워졌습니다.