Feat/component/input/DEVING-34 Input 컴포넌트 개발 #15
Conversation
…mService/Moim-FE into feat/component/input/DEVING-34
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough이번 변경 사항은 두 개의 컴포넌트 업데이트를 포함합니다. 첫 번째로, Changes
Sequence Diagram(s)sequenceDiagram
participant U as 사용자
participant I as Input 컴포넌트
participant V as 가시성 토글 아이콘
U->>I: 비밀번호 입력 필드 활성화
U->>V: 토글 아이콘 클릭
V->>I: 토글 이벤트 전달
I->>I: 입력 타입 상태 업데이트 (password ↔ text)
I-->>U: 업데이트된 입력 렌더링
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
CI Status Report검사 결과
❌ 일부 검사가 실패했습니다. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ |
CI Status Report검사 결과
✅ 모든 검사가 통과되었습니다. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
src/app/preview/input/page.tsx (1)
8-137: 코드 중복을 제거하여 유지보수성을 개선해야 합니다.반복되는 컨테이너 너비 정의를 상수로 추출하고, 반복되는 Input 컴포넌트 렌더링을 map 함수를 사용하여 리팩토링하는 것이 좋습니다.
다음과 같이 수정하는 것을 제안합니다:
const CONTAINER_WIDTH = 'w-[460px]'; const INPUT_VARIANTS = [ { title: 'normal', props: { placeholder: 'Default' } }, { title: 'typing', props: { placeholder: 'Default', defaultValue: 'Typing' } }, { title: 'done', props: { defaultValue: 'Done', state: 'success' } }, { title: 'error', props: { defaultValue: 'Error', errorMessage: '필수 입력 사항입니다.' } }, { title: 'disable', props: { placeholder: 'Disabled', disabled: true } }, ] as const; // 사용 예: {INPUT_VARIANTS.map(({ title, props }) => ( <React.Fragment key={title}> <h2>large {title}</h2> <div className={`mb-4 ${CONTAINER_WIDTH}`}> <Input {...props} /> </div> </React.Fragment> ))}src/components/common/Header.tsx (1)
96-135: 접근성 개선 필요NavLinks 컴포넌트의 접근성을 개선할 필요가 있습니다. 현재 구현에는 ARIA 레이블이나 역할이 정의되어 있지 않습니다.
다음과 같이 수정하는 것을 제안합니다:
<ul + role="navigation" + aria-label="메인 메뉴" className={`${!isMobile ? 'hidden items-center text-Cgray700 lg:flex' : 'text-Cgray400'}`} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/assets/icon/logo.svgis excluded by!**/*.svgsrc/assets/icon/profile.svgis excluded by!**/*.svgsrc/fonts/PretendardVariable.woff2is excluded by!**/*.woff2
📒 Files selected for processing (12)
.coderabbit.yaml(0 hunks)next.config.mjs(1 hunks)package.json(1 hunks)src/app/layout.tsx(2 hunks)src/app/preview/dropdown/page.tsx(1 hunks)src/app/preview/gnb/page.tsx(1 hunks)src/app/preview/input/page.tsx(1 hunks)src/components/common/Header.tsx(1 hunks)src/components/ui/Input.tsx(1 hunks)src/types/global.d.ts(1 hunks)tailwind.config.ts(3 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- .coderabbit.yaml
🧰 Additional context used
🪛 GitHub Check: check
src/components/common/Header.tsx
[warning] 30-30:
Unexpected console statement
[warning] 69-69:
Unexpected console statement
🔇 Additional comments (6)
src/types/global.d.ts (1)
1-5: SVG 모듈 선언이 적절하게 구현되었습니다!React 컴포넌트로 SVG를 임포트할 수 있도록 타입이 정확하게 정의되어 있습니다.
next.config.mjs (1)
2-10: Webpack 설정이 적절하게 구현되었습니다!SVG 파일을 React 컴포넌트로 처리하기 위한 webpack 설정이 정확하게 구현되어 있습니다.
src/app/layout.tsx (1)
12-17: 폰트 설정이 적절하게 구현되었습니다!Pretendard 폰트의 variable font 설정과 display swap 옵션이 성능 최적화를 위해 올바르게 구성되어 있습니다.
tailwind.config.ts (1)
13-16: 브레이크포인트 값을 검토해주세요.현재 설정된 브레이크포인트 값이 일반적인 모바일/태블릿 기준과 다릅니다:
- md: 376px (일반적으로 768px)
- lg: 745px (일반적으로 1024px)
디자인 시스템과 일치하는지 확인이 필요합니다.
tsconfig.json (1)
34-40: 설정이 적절해 보입니다!src 디렉토리를 포함하도록 설정을 추가한 것이 적절합니다. TypeScript 컴파일러가 올바르게 소스 파일을 찾을 수 있게 됩니다.
package.json (1)
17-18:Details
✅ Verification successful
의존성 추가가 적절해 보입니다!
@svgr/webpack과 @radix-ui/react-slot의 추가가 적절합니다. 다만, 보안을 위해 최신 버전인지 확인이 필요합니다.
🌐 Web query:
What is the latest version of @svgr/webpack and @radix-ui/react-slot?💡 Result:
The latest versions of the packages based on current information are:
@svgr/webpack: Version 8.1.0
@radix-ui/react-slot: Version 1.1.2Details:
- @svgr/webpack: The npm registry lists
@svgr/webpackat v8.1.0 as its latest stable release[10]. This aligns with usage examples in projects like Gatsby[7].- @radix-ui/react-slot: The npm registry shows v1.1.2 as the latest stable release (published 11 days ago)[2][8]. Projects like PasteBarApp also use this version[3].
For production environments, ensure compatibility with peer dependencies like Webpack (v5+ for
@svgr/webpack)[1][7].Citations:
- 1: https://react-svgr.com/docs/migrate/
- 2: https://www.npmjs.com/package/@radix-ui/react-slot?activeTab=versions
- 3: https://github.com/PasteBar/PasteBarApp/blob/main/package.json
- 4: https://github.com/gregberge/svgr/blob/main/CHANGELOG.md
- 5: https://www.radix-ui.com/primitives/docs/overview/releases
- 6: radix-ui/primitives#2769
- 7: https://www.gatsbyjs.com/plugins/gatsby-plugin-svgr/
- 8: https://www.npmjs.com/package/@radix-ui/react-slot
- 9: https://answers.netlify.com/t/next-js-unable-to-get-sharp-working/123224
- 10: https://www.npmjs.com/package/@svgr/webpack
의존성 버전 검증 완료 – 모든 것이 최신 상태입니다.
- 현재 사용 중인 버전:
@svgr/webpack:^8.1.0@radix-ui/react-slot:^1.1.2웹 상의 최신 정보와 일치하므로, 해당 의존성 추가는 보안 및 기능 면에서 적절합니다.
| export default function Home() { | ||
| return ( | ||
| <div className="mt-40"> | ||
| <Header isLogIn /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
isLogIn prop의 상태 관리 개선이 필요합니다.
하드코딩된 isLogIn 값은 실제 로그인 상태와 동기화되지 않을 수 있습니다. 전역 상태 관리나 Context를 통해 실제 로그인 상태를 관리하는 것이 좋습니다.
- <Header isLogIn />
+ <Header isLogIn={useAuth().isLoggedIn} />There was a problem hiding this comment.
@dbswl701 알겠습니다. 로그인 기능 구현 후 연동하시면 되겠습니다.
🧠 Learnings used
Learnt from: dbswl701
PR: MoimService/Moim-FE#13
File: src/app/preview/gnb/page.tsx:8-8
Timestamp: 2025-02-18T07:37:44.563Z
Learning: The hardcoded `isLogIn` prop in the GNB preview page (`src/app/preview/gnb/page.tsx`) is temporary and will be replaced with actual authentication state when login functionality is implemented.
| <body>{children}</body> | ||
| <html lang="ko" className={pretendard.variable}> | ||
| <body> | ||
| <Header /> |
There was a problem hiding this comment.
Header 컴포넌트의 props 검토가 필요합니다.
layout에서 사용되는 Header 컴포넌트에 isLogIn prop이 전달되지 않았습니다. 이는 preview/gnb 페이지의 구현과 일관성이 없습니다.
- <Header />
+ <Header isLogIn={useAuth().isLoggedIn} />📝 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.
| <Header /> | |
| <Header isLogIn={useAuth().isLoggedIn} /> |
|
|
||
| export default function Home() { | ||
| const [selectedFilter, setSelectedFilter] = useState<string | null>(null); | ||
| const [selectedFilter] = useState<string | null>(null); |
There was a problem hiding this comment.
상태 업데이트 로직이 누락되었습니다.
selectedFilter의 상태를 업데이트할 수 있는 setSelectedFilter 함수가 제거되어 필터 기능이 동작하지 않을 수 있습니다.
다음과 같이 수정하는 것을 제안합니다:
-const [selectedFilter] = useState<string | null>(null);
+const [selectedFilter, setSelectedFilter] = useState<string | null>(null);📝 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.
| const [selectedFilter] = useState<string | null>(null); | |
| const [selectedFilter, setSelectedFilter] = useState<string | null>(null); |
There was a problem hiding this comment.
@dbswl701 네, 이해했습니다. preview 페이지에서 실제 필터링 기능이 구현되지 않아 상태 업데이트가 불필요하다는 판단이 맞습니다.
✏️ Learnings added
Learnt from: dbswl701
PR: MoimService/Moim-FE#15
File: src/app/preview/dropdown/page.tsx:0-0
Timestamp: 2025-02-19T02:36:43.049Z
Learning: In preview/example pages where components are only displayed for demonstration purposes without actual functionality, state update functions can be omitted if they're not being used to update the UI.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| <button | ||
| onClick={() => setIsVisible((prev) => !prev)} | ||
| className={`absolute right-[16px] top-1/2 -translate-y-1/2 text-Cgray500 focus-within:text-Cgray700 ${type !== 'password' && 'hidden'} | ||
| `} | ||
| > | ||
| {!isVisible ? ( | ||
| <EyeOff className={`size-5 `} /> | ||
| ) : ( | ||
| <Eye className={`size-5 `} /> | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성 개선이 필요합니다.
비밀번호 표시/숨기기 버튼에 대한 접근성 개선이 필요합니다:
다음과 같이 수정하는 것을 제안합니다:
<button
onClick={() => setIsVisible((prev) => !prev)}
+ type="button"
+ aria-label={isVisible ? "비밀번호 숨기기" : "비밀번호 표시"}
className={`absolute right-[16px] top-1/2 -translate-y-1/2 text-Cgray500 focus-within:text-Cgray700 ${type !== 'password' && 'hidden'}`}
>📝 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.
| <button | |
| onClick={() => setIsVisible((prev) => !prev)} | |
| className={`absolute right-[16px] top-1/2 -translate-y-1/2 text-Cgray500 focus-within:text-Cgray700 ${type !== 'password' && 'hidden'} | |
| `} | |
| > | |
| {!isVisible ? ( | |
| <EyeOff className={`size-5 `} /> | |
| ) : ( | |
| <Eye className={`size-5 `} /> | |
| )} | |
| </button> | |
| <button | |
| onClick={() => setIsVisible((prev) => !prev)} | |
| + type="button" | |
| + aria-label={isVisible ? "비밀번호 숨기기" : "비밀번호 표시"} | |
| className={`absolute right-[16px] top-1/2 -translate-y-1/2 text-Cgray500 focus-within:text-Cgray700 ${type !== 'password' && 'hidden'}`} | |
| > | |
| {!isVisible ? ( | |
| <EyeOff className={`size-5 `} /> | |
| ) : ( | |
| <Eye className={`size-5 `} /> | |
| )} | |
| </button> |
| <input | ||
| type={ | ||
| type === 'password' ? (isVisible ? 'text' : 'password') : 'text' | ||
| } | ||
| className={cn( | ||
| inputVariants({ state, inputSize }), | ||
| !!errorMessage && | ||
| 'border border-warning text-warning caret-warning', | ||
| )} | ||
| ref={ref} | ||
| {...props} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
입력 필드의 접근성을 개선해야 합니다.
입력 필드에 aria-invalid 및 aria-describedby 속성이 누락되었습니다.
다음과 같이 수정하는 것을 제안합니다:
<input
type={type === 'password' ? (isVisible ? 'text' : 'password') : 'text'}
+ aria-invalid={state === 'error' || !!errorMessage}
+ aria-describedby={errorMessage ? `${props.id}-error` : undefined}
className={cn(
inputVariants({ state, inputSize }),
!!errorMessage && 'border border-warning text-warning caret-warning',
)}
ref={ref}
{...props}
/>그리고 에러 메시지에 ID를 추가합니다:
{errorMessage && (
- <p className={cn(errorTextVariants({ inputSize, className }))}>
+ <p id={`${props.id}-error`} className={cn(errorTextVariants({ inputSize, className }))}>
{errorMessage}
</p>
)}📝 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.
| <input | |
| type={ | |
| type === 'password' ? (isVisible ? 'text' : 'password') : 'text' | |
| } | |
| className={cn( | |
| inputVariants({ state, inputSize }), | |
| !!errorMessage && | |
| 'border border-warning text-warning caret-warning', | |
| )} | |
| ref={ref} | |
| {...props} | |
| /> | |
| <input | |
| type={ | |
| type === 'password' ? (isVisible ? 'text' : 'password') : 'text' | |
| } | |
| aria-invalid={state === 'error' || !!errorMessage} | |
| aria-describedby={errorMessage ? `${props.id}-error` : undefined} | |
| className={cn( | |
| inputVariants({ state, inputSize }), | |
| !!errorMessage && | |
| 'border border-warning text-warning caret-warning', | |
| )} | |
| ref={ref} | |
| {...props} | |
| /> | |
| {errorMessage && ( | |
| <p id={`${props.id}-error`} className={cn(errorTextVariants({ inputSize, className }))}> | |
| {errorMessage} | |
| </p> | |
| )} |
There was a problem hiding this comment.
aria 라벨 중에서 label, invalid, describedby 는 어떤 역할을 하기에 웹 접근성을 위한 속성이라는거야?
There was a problem hiding this comment.
@dbswl701 각 ARIA 속성의 역할을 설명해드리겠습니다:
-
aria-label: 시각적으로 보이지 않는 요소에 대한 설명을 제공합니다. 예를 들어, 아이콘 버튼에 "비밀번호 보기" 같은 레이블을 제공할 수 있습니다. -
aria-invalid: 입력값이 유효하지 않음을 스크린 리더에 알립니다. 에러 상태일 때 스크린 리더가 "유효하지 않은 입력"이라고 읽어줍니다. -
aria-describedby: 입력 필드와 관련된 추가 설명(예: 에러 메시지)을 연결합니다. 스크린 리더가 입력 필드에 포커스될 때 연결된 설명을 함께 읽어줍니다.
이러한 속성들은 시각 장애인이 스크린 리더를 통해 웹 페이지를 더 잘 이해하고 사용할 수 있도록 도와줍니다.
(｀･ω･´)ゞ
✏️ Learnings added
Learnt from: dbswl701
PR: MoimService/Moim-FE#15
File: src/components/ui/Input.tsx:104-115
Timestamp: 2025-02-19T02:38:34.072Z
Learning: ARIA 속성들의 주요 역할:
- aria-label: 시각적으로 보이지 않는 요소에 대한 설명 제공
- aria-invalid: 입력값의 유효성 상태를 스크린 리더에 전달
- aria-describedby: 입력 필드와 관련 설명(에러 메시지 등)을 연결하여 스크린 리더가 함께 읽도록 함
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
에러 메시지에 id를 추가하는 이유가 뭐야?
There was a problem hiding this comment.
@dbswl701 에러 메시지에 ID를 추가하는 이유는 aria-describedby 속성과 함께 작동하기 위해서입니다. 입력 필드의 aria-describedby가 에러 메시지의 ID를 참조하면, 스크린 리더는 이 두 요소가 연결되어 있다는 것을 인식합니다. 그래서 사용자가 입력 필드에 포커스할 때, 스크린 리더가 입력 필드와 함께 관련된 에러 메시지도 읽어줄 수 있게 됩니다.
✏️ Learnings added
Learnt from: dbswl701
PR: MoimService/Moim-FE#15
File: src/components/ui/Input.tsx:104-115
Timestamp: 2025-02-19T02:43:36.851Z
Learning: HTML 접근성에서 `aria-describedby`와 요소 ID의 관계:
- 에러 메시지와 같은 설명 텍스트에는 고유한 ID가 필요함
- 입력 필드의 `aria-describedby` 속성은 이 ID를 참조하여 스크린 리더가 두 요소를 연결하여 읽도록 함
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
그럼 id를 props로 꼭 받아야하는거네?
There was a problem hiding this comment.
@dbswl701 네, 접근성을 위해서는 id가 필요합니다. 하지만 컴포넌트를 더 사용하기 쉽게 만들기 위해, id가 제공되지 않았을 때는 내부적으로 고유한 ID를 생성하도록 개선할 수 있습니다. 예를 들어:
const Input = React.forwardRef<HTMLInputElement, IInputProps>(
({ className, type, state = 'default', errorMessage, inputSize = 'l', id, ...props }, ref) => {
const [isVisible, setIsVisible] = React.useState(false);
const uniqueId = React.useId(); // 내부적으로 고유한 ID 생성
const inputId = id || uniqueId;
return (
<div className={`w-full ${className}`}>
<div className="relative focus-within:text-Cgray700">
<input
id={inputId}
type={type === 'password' ? (isVisible ? 'text' : 'password') : 'text'}
aria-invalid={state === 'error' || !!errorMessage}
aria-describedby={errorMessage ? `${inputId}-error` : undefined}
// ... 나머지 코드
/>
{errorMessage && (
<p id={`${inputId}-error`} className={cn(errorTextVariants({ inputSize, className }))}>
{errorMessage}
</p>
)}
</div>
</div>
);
}
);✏️ Learnings added
Learnt from: dbswl701
PR: MoimService/Moim-FE#15
File: src/components/ui/Input.tsx:104-115
Timestamp: 2025-02-19T02:46:56.222Z
Learning: Input 컴포넌트에서 접근성을 위한 ID 처리:
- 에러 메시지와 입력 필드를 연결하기 위해 ID가 필요함
- ID가 제공되지 않은 경우 React.useId()를 사용하여 내부적으로 고유한 ID를 생성할 수 있음
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| const menu = [ | ||
| { label: '내 모임', onSelect: () => router.push('/my-meeting') }, | ||
| { label: '마이페이지', onSelect: () => router.push('/my-page') }, | ||
| { label: '로그아웃', onSelect: () => console.log('로그아웃') }, |
There was a problem hiding this comment.
콘솔 로그 제거 필요
프로덕션 환경에서 콘솔 로그를 사용하는 것은 바람직하지 않습니다. 로그아웃 기능 구현 시 실제 로그아웃 로직으로 대체해야 합니다.
다음과 같이 수정하는 것을 제안합니다:
- { label: '로그아웃', onSelect: () => console.log('로그아웃') },
+ { label: '로그아웃', onSelect: () => handleLogout() },- onClick={() => console.log('로그아웃')}
+ onClick={handleLogout}Also applies to: 69-69
🧰 Tools
🪛 GitHub Check: check
[warning] 30-30:
Unexpected console statement
There was a problem hiding this comment.
임시로 작성한 내용. 추구 제거 예정
There was a problem hiding this comment.
@dbswl701 알겠습니다. 추후 로그아웃 기능 구현 시 제거하시면 되겠습니다.
✏️ Learnings added
Learnt from: dbswl701
PR: MoimService/Moim-FE#15
File: src/components/common/Header.tsx:0-0
Timestamp: 2025-02-19T02:47:47.477Z
Learning: The console.log statements in Header.tsx for logout functionality are temporary placeholders that will be replaced with actual logout logic in a future update.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| <div className="flex items-center"> | ||
| <Dropdown items={menu} trigger={<Profile />} /> | ||
| <span className="typo-head3 m-auto w-[77px] text-center text-white"> | ||
| 김밤식 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
하드코딩된 사용자 이름
사용자 이름이 하드코딩되어 있습니다. 실제 사용자 정보를 props로 전달받아 사용하도록 수정해야 합니다.
다음과 같이 수정하는 것을 제안합니다:
- <span className="typo-head3 m-auto w-[77px] text-center text-white">
- 김밤식
- </span>
+ <span className="typo-head3 m-auto w-[77px] text-center text-white">
+ {userName}
+ </span>Also applies to: 76-76
| <Logo /> | ||
| </Link> | ||
| <NavLinks /> | ||
| {isLogIn ? <BeforeLogin /> : <AfterLogin />} |
There was a problem hiding this comment.
로그인 상태 렌더링 로직 오류
isLogIn prop의 사용이 반대로 되어 있습니다. 현재 로직은 로그인된 상태에서 BeforeLogin 컴포넌트를, 로그아웃된 상태에서 AfterLogin 컴포넌트를 보여주고 있습니다.
다음과 같이 수정하는 것을 제안합니다:
- {isLogIn ? <BeforeLogin /> : <AfterLogin />}
+ {isLogIn ? <AfterLogin /> : <BeforeLogin />}📝 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.
| {isLogIn ? <BeforeLogin /> : <AfterLogin />} | |
| {isLogIn ? <AfterLogin /> : <BeforeLogin />} |
dbswl701
force-pushed
the
feat/component/input/DEVING-34
branch
from
2a76416 to
344177e
Compare
CI Status Report검사 결과
✅ 모든 검사가 통과되었습니다. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/components/ui/Input.tsx (1)
104-126:⚠️ Potential issue접근성 개선이 여전히 필요합니다.
입력 필드와 비밀번호 표시/숨기기 버튼에 대한 접근성 속성이 누락되어 있습니다.
이전 리뷰에서 제안된 다음과 같은 수정사항을 적용해주세요:
- 입력 필드에
aria-invalid및aria-describedby속성 추가- 비밀번호 표시/숨기기 버튼에
type="button"및aria-label속성 추가
🧹 Nitpick comments (1)
src/components/ui/Input.tsx (1)
8-28: 타입 안전성 개선이 필요합니다.
state와inputSize속성에 대해 리터럴 유니온 타입을 별도로 정의하면 코드의 재사용성과 유지보수성이 향상될 것 같습니다.다음과 같이 수정하는 것을 제안합니다:
+type InputState = 'default' | 'success' | 'error'; +type InputSize = 'l' | 's'; + interface IInputProps extends React.InputHTMLAttributes<HTMLInputElement>, VariantProps<typeof inputVariants> { /** * 입력 필드의 상태 (기본값: `'default'`) * - `'default'` : 기본 상태 (테두리 없음) * - `'success'` : 성공 상태 (메인 컬러 테두리 적용) * - `'error'` : 에러 상태 (경고 컬러 테두리 및 텍스트 적용) */ - state?: 'default' | 'success' | 'error'; + state?: InputState; /** * 입력 필드의 크기 (기본값: `'l'`) * - `'s'` : 작은 크기 (36px) * - `'l'` : 기본 크기 (50px) */ - inputSize?: 'l' | 's'; + inputSize?: InputSize; /** * 에러 메시지 (에러 상태일 때 표시) */ errorMessage?: string; }
| const errorTextVariants = cva('px-[10px] text-warning', { | ||
| variants: { | ||
| inputSize: { | ||
| s: 'typo-caption2 mt-[8px]', | ||
| l: 'typo-caption1 mt-[10px]', | ||
| }, | ||
| defaultVariants: { | ||
| inputSize: 'l', | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
오류 텍스트 스타일의 기본값 설정에 버그가 있습니다.
errorTextVariants의 defaultVariants 설정이 잘못된 위치에 있습니다. 현재는 variants 객체 내부에 있어 작동하지 않습니다.
다음과 같이 수정하는 것을 제안합니다:
const errorTextVariants = cva('px-[10px] text-warning', {
variants: {
inputSize: {
s: 'typo-caption2 mt-[8px]',
l: 'typo-caption1 mt-[10px]',
},
- defaultVariants: {
- inputSize: 'l',
- },
},
+ defaultVariants: {
+ inputSize: 'l',
+ },
});📝 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.
| const errorTextVariants = cva('px-[10px] text-warning', { | |
| variants: { | |
| inputSize: { | |
| s: 'typo-caption2 mt-[8px]', | |
| l: 'typo-caption1 mt-[10px]', | |
| }, | |
| defaultVariants: { | |
| inputSize: 'l', | |
| }, | |
| }, | |
| }); | |
| const errorTextVariants = cva('px-[10px] text-warning', { | |
| variants: { | |
| inputSize: { | |
| s: 'typo-caption2 mt-[8px]', | |
| l: 'typo-caption1 mt-[10px]', | |
| }, | |
| }, | |
| defaultVariants: { | |
| inputSize: 'l', | |
| }, | |
| }); |
| className={cn( | ||
| inputVariants({ state, inputSize }), | ||
| !!errorMessage && | ||
| 'border border-warning text-warning caret-warning', | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 상태 처리 로직이 중복됩니다.
state가 'error'일 때와 errorMessage가 있을 때 모두 동일한 에러 스타일이 적용되고 있습니다. 이는 일관성 없는 상태를 초래할 수 있습니다.
다음과 같이 수정하는 것을 제안합니다:
className={cn(
- inputVariants({ state, inputSize }),
- !!errorMessage &&
- 'border border-warning text-warning caret-warning',
+ inputVariants({
+ state: !!errorMessage ? 'error' : state,
+ inputSize
+ }),
)}📝 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.
| className={cn( | |
| inputVariants({ state, inputSize }), | |
| !!errorMessage && | |
| 'border border-warning text-warning caret-warning', | |
| )} | |
| className={cn( | |
| inputVariants({ | |
| state: !!errorMessage ? 'error' : state, | |
| inputSize | |
| }), | |
| )} |
CI Status Report검사 결과
✅ 모든 검사가 통과되었습니다. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/components/ui/Input.tsx (1)
104-116:⚠️ Potential issueARIA 속성 설정이 잘못되었습니다.
aria-describedby가 입력 필드 대신 버튼에 설정되어 있습니다.- 에러 상태 처리가 중복됩니다.
<input type={type === 'password' ? (isVisible ? 'text' : 'password') : 'text'} aria-invalid={!!errorMessage} + aria-describedby={errorMessage ? `${id}-error` : undefined} className={cn( - inputVariants({ state, inputSize }), - !!errorMessage && 'border border-warning text-warning caret-warning', + inputVariants({ + state: errorMessage ? 'error' : state, + inputSize + }), )} ref={ref} {...props} />
🧹 Nitpick comments (1)
src/components/ui/Input.tsx (1)
8-27: 인터페이스 정의를 개선해야 합니다.다음과 같은 개선사항을 제안합니다:
state타입에 'error' 상태가 누락되었습니다 (예제에서 사용됨).inputSize대신size를 사용하면 더 간결할 것 같습니다.- state?: 'default' | 'success'; + state?: 'default' | 'success' | 'error'; - inputSize?: 'l' | 's'; + size?: 'l' | 's';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/ui/Input.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/components/ui/Input.tsx (3)
Learnt from: dbswl701
PR: MoimService/Moim-FE#15
File: src/components/ui/Input.tsx:104-115
Timestamp: 2025-02-19T02:46:56.460Z
Learning: Input 컴포넌트에서 접근성을 위한 ID 처리:
- 에러 메시지와 입력 필드를 연결하기 위해 ID가 필요함
- ID가 제공되지 않은 경우 React.useId()를 사용하여 내부적으로 고유한 ID를 생성할 수 있음
Learnt from: dbswl701
PR: MoimService/Moim-FE#15
File: src/components/ui/Input.tsx:104-115
Timestamp: 2025-02-19T02:43:36.967Z
Learning: HTML 접근성에서 `aria-describedby`와 요소 ID의 관계:
- 에러 메시지와 같은 설명 텍스트에는 고유한 ID가 필요함
- 입력 필드의 `aria-describedby` 속성은 이 ID를 참조하여 스크린 리더가 두 요소를 연결하여 읽도록 함
Learnt from: dbswl701
PR: MoimService/Moim-FE#15
File: src/components/ui/Input.tsx:104-115
Timestamp: 2025-02-19T02:38:34.203Z
Learning: ARIA 속성들의 주요 역할:
- aria-label: 시각적으로 보이지 않는 요소에 대한 설명 제공
- aria-invalid: 입력값의 유효성 상태를 스크린 리더에 전달
- aria-describedby: 입력 필드와 관련 설명(에러 메시지 등)을 연결하여 스크린 리더가 함께 읽도록 함
🔇 Additional comments (1)
src/components/ui/Input.tsx (1)
48-58: 오류 텍스트 스타일의 기본값 설정에 버그가 있습니다.
errorTextVariants의defaultVariants설정이 잘못된 위치에 있어 작동하지 않습니다.
| <button | ||
| onClick={() => setIsVisible((prev) => !prev)} | ||
| type="button" | ||
| aria-label={isVisible ? '비밀번호 숨기기' : '비밀번호 표시'} | ||
| aria-describedby={errorMessage ? `${id}-error` : undefined} | ||
| className={`absolute right-[16px] top-1/2 -translate-y-1/2 text-Cgray500 focus-within:text-Cgray700 ${type !== 'password' && 'hidden'} | ||
| `} | ||
| > | ||
| {!isVisible ? ( | ||
| <EyeOff className={`size-5 `} /> | ||
| ) : ( | ||
| <Eye className={`size-5 `} /> | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
비밀번호 토글 버튼의 ARIA 속성을 수정해야 합니다.
aria-describedby가 버튼에서 제거되어야 합니다.
<button
onClick={() => setIsVisible((prev) => !prev)}
type="button"
aria-label={isVisible ? '비밀번호 숨기기' : '비밀번호 표시'}
- aria-describedby={errorMessage ? `${id}-error` : undefined}
className={`absolute right-[16px] top-1/2 -translate-y-1/2 text-Cgray500 focus-within:text-Cgray700 ${
type !== 'password' && 'hidden'
}`}
>📝 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.
| <button | |
| onClick={() => setIsVisible((prev) => !prev)} | |
| type="button" | |
| aria-label={isVisible ? '비밀번호 숨기기' : '비밀번호 표시'} | |
| aria-describedby={errorMessage ? `${id}-error` : undefined} | |
| className={`absolute right-[16px] top-1/2 -translate-y-1/2 text-Cgray500 focus-within:text-Cgray700 ${type !== 'password' && 'hidden'} | |
| `} | |
| > | |
| {!isVisible ? ( | |
| <EyeOff className={`size-5 `} /> | |
| ) : ( | |
| <Eye className={`size-5 `} /> | |
| )} | |
| </button> | |
| <button | |
| onClick={() => setIsVisible((prev) => !prev)} | |
| type="button" | |
| aria-label={isVisible ? '비밀번호 숨기기' : '비밀번호 표시'} | |
| className={`absolute right-[16px] top-1/2 -translate-y-1/2 text-Cgray500 focus-within:text-Cgray700 ${type !== 'password' && 'hidden'} | |
| `} | |
| > | |
| {!isVisible ? ( | |
| <EyeOff className={`size-5 `} /> | |
| ) : ( | |
| <Eye className={`size-5 `} /> | |
| )} | |
| </button> |
| const Input = React.forwardRef<HTMLInputElement, IInputProps>( | ||
| ( | ||
| { | ||
| className, | ||
| type, | ||
| state = 'default', | ||
| errorMessage, | ||
| inputSize = 'l', | ||
| id, | ||
| ...props | ||
| }, | ||
| ref, | ||
| ) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성을 위한 ID 처리를 개선해야 합니다.
id가 필수적으로 필요하지만 제공되지 않았을 때의 처리가 누락되었습니다.
const Input = React.forwardRef<HTMLInputElement, IInputProps>(
(
{
className,
type,
state = 'default',
errorMessage,
inputSize = 'l',
- id,
+ id: providedId,
...props
},
ref,
) => {
+ const uniqueId = React.useId();
+ const id = providedId || uniqueId;📝 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.
| const Input = React.forwardRef<HTMLInputElement, IInputProps>( | |
| ( | |
| { | |
| className, | |
| type, | |
| state = 'default', | |
| errorMessage, | |
| inputSize = 'l', | |
| id, | |
| ...props | |
| }, | |
| ref, | |
| ) => { | |
| const Input = React.forwardRef<HTMLInputElement, IInputProps>( | |
| ( | |
| { | |
| className, | |
| type, | |
| state = 'default', | |
| errorMessage, | |
| inputSize = 'l', | |
| id: providedId, | |
| ...props | |
| }, | |
| ref, | |
| ) => { | |
| const uniqueId = React.useId(); | |
| const id = providedId || uniqueId; | |
| // ... rest of the component implementation |
📝 주요 작업 내용
📺 스크린샷
🔗 참고 사항
💬 리뷰 요구사항
인풋 컴포넌트를 가져다 쓰는 곳에서의 사용성이 어떨지 고려해서 작성하였는데, 제가 생각치 못한 부분이 있다면 말씀 부탁드립니다.
📃 관련 이슈
DEVING-34
Summary by CodeRabbit