Skip to content
Merged
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { cn } from "@repo/timo-design-system/utils";

export interface OnboardingSelectCardProps {
size: "sm" | "lg";
selected: boolean;
label: string;
sublabel: string;
onClick?: () => void;
}

export const OnboardingSelectCard = ({
size,
selected,
label,
sublabel,
onClick,
}: OnboardingSelectCardProps) => {
const radioIcon = (
<div
className={cn(
"size-4.5 shrink-0 rounded-full bg-white",
selected
? "border-timo-blue-300 border-[5px]"
: "border-timo-gray-500 border-2",
)}
/>
);
Comment on lines +18 to +27

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

선택 상태를 스크린리더에도 알려주면 더 좋아요.

현재는 색상/테두리로만 selected 상태를 표현해서, 스크린리더 사용자는 카드가 선택됐는지 알 방법이 없습니다. 라디오 형태의 선택 UI이므로 buttonaria-pressed={selected} (혹은 role="radio" aria-checked={selected})를 추가하고, 순수 장식용인 radioIcon에는 aria-hidden="true"를 붙여주는 걸 제안합니다.

♿ 제안 diff
   const radioIcon = (
     <div
+      aria-hidden="true"
       className={cn(
     <button
       type="button"
+      aria-pressed={selected}
       onClick={onClick}

관련해서 WAI-ARIA의 button role 패턴을 참고하시면 좋아요.

[accessibility]

Also applies to: 30-42

🤖 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 `@apps/timo-web/app/`[locale]/onboarding/_components/OnboardingSelectCard.tsx
around lines 18 - 27, The selected state is only conveyed visually in
OnboardingSelectCard, so make the card’s interactive element expose selection to
assistive tech by adding either aria-pressed={selected} or role="radio" with
aria-checked={selected} on the button wrapper used in this component. Also mark
the radioIcon element as purely decorative with aria-hidden="true" so screen
readers ignore it. Update the related rendering logic in OnboardingSelectCard to
keep the accessible state in sync wherever selected is used.


return (
<button
type="button"
onClick={onClick}
className={cn(
"flex rounded-[4px] border px-4 py-2 text-left",
selected
? "border-timo-blue-300 bg-timo-blue-50"
: "border-timo-gray-500 bg-white",
size === "sm" ? "w-37.5" : "w-76 flex-row items-center justify-between",
)}
>
{size === "sm" ? (
<div className="flex w-full flex-col items-end gap-1">
<div className="flex w-full flex-col">
<span
className={cn(
"typo-headline-b-16",
selected ? "text-timo-blue-300" : "text-timo-gray-900",
)}
>
{label}
</span>
<span
className={cn(
"typo-body-m-12",
selected ? "text-timo-blue-300" : "text-timo-gray-700",
)}
>
{sublabel}
</span>
</div>
{radioIcon}
</div>
) : (
<>
<div className="flex flex-col">
<span
className={cn(
"typo-headline-b-16",
selected ? "text-timo-blue-300" : "text-timo-gray-900",
)}
>
{label}
</span>
<span
className={cn(
"typo-body-m-12",
selected ? "text-timo-blue-300" : "text-timo-gray-700",
)}
>
{sublabel}
</span>
</div>
{radioIcon}
</>
)}
Comment on lines +41 to +85

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

label/sublabel 렌더링 블록 중복 제거를 제안합니다.

sm(라인 45-62)과 lg(라인 67-84) 분기의 내부 콘텐츠가 wrapper만 다르고 완전히 동일합니다. 한쪽만 수정하고 다른 쪽을 놓치는 실수가 생기기 쉬운 구조입니다. 공통 콘텐츠를 변수로 뽑아 wrapper만 분기하면 훨씬 안전합니다.

♻️ 제안 리팩터
+  const content = (
+    <div className={cn("flex", size === "sm" ? "w-full flex-col" : "flex-col")}>
+      <span
+        className={cn(
+          "typo-headline-b-16",
+          selected ? "text-timo-blue-300" : "text-timo-gray-900",
+        )}
+      >
+        {label}
+      </span>
+      <span
+        className={cn(
+          "typo-body-m-12",
+          selected ? "text-timo-blue-300" : "text-timo-gray-700",
+        )}
+      >
+        {sublabel}
+      </span>
+    </div>
+  );
+
   return (
     <button
       ...
     >
       {size === "sm" ? (
         <div className="flex w-full flex-col items-end gap-1">
-          <div className="flex w-full flex-col">
-            <span ...>{label}</span>
-            <span ...>{sublabel}</span>
-          </div>
+          {content}
           {radioIcon}
         </div>
       ) : (
         <>
-          <div className="flex flex-col">
-            <span ...>{label}</span>
-            <span ...>{sublabel}</span>
-          </div>
+          {content}
           {radioIcon}
         </>
       )}
     </button>
   );

참고로 React 공식 문서의 Conditional Rendering 가이드에서도 공통 JSX는 변수로 추출해 재사용하는 패턴을 권장합니다.

📝 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
{size === "sm" ? (
<div className="flex w-full flex-col items-end gap-1">
<div className="flex w-full flex-col">
<span
className={cn(
"typo-headline-b-16",
selected ? "text-timo-blue-300" : "text-timo-gray-900",
)}
>
{label}
</span>
<span
className={cn(
"typo-body-m-12",
selected ? "text-timo-blue-300" : "text-timo-gray-700",
)}
>
{sublabel}
</span>
</div>
{radioIcon}
</div>
) : (
<>
<div className="flex flex-col">
<span
className={cn(
"typo-headline-b-16",
selected ? "text-timo-blue-300" : "text-timo-gray-900",
)}
>
{label}
</span>
<span
className={cn(
"typo-body-m-12",
selected ? "text-timo-blue-300" : "text-timo-gray-700",
)}
>
{sublabel}
</span>
</div>
{radioIcon}
</>
)}
const content = (
<div className={cn("flex", size === "sm" ? "w-full flex-col" : "flex-col")}>
<span
className={cn(
"typo-headline-b-16",
selected ? "text-timo-blue-300" : "text-timo-gray-900",
)}
>
{label}
</span>
<span
className={cn(
"typo-body-m-12",
selected ? "text-timo-blue-300" : "text-timo-gray-700",
)}
>
{sublabel}
</span>
</div>
);
return (
<button>
{size === "sm" ? (
<div className="flex w-full flex-col items-end gap-1">
{content}
{radioIcon}
</div>
) : (
<>
{content}
{radioIcon}
</>
)}
</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 `@apps/timo-web/app/`[locale]/onboarding/_components/OnboardingSelectCard.tsx
around lines 43 - 87, The label/sublabel JSX in OnboardingSelectCard is
duplicated across the sm and lg branches, so extract the shared content into a
single reusable fragment/variable and keep only the wrapper layout conditional;
update the selected/text color logic once in that shared block so future changes
to label, sublabel, or cn usage stay in sync.

</button>
);
};
Loading