[FEAT] Tab 컴포넌트 구현#79
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough디자인 시스템에 ChangesTab 컴포넌트 구현
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Tab
participant ParentComponent
User->>Tab: 클릭
Tab->>ParentComponent: onClick 호출
ParentComponent-->>Tab: isSelected 값 갱신
Tab->>Tab: 변형 클래스/아이콘 재계산
Suggested labels: Suggested reviewers: 짧은 한마디: Tab이 이제 “눌릴 준비 완료” 상태네요. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/timo-design-system/src/components/tab/Tab.tsx`:
- Around line 34-49: The Tab component currently exposes selection only through
styling, so add an accessible state to the button in Tab so assistive tech can
announce whether it is selected. Update the Tab component’s button markup to
include either aria-pressed or the appropriate role="tab" with aria-selected,
using the existing variant/label rendering in Tab.tsx. Make sure the chosen
attribute reflects the selected state passed into the Tab component and keep the
current click handling and icon/label structure unchanged.
- Line 5: `TabVariant` is a literal union type alias that violates the naming
convention; rename it to `TabVariantTypes` and update every reference in
`Tab.tsx` accordingly, including `TAB_VARIANT_CLASS_NAME:
Record<TabVariantTypes, string>`, the `variant` variable annotation, and any
other usages so the type name stays consistent throughout the component.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: b2391da9-0b98-404b-8eda-1829c88494a2
📒 Files selected for processing (3)
packages/timo-design-system/src/components/index.tspackages/timo-design-system/src/components/tab/Tab.stories.tsxpackages/timo-design-system/src/components/tab/Tab.tsx
|
|
||
| import type { ReactNode } from "react"; | ||
|
|
||
| type TabVariant = "default" | "selected"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
타입 별칭 네이밍 컨벤션 위반: TabVariant → TabVariantTypes
리터럴 유니언 타입에는 접미사 Types를 붙이는 것이 컨벤션이에요. 지금 이름은 규칙에서 살짝 벗어나 있네요 🐰
✏️ 제안 diff
-type TabVariant = "default" | "selected";
+type TabVariantTypes = "default" | "selected";이후 TAB_VARIANT_CLASS_NAME: Record<TabVariantTypes, string>, const variant: TabVariantTypes = ... 등 참조부도 함께 변경해야 합니다.
As per path instructions, "type alias는 유니언·튜플·리터럴에만 사용, 접미사 Types".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/timo-design-system/src/components/tab/Tab.tsx` at line 5,
`TabVariant` is a literal union type alias that violates the naming convention;
rename it to `TabVariantTypes` and update every reference in `Tab.tsx`
accordingly, including `TAB_VARIANT_CLASS_NAME: Record<TabVariantTypes,
string>`, the `variant` variable annotation, and any other usages so the type
name stays consistent throughout the component.
Source: Path instructions
| return ( | ||
| <button | ||
| type="button" | ||
| onClick={onClick} | ||
| className={cn(baseButtonClassName, TAB_VARIANT_CLASS_NAME[variant])} | ||
| > | ||
| {hasHoverIcon ? ( | ||
| <> | ||
| <span className="group-hover:hidden">{icon}</span> | ||
| <span className="hidden group-hover:block">{hoverIcon}</span> | ||
| </> | ||
| ) : ( | ||
| icon | ||
| )} | ||
| <span>{label}</span> | ||
| </button> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
선택 상태를 스크린리더에도 알려주면 완벽할 것 같아요.
현재 선택 여부는 배경색으로만 표현되어 있어서, 보조기술 사용자는 어떤 탭이 선택됐는지 알기 어려워요. aria-pressed 또는 role="tab" + aria-selected 속성을 추가하면 접근성이 한층 좋아집니다.
♿ 제안 diff
<button
type="button"
onClick={onClick}
+ aria-pressed={isSelected}
className={cn(baseButtonClassName, TAB_VARIANT_CLASS_NAME[variant])}
>관련 문서: WAI-ARIA aria-pressed
📝 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.
| return ( | |
| <button | |
| type="button" | |
| onClick={onClick} | |
| className={cn(baseButtonClassName, TAB_VARIANT_CLASS_NAME[variant])} | |
| > | |
| {hasHoverIcon ? ( | |
| <> | |
| <span className="group-hover:hidden">{icon}</span> | |
| <span className="hidden group-hover:block">{hoverIcon}</span> | |
| </> | |
| ) : ( | |
| icon | |
| )} | |
| <span>{label}</span> | |
| </button> | |
| return ( | |
| <button | |
| type="button" | |
| onClick={onClick} | |
| aria-pressed={isSelected} | |
| className={cn(baseButtonClassName, TAB_VARIANT_CLASS_NAME[variant])} | |
| > | |
| {hasHoverIcon ? ( | |
| <> | |
| <span className="group-hover:hidden">{icon}</span> | |
| <span className="hidden group-hover:block">{hoverIcon}</span> | |
| </> | |
| ) : ( | |
| icon | |
| )} | |
| <span>{label}</span> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/timo-design-system/src/components/tab/Tab.tsx` around lines 34 - 49,
The Tab component currently exposes selection only through styling, so add an
accessible state to the button in Tab so assistive tech can announce whether it
is selected. Update the Tab component’s button markup to include either
aria-pressed or the appropriate role="tab" with aria-selected, using the
existing variant/label rendering in Tab.tsx. Make sure the chosen attribute
reflects the selected state passed into the Tab component and keep the current
click handling and icon/label structure unchanged.
Timo Performance ReportBundle Size — timo-web
Lighthouse — timo-web
Image Optimization — timo-web
측정 커밋: |
jjangminii
left a comment
jjangminii
left a comment
There was a problem hiding this comment.
제어 컴포넌트 방식으로 설계하셔서 Tab이 도메인을 직접 알 필요 없이 순수 UI 역할만 할 수 있게 분리하니 깔끔하네요 🙂 선택 상태는 props로, hover는 CSS group-hover로 역할을 나눈 부분도 깔끔했습니다! hover 시 배경만 바뀌는 문제를 발견하고 hoverIcon prop으로 아이콘 자체를 교체하는 방식으로 해결하신 점도 꼼꼼하게 챙겨주셨네요 고생하셨습니다 😊
이제 진짜 나루토가 되었네요 👍 👍
| onClick, | ||
| }: TabProps) => { | ||
| const baseButtonClassName = | ||
| "group flex w-45 items-center gap-2 rounded-8 px-3 py-1"; |
There was a problem hiding this comment.
rounded는 rem 영향을 받는 속성이 아니니 px 단위로 표기해도 괜찮을 것 같아요 🙂
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/timo-design-system/src/components/tab/Tab.tsx (2)
32-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
hasHoverIcon이 boolean이 아니에요.
hoverIcon && !isSelected의 타입은false | ReactNode라서has접두사가 암시하는 실제 boolean과 달라요. 현재는 삼항 연산의 truthy 체크로만 쓰여서 동작엔 문제없지만, 컨벤션과 타입 정확성을 위해 명시적으로 boolean 변환을 해주는 게 깔끔합니다.As per path instructions, "Boolean 변수명:
is,has,should,can접두사".🔧 제안 diff
- const hasHoverIcon = hoverIcon && !isSelected; + const hasHoverIcon = Boolean(hoverIcon) && !isSelected;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/timo-design-system/src/components/tab/Tab.tsx` at line 32, `hasHoverIcon` in `Tab` is named like a boolean but currently has a `false | ReactNode` type because it uses `hoverIcon && !isSelected`; update the `Tab.tsx` logic so `hasHoverIcon` is explicitly a boolean derived from the hover-icon condition, keeping the `has` prefix accurate and preserving the existing conditional rendering behavior in `Tab`.Source: Path instructions
40-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff호버 아이콘이 키보드 포커스 시엔 전환되지 않아요.
group-hover만 사용해서 마우스 오버에서만 아이콘이 바뀌고, 키보드로 포커스했을 때는 그대로예요. 완전한 대칭은 아니지만group-focus-visible:변형도 함께 추가하면 키보드 사용자 경험이 더 일관돼요.참고: Tailwind CSS group-hover 문서
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/timo-design-system/src/components/tab/Tab.tsx` around lines 40 - 44, The hover icon swap in Tab is only wired to mouse hover, so keyboard focus does not trigger the alternate icon. Update the icon rendering in Tab.tsx to include a keyboard-focus variant alongside the existing group-hover behavior, using the same hoverIcon/icon spans in the Tab component so the state change also occurs on group-focus-visible. Keep the logic aligned with the current hasHoverIcon conditional and preserve the existing hover behavior while extending it for focus-visible.packages/timo-design-system/src/components/tab/Tab.stories.tsx (1)
58-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win아이콘 크기
24가 반복적으로 하드코딩돼 있어요.
width={24} height={24}가 스토리 전체에서 20번 가까이 반복돼요. 상수로 뽑아두면 나중에 크기 조정할 때 한 곳만 고치면 됩니다 🙂♻️ 제안 diff (일부만 예시)
+const TAB_ICON_SIZE = 24; + export const AllStates: Story = { ... - <Tab label="홈" icon={<HomeOnIcon width={24} height={24} />} isSelected /> + <Tab label="홈" icon={<HomeOnIcon width={TAB_ICON_SIZE} height={TAB_ICON_SIZE} />} isSelected />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/timo-design-system/src/components/tab/Tab.stories.tsx` around lines 58 - 129, The icon size is hardcoded repeatedly in the Tab stories, making future size changes tedious. In Tab.stories.tsx, extract the repeated 24 value used for icon width/height into a shared constant and reuse it for all icon instances. Update the Tab story blocks to reference that constant instead of inline width and height values so the sizing is centralized and easier to maintain.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/timo-design-system/src/components/tab/Tab.stories.tsx`:
- Around line 58-129: The icon size is hardcoded repeatedly in the Tab stories,
making future size changes tedious. In Tab.stories.tsx, extract the repeated 24
value used for icon width/height into a shared constant and reuse it for all
icon instances. Update the Tab story blocks to reference that constant instead
of inline width and height values so the sizing is centralized and easier to
maintain.
In `@packages/timo-design-system/src/components/tab/Tab.tsx`:
- Line 32: `hasHoverIcon` in `Tab` is named like a boolean but currently has a
`false | ReactNode` type because it uses `hoverIcon && !isSelected`; update the
`Tab.tsx` logic so `hasHoverIcon` is explicitly a boolean derived from the
hover-icon condition, keeping the `has` prefix accurate and preserving the
existing conditional rendering behavior in `Tab`.
- Around line 40-44: The hover icon swap in Tab is only wired to mouse hover, so
keyboard focus does not trigger the alternate icon. Update the icon rendering in
Tab.tsx to include a keyboard-focus variant alongside the existing group-hover
behavior, using the same hoverIcon/icon spans in the Tab component so the state
change also occurs on group-focus-visible. Keep the logic aligned with the
current hasHoverIcon conditional and preserve the existing hover behavior while
extending it for focus-visible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1f084470-2103-4fae-bb5a-e20c851d8079
📒 Files selected for processing (2)
packages/timo-design-system/src/components/tab/Tab.stories.tsxpackages/timo-design-system/src/components/tab/Tab.tsx
kimminna
left a comment
kimminna
left a comment
There was a problem hiding this comment.
고생하셨어요! 넘넘 잘했다~
다만 지금 구현된 걸로는 Next 앱에서 next/link 를 못 쓸 것 같아요.
<Link> 는 <a>를 렌더링하고, 지금 구현된 탭 버튼은 <button>을 렌더링해서, <a> 안에 <button>을 중첩하는 건 유효하지 않은 태그 방식이라 사용할 수 없을 것 같아요. 그래서 후속 작업으로 넘기자면, 실제로 사이드바 컨테이너를 구현할 때 어떤 방식으로 구현할지 결정해 주면 좋을 것 같습니다!
지금 생각나는 대로 구현 방식을 생각해 보자면, 겉모습을 결정하는 스타일 부분만 콘텐츠 전용 컴포넌트로 빼고, apps/timo-web 에 사이드바 전용 컨테이너를 만들어서 <Link href = ''> 내부에서 콘텐츠 전용 컴포넌트를 재사용하는 방식이 있을 것 같아요! 이건 고려만 해 주시면 되겠습니다 😎
| onClick: () => void; | ||
| } | ||
|
|
||
| export const Tab = ({ |
There was a problem hiding this comment.
Tab이라는 컴포넌트명보단, 지금 구현된 걸 보니까 버튼 컴포넌트인 것 같아요! 컴포넌트명 수정해서 button 내부에 구조화해 주면 좋을 것 같아요.
| } from "@icons"; | ||
| import { useState } from "react"; | ||
|
|
||
| import { Tab } from "./Tab"; |
3 participants
ISSUE 🔗
close #58
What is this PR? 🔍
디자인 시스템에서 재사용할 수 있는
Tab공통 컴포넌트를 구현했습니다.선택 상태는 부모가 제어하고,
Tab은 전달받은 상태를 표현하는 방식으로 구성했어요.정민님 아티클을 참고해
Tab을 제어 컴포넌트 방식으로 구현했습니다.Tab내부에서 선택 상태를 직접 관리하지 않고, 부모가isSelected와onClick을 넘겨주는 구조로 만들었습니다. 이렇게 하면 실제 페이지에서는 라우팅 상태, 현재 선택된 메뉴, 탭 전환 로직을 부모가 관리할 수 있고,Tab은 공통 UI로서 상태를 표현하는 책임만 가지게 됩니다.이 구조 덕분에 Tab이 home, today, focus 같은 특정 도메인을 직접 알 필요가 없어졌고, 다른 화면에서도 같은 컴포넌트를 재사용할 수 있습니다!
Record 기반 상태 스타일 분리
selected / default 상태별 스타일은 Record로 분리했습니다.
처음에는 isSelected ? ... : ... 형태로 바로 className을 나누었었는데, 상태별 스타일이 컴포넌트 내부에 흩어지면 읽기 어려워질 수 있다고 판단했습니다. 그래서 TabVariant와
Record<TabVariant, string>을 사용해 상태별 스타일을 한 곳에서 관리했습니다.현재는 default, selected 두 상태만 사용합니다. disabled는 실제로 클릭할 수 없는 Tab 상황이 없다고 판단해서 제외했어요.
Hover 처리
hover 배경 처리는 CSS hover: 클래스로 처리했습니다.
hover는 부모가 넘겨주는 앱 상태라기보다 사용자의 마우스 상태에 가까워서, props로 isHovered를 받지 않았어요. 대신 기본 상태의 className에 hover:bg-timo-gray-500을 포함해 브라우저 hover 상태에서만 배경이 바뀌도록 했습니다.
즉, 선택 여부는 React props로 제어하고, hover는 CSS로 처리하도록 역할을 나눴습니다.
Hover 아이콘 교체
구현 중 hover 시 배경은 바뀌지만 아이콘이 바뀌지 않는 문제가 있었습니다.
home-off, home-hover, home-on처럼 상태별 SVG가 따로 제공되고 있어서, hover 시 아이콘 컴포넌트 자체를 교체하는 방식으로 해결했습니다.
이를 위해 hoverIcon props를 추가했고, group-hover를 사용해 기본 아이콘과 hover 아이콘을 전환하도록 구성했습니다.
To Reviewers
Storybook에는 AllStates와 Interactive 두 가지 예시를 추가했습니다.
AllStates는 피그마처럼 selected / hover 가능 / default 상태를 한 화면에서 비교하기 위한 전시용 Story입니다.
Interactive는 간단한 인터렉션을 확인할 수 있게 Storybook 안에서 임시 부모 컴포넌트를 만들어 클릭 동작을 확인할 수 있도록 구성했습니다. useState로 선택 상태를 관리하고, 홈 탭을 클릭하면 selected 상태가 토글됩니다.
Screenshot 📷
2026-07-02.4.31.08.mov
Test Checklist ✔