[FEAT] 온보딩 화면 OnboardingSelectCard 컴포넌트 구현#117
Merged
Conversation
- sm/lg 사이즈 props로 레이아웃을 제어했습니다 - selected 상태에 따라 배경·보더·텍스트 색상을 변경했습니다 - 라디오 아이콘을 별도 컴포넌트 없이 직접 구현했습니다
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Walkthrough온보딩 화면용 ChangesOnboardingSelectCard 컴포넌트
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
ehye1
changed the title
![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 `@apps/timo-web/app/`[locale]/onboarding/_components/OnboardingSelectCard.tsx:
- Around line 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.
- Around line 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.
🪄 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: 4e7647ad-9c51-41cb-9e43-ae72d191f181
📒 Files selected for processing (1)
apps/timo-web/app/[locale]/onboarding/_components/OnboardingSelectCard.tsx
Comment on lines
+18
to
+27
| const radioIcon = ( | ||
| <div | ||
| className={cn( | ||
| "size-[18px] shrink-0 rounded-full bg-white", | ||
| selected | ||
| ? "border-timo-blue-300 border-[5px]" | ||
| : "border-timo-gray-500 border-2", | ||
| )} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
선택 상태를 스크린리더에도 알려주면 더 좋아요.
현재는 색상/테두리로만 selected 상태를 표현해서, 스크린리더 사용자는 카드가 선택됐는지 알 방법이 없습니다. 라디오 형태의 선택 UI이므로 button에 aria-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.
Comment on lines
+43
to
+87
| {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} | ||
| </> | ||
| )} |
There was a problem hiding this comment.
📐 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.
Timo Performance ReportBundle Size — timo-web
Lighthouse — timo-web
Image Optimization — timo-web
측정 커밋: |
kimminna
approved these changes
Jul 8, 2026
jjangminii
approved these changes
Jul 8, 2026
jjangminii
left a comment
jjangminii
left a comment
Contributor
There was a problem hiding this comment.
깔끔하고 이쁘네요 간단한 코멘트만 확인해주세요~
| const radioIcon = ( | ||
| <div | ||
| className={cn( | ||
| "size-[18px] shrink-0 rounded-full bg-white", |
Comment on lines
+39
to
+40
| ? "w-[150px]" | ||
| : "w-[304px] flex-row items-center justify-between", |
Contributor
There was a problem hiding this comment.
이것도 px말고 테일윈드 토큰 사이즈로 바꿔주세여
- size-[18px]을 size-4.5로 교체했습니다 - w-[150px]을 w-37.5로 교체했습니다 - w-[304px]을 w-76으로 교체했습니다
5 tasks
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 #108
What is this PR? 🔍
온보딩 화면에서 언어 및 활동 유형 선택 시 공통으로 사용하는
OnboardingSelectCard컴포넌트를 구현했습니다.배경
size,selected,label,sublabelprops로 상태와 레이아웃을 외부에서 주입받는 순수 UI 컴포넌트로 구현했습니다.OnboardingSelectCard
sm/lg두 가지 사이즈와selected상태를 props로 제어하는 선택 카드 컴포넌트를 추가했습니다.size="sm"은 column 레이아웃(텍스트 좌상단, 라디오 우하단),size="lg"는 row + justify-between 레이아웃으로 분기합니다. 라디오 아이콘은 별도 아이콘 컴포넌트 없이border-[5px]두꺼운 테두리로 선택 상태의 링(도넛) 모양을 직접 구현했습니다.selected·onClick을 부모에서 주입받는 순수 UI로 유지했습니다.To Reviewers
라디오 아이콘을 아이콘 컴포넌트 없이 CSS
border-[5px]로 링 모양을 직접 구현했습니다.<button>요소의 UA 기본 스타일(text-align: center)을text-left로 재정의했습니다.Screenshot 📷
Timo.-.Chrome.2026-07-08.04-19-10.mp4
Test Checklist ✔
pnpm check-types)pnpm lint)sm/lg사이즈 레이아웃 확인selected상태 전환 시 색상·라디오 아이콘 변경 확인