[FEAT] TogglePanel 컴포넌트 추가#76
Open
jjangminii wants to merge 4 commits into
Open
Conversation
- Timebox/Timer 두 옵션을 전환하는 TogglePanel 컴포넌트를 추가했습니다 - 너비는 w-full로 두고 부모가 padding/wrapper로 크기를 제어하도록 했습니다 - Default/Big/AllSizes Storybook 스토리를 추가했습니다
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough
ChangesTogglePanel 컴포넌트 추가
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/index.ts`:
- Line 6: The barrel export in the index module is using a relative path, but
this public entrypoint should follow the repo convention and use the absolute
alias path instead. Update the export in the component index barrel for
TogglePanel to reference it through the package alias rather than a relative
import, keeping the public API shape unchanged.
In
`@packages/timo-design-system/src/components/toggle-panel/TogglePanel.stories.tsx`:
- Around line 3-6: The story file is using relative imports for TogglePanel and
TogglePanelValue instead of the package alias convention. Update the imports in
TogglePanel.stories.tsx to use the same alias-based path style used across the
design system, and keep the existing Meta and StoryObj imports unchanged.
- Around line 23-33: PlaygroundTogglePanel currently initializes local state
from args.value only once, so Storybook control changes to value do not
propagate into the TogglePanel state. Update PlaygroundTogglePanel to keep its
internal value synchronized with args.value changes, using the existing
TogglePanelValue state and the component’s onChange handling so the preview
reflects control updates reliably.
🪄 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: 3b8073e9-13de-46d0-ae3d-345517d6c858
📒 Files selected for processing (3)
packages/timo-design-system/src/components/index.tspackages/timo-design-system/src/components/toggle-panel/TogglePanel.stories.tsxpackages/timo-design-system/src/components/toggle-panel/TogglePanel.tsx
…panel # Conflicts: # packages/timo-design-system/src/components/index.ts
- PlaygroundTogglePanel이 args.value를 마운트 시점에만 useState 초기값으로 읽어서 Storybook Controls에서 value를 바꿔도 반영되지 않던 문제를 수정했습니다 - useEffect로 args.value 변경 시 내부 state를 동기화하도록 했습니다
kimminna
reviewed
Jul 2, 2026
- 리뷰 반영: aria-pressed 대신 role="tablist"/"tab" + aria-selected로 tab 시맨틱을 적용했습니다 - 좌우 화살표 키보드 네비게이션과 roving tabindex를 추가했습니다 - 두 버튼의 중복 마크업을 options 배열 기반 렌더링으로 정리했습니다 - id prop을 추가해 외부 tabpanel과 aria-controls/aria-labelledby로 연결할 수 있게 하고, Storybook에 WithPanel 예시를 추가했습니다
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/toggle-panel/TogglePanel.tsx`:
- Around line 56-61: The tablist container in TogglePanel lacks an accessible
name, so add an aria-label prop (or aria-labelledby support) to the div with
role="tablist" and pass it through from the component API. Update TogglePanel so
consumers can provide a contextual label for the tab group while keeping the
existing keyboard handling and tablist behavior intact.
- Around line 45-53: The handleKeyDown logic in TogglePanel currently ignores
arrow direction and always picks the first option whose value differs from the
current one. Update the keyboard handling in handleKeyDown to derive the current
index from options and choose the next or previous option based on ArrowRight
versus ArrowLeft, then call onChange and focus the selected option’s ref. Keep
the existing symbols handleKeyDown, options, value, onChange, and next/ref-based
focus flow, but make the selection index-based so it remains correct if more
options are added later.
- Line 6: Rename the union type alias `TogglePanelValue` to follow the project
naming convention by using the `Types` suffix, and update all references
accordingly. Check the `TogglePanel` component and any Storybook or related
files that import or use this alias, since the rename will affect those symbols
as well.
🪄 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: b6efeafb-a88f-43e2-b347-9ad2652ce28e
📒 Files selected for processing (2)
packages/timo-design-system/src/components/toggle-panel/TogglePanel.stories.tsxpackages/timo-design-system/src/components/toggle-panel/TogglePanel.tsx
|
|
||
| import type { KeyboardEvent, RefObject } from "react"; | ||
|
|
||
| export type TogglePanelValue = "timebox" | "timer"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
type alias 네이밍 컨벤션 확인해주세요.
TogglePanelValue는 유니언 타입인데, 컨벤션 상 type alias는 Types 접미사를 붙이도록 되어 있어요. TogglePanelValueTypes처럼 이름을 맞춰주시면 좋을 것 같습니다 (스토리 파일에서도 함께 사용되니 리네이밍 시 영향 범위 확인 필요해요).
As per path instructions, "type alias는 유니언·튜플·리터럴에만 사용, 접미사 Types"를 따라야 합니다.
♻️ 제안
-export type TogglePanelValue = "timebox" | "timer";
+export type TogglePanelValueTypes = "timebox" | "timer";📝 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 type TogglePanelValue = "timebox" | "timer"; | |
| export type TogglePanelValueTypes = "timebox" | "timer"; |
🤖 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/toggle-panel/TogglePanel.tsx` at
line 6, Rename the union type alias `TogglePanelValue` to follow the project
naming convention by using the `Types` suffix, and update all references
accordingly. Check the `TogglePanel` component and any Storybook or related
files that import or use this alias, since the rename will affect those symbols
as well.
Source: Path instructions
Comment on lines
+45
to
+53
| const handleKeyDown = (event: KeyboardEvent<HTMLDivElement>) => { | ||
| if (event.key !== "ArrowLeft" && event.key !== "ArrowRight") return; | ||
|
|
||
| event.preventDefault(); | ||
| const next = options.find((option) => option.value !== value); | ||
| if (!next) return; | ||
| onChange(next.value); | ||
| next.ref.current?.focus(); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
방향 구분 없이 항상 "다른 옵션"으로 이동합니다.
options.find((option) => option.value !== value)는 ArrowLeft/ArrowRight 방향을 구분하지 않고 항상 현재값이 아닌 옵션을 찾아요. 지금은 옵션이 2개뿐이라 결과적으론 정상 동작하지만, 나중에 옵션이 3개 이상으로 늘어나면 방향과 무관하게 동일한 옵션으로만 이동하는 버그가 생길 수 있어요.
인덱스 기반으로 방향에 맞게 다음/이전 옵션을 계산하도록 방어적으로 구현해두면 향후 확장에도 안전할 것 같습니다.
🛡️ 제안
const handleKeyDown = (event: KeyboardEvent<HTMLDivElement>) => {
if (event.key !== "ArrowLeft" && event.key !== "ArrowRight") return;
event.preventDefault();
- const next = options.find((option) => option.value !== value);
- if (!next) return;
+ const currentIndex = options.findIndex((option) => option.value === value);
+ if (currentIndex === -1) return;
+ const delta = event.key === "ArrowRight" ? 1 : -1;
+ const nextIndex = (currentIndex + delta + options.length) % options.length;
+ const next = options[nextIndex];
onChange(next.value);
next.ref.current?.focus();
};📝 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
| const handleKeyDown = (event: KeyboardEvent<HTMLDivElement>) => { | |
| if (event.key !== "ArrowLeft" && event.key !== "ArrowRight") return; | |
| event.preventDefault(); | |
| const next = options.find((option) => option.value !== value); | |
| if (!next) return; | |
| onChange(next.value); | |
| next.ref.current?.focus(); | |
| }; | |
| const handleKeyDown = (event: KeyboardEvent<HTMLDivElement>) => { | |
| if (event.key !== "ArrowLeft" && event.key !== "ArrowRight") return; | |
| event.preventDefault(); | |
| const currentIndex = options.findIndex((option) => option.value === value); | |
| if (currentIndex === -1) return; | |
| const delta = event.key === "ArrowRight" ? 1 : -1; | |
| const nextIndex = (currentIndex + delta + options.length) % options.length; | |
| const next = options[nextIndex]; | |
| onChange(next.value); | |
| next.ref.current?.focus(); | |
| }; |
🤖 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/toggle-panel/TogglePanel.tsx`
around lines 45 - 53, The handleKeyDown logic in TogglePanel currently ignores
arrow direction and always picks the first option whose value differs from the
current one. Update the keyboard handling in handleKeyDown to derive the current
index from options and choose the next or previous option based on ArrowRight
versus ArrowLeft, then call onChange and focus the selected option’s ref. Keep
the existing symbols handleKeyDown, options, value, onChange, and next/ref-based
focus flow, but make the selection index-based so it remains correct if more
options are added later.
Comment on lines
+56
to
+61
| <div | ||
| role="tablist" | ||
| tabIndex={-1} | ||
| onKeyDown={handleKeyDown} | ||
| className="flex h-7.5 w-full overflow-hidden rounded-[4px]" | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
tablist 컨테이너에 접근성 이름이 없어요.
role="tablist" 컨테이너에 aria-label(또는 aria-labelledby)이 없어서 스크린리더 사용자는 이 탭 그룹이 무엇을 위한 것인지 알기 어려울 수 있어요. WAI-ARIA Tabs 패턴에서는 tablist에 접근 가능한 이름을 제공할 것을 권장합니다.
aria-label prop을 추가해서 소비자가 컨텍스트에 맞는 라벨을 지정할 수 있게 하면 좋을 것 같아요.
🤖 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/toggle-panel/TogglePanel.tsx`
around lines 56 - 61, The tablist container in TogglePanel lacks an accessible
name, so add an aria-label prop (or aria-labelledby support) to the div with
role="tablist" and pass it through from the component API. Update TogglePanel so
consumers can provide a contextual label for the tab group while keeping the
existing keyboard handling and tablist behavior intact.
yumin-kim2
approved these changes
Jul 2, 2026
yumin-kim2
left a comment
yumin-kim2
left a comment
Contributor
There was a problem hiding this comment.
size variant를 컴포넌트 안에 두지 않고 부모가 wrapper 너비로 제어하게 바꾸신 것, 컴포넌트 책임 범위를 딱 필요한 만큼만 남긴 선택이라고 생각해요!👍 덕분에 TogglePanelProps가 value/onChange 두 개뿐이라 인터페이스가 읽기 쉬운 것 같습니다. 수고하셨써요~~!!
아 그리고 사소한거지만 PR 라벨이 누락된 것 같습니다!
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
ISSUE 🔗
close #63
What is this PR? 🔍
Timebox/Timer 두 옵션을 전환하는
TogglePanel컴포넌트를 디자인 시스템에 추가했습니다.배경
toggle_panel(268×30 고정)과toggle_panel_big(404 Hug×30 Hug) 두 사이즈 variant가 있어, 이를 컴포넌트 레벨에서 어떻게 표현할지 결정이 필요했습니다.Record기반 사이즈 variant로 구현했지만, 논의 끝에 컴포넌트는w-full만 갖고 실제 크기는 부모가 wrapper 너비로 제어하는 방향으로 단순화했습니다.TogglePanel 컴포넌트
value: "timebox" | "timer",onChange만 받는 controlled 컴포넌트로 구현했습니다.sizeprop을 없앴습니다. cva 도입도 검토했지만, 이미PriorityIcon등에서 쓰는cn()+ 조건부 클래스 패턴만으로 충분해 새 의존성을 추가하지 않았습니다.<button>을flex로 나란히 배치하고, 각 버튼은flex-1로 폭을 균등 분배합니다. 활성 옵션은bg-timo-black text-white, 비활성은bg-timo-gray-300 text-timo-black을cn()조건부 클래스로 적용했습니다. 폰트는typo-body-sb-12, 활성 상태는aria-pressed로 노출했습니다. 컨테이너는w-full이라 크기는 상위에서 wrapper 너비(w-67,w-101등)로 제어합니다.Storybook 스토리
Default(268px wrapper),Big(404px wrapper),AllSizes(두 크기를 라벨과 함께 한 화면에 나열) 세 스토리를 추가했습니다.To Reviewers
sizeprop을 없애고 부모가 너비를 결정하는 방식이 실제 페이지에서 쓰기에 자연스러운지 한번 봐주세요. 이 컴포넌트를 실제 화면에 배치하는 작업은 이번 PR 범위에 포함되지 않았습니다.Screenshot 📷
Test Checklist ✔
pnpm --filter @repo/timo-design-system check-types통과pnpm --filter @repo/timo-design-system lint통과pnpm --filter @repo/timo-design-system build:storybook실행 → Default/Big/AllSizes 스토리 포함 빌드 성공pnpm build— 미실행: CI에서 확인 예정