Skip to content
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] Tabs 컴포넌트 개발, 스토리북 제작 #11

Merged
merged 14 commits into from
Jan 24, 2025
Merged

Conversation

yougyung
Copy link
Collaborator

@yougyung yougyung commented Jan 19, 2025

🔥 연관 이슈

🚀 작업 내용

  • Tabs 공통 컴포넌트 개발
스크린샷 2025-01-19 오후 3 01 40

💬 리뷰 중점사항

- ColorPalette를 활용해서 작업을 진행했어요. 적용된 화면을 확인하시려면, local에서 (color 설정 branch style/#4-ColorPalette)를 pull받는 것을 권장드려요.

Summary by CodeRabbit

  • 새로운 기능

    • 탭 컴포넌트 추가
    • Storybook에 탭 컴포넌트 문서화
    • 탭 상태 관리를 위한 컨텍스트 추가
  • 종속성 업데이트

    • Framer Motion 라이브러리 버전 업그레이드 (v11.15.0 → v11.18.2)
  • 개발 환경

    • Storybook 정적 파일을 Git 추적 제외

@yougyung yougyung self-assigned this Jan 19, 2025
Copy link

coderabbitai bot commented Jan 19, 2025

워크스루

이 풀 리퀘스트는 Tabs 컴포넌트의 전체 구현을 포함하고 있습니다. 새로운 컴포넌트는 탭 인터페이스를 관리하며, Storybook을 통해 문서화되었습니다. 컨텍스트, 루트 및 아이템 컴포넌트를 포함하여 동적이고 유연한 탭 시스템을 제공합니다. 또한 framer-motion 라이브러리의 버전을 업데이트하고 Storybook 정적 파일을 .gitignore에 추가했습니다.

변경 사항

파일 변경 요약
.gitignore storybook-static 디렉토리 추가
src/shared/ui/Tabs/Tabs.stories.tsx Tabs 컴포넌트를 위한 Storybook 스토리 추가
src/shared/ui/Tabs/index.tsx TabsItem 컴포넌트 내보내기
src/shared/ui/Tabs/Tabs.context.ts 탭 상태 관리를 위한 컨텍스트 생성
src/shared/ui/Tabs/TabsItem.tsx 개별 탭 아이템 컴포넌트 구현
src/shared/ui/Tabs/TabsRoot.tsx 탭 루트 컴포넌트 구현
package.json framer-motion 버전을 11.15.0에서 11.18.2로 업데이트

연결된 이슈에 대한 평가

목표 해결 여부 설명
Tabs 컴포넌트 구현
Tabs 스토리북 구현

관련될 수 있는 풀 리퀘스트

제안된 리뷰어

  • ujinsim

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

github-actions bot commented Jan 19, 2025

Update: 2025년 01월 23일 13시 32분 56초
Storybook 미리보기: https://677a9a76c58a295e9421d3ef-hogyqcwnwy.chromatic.com/

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
src/shared/ui/Tabs/TabRoot.tsx (1)

24-31: 접근성(a11y) 개선이 필요합니다.

WAI-ARIA 역할과 속성을 추가하여 접근성을 개선해주세요.

return (
  <TabContext.Provider value={{ activeLabel, setActiveLabel, labels }}>
-   <div className="w-full">
+   <div className="w-full" role="tabpanel">
      <TabNav />
      {children}
    </div>
  </TabContext.Provider>
);
src/shared/ui/Tabs/Tabs.stories.tsx (3)

3-3: 상대 경로 대신 절대 경로 사용을 고려해주세요.

상대 경로('.')를 사용하면 파일 구조가 변경될 때 문제가 발생할 수 있습니다. 유지보수성을 높이기 위해 절대 경로를 사용하는 것이 좋습니다.

-import Tabs from '.';
+import Tabs from '@/shared/ui/Tabs';

11-14: 컴포넌트 문서를 더 자세히 작성해주세요.

현재 문서에는 기본적인 설명만 포함되어 있습니다. 다음과 같은 내용을 추가하면 좋을 것 같습니다:

  • TabItem의 속성과 사용 방법
  • 각 prop의 타입 정보
  • 사용 예시 코드

26-40: 스토리 구현을 개선해주세요.

  1. 다양한 사용 사례를 보여주기 위해 여러 스토리 변형을 추가해주세요:

    • 다른 defaultIndex 값을 가진 예시
    • 다양한 스타일 변형
    • 에러 상태나 경계 조건
  2. 인라인 스타일을 컴포넌트로 이동하는 것이 좋습니다:

-<div className="my-4 text-center">1번 탭 컴포넌트</div>
+<TabContent>1번 탭 컴포넌트</TabContent>
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e8fd3bd and c555f7f.

📒 Files selected for processing (7)
  • .gitignore (1 hunks)
  • src/shared/ui/Tabs/Tab.context.ts (1 hunks)
  • src/shared/ui/Tabs/TabItem.tsx (1 hunks)
  • src/shared/ui/Tabs/TabNav.tsx (1 hunks)
  • src/shared/ui/Tabs/TabRoot.tsx (1 hunks)
  • src/shared/ui/Tabs/Tabs.stories.tsx (1 hunks)
  • src/shared/ui/Tabs/index.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • .gitignore
  • src/shared/ui/Tabs/index.tsx
🔇 Additional comments (2)
src/shared/ui/Tabs/Tab.context.ts (1)

3-7: 타입 정의가 명확하고 잘 구성되어 있습니다!

탭 컴포넌트에 필요한 모든 속성이 포함되어 있으며, 타입 안정성이 잘 보장되어 있습니다.

src/shared/ui/Tabs/TabRoot.tsx (1)

12-22: defaultIndex 유효성 검사와 에러 처리를 추가해주세요.

  1. defaultIndex가 labels 배열의 범위를 벗어날 경우에 대한 처리가 필요합니다.
  2. 유효하지 않은 children이 전달될 경우에 대한 에러 처리가 필요합니다.

다음 사항들을 확인해주세요:

  1. defaultIndex가 음수이거나 labels.length보다 큰 경우
  2. 모든 자식 컴포넌트가 TabItem이 아닌 경우
  3. label prop이 없는 자식 컴포넌트가 있는 경우
export function TabRoot({ children, defaultIndex = 0 }: Props) {
  const labels = useMemo(
    () =>
      Children.toArray(children)
        .filter((child): child is ReactElement<{ label: string }> => isValidElement(child))
        .map((child) => child.props.label),
    [children]
  );

+ if (labels.length === 0) {
+   throw new Error('TabRoot must have at least one TabItem child');
+ }

+ const safeDefaultIndex = Math.max(0, Math.min(defaultIndex, labels.length - 1));
  type LabelType = (typeof labels)[number];
- const [activeLabel, setActiveLabel] = useState<LabelType>(labels[defaultIndex]);
+ const [activeLabel, setActiveLabel] = useState<LabelType>(labels[safeDefaultIndex]);
✅ Verification successful

TabRoot 컴포넌트의 유효성 검사 추가가 필요합니다

리뷰 코멘트에서 제안된 수정사항이 적절합니다:

  • defaultIndex 범위 검사와 안전한 인덱스 계산
  • 최소 하나 이상의 TabItem 존재 여부 검증
  • 제안된 에러 처리 로직 추가

스토리북 예제와 TabItem 구현을 검토한 결과, 제안된 수정사항들이 컴포넌트의 안정성을 높이는데 필수적입니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Find TabItem component definition and related files
echo "=== Finding TabItem and related components ==="
fd "Tab.*\.tsx?" src/shared/ui/Tabs

# Look for TabRoot usage patterns
echo -e "\n=== Finding TabRoot usage patterns ==="
rg "TabRoot" -A 3 -B 3

# Check for error handling patterns in Tabs
echo -e "\n=== Checking error handling patterns ==="
rg "throw new Error|console\.error" src/shared/ui/Tabs

Length of output: 1293


Script:

#!/bin/bash
# Check TabItem implementation
echo "=== TabItem Implementation ==="
cat src/shared/ui/Tabs/TabItem.tsx

# Check Tabs.stories.tsx for usage examples
echo -e "\n=== Usage Examples ==="
cat src/shared/ui/Tabs/Tabs.stories.tsx

Length of output: 1476

Comment on lines 10 to 14
export function TabItem({ label, children }: Props) {
const content = useContext(TabContext);
const isActive = content?.activeLabel === label;

return <div className={`${!isActive && 'hidden'}`}>{children}</div>;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

컨텍스트 null 체크와 className 처리를 개선해주세요.

  1. 컨텍스트가 undefined일 경우에 대한 처리가 필요합니다.
  2. className 문자열 연결 대신 cn 유틸리티를 사용하면 더 깔끔해질 것 같습니다.

다음과 같이 수정해보세요:

export function TabItem({ label, children }: Props) {
  const content = useContext(TabContext);
+ if (!content) return null;
  const isActive = content?.activeLabel === label;

- return <div className={`${!isActive && 'hidden'}`}>{children}</div>;
+ return <div className={cn({ hidden: !isActive })}>{children}</div>;
}

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 14 to 26
{labels.map((label, index) => (
<button
key={index}
onClick={() => setActiveLabel(label)}
className={`grow p-3 font-semibold w-1/${labels.length} ${
activeLabel === label
? 'border-primary-300 text-primary-300 hover:bg-primary-50 border-b-2'
: 'text-gray-500 hover:bg-gray-50'
} md:p-4 md:text-xl`}
>
{label}
</button>
))}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

배열 인덱스를 key로 사용하는 것과 className 처리를 개선해주세요.

  1. 배열의 인덱스를 key로 사용하면 탭의 순서가 변경될 때 문제가 발생할 수 있습니다.
  2. className 문자열이 복잡하여 가독성이 떨어집니다.

다음과 같이 수정해보세요:

- {labels.map((label, index) => (
+ {labels.map((label) => (
    <button
-     key={index}
+     key={label}
      onClick={() => setActiveLabel(label)}
-     className={`grow p-3 font-semibold w-1/${labels.length} ${
-       activeLabel === label
-         ? 'border-primary-300 text-primary-300 hover:bg-primary-50 border-b-2'
-         : 'text-gray-500 hover:bg-gray-50'
-     } md:p-4 md:text-xl`}
+     className={cn(
+       'grow p-3 font-semibold md:p-4 md:text-xl',
+       `w-1/${labels.length}`,
+       activeLabel === label
+         ? 'border-primary-300 text-primary-300 hover:bg-primary-50 border-b-2'
+         : 'text-gray-500 hover:bg-gray-50'
+     )}
    >

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 28 to 38
<Tabs defaultIndex={0}>
<Tabs.Item label="공지사항">
<div className="my-4 text-center">1번 탭 컴포넌트</div>
</Tabs.Item>
<Tabs.Item label="피드">
<div className="my-4 text-center">2번 탭 컴포넌트</div>
</Tabs.Item>
<Tabs.Item label="활동보고서">
<div className="my-4 text-center">3번 탭 컴포넌트</div>
</Tabs.Item>
</Tabs>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

접근성(a11y) 속성을 추가해주세요.

탭 컴포넌트의 접근성을 향상시키기 위해 ARIA 속성을 추가하는 것이 좋습니다:

  • aria-selected
  • aria-controls
  • role="tabpanel"

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
src/shared/ui/Tabs/Tabs.context.ts (1)

9-9: 컨텍스트 초기값 처리 개선이 필요합니다.

undefined 대신 기본값을 제공하면 컨텍스트 사용 시 매번 undefined 체크를 하지 않아도 됩니다.

-export const TabsContext = createContext<TabsContextType | undefined>(undefined);
+export const TabsContext = createContext<TabsContextType>({
+  activeLabel: '',
+  setActiveLabel: () => {},
+  labels: [],
+});
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c555f7f and e6d802c.

📒 Files selected for processing (5)
  • src/shared/ui/Tabs/Tabs.context.ts (1 hunks)
  • src/shared/ui/Tabs/TabsItem.tsx (1 hunks)
  • src/shared/ui/Tabs/TabsNav.tsx (1 hunks)
  • src/shared/ui/Tabs/TabsRoot.tsx (1 hunks)
  • src/shared/ui/Tabs/index.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/shared/ui/Tabs/index.tsx
🔇 Additional comments (1)
src/shared/ui/Tabs/Tabs.context.ts (1)

3-7: 타입 정의가 명확하고 간결합니다.

타입스크립트를 활용하여 컨텍스트의 구조를 명확하게 정의했습니다.

const { activeLabel, setActiveLabel, labels } = content;

return (
<nav className="flex w-full">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

nav 요소에 ARIA 랜드마크 추가가 필요합니다.

탭 네비게이션의 의미를 명확히 하기 위해 ARIA 속성을 추가하세요.

-<nav className="flex w-full">
+<nav className="flex w-full" role="tablist" aria-label="탭 네비게이션">
📝 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
<nav className="flex w-full">
<nav className="flex w-full" role="tablist" aria-label="탭 네비게이션">

Comment on lines 13 to 23
<button
key={index}
onClick={() => setActiveLabel(label)}
className={`grow p-3 font-semibold w-1/${labels.length} ${
activeLabel === label
? 'border-primary-300 text-primary-300 hover:bg-primary-50 border-b-2'
: 'text-gray-500 hover:bg-gray-50'
} md:p-4 md:text-xl`}
>
{label}
</button>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

접근성(a11y) 개선이 필요합니다.

탭 네비게이션의 접근성을 높이기 위해 ARIA 속성을 추가하는 것이 좋습니다.

 <button
   key={label}
   onClick={() => setActiveLabel(label)}
+  role="tab"
+  aria-selected={activeLabel === label}
+  aria-controls={`${label}-panel`}
   className={`grow p-3 font-semibold w-1/${labels.length} ${
     activeLabel === label
       ? 'border-primary-300 text-primary-300 hover:bg-primary-50 border-b-2'
       : 'text-gray-500 hover:bg-gray-50'
   } md:p-4 md:text-xl`}
 >
   {label}
 </button>
📝 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
<button
key={index}
onClick={() => setActiveLabel(label)}
className={`grow p-3 font-semibold w-1/${labels.length} ${
activeLabel === label
? 'border-primary-300 text-primary-300 hover:bg-primary-50 border-b-2'
: 'text-gray-500 hover:bg-gray-50'
} md:p-4 md:text-xl`}
>
{label}
</button>
<button
key={index}
onClick={() => setActiveLabel(label)}
role="tab"
aria-selected={activeLabel === label}
aria-controls={`${label}-panel`}
className={`grow p-3 font-semibold w-1/${labels.length} ${
activeLabel === label
? 'border-primary-300 text-primary-300 hover:bg-primary-50 border-b-2'
: 'text-gray-500 hover:bg-gray-50'
} md:p-4 md:text-xl`}
>
{label}
</button>

<nav className="flex w-full">
{labels.map((label, index) => (
<button
key={index}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

배열 인덱스를 key로 사용하는 것은 피해야 합니다.

동적으로 탭이 변경될 경우 React의 재조정(reconciliation) 과정에서 문제가 발생할 수 있습니다. 대신 고유한 값인 label을 key로 사용하세요.

-key={index}
+key={label}

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 12 to 19
export function TabsRoot({ children, defaultIndex = 0 }: Props) {
const labels = useMemo(
() =>
Children.toArray(children)
.filter((child): child is ReactElement<{ label: string }> => isValidElement(child))
.map((child) => child.props.label),
[children]
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

자식 컴포넌트 타입 검증 강화가 필요합니다.

현재 구현은 모든 자식 컴포넌트가 label prop을 가지고 있다고 가정합니다. TabsItem 컴포넌트만 허용하도록 타입을 제한하는 것이 안전합니다.

+import { TabsItem } from './TabsItem';
+
+type Props = {
+  children: React.ReactElement<typeof TabsItem> | React.ReactElement<typeof TabsItem>[];
+  defaultIndex?: number;
+};

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +21 to +22
type LabelType = (typeof labels)[number];
const [activeLabel, setActiveLabel] = useState<LabelType>(labels[defaultIndex]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

defaultIndex 유효성 검사가 필요합니다.

defaultIndex가 유효한 범위를 벗어날 경우 처리가 누락되어 있습니다.

-const [activeLabel, setActiveLabel] = useState<LabelType>(labels[defaultIndex]);
+const validDefaultIndex = defaultIndex >= 0 && defaultIndex < labels.length ? defaultIndex : 0;
+const [activeLabel, setActiveLabel] = useState<LabelType>(labels[validDefaultIndex]);
📝 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
type LabelType = (typeof labels)[number];
const [activeLabel, setActiveLabel] = useState<LabelType>(labels[defaultIndex]);
type LabelType = (typeof labels)[number];
const validDefaultIndex = defaultIndex >= 0 && defaultIndex < labels.length ? defaultIndex : 0;
const [activeLabel, setActiveLabel] = useState<LabelType>(labels[validDefaultIndex]);

Comment on lines 10 to 15
export function TabsItem({ label, children }: Props) {
const content = useContext(TabsContext);
const isActive = content?.activeLabel === label;

return <div className={`${!isActive && 'hidden'}`}>{children}</div>;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

탭 패널의 접근성과 성능 개선이 필요합니다.

  1. ARIA 속성이 누락되어 있습니다.
  2. 컨텍스트가 undefined일 때의 처리가 필요합니다.
  3. hidden 클래스 대신 조건부 렌더링을 사용하면 성능이 개선될 수 있습니다.
 export function TabsItem({ label, children }: Props) {
   const content = useContext(TabsContext);
+  if (!content) return null;
   const isActive = content?.activeLabel === label;
 
-  return <div className={`${!isActive && 'hidden'}`}>{children}</div>;
+  return isActive ? (
+    <div
+      role="tabpanel"
+      aria-labelledby={`${label}-tab`}
+      id={`${label}-panel`}
+    >
+      {children}
+    </div>
+  ) : null;
 }
📝 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 TabsItem({ label, children }: Props) {
const content = useContext(TabsContext);
const isActive = content?.activeLabel === label;
return <div className={`${!isActive && 'hidden'}`}>{children}</div>;
}
export function TabsItem({ label, children }: Props) {
const content = useContext(TabsContext);
if (!content) return null;
const isActive = content?.activeLabel === label;
return isActive ? (
<div
role="tabpanel"
aria-labelledby={`${label}-tab`}
id={`${label}-panel`}
>
{children}
</div>
) : null;
}

@yougyung yougyung added the feat label Jan 19, 2025
Copy link
Contributor

@keemsebin keemsebin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다~!

labels: string[];
};

export const TabsContext = createContext<TabsContextType | undefined>(undefined);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undefined를 설정하신 이유가 궁금합니다~!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

할당 이전과 이후상태의 구분을 목적으로 undefined로 초기화를 진행했습니다 !

src/shared/ui/Tabs/TabsNav.tsx Outdated Show resolved Hide resolved
src/shared/ui/Tabs/TabsNav.tsx Outdated Show resolved Hide resolved
src/shared/ui/Tabs/TabsRoot.tsx Outdated Show resolved Hide resolved
return (
<TabsContext.Provider value={{ activeLabel, setActiveLabel, labels }}>
<div className="w-full">
<TabsNav />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 파일들이 많이 분산되어 있어서 TabsRoot 내부에 TabsNav를 두는 것도 더 응집력 있는 설계일 수 있다는 생각이 들어요! 어떻게 생각하시나용

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

파일이 재사용되지 않고 Tabs에만 사용된다면 TabsRoot 내부에 TabsNav를 두어도 좋아보입니다

Copy link
Collaborator Author

@yougyung yougyung Jan 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저희팀의 가독성을 위해 분리했던 부분이었기 때문에, 팀원분들 의견 반영해서 TabsRoot 내부로 위치 변경 진행했어요 4970f2a

Copy link
Collaborator

@ujinsim ujinsim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다 !

src/shared/ui/Tabs/TabsNav.tsx Outdated Show resolved Hide resolved
return (
<TabsContext.Provider value={{ activeLabel, setActiveLabel, labels }}>
<div className="w-full">
<TabsNav />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

파일이 재사용되지 않고 Tabs에만 사용된다면 TabsRoot 내부에 TabsNav를 두어도 좋아보입니다

@github-actions github-actions bot requested review from keemsebin and ujinsim January 22, 2025 16:30
Copy link

@coderabbitai coderabbitai bot left a 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/Tabs/TabsRoot.tsx (1)

27-48: 접근성 개선이 필요합니다.

탭 네비게이션에 다음 접근성 개선사항을 추가하는 것이 좋습니다:

  1. role="tablist"를 nav 요소에 추가
  2. role="tab"을 각 버튼에 추가
  3. aria-selected 속성을 활성 탭에 추가
-        <nav className="flex w-full">
+        <nav className="flex w-full" role="tablist">
           {labels.map((label) => (
             <motion.button
               key={label}
+              role="tab"
+              aria-selected={activeLabel === label}
               className={`grow transform p-3 font-semibold w-1/${labels.length} ${
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e6d802c and adff787.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (6)
  • package.json (1 hunks)
  • src/shared/ui/Tabs/Tabs.context.ts (1 hunks)
  • src/shared/ui/Tabs/Tabs.stories.tsx (1 hunks)
  • src/shared/ui/Tabs/TabsItem.tsx (1 hunks)
  • src/shared/ui/Tabs/TabsRoot.tsx (1 hunks)
  • src/shared/ui/Tabs/index.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/shared/ui/Tabs/TabsItem.tsx
  • src/shared/ui/Tabs/index.tsx
  • src/shared/ui/Tabs/Tabs.context.ts
  • src/shared/ui/Tabs/Tabs.stories.tsx
🔇 Additional comments (4)
src/shared/ui/Tabs/TabsRoot.tsx (3)

7-12: Props 타입 정의가 잘 되어있습니다! 👍

HTML div 요소의 속성들을 상속받아 확장성 있게 구현되었습니다.


13-19: 자식 컴포넌트 타입 검증 강화가 필요합니다.

현재 구현은 모든 자식 컴포넌트가 label prop을 가지고 있다고 가정합니다. TabsItem 컴포넌트만 허용하도록 타입을 제한하는 것이 안전합니다.


21-22: defaultIndex 유효성 검사가 필요합니다.

defaultIndex가 유효한 범위를 벗어날 경우 처리가 누락되어 있습니다.

package.json (1)

21-21: framer-motion 버전 호환성 확인이 필요합니다.

최신 버전의 framer-motion이 프로젝트의 다른 의존성들과 호환되는지 확인해주세요.

✅ Verification successful

framer-motion 버전 호환성이 확인되었습니다

프로젝트의 React 버전(v19.0.0)이 framer-motion v11.18.2의 peer dependency 요구사항(^18.0.0 || ^19.0.0)을 충족합니다. 호환성 문제 없이 안전하게 사용하실 수 있습니다.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# framer-motion 호환성 검사

# package-lock.json에서 peer dependencies 확인
if [ -f "package-lock.json" ]; then
  echo "Checking peer dependencies..."
  cat package-lock.json | jq '.packages["node_modules/framer-motion"].peerDependencies'
fi

# npm으로 호환성 체크
npm ls framer-motion

Length of output: 381


Script:

#!/bin/bash
# React 버전 확인
echo "Checking React version..."
npm ls react react-dom

# package.json에서 React 버전 확인
echo -e "\nReact version in package.json:"
cat package.json | jq '.dependencies.react, .dependencies["react-dom"]'

Length of output: 1703

Copy link

@coderabbitai coderabbitai bot left a 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 (2)
src/shared/ui/Tabs/TabsItem.tsx (2)

11-13: JSDoc 주석을 더 명확하게 개선해주세요.

children prop의 JSDoc 주석이 모호합니다. 탭 패널의 컨텐츠임을 명확히 설명해주세요.

-   * if activated, exposed element.
+   * 탭이 활성화되었을 때 표시될 패널의 컨텐츠입니다.

21-21: props 전달 방식을 개선해주세요.

...props를 통해 전달되는 속성들이 컴포넌트 내부에서 사용되는 className과 충돌할 수 있습니다. className을 전달받아 병합하는 방식으로 개선하는 것이 좋습니다.

-    <div className={`${!isActive && 'hidden'}`} {...props}>
+    <div 
+      className={`${!isActive && 'hidden'} ${props.className || ''}`}
+      {...props}
+    >
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between adff787 and f19699a.

📒 Files selected for processing (2)
  • src/shared/ui/Tabs/TabsItem.tsx (1 hunks)
  • src/shared/ui/Tabs/TabsRoot.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/shared/ui/Tabs/TabsRoot.tsx
🔇 Additional comments (1)
src/shared/ui/Tabs/TabsItem.tsx (1)

16-24: 탭 패널의 접근성과 성능 개선이 필요합니다.

이전 리뷰에서 지적된 문제들이 아직 해결되지 않았습니다:

  1. ARIA 속성이 누락되어 있습니다.
  2. 컨텍스트가 undefined일 때의 처리가 필요합니다.
  3. hidden 클래스 대신 조건부 렌더링을 사용하면 성능이 개선될 수 있습니다.

Copy link
Contributor

@keemsebin keemsebin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tab에 생동감이 생겼네용 수고하셨습니다~!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] Tabs 컴포넌트, 스토리북
3 participants