Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/timo-design-system/src/components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ export { Tag } from "@components/tag/Tag";
export { PriorityIcon } from "@components/priority-icon/PriorityIcon";
export { CreateButton } from "@components/button/create-button/CreateButton";
export { TodayBadge } from "@components/badge/today-badge/TodayBadge";
export { TogglePanel } from "@components/toggle-panel/TogglePanel";
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import {
TogglePanel,
TogglePanelValue,
} from "@components/toggle-panel/TogglePanel";
import { useEffect, useState } from "react";

import type { Meta, StoryObj } from "@storybook/react";

const meta = {
title: "Components/TogglePanel",
component: TogglePanel,
parameters: {
layout: "centered",
},
args: {
value: "timebox",
onChange: () => {},
},
} satisfies Meta<typeof TogglePanel>;

export default meta;
type Story = StoryObj<typeof meta>;

const PlaygroundTogglePanel = ({
width,
...args
}: React.ComponentProps<typeof TogglePanel> & { width: string }) => {
const [value, setValue] = useState<TogglePanelValue>(args.value);

useEffect(() => {
setValue(args.value);
}, [args.value]);

return (
<div className={width}>
<TogglePanel {...args} value={value} onChange={setValue} />
</div>
);
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

export const Default: Story = {
render: (args) => <PlaygroundTogglePanel {...args} width="w-67" />,
};

export const Big: Story = {
args: {
value: "timer",
},
render: (args) => <PlaygroundTogglePanel {...args} width="w-101" />,
};

const TogglePanelWithPanel = (
args: React.ComponentProps<typeof TogglePanel>,
) => {
const [value, setValue] = useState<TogglePanelValue>(args.value);

useEffect(() => {
setValue(args.value);
}, [args.value]);

return (
<div className="w-67">
<TogglePanel
{...args}
id="toggle-panel-demo"
value={value}
onChange={setValue}
timeboxControls="toggle-panel-demo-timebox-panel"
timerControls="toggle-panel-demo-timer-panel"
/>
<div
id="toggle-panel-demo-timebox-panel"
role="tabpanel"
aria-labelledby="toggle-panel-demo-timebox-tab"
hidden={value !== "timebox"}
className="bg-timo-blue-100 mt-4 rounded-lg p-4 text-sm"
>
Timebox 관련 콘텐츠 영역
</div>
<div
id="toggle-panel-demo-timer-panel"
role="tabpanel"
aria-labelledby="toggle-panel-demo-timer-tab"
hidden={value !== "timer"}
className="border-timo-gray-500 mt-4 rounded-lg border p-4 text-sm"
>
Timer 관련 콘텐츠 영역
</div>
</div>
);
};

export const WithPanel: Story = {
render: (args) => <TogglePanelWithPanel {...args} />,
};

export const AllSizes: Story = {
render: () => (
<div className="flex flex-col items-start gap-6">
<div className="flex flex-col items-start gap-2">
<p className="text-xs">Default (268px)</p>
<div className="w-67">
<TogglePanel value="timebox" onChange={() => {}} />
</div>
</div>
<div className="flex flex-col items-start gap-2">
<p className="text-xs">Big (404px)</p>
<div className="w-101">
<TogglePanel value="timer" onChange={() => {}} />
</div>
</div>
</div>
),
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { cn } from "@lib";
import { useId, useRef } from "react";

import type { KeyboardEvent, RefObject } from "react";

export type TogglePanelValue = "timebox" | "timer";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 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


export interface TogglePanelProps {
id?: string;
value: TogglePanelValue;
onChange: (value: TogglePanelValue) => void;
timeboxControls?: string;
timerControls?: string;
}

interface TogglePanelOption {
value: TogglePanelValue;
label: string;
ref: RefObject<HTMLButtonElement | null>;
controls?: string;
}

export const TogglePanel = ({
id: idProp,
value,
onChange,
timeboxControls,
timerControls,
}: TogglePanelProps) => {
const generatedId = useId();
const id = idProp ?? generatedId;
const timeboxRef = useRef<HTMLButtonElement>(null);
const timerRef = useRef<HTMLButtonElement>(null);

const options: TogglePanelOption[] = [
{
value: "timebox",
label: "Timebox",
ref: timeboxRef,
controls: timeboxControls,
},
{ value: "timer", label: "Timer", ref: timerRef, controls: timerControls },
];

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();
};
Comment on lines +45 to +53

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 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.


return (
<div
role="tablist"
tabIndex={-1}
onKeyDown={handleKeyDown}
className="flex h-7.5 w-full overflow-hidden rounded-[4px]"
>
Comment on lines +56 to +61

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 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.

{options.map((option) => (
<button
key={option.value}
ref={option.ref}
type="button"
id={`${id}-${option.value}-tab`}
role="tab"
aria-selected={value === option.value}
aria-controls={option.controls}
tabIndex={value === option.value ? 0 : -1}
onClick={() => onChange(option.value)}
className={cn(
"typo-body-sb-12 flex flex-1 items-center justify-center px-4 whitespace-nowrap",
value === option.value
? "bg-timo-black text-white"
: "bg-timo-gray-300 text-timo-black",
)}
>
{option.label}
</button>
))}
</div>
);
};