-
Notifications
You must be signed in to change notification settings - Fork 1
Feature/#7 공통 컴포넌트 구현 #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- href fallback 제거
Walkthrough이 PR은 UI 컴포넌트와 스타일 설정을 소규모로 수정합니다. Button에 잔여 props 전달을 추가하고, Chip의 공개 prop 이름을 변경하며, IconMap의 키를 정정했습니다. SearchBar의 href 처리와 접근성 라벨을 조정했고, Textarea에 useId 기반 name을 추가했습니다. styles.css의 Tailwind 설정 로딩 순서를 재구성했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui/src/components/Button/Button.tsx (1)
28-29: form 내부 기본 제출 방지를 위해 기본 type="button" 지정 권장기본
as가'button'인 경우, 명시적으로type을 지정하지 않으면 브라우저 기본값이submit이어서 폼 내에서 예기치 않은 제출이 발생할 수 있습니다. 기본을button으로 지정하고, 사용자가type을 넘기면 덮어쓰도록 하는 패턴을 추천합니다.아래와 같이 조건부로 기본 type을 주입하는 방안을 제안합니다.
const Component = as || 'button' return ( <Component className={cn( 'ui:flex', 'ui:justify-center', 'ui:items-center', 'ui:bg-main', 'ui:text-white', 'ui:rounded-lg', BUTTON_SIZE[size], className, )} - {...restProps} + {...((Component === 'button' && !(restProps as any)?.type) + ? ({ type: 'button' } as const) + : {})} + {...restProps} > <Text as='span' variant={BUTTON_FONT_SIZE[size]}> {children} </Text> </Component> )
- 조건부로만
type을 추가하여as="a"등 다른 엘리먼트에는 불필요한 속성이 전달되지 않습니다.{...restProps}를 마지막에 유지하여 사용자가 명시적으로 전달한type이 우선하도록 합니다.Also applies to: 31-43
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/ui/src/components/Button/Button.tsx(2 hunks)packages/ui/src/components/Chip/Chip.tsx(3 hunks)packages/ui/src/components/Icon/IconMap.ts(1 hunks)packages/ui/src/components/SearchBar/SearchBar.tsx(1 hunks)packages/ui/src/components/Textarea/Textarea.tsx(3 hunks)packages/ui/src/styles.css(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/ui/src/components/Chip/Chip.tsx (1)
packages/ui/src/components/Chip/Chip.stories.tsx (4)
type(35-43)console(39-41)Flex(26-45)Flex(15-22)
packages/ui/src/components/Icon/IconMap.ts (3)
packages/ui/src/components/Icon/assets/icons/kakaoLogo.tsx (1)
KakaoLogo(3-30)packages/ui/src/components/Icon/Icon.tsx (1)
Icon(40-51)packages/ui/src/components/Header/Header.tsx (1)
icon(23-34)
packages/ui/src/components/Button/Button.tsx (7)
packages/ui/src/components/Text/Text.tsx (1)
Text(115-138)packages/ui/src/components/Button/consts.ts (1)
BUTTON_FONT_SIZE(10-16)packages/ui/src/components/Layout/Flex/Flex.tsx (1)
restProps(14-25)packages/ui/src/components/Layout/JustifyBetween/JustifyBetween.tsx (1)
restProps(4-21)packages/ui/src/components/Layout/Column/Column.tsx (1)
restProps(4-16)packages/ui/src/components/Layout/JustifyAround/JustifyAround.tsx (1)
restProps(4-21)packages/ui/src/components/Layout/JustifyEnd/JustifyEnd.tsx (1)
restProps(4-21)
packages/ui/src/components/SearchBar/SearchBar.tsx (1)
packages/ui/src/utils/cn.ts (1)
cn(19-21)
🪛 Biome (2.1.2)
packages/ui/src/styles.css
[error] 3-3: expected , but instead found (
Remove (
(parse)
[error] 3-3: This @import is in the wrong position.
Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.
Consider moving import position.
(lint/correctness/noInvalidPositionAtImportRule)
[error] 3-3: Don't use unknown media feature names.
Unexpected unknown media feature name.
You should use media feature names defined in the CSS Specifications.
(lint/correctness/noUnknownMediaFeatureName)
[error] 4-4: This @import is in the wrong position.
Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.
Consider moving import position.
(lint/correctness/noInvalidPositionAtImportRule)
🔇 Additional comments (12)
packages/ui/src/components/Textarea/Textarea.tsx (3)
3-3: useId 추가가 적절합니다.React의 useId 훅을 추가하여 각 인스턴스마다 고유한 식별자를 생성하는 것은 좋은 접근입니다.
39-39: 고유한 ID 생성이 올바릅니다.useId를 사용하여 각 컴포넌트 인스턴스마다 고유한 ID를 생성하는 것은 SSR 환경에서 hydration mismatch를 방지하고 접근성을 개선하는 모범 사례입니다.
58-58: name 속성 추가가 유용합니다.각 textarea 인스턴스에 고유한 name 속성을 추가한 것은 폼 제출과 테스팅 시 유용합니다.
message-${id}형식으로 생성된 이름은 각 인스턴스를 고유하게 식별할 수 있게 해줍니다.packages/ui/src/components/SearchBar/SearchBar.tsx (2)
10-10: href 처리 개선이 적절합니다.빈 문자열로의 fallback을 제거하고 href를 그대로 사용하는 것이 더 명확합니다. 타입 정의에서 href가 string으로 지정되어 있어 항상 값이 제공될 것으로 예상됩니다.
18-18: 접근성 개선이 좋습니다.aria-label을 추가하여 스크린 리더 사용자에게 링크의 목적을 명확히 전달하는 것은 훌륭한 접근성 개선입니다.
packages/ui/src/components/Icon/IconMap.ts (1)
96-96: kakaoLogo 키 이름 변경 검증 완료검토 결과,
kakkoLogo에서kakaoLogo로 키를 수정한 후 전체 코드베이스를 검색한 결과 옛 키(kakkoLogo)를 참조하는 곳은 없으며, 아래 위치에서만 정상적으로 사용되고 있습니다.
- packages/ui/src/components/Icon/IconMap.ts
- import:
import { KakaoLogo } from './assets/icons/kakaoLogo'- iconMap:
kakaoLogo: KakaoLogo,다른 파일에서 참조되지 않아 변경에 따른 영향이 없습니다.
packages/ui/src/components/Chip/Chip.tsx (2)
33-33: prop 이름 변경이 일관성을 개선합니다.
type에서chipType으로 prop 이름을 변경한 것은 좋은 개선입니다.type은 HTML의 기본 속성과 충돌할 수 있어 더 명시적인 이름이 적절합니다.
53-53: JSDoc 업데이트가 정확합니다.param 주석을
type에서chipType으로 업데이트한 것이 코드 변경과 일치합니다.packages/ui/src/styles.css (1)
2-2: Tailwind 설정 순서가 올바르게 적용되었으나, CSS 파싱 오류가 계속 발생하고 있어 추가 확인이 필요합니다.패키지 내 설정 파일과 경로는 올바르게 존재함을 확인했습니다.
다음 사항들을 점검해 주세요:
- packages/ui/src/styles.css
@config "../tailwind.config.ts";→packages/ui/tailwind.config.ts경로 확인 완료@import "tailwindcss" prefix(ui);@import "@repo/tailwind-config";- 정적 분석 도구(stylelint/PostCSS 등)의 Tailwind 전용 At-rule 인식 여부
- postcss-import, tailwindcss/nesting 플러그인 및 stylelint-plugin-tailwindcss 같은 확장 플러그인이 설정되어 있는지 확인
- 빌드 파이프라인에서
.ts확장자 설정
- Tailwind 설정을 TypeScript로 작성할 경우 ts-node 또는 esbuild 등으로 로드되도록 구성되어 있는지 검증
- 필요 시 tailwind.config.js로 변환하거나 적절한 로더를 추가
위 점검 후에도 파싱 오류가 지속된다면, 실제 빌드 과정을 재현(예:
npx stylelint packages/ui/src/styles.css,npx postcss packages/ui/src/styles.css -o /dev/null)하여 원인을 파악해 주세요.packages/ui/src/components/Button/Button.tsx (3)
21-27: restProps 전달 추가 잘 반영됨Polymorphic 컴포넌트 패턴(Text, Flex 등)과 일관되게 잔여 props를 받아 전달하도록 변경되어 확장성과 사용성이 좋아졌습니다.
42-42: restProps 스프레드 위치 적절이미 상단에서 className을 분리했기 때문에, 이후
{...restProps}로 className이 덮어써지는 문제도 없습니다. DOM 속성/이벤트, ARIA 속성 전달이 가능해져 유용합니다.
44-44: 사소: 문자열 리터럴로 통일해 가독성 소폭 향상
{'span'}→'span'변경은 동작에는 영향 없고, 코드 일관성과 가독성에 긍정적입니다.
| as, | ||
| className, | ||
| type, | ||
| chipType, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
prop 사용처 업데이트가 완료되었습니다.
destructuring과 내부 사용처에서 type을 chipType으로 일관되게 변경했습니다.
Chip.stories.tsx 파일에서 이 변경사항을 반영했는지 확인해보겠습니다.
Also applies to: 70-70
🏁 Script executed:
#!/bin/bash
# 설명: Chip 컴포넌트의 type prop 사용처를 확인합니다.
# Chip 컴포넌트에서 type prop 사용 검색 (구 이름)
echo "=== type prop 사용처 (구 이름) ==="
rg -nC3 '<Chip[^>]*type=' --type=tsx --type=ts
# chipType prop 사용 검색 (새 이름)
echo "=== chipType prop 사용처 (새 이름) ==="
rg -nC3 '<Chip[^>]*chipType=' --type=tsx --type=tsLength of output: 299
ripgrep의 --type=tsx 설정이 인식되지 않아 검색이 실패했습니다. glob 패턴으로 .tsx/.ts 파일을 직접 지정해 다시 확인해보겠습니다.
🏁 Script executed:
#!/bin/bash
echo "=== type prop 사용처 (구 이름) ==="
rg -nC3 -g '*.tsx' -g '*.ts' '<Chip[^>]*type='
echo "=== chipType prop 사용처 (새 이름) ==="
rg -nC3 -g '*.tsx' -g '*.ts' '<Chip[^>]*chipType='Length of output: 1472
Chip.stories.tsx에서 type → chipType 프로퍼티 명 변경 필요
스토리 파일에 아직 type= 사용 예시가 남아 있어, 새로 도입된 chipType=으로 수정해야 합니다.
• packages/ui/src/components/Chip/Chip.stories.tsx
– 줄 17: <Chip type='SOLO_FRIENDLY' />
– 줄 18: <Chip type='GOOD_AMBIENCE' />
– 줄 19: <Chip type='VALUE_FOR_MONEY' />
– 줄 20: <Chip type='KIND_SERVICE' />
- <Chip type='SOLO_FRIENDLY' />
+ <Chip chipType='SOLO_FRIENDLY' />Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/ui/src/components/Chip/Chip.stories.tsx around lines 17 to 20, the
story examples still use the old prop name `type=`; update each instance to use
the new prop name `chipType=` (e.g. change `<Chip type='SOLO_FRIENDLY' />` to
`<Chip chipType='SOLO_FRIENDLY' />`, and similarly for the other three lines) so
the stories match the component's `chipType` prop.
#️⃣연관된 이슈
📝작업 내용
Button 컴포넌트에 restProps 추가 3ebc9f6 f3ada25
Chip 컴포넌트에서 type을 chipType으로 변경 389eedf
Tailwind 설정 로딩을 위한 CSS import 순서 수정 6d6ba78
config지시문을import구문보다 앞으로 이동Textarea 컴포넌트에 useId 훅 추가 및 name 속성 설정
스크린샷 (선택)
💬리뷰 요구사항(선택)
Summary by CodeRabbit