[2단계 - 페이먼츠 모듈] 수이(김수연) 미션 제출합니다. #140
Conversation
Summary by CodeRabbit
Walkthrough이번 변경에서는 모달, 버튼, 입력창 등 UI 컴포넌트와 카드 번호 입력 관련 훅 및 유틸리티가 새롭게 추가 및 개선되었습니다. Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
🧹 Nitpick comments (26)
.github/pull_request_template.md (1)
51-52: 모듈 배포 체크포인트 추가 제안
현재 ‘npm 배포를 위한 설정’만으로도 충분하지만, 실제 설치와 동작 확인을 위한 체크리스트(예:npm install,import후 번들링/타입 검증 등) 항목을 부가하면 배포 신뢰도를 더욱 높일 수 있습니다.hooks/vite.config.ts (1)
25-31: TypeScript 선언 파일 생성을 위한 dts 플러그인 설정 개선dts 플러그인에
include및tsconfigPath옵션을 명시적으로 설정한 것은 좋은 접근입니다. 이를 통해 타입 선언 파일 생성 범위를 정확하게 지정하고 올바른 타입스크립트 설정 파일을 참조할 수 있습니다. 향후 패키지 사용자에게 더 정확한 타입 정보를 제공할 수 있을 것입니다.다만, 프로젝트 내 다른 패키지(components)와 설정 일관성을 유지하기 위해, 빌드 구성에서 entry 경로와 dts include 경로가 일치하는지 확인하는 것이 좋습니다. 현재 entry는
"src/lib/index.ts"이고 include는["src"]로 되어 있어 불필요한 타입 선언이 생성될 수 있습니다.hooks/package.json (1)
2-3: 패키지 이름과 버전 업데이트 확인"supe-hooks"라는 패키지 이름과 버전 0.0.5로 업데이트한 것은 npm에 배포하기 위한 좋은 준비입니다. 다만, 패키지 버전 관리 전략을 명확히 하는 것이 좋습니다. 언제 마이너/패치 버전을 증가시킬지에 대한 규칙을 README에 명시하면 좋을 것 같습니다.
components/src/App.tsx (2)
6-9: 모달 상태 관리 로직 구현useState를 사용하여 모달의 열림/닫힘 상태를 관리하고, handleClose 함수를 통해 모달을 닫는 로직이 잘 구현되어 있습니다. 이는 모달 컴포넌트의 기본 사용법을 보여주는 좋은 예시입니다.
다만 이 App 컴포넌트가 단순히 모달 사용 예시를 보여주는 것이 목적이라면, 처음부터 모달을 열어두는 것보다 버튼을 클릭하여 모달을 열 수 있는 상호작용 요소를 추가하는 것이 사용자 경험 측면에서 더 자연스러울 수 있습니다.
14-14: 모달 컴포넌트 사용Modal 컴포넌트를 적절히 사용하고 있으며, 필요한 props(isOpen, onClose, title)를 전달하고 있습니다. 이를 통해 모달의 기본 기능을 잘 보여주고 있습니다.
다만, PR 목표에서 언급된 다양한 모달 유형(alert, confirm, prompt)과 위치 조정 기능을 시연하기 위해 추가 props를 활용하는 예시를 더 포함하면 좋을 것 같습니다. 또한 모달 내용(children)이 없는 상태인데, 실제 사용 예시에서는 의미 있는 내용을 포함하는 것이 더 도움이 될 것입니다.
components/src/components/common/CloseIcon.tsx (1)
1-20: SVG 컴포넌트 구현이 잘 되었습니다.CloseIcon 컴포넌트가 React와 TypeScript의 모범 사례를 따르고 있습니다:
- props를 통한 확장성 확보
- 타입 정의의 적절한 사용
- SVG 마크업의 깔끔한 구현
한 가지 고려할 점은 크기 및 색상을 props로 받아 더 유연하게 사용할 수 있도록 구현하는 것입니다.
색상과 크기를 props로 받아 더 유연한 컴포넌트로 만들 수 있습니다:
export default function CloseIcon( - props: React.HTMLAttributes<HTMLOrSVGElement> + props: React.HTMLAttributes<HTMLOrSVGElement> & { + width?: number; + height?: number; + fill?: string; + } ) { + const { width = 15, height = 14, fill = "black", ...restProps } = props; return ( <svg - width="15" - height="14" viewBox="0 0 15 14" fill="none" xmlns="http://www.w3.org/2000/svg" style={{ marginLeft: "auto" }} - {...props} + width={width} + height={height} + {...restProps} > <path d="M14.8167 1.41L13.4067 0L7.81665 5.59L2.22665 0L0.81665 1.41L6.40665 7L0.81665 12.59L2.22665 14L7.81665 8.41L13.4067 14L14.8167 12.59L9.22665 7L14.8167 1.41Z" - fill="black" + fill={fill} /> </svg> ); }components/tsconfig.node.json (1)
21-28: TypeScript 선언 파일 생성 설정이 적절히 추가되었습니다.타입 선언 파일 생성과 관련된 설정이 적절하게 추가되었습니다. 이는 라이브러리 사용자에게 타입 정보를 제공하여 개발 경험을 향상시키는 중요한 설정입니다.
다만, 설정 간의 관계가 더 명확하게 주석으로 설명되면 좋을 것 같습니다.
다음과 같이 주석을 추가하여 설정 의도를 더 명확히 할 수 있습니다:
"noUnusedParameters": true, "noFallthroughCasesInSwitch": true, "noUncheckedSideEffectImports": true, + /* Type Declaration Generation */ "allowJs": true, "declaration": true, "declarationDir": "dist", "emitDeclarationOnly": true, "outDir": "dist", "declarationMap": true🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 21-21: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 24-24: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 24-24: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 24-24: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 24-24: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 25-25: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 25-25: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 25-25: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 25-25: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 26-26: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 26-26: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 26-26: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 26-26: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 27-27: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 27-27: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 27-27: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 27-27: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 28-28: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 28-28: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 28-28: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
hooks/src/lib/useCvcNumber.ts (1)
1-38: 카드 검증 로직 구현이 잘 되어 있습니다CVC 번호 검증을 위한 커스텀 훅이 명확하게 구현되어 있습니다. 정규식을 사용한 입력 검증과 조건부 오류 메시지 처리가 깔끔합니다.
한 가지 제안: 현재 CVC가 3자리 미만일 때만 오류를 표시하는데, 일부 카드(AMEX)는 4자리 CVC를 사용합니다. 카드 타입에 따라 유효성 검사 로직을 조정하는 것이 좋을 것 같습니다.
import { useState } from "react"; const numberRegex = /^[0-9]*$/; interface CustomErrorMessagesType { length?: string; } + interface UseCvcNumberProps { + customErrorMessage?: CustomErrorMessagesType; + isAmex?: boolean; + } export default function useCvcNumber( - customErrorMessage?: CustomErrorMessagesType + { customErrorMessage, isAmex = false }: UseCvcNumberProps = {} ) { const [cvcNumber, setCvcNumber] = useState(""); const [errorMessage, setErrorMessage] = useState(""); const isValid = !errorMessage; const handleCvcNumberChange = ( event: React.ChangeEvent<HTMLInputElement> ) => { const value = event.target.value; if (!numberRegex.test(value)) return; setCvcNumber(value); + const requiredLength = isAmex ? 4 : 3; + const errorMsg = isAmex + ? customErrorMessage?.length ?? "4자리 숫자를 입력해 주세요." + : customErrorMessage?.length ?? "3자리 숫자를 입력해 주세요."; - if (value.length < 3) { + if (value.length < requiredLength) { setErrorMessage( - customErrorMessage?.length ?? "3자리 숫자를 입력해 주세요." + errorMsg ); return; } setErrorMessage(""); }; return { errorMessage, isValid, cvcNumber, handleCvcNumberChange }; }README.md (1)
2-27: 문서 구조 개선이 필요합니다기능 목록과 설명이 잘 정리되어 있지만, 마크다운 문법에 따르면 헤딩 레벨이 순차적으로 증가해야 합니다.
h1다음에 바로h3를 사용하는 것보다h2를 먼저 사용하는 것이 권장됩니다.또한, 사용 예시나 설치 방법 등 더 자세한 정보를 추가하면 사용자에게 도움이 될 것 같습니다.
# react-modules -### Modal +## 구성 요소 + +### Modal - [x] size(small, medium, large) - [x] Button 컴포넌트 - [x] 확인 - [x] 취소 - [x] Input 컴포넌트 - [x] 반응형 - [x] 포커스 제어 -### hook +## 훅(hooks) + +### 카드 관련 훅 - [x] 카드사 식별 - [x] 카드 번호 포맷팅 - [x] MasterCard: 51~55로 시작하는 16자리 숫자 예시 5111 1111 1111 1111 - [x] Diners: 36으로 시작하는 14자리 숫자 예시 3612 345678 9012 - [x] AMEX: 34, 37로 시작하는 15자리 숫자 예시 (34로 시작): 3412 345678 90123 예시 (37로 시작): 3712 345678 90123 - [x] 유니온페이: 카드의 앞 번호가 아래 3가지 조건을 만족하는 16자리 숫자 예시 622126~622925로 시작하는 경우: 6221 2612 3456 7890 예시 624~626로 시작하는 경우: 6240 1234 5678 9012 예시 6282~6288로 시작하는 경우: 6282 1234 5678 9012 + +## 설치 방법 + +```bash +npm install supe-modal supe-hooks +```🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3(MD001, heading-increment)
components/.storybook/preview.ts (1)
2-2: 스토리북 설정 개선이 잘 이루어졌습니다.뷰포트와 배경색 설정을 추가하여 다양한 환경에서 컴포넌트를 테스트할 수 있도록 개선했습니다. 반응형 테스트와 컴포넌트 시각화에 큰 도움이 될 것입니다.
주석 처리된
defaultViewport설정은 사용하지 않을 경우 제거하는 것이 더 깔끔할 것 같습니다:viewport: { viewports: INITIAL_VIEWPORTS, - // defaultViewport: "iphone12", },Also applies to: 12-23
components/src/components/common/Input/Input.tsx (1)
8-26: 컴포넌트 기능 확장 고려Input 컴포넌트가 기본적인 기능만 제공하고 있습니다. 재사용성을 높이고 실제 사용 사례를 더 잘 지원하기 위해 기능을 확장하는 것을 고려해보세요.
다음과 같은 기능을 추가하면 유용할 것 같습니다:
- 오류 상태 표시 (에러 메시지, 테두리 색상 등)
- 레이블 통합
- 접두사/접미사 아이콘 지원
- 포커스, 호버 상태 스타일링
interface InputInterface extends React.InputHTMLAttributes<HTMLInputElement> { height?: string; error?: string; label?: string; prefix?: React.ReactNode; suffix?: React.ReactNode; } export default function Input({ height, value, onChange, error, label, prefix, suffix, ...rest }: InputInterface) { return ( <InputWrapper> {label && <Label>{label}</Label>} <InputContainer> {prefix && <PrefixContainer>{prefix}</PrefixContainer>} <StyledInput height={height} value={value} onChange={onChange} hasError={!!error} {...rest} /> {suffix && <SuffixContainer>{suffix}</SuffixContainer>} </InputContainer> {error && <ErrorMessage>{error}</ErrorMessage>} </InputWrapper> ); }이런 방식으로 확장하면 다양한 UI 요구사항에 더 잘 대응할 수 있습니다.
hooks/src/lib/useCvcNumber.test.ts (1)
19-57: 입력 처리 테스트 관련 작은 개선점입력 처리에 대한 테스트가 잘 작성되어 있지만, 테스트 케이스를 추가하면 더 완벽할 것 같습니다.
3자리 이상의 숫자 입력에 대한 테스트 케이스를 추가하는 것이 좋겠습니다. 현재 구현에서는 3자리 이상도 허용하는지, 아니면 3자리 정확히만 허용하는지 명확하지 않습니다.
test("3자리 이상의 숫자를 입력하면 어떻게 처리되는지 확인한다.", () => { const { result } = renderHook(() => useCvcNumber()); const { handleCvcNumberChange } = result.current; const event = { target: { value: "1234", }, }; act(() => handleCvcNumberChange( event as unknown as React.ChangeEvent<HTMLInputElement> ) ); // 여기서 예상되는 동작에 따라 assertion을 작성 // 만약 3자리만 허용한다면: // expect(result.current.cvcNumber).toBe("123"); // 또는 에러를 표시해야 한다면: // expect(result.current.isValid).toBeFalsy(); });hooks/src/lib/usePassword.test.ts (1)
19-57: 반복되는 테스트 코드 개선 가능입력 처리 테스트가 잘 작성되어 있으나,
useCvcNumber.test.ts와 상당히 유사한 구조로 중복 코드가 많습니다.여러 훅 테스트 파일에서 반복되는 테스트 패턴을 재사용 가능한 테스트 유틸리티 함수로 추출하는 것을 고려해보세요. 예를 들어:
// hooks/src/test/utils.ts export function testInputChange<T extends Record<string, any>>({ hook, value, handler, stateKey, expectedValue, }: { hook: () => T; value: string; handler: keyof T; stateKey: keyof T; expectedValue: string; }) { const { result } = renderHook(() => hook()); const event = { target: { value }, }; act(() => result.current[handler]( event as unknown as React.ChangeEvent<HTMLInputElement> ) ); expect(result.current[stateKey]).toBe(expectedValue); } // 사용 예시 test("2자리 숫자를 입력하면 반영된다.", () => { testInputChange({ hook: usePassword, value: "11", handler: "handlePasswordChange", stateKey: "password", expectedValue: "11" }); });이런 패턴을 사용하면 코드 중복을 줄이고 테스트 로직을 더 명확하게 표현할 수 있습니다.
hooks/src/lib/useExpiryDate.test.ts (1)
1-238: 테스트 코드가 체계적으로 구성되어 있습니다.이 테스트 코드는
useExpiryDate훅의 초기 상태, 입력 처리, 유효성 검증을 체계적으로 검증하고 있습니다. 특히 경계값 테스트(1~12 범위의 월, 현재/과거/미래 년도)와 오류 조건이 잘 커버되어 있습니다.한 가지 제안하자면, 테스트 데이터 생성 로직을 분리하여 테스트 가독성을 높이는 것이 좋겠습니다. 예를 들어, 반복되는 event 객체 생성을 테스트 헬퍼 함수로 추출할 수 있습니다:
function createInputEvent(value: string | number) { return { target: { value } } as unknown as React.ChangeEvent<HTMLInputElement>; }이렇게 하면 테스트 코드가 더 간결해지고 의도가 명확해집니다.
hooks/src/lib/usePassword.ts (1)
18-33: 유효성 검사 로직을 최적화할 수 있습니다.현재 코드에서는 비밀번호 길이 검증을 위해 조건문에서 return을 사용하고 있습니다. 이 방식도 작동하지만, 조건문 구조를 단순화하고 가독성을 높일 수 있는 방법이 있습니다.
아래와 같이 리팩토링하면 코드의 흐름이 더 명확해집니다:
const handlePasswordChange = (event: React.ChangeEvent<HTMLInputElement>) => { const value = event.target.value; if (!numberRegex.test(value)) return; setPassword(value); if (value.length < 2) { setErrorMessage( customErrorMessage?.length ?? "2자리 숫자를 입력해 주세요." ); } else { setErrorMessage(""); } };이렇게 early return 패턴과 if/else 구조를 함께 사용하면 코드의 의도가 더 명확하게 전달됩니다.
hooks/src/lib/useCardNumber.ts (2)
33-42: 에러 메시지 상수화 및 코드 분리 권장에러 메시지 로직이 다소 복잡하고, 문자열이 코드에 직접 포함되어 있습니다. 에러 메시지를 상수로 분리하고 조건부 로직을 단순화하면 가독성과 유지보수성이 향상됩니다.
+ const DEFAULT_LENGTH_ERROR_MESSAGE = (length: number) => `${length}자리를 입력해 주세요.`; const validateCardNumberLength = ( value: string, cardNumberLength: number | null ) => { if (isExceedCardNumberLength(value, cardNumberLength)) return; setCardNumber(value); if (isShortOfCardNumberLength(value, cardNumberLength)) { - setErrorMessage( - customErrorMessage - ? `${cardNumberLength}${customErrorMessage.length}` - : `${cardNumberLength}자리를 입력해 주세요.` - ); + const errorMsg = customErrorMessage?.length + ? `${cardNumberLength}${customErrorMessage.length}` + : DEFAULT_LENGTH_ERROR_MESSAGE(cardNumberLength!); + setErrorMessage(errorMsg); return; } setErrorMessage(""); };
57-65: 브랜드 및 포맷 로직의 메모이제이션 권장
cardBrand와formattedCardNumber함수는 렌더링마다 새로 계산됩니다. 이 값들을 useMemo로 메모이제이션하면 불필요한 계산을 방지할 수 있습니다.+ import { useState, useMemo } from "react"; // ... - const cardBrand = (): CardBrandType => { - return getCardBrand(cardNumber); - }; + const cardBrand = useMemo<() => CardBrandType>( + () => () => getCardBrand(cardNumber), + [cardNumber] + ); - const formattedCardNumber = () => { - const brand = cardBrand(); - const pattern = CARD_BRAND[brand].format; - return formatCardNumber(cardNumber, brand, pattern); - }; + const formattedCardNumber = useMemo( + () => { + const brand = cardBrand(); + const pattern = CARD_BRAND[brand].format; + return formatCardNumber(cardNumber, brand, pattern); + }, + [cardNumber, cardBrand] + );components/src/components/Modal/Modal.tsx (3)
115-115: 조건부 반환 시 명시적 null 반환 권장React 컴포넌트에서 조건부로 렌더링하지 않을 때는
undefined가 아닌null을 명시적으로 반환하는 것이 좋습니다.- if (!isOpen) return; + if (!isOpen) return null;
85-98: 인터페이스 속성 그룹화 및 필수 속성 분리 권장인터페이스의 속성들이 많아 가독성이 떨어집니다. 관련 속성을 그룹화하고 필수 속성과 선택적 속성을 명확히 구분하면 가독성이 향상됩니다.
interface ModalInterface { // 필수 속성 onClose: () => void; isOpen: boolean; size: "small" | "medium" | "large"; // 레이아웃 관련 속성 position?: "center" | "bottom"; // 버튼 관련 속성 closeButton?: boolean; confirmButton?: boolean; cancelButton?: boolean; onConfirm?: () => void; // 콘텐츠 관련 속성 title?: string; // 입력 필드 관련 속성 input?: boolean; inputValue?: string; onInputChange?: (e: React.ChangeEvent<HTMLInputElement>) => void; }
100-114: prop 수가 많을 때 컴포넌트 분리 또는 패턴 적용 고려현재 Modal 컴포넌트는 12개의 props를 받고 있어 복잡성이 높습니다. 컴포넌트 합성 패턴(Compound Component)이나 Context API를 활용하여 Modal 구현을 다음과 같이 재구성할 수 있습니다:
// 예시: 컴포넌트 합성 패턴 적용 <Modal isOpen={isOpen} onClose={onClose} position="center" size="medium"> <Modal.Header title="제목" showCloseButton /> <Modal.Content> {input && <Input value={inputValue} onChange={onInputChange} />} {children} </Modal.Content> <Modal.Footer showConfirm showCancel onConfirm={onConfirm} /> </Modal>이 패턴은 props 수를 줄이고 컴포넌트 구조를 더 명확하게 만들어줍니다. 코드 복잡성이 증가하는 단점이 있지만, 향후 확장성과 유지보수성이 크게 향상됩니다.
hooks/src/lib/useCardNumber.test.ts (1)
5-16: 테스트 헬퍼 함수 개선 권장
changeCardNumber함수가 제네릭하지 않고 특정 훅에 종속되어 있습니다. 더 범용적인 헬퍼 함수로 만들어 다른 입력 관련 훅 테스트에서도 재사용할 수 있게 해보세요.제네릭 타입을 사용하여 다양한 훅 타입에 적용 가능한 헬퍼 함수로 개선:
function changeInputValue<T extends { handleCardNumberChange?: (e: any) => void }>( result: { current: T }, rerender: () => void, value: string, handlerName: keyof T & string = 'handleCardNumberChange' ) { const handler = result.current[handlerName]; if (typeof handler === 'function') { act(() => { handler({ target: { value }, } as React.ChangeEvent<HTMLInputElement>); }); rerender(); } }이렇게 하면 다른 입력 관련 훅 테스트에서도 해당 함수를 재사용할 수 있습니다.
components/src/components/common/Button/Button.tsx (2)
56-56: 빈 텍스트에 대한 처리 보완 필요현재 구현에서는 variant가 없고 text가 undefined인 경우 빈 문자열("")이 표시됩니다. 이는 사용자에게 혼란을 줄 수 있으므로, 기본 텍스트를 제공하거나 경고를 표시하는 것이 좋겠습니다.
- const buttonText = variant ? textMap[variant] : text ?? ""; + const buttonText = variant ? textMap[variant] : text ?? "버튼";또는 개발 모드에서만 경고를 표시하는 방법도 고려해볼 수 있습니다:
const buttonText = variant ? textMap[variant] : text ?? ""; + // 개발 모드에서만 경고 표시 + if (process.env.NODE_ENV !== 'production' && !buttonText) { + console.warn('Button component: No text provided for button'); + }이렇게 하면 버튼에 항상 텍스트가 표시되어 사용자 경험이 향상되고, 개발자가 텍스트를 잊어버린 경우에도 알림을 받을 수 있습니다.
28-34: 호버/액티브 상태 스타일 개선 필요현재 모든 버튼 variants에 대해 동일한 hover/active 스타일이 적용되고 있습니다. 이로 인해 "confirm" 버튼에서는 검은색 배경에서 회색으로 변경되어 시각적으로 부자연스러울 수 있습니다. variant에 따라 다른 호버/액티브 스타일을 적용하는 것이 좋겠습니다.
- &:hover { - background-color: lightgray; - } - - &:active { - background-color: gray; - } + ${({ variant }) => + variant === "confirm" + ? css` + &:hover { + background-color: #333; + } + &:active { + background-color: #555; + } + ` + : css` + &:hover { + background-color: #f5f5f5; + } + &:active { + background-color: #e0e0e0; + } + `}이렇게 하면 각 버튼 variant에 맞는 자연스러운 상태 변화를 제공하여 사용자 경험을 향상시킬 수 있습니다.
hooks/src/�utils/card.ts (3)
15-34: 카드 번호 포맷팅 함수의 견고성 개선포맷팅 함수가 잘 구현되어 있지만, 몇 가지 개선 사항이 있습니다:
pattern: number[] | never[]의 타입 정의가 다소 특이합니다. 일반적으로는number[]로만 정의하는 것이 충분합니다.never[]는 특별한 용도가 있는지 검토해보세요.입력 문자열이 패턴보다 짧을 경우에 대한 처리가 없습니다. 이는 불완전한 결과를 반환할 수 있습니다.
더 견고한 구현을 위해 다음과 같이 수정해 보세요:
export const formatCardNumber = ( cardNumber: string, - brand: CardBrandType, - pattern: number[] | never[] + brand: CardBrandType, + pattern: number[] ): string => { let start = 0; if (brand === "none") { return cardNumber; } + // 입력된 카드 번호의 길이가 충분한지 확인 + const totalLength = pattern.reduce((sum, len) => sum + len, 0); + if (cardNumber.length < totalLength) { + // 길이가 충분하지 않은 경우 처리 (옵션) + } return pattern .map((len) => { const number = cardNumber.slice(start, start + len); start += len; return number; }) .join(" ") .trim(); };
36-50: 검증 함수의 명칭 및 재사용성 개선카드 번호 길이 검증 함수들의 이름이 다소 길고 특정 용도에 한정되어 있습니다. 이러한 함수들은 더 일반적인 목적으로 재사용할 수 있도록 리팩토링하는 것이 좋습니다.
-export const isExceedCardNumberLength = ( +export const exceedsLength = ( value: string, - cardNumberLength: number | null + maxLength: number | null ): boolean => { - return cardNumberLength !== null && value.length > cardNumberLength; + return maxLength !== null && value.length > maxLength; }; -export const isShortOfCardNumberLength = ( +export const belowLength = ( value: string, - cardNumberLength: number | null + minLength: number | null ): boolean => { - return cardNumberLength !== null && value.length < cardNumberLength; + return minLength !== null && value.length < minLength; };이러한 변경은 함수의 용도를 더 일반화하고, 카드 번호 이외의 문자열 검증에도 사용할 수 있게 합니다. 이는 코드 재사용성을 높이고 추후 유지보수를 용이하게 합니다.
1-51: 전반적인 코드 구조 및 사용성에 대한 피드백카드 관련 유틸리티 구현이 전반적으로 잘 되어 있습니다. 다만 몇 가지 고려할 점들이 있습니다:
카드 브랜드별 검증 로직이 CARD_BRAND 상수에 의존하는데, 이 상수의 구조가 변경되면 전체 유틸리티에 영향을 미칠 수 있습니다. 좀 더 견고한 타입 정의와 인터페이스 설계를 고려해보세요.
각 유틸리티 함수가 단일 책임을 잘 지키고 있지만, 일부 카드 검증 로직은 더 복잡한 실제 시나리오(예: Luhn 알고리즘 검증)를 포함하지 않고 있습니다. 필요에 따라 확장할 수 있는 구조인지 검토해보세요.
문서화를 JSDoc 형태로 추가하면 API 사용자에게 더 명확한 가이드를 제공할 수 있을 것입니다.
이 유틸리티 함수들은 useCardNumber 훅과 연계하여 사용되는 것으로 보이며, 해당 훅의 구현과 테스트를 통해 전체 기능의 정확성을 확인하는 것이 중요합니다.
🧰 Tools
🪛 Biome (1.9.4)
[error] 8-8: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
components/package-lock.jsonis excluded by!**/package-lock.jsonhooks/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (35)
.github/pull_request_template.md(2 hunks).gitignore(1 hunks)README.md(1 hunks)components/.storybook/preview.ts(2 hunks)components/package.json(1 hunks)components/src/App.tsx(1 hunks)components/src/components/Modal/Modal.stories.tsx(1 hunks)components/src/components/Modal/Modal.tsx(1 hunks)components/src/components/Modal/useFocus.ts(1 hunks)components/src/components/common/Button/Button.stories.tsx(1 hunks)components/src/components/common/Button/Button.tsx(1 hunks)components/src/components/common/CloseIcon.tsx(1 hunks)components/src/components/common/Input/Input.stories.tsx(1 hunks)components/src/components/common/Input/Input.tsx(1 hunks)components/src/components/common/index.ts(1 hunks)components/src/components/index.ts(1 hunks)components/tsconfig.app.json(1 hunks)components/tsconfig.node.json(1 hunks)components/vite.config.ts(2 hunks)hooks/package.json(2 hunks)hooks/src/�utils/card.ts(1 hunks)hooks/src/App.tsx(1 hunks)hooks/src/constants/cardBrand.ts(1 hunks)hooks/src/lib/index.ts(1 hunks)hooks/src/lib/useCardNumber.test.ts(1 hunks)hooks/src/lib/useCardNumber.ts(1 hunks)hooks/src/lib/useCvcNumber.test.ts(1 hunks)hooks/src/lib/useCvcNumber.ts(1 hunks)hooks/src/lib/useExpiryDate.test.ts(1 hunks)hooks/src/lib/useExpiryDate.ts(1 hunks)hooks/src/lib/usePassword.test.ts(1 hunks)hooks/src/lib/usePassword.ts(1 hunks)hooks/tsconfig.app.json(1 hunks)hooks/tsconfig.node.json(1 hunks)hooks/vite.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.ts`: You MUST follow these guidelines: - Please leave 1–2 key review comments that address recurring mistakes or pain points the code author frequently makes. If possible, re...
**/*.ts: You MUST follow these guidelines:
- Please leave 1–2 key review comments that address recurring mistakes or pain points the code author frequently makes. If possible, reference official documentation that can help resolve these issues
- Analyze the commit history to infer the cognitive process the developer went through while solving the problem. Summarize their thought progression: how they understood the problem, iterated on solutions, and refined the code. Focus on their reasoning patterns, learning moments, or shifts in approach. If applicable, relate their process to established best practices in the React ecosystem.
- Do not provide complete solutions unless explicitly requested.
- Guide the programmer in problem-solving instead of providing direct answers.
- When asked about programming concepts (e.g., "What is a hook?"), give direct and clear explanations.
- Break problems into smaller, manageable steps to help think through them logically.
- Ask leading questions and provide hints instead of just telling the answer.
- Encourage independent debugging before offering suggestions.
- Refer to relevant documentation instead of providing ready-made solutions.
- Promote modular thinking—breaking problems into reusable components.
- Remind the programmer to reflect on lessons learned after solving an issue.
- If explicitly asked for code (e.g., "Give me the code"), then provide it.
components/src/components/index.tscomponents/src/components/common/index.tscomponents/vite.config.tshooks/vite.config.tshooks/src/lib/index.tshooks/src/lib/useCvcNumber.test.tshooks/src/lib/usePassword.test.tshooks/src/lib/useExpiryDate.test.tshooks/src/constants/cardBrand.tshooks/src/lib/useCvcNumber.tshooks/src/lib/useCardNumber.tshooks/src/lib/usePassword.tshooks/src/lib/useCardNumber.test.tshooks/src/lib/useExpiryDate.tshooks/src/�utils/card.tscomponents/src/components/Modal/useFocus.ts
`**/*.tsx`: You MUST follow these guidelines: - Please leave 1–2 key review comments that address recurring mistakes or pain points the code author frequently makes. If possible, r...
**/*.tsx: You MUST follow these guidelines:
- Please leave 1–2 key review comments that address recurring mistakes or pain points the code author frequently makes. If possible, reference official documentation that can help resolve these issues
- Analyze the commit history to infer the cognitive process the developer went through while solving the problem. Summarize their thought progression: how they understood the problem, iterated on solutions, and refined the code. Focus on their reasoning patterns, learning moments, or shifts in approach. If applicable, relate their process to established best practices in the React ecosystem.
- Do not provide complete solutions unless explicitly requested.
- Guide the programmer in problem-solving instead of providing direct answers.
- When asked about programming concepts (e.g., "What is a hook?"), give direct and clear explanations.
- Break problems into smaller, manageable steps to help think through them logically.
- Ask leading questions and provide hints instead of just telling the answer.
- Encourage independent debugging before offering suggestions.
- Refer to relevant documentation instead of providing ready-made solutions.
- Promote modular thinking—breaking problems into reusable components.
- Remind the programmer to reflect on lessons learned after solving an issue.
- If explicitly asked for code (e.g., "Give me the code"), then provide it.
components/src/App.tsxhooks/src/App.tsxcomponents/src/components/common/Input/Input.stories.tsxcomponents/src/components/common/Input/Input.tsxcomponents/src/components/common/Button/Button.stories.tsxcomponents/src/components/common/CloseIcon.tsxcomponents/src/components/Modal/Modal.stories.tsxcomponents/src/components/Modal/Modal.tsxcomponents/src/components/common/Button/Button.tsx
🧬 Code Graph Analysis (6)
components/src/components/common/Input/Input.stories.tsx (1)
components/src/components/common/Input/Input.tsx (1)
Input(12-26)
hooks/src/lib/useCvcNumber.test.ts (1)
hooks/src/lib/useCvcNumber.ts (1)
useCvcNumber(9-38)
components/src/components/common/Button/Button.stories.tsx (1)
components/src/components/common/Button/Button.tsx (1)
Button(45-63)
hooks/src/lib/usePassword.test.ts (1)
hooks/src/lib/usePassword.ts (1)
usePassword(9-36)
components/src/components/Modal/Modal.stories.tsx (2)
components/src/components/Modal/Modal.tsx (1)
Modal(100-139)components/src/components/common/Button/Button.tsx (1)
Button(45-63)
components/src/components/Modal/Modal.tsx (3)
components/src/components/common/Input/Input.tsx (1)
Input(12-26)components/src/components/common/CloseIcon.tsx (1)
CloseIcon(1-20)components/src/components/common/Button/Button.tsx (1)
Button(45-63)
🪛 Biome (1.9.4)
components/tsconfig.node.json
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
hooks/tsconfig.node.json
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: JSON standard does not allow comments.
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
components/tsconfig.app.json
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: JSON standard does not allow comments.
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 32-32: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 33-33: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 33-33: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 33-33: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
hooks/tsconfig.app.json
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: JSON standard does not allow comments.
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 27-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 28-28: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 29-29: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 30-30: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 31-31: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 32-32: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 32-32: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 32-32: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
hooks/src/�utils/card.ts
[error] 8-8: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 markdownlint-cli2 (0.17.2)
README.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (27)
.github/pull_request_template.md (2)
7-8: 체크리스트 항목의 간결화가 적절합니다.
불필요한 예시를 제거하고 핵심 요구사항만 강조하여 읽기 쉽고 이해하기 편해졌습니다.
47-48: 커스텀 훅 책임 분리 및 반환 항목 강조가 명확합니다.
각 훅의 역할 분리와 유효성 검사 결과 및 에러 메시지 반환이라는 핵심 포인트를 잘 담아냈습니다.components/src/components/common/index.ts (1)
1-1: CloseIcon 컴포넌트 내보내기가 정상적으로 설정되었습니다.Modal 컴포넌트에서 사용할 닫기 아이콘을 공통 컴포넌트로 내보내는 작업이 잘 이루어졌습니다. 향후 다른 공통 컴포넌트들도 이 파일을 통해 일관된 방식으로 내보내면 좋을 것 같습니다.
components/src/components/index.ts (1)
1-1: Modal 컴포넌트 내보내기가 올바르게 구성되었습니다.Modal 컴포넌트를 패키지의 루트 엔트리 포인트에서 내보내는 것은 좋은 접근법입니다. PR 설명에서 언급하신 12개 props에 대한 고민은 이해합니다만, 현재 방식처럼 명확한 기본값과 선택적 props를 제공하는 것이 사용자 입장에서는 오히려 편리할 수 있습니다. 컴포넌트 합성 패턴을 적용하지 않은 것에 대한 고민도 있으신 것 같은데, 현재 구현된 모달 종류(alert, confirm, prompt)가 제한적이라면 현재 접근 방식도 충분히 합리적입니다. 필요에 따라 추후 확장성을 고려한 리팩토링을 진행하실 수 있을 것입니다.
.gitignore (1)
26-27: 테스트 커버리지 및 Storybook 정적 파일 제외 설정이 적절합니다.테스트 커버리지 보고서와 Storybook 빌드 결과물을 Git에서 제외하는 것은 좋은 관행입니다. 이러한 생성된 파일들은 버전 관리에 포함될 필요가 없으며, 이는 저장소 크기를 줄이고 불필요한 충돌을 방지하는 데 도움이 됩니다.
components/vite.config.ts (2)
9-9: 모듈 진입점 경로가 적절하게 변경되었습니다.기존
src/lib/index.ts에서src/components/index.ts로 entry point를 변경한 것은 패키지 구조를 명확하게 하는 좋은 접근입니다. 이는 패키지명(supe-modal)과도 잘 일치하며, 사용자에게 더 직관적인 import 경험을 제공할 것입니다.
25-31: TypeScript 선언 파일 생성 설정이 개선되었습니다.
dts플러그인 설정을 세부적으로 구성한 것은 타입 선언 파일 생성 프로세스를 더 명확하게 제어할 수 있게 해줍니다.include옵션으로 src 디렉토리를 명시하고, 별도의 tsconfig 파일을 지정함으로써 타입 선언 생성 과정이 더 안정적으로 이루어질 것입니다.React 컴포넌트 라이브러리를 위한 TypeScript 타입 선언은 사용자 경험에 중요한 부분이므로, 이런 개선은 큰 가치가 있습니다.
hooks/package.json (1)
6-8: 패키지 진입점 경로 수정타입 선언 파일과 메인 모듈 경로를
dist/lib/index.*로 변경한 것은vite.config.ts의 라이브러리 진입점 설정(src/lib/index.ts)과 일치하여 적절합니다. 이로써 패키지 사용자가 올바른 타입 정보와 모듈에 접근할 수 있습니다.components/package.json (3)
2-3: 패키지 이름과 버전 정보 업데이트"supe-modal"이라는 패키지 이름과 0.0.5 버전으로 업데이트한 것은 npm에 배포하기 위한 적절한 준비입니다. PR 목표에 맞게 모달 기능에 중점을 둔 이름을 선택한 것이 좋습니다. 다만, 관련 hooks 패키지와 마찬가지로 버전 관리 전략을 README에 명시하면 사용자와 기여자에게 도움이 될 것입니다.
6-8: 모듈 진입점 경로 수정타입 선언 파일과 메인 모듈 경로를
dist/components/index.*로 변경한 것은 적절합니다. 이는 패키지 구조와 빌드 설정을 일관되게 유지하는 좋은 접근입니다. 다만, 이 경로가vite.config.ts의 라이브러리 진입점 설정과 일치하는지 확인하는 것이 중요합니다.
13-13: 개발 스크립트에 호스트 옵션 추가
vite --host명령으로 변경한 것은 네트워크를 통해 다른 장치에서도 개발 서버에 접근할 수 있게 해주는 좋은 변경사항입니다. 다만, 보안 측면에서는 개발 중인 서버가 로컬 네트워크에 노출되므로 필요할 때만 이 옵션을 사용하는 것이 좋습니다.components/src/App.tsx (1)
1-3: 필요한 의존성 가져오기 및 구조 개선React의 useState를 사용하여 모달 상태를 관리하고, pesu-modal 패키지에서 Modal 컴포넌트를 가져온 것은 적절합니다. 상태 관리 로직을 명확하게 구성했습니다.
다만, 현재 개발 중인 supe-modal 패키지 내에서 pesu-modal을 사용하는 것이 의도적인지 확인이 필요합니다. 일반적으로 패키지 개발 시에는 내부 컴포넌트를 직접 사용하여 테스트하는 것이 더 적합할 수 있습니다.
hooks/src/lib/index.ts (1)
1-4: 모듈 내보내기 정리가 깔끔합니다.각 훅이 명확하게 내보내지고 있으며, 네이밍 컨벤션도 일관성 있게 유지되고 있습니다. 이런 중앙 집중식 내보내기 방식은 사용자가 필요한 훅을 쉽게 가져올 수 있게 해줍니다.
hooks/tsconfig.node.json (2)
16-17: npm 패키지 배포를 위한 타입 선언 파일 설정 추가 👏TypeScript 구성이 npm 패키지 배포를 위해 적절하게 개선되었습니다.
esModuleInterop과 선언 파일 생성 관련 옵션들은 라이브러리 사용자에게 좋은 개발 경험을 제공할 것입니다.Also applies to: 25-30
🧰 Tools
🪛 Biome (1.9.4)
[error] 16-16: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 16-16: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 16-16: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 16-18: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
23-24: 스타일 일관성을 위한 변경후행 쉼표(trailing comma)를 추가하여 JSON 구성 파일의 일관성을 유지하는 것은 좋은 습관입니다.
🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 23-23: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
components/tsconfig.app.json (3)
15-17: Emotion 스타일링 지원 설정이 추가되었습니다
noEmit옵션을false로 변경하고jsxImportSource를@emotion/react로 설정한 것은 컴포넌트 스타일링을 위한 좋은 접근입니다. 이로써 CSS-in-JS 스타일링이 가능해졌습니다.한 가지 고려할 점: 이 설정은 프로젝트의 모든 JSX 파일에 영향을 미칩니다. 혹시 특정 파일에만 Emotion을 사용하고 싶다면 해당 파일의 상단에
/** @jsxImportSource @emotion/react */주석을 추가하는 방법도 있습니다.🧰 Tools
🪛 Biome (1.9.4)
[error] 15-15: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 15-15: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 15-15: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 15-15: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 16-16: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 16-16: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 16-16: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 16-16: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 17-17: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 17-17: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 17-17: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
26-31: 타입 선언 파일 생성 설정이 적절합니다npm 패키지로 배포하기 위한 타입 선언 파일 설정이 잘 되어 있습니다.
declaration,emitDeclarationOnly,declarationMap등의 옵션을 통해 사용자에게 타입 정의를 제공하는 것은 훌륭한 개발 경험을 제공합니다.🧰 Tools
🪛 Biome (1.9.4)
[error] 26-26: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 26-26: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 26-26: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 26-26: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 27-27: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 27-27: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 27-27: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 27-27: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 28-28: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 28-28: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 28-28: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 28-28: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 29-29: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 29-29: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 29-29: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 29-29: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 30-30: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 30-30: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 30-30: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 30-30: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 31-31: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 31-31: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 31-31: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
33-33: 소스 파일 포함 패턴 개선
include패턴을"src/**/*"로 변경한 것은 모든 하위 디렉토리의 파일을 포함하기 위한 적절한 수정입니다. 이전 설정("src")은src디렉토리의 최상위 파일만 포함했을 수 있습니다.🧰 Tools
🪛 Biome (1.9.4)
[error] 33-33: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 33-33: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 33-33: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 33-34: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
hooks/src/lib/useCvcNumber.test.ts (2)
8-16: 초기 상태 테스트 커버리지 적절초기 상태를 확인하는 테스트가 잘 작성되어 있습니다.
cvcNumber,errorMessage, 그리고isValid모두 올바른 초기값을 검증하고 있습니다.
59-112: 유효성 검증 테스트 구조 잘 작성됨CVC 번호의 유효성 검증 테스트가 체계적으로 잘 작성되어 있습니다. 특히 에러 상태와 에러 메시지를 모두 검증하는 것이 좋습니다.
다만, 커스텀 에러 메시지를 전달했을 때의 동작을 테스트하는 케이스를 추가하면 더 완벽할 것 같습니다.
test("커스텀 에러 메시지가 제대로 표시되는지 확인한다.", () => { const customErrorMessage = "커스텀 에러 메시지"; const { result } = renderHook(() => useCvcNumber({ length: customErrorMessage }) ); const { handleCvcNumberChange } = result.current; const event = { target: { value: "11", }, }; act(() => { handleCvcNumberChange( event as unknown as React.ChangeEvent<HTMLInputElement> ); }); expect(result.current.errorMessage).toBe(customErrorMessage); });hooks/src/lib/usePassword.test.ts (1)
5-17: 초기 상태 테스트 적절히 구현됨비밀번호 훅의 초기 상태를 확인하는 테스트가 적절히 구현되어 있습니다. 모든 중요한 초기값(
password,errorMessage,isValid)을 확인하고 있습니다.hooks/tsconfig.app.json (3)
18-19: esModuleInterop 설정 추가 적절함
esModuleInterop: true설정이 추가된 것은 CommonJS 모듈과 ES 모듈 간의 호환성을 위해 적절한 변경입니다.🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 18-18: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 18-18: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 18-20: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
25-26: 사이드 이펙트 검사 설정 추가
noUncheckedSideEffectImports: true설정을 추가한 것은 좋은 선택입니다. 이 설정은 트리 쉐이킹(tree shaking)에 도움이 되어 번들 크기를 최적화하는 데 기여합니다.🧰 Tools
🪛 Biome (1.9.4)
[error] 25-25: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 25-25: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 25-25: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 25-25: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
27-32: 타입 선언 파일 생성 설정 추가타입 선언 파일 생성을 위한 설정들이 적절하게 추가되었습니다. 이는 라이브러리 배포 시 중요한 부분입니다.
다만, 배포를 위한 추가 설정으로
package.json에types필드도 함께 설정했는지 확인해보세요. 일반적인 설정은 다음과 같습니다:{ "name": "supe-hooks", "main": "dist/index.js", "types": "dist/index.d.ts" }이렇게 하면 TypeScript 사용자가 패키지를 더 쉽게 사용할 수 있습니다.
🧰 Tools
🪛 Biome (1.9.4)
[error] 27-27: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 27-27: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 27-27: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 27-27: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 28-28: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 28-28: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 28-28: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 28-28: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 29-29: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 29-29: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 29-29: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 29-29: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 30-30: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 30-30: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 30-30: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 30-30: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 31-31: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 31-31: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 31-31: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 31-31: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 32-32: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 32-32: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
[error] 32-32: End of file expected
Use an array for a sequence of values:
[1, 2](parse)
components/src/components/Modal/Modal.stories.tsx (2)
174-174: 비어있는 콜백 함수는 정말로 의도하신 건가요?ConfirmModal에서
onConfirm={() => {}}부분이 비어있는 콜백 함수로 설정되어 있습니다. 이것이 의도적인 것인지 확인해주세요.스토리북 예제에서도 실제 사용 사례를 보여주는 것이 좋습니다. 예를 들어:
onConfirm={() => { closeModal(); console.log('사용자가 확인 버튼을 클릭했습니다.'); }}이렇게 하면 확인 버튼의 실제 사용 방법을 더 명확하게 보여줄 수 있습니다.
208-208: 비어있는 콜백 함수는 정말로 의도하신 건가요?PromptModal에서도
onConfirm={() => {}}부분이 비어있는 콜백 함수로 설정되어 있습니다. 이것이 의도적인 것인지 확인해주세요.입력 필드가 있는 모달에서는 사용자 입력을 처리하는 로직을 보여주는 것이 더 도움이 될 것입니다. 예를 들어:
onConfirm={() => { console.log('입력된 쿠폰 번호:', inputValue); closeModal(); }}이렇게 하면 입력값을 활용하는 방법을 더 명확하게 보여줄 수 있습니다.
hooks/src/lib/useCardNumber.ts (1)
28-28: 🛠️ Refactor suggestion불필요한 early return 패턴 수정 필요
isExceedCardNumberLength가 true를 반환할 때 early return하고 있지만, 이 함수는 아무것도 반환하지 않습니다. early return 시 명확한 의도를 위해 다음과 같이 변경하는 것이 좋습니다:- if (isExceedCardNumberLength(value, cardNumberLength)) return; + if (isExceedCardNumberLength(value, cardNumberLength)) { + return; // 최대 길이 초과 시 상태 업데이트하지 않음 + }Likely an incorrect or invalid review comment.
| "@testing-library/dom": "^10.4.0", | ||
| "pesu-hooks": "^0.0.3", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
종속성 추가 고려사항
@testing-library/dom과 pesu-hooks를 추가한 것은 필요한 기능 구현을 위한 적절한 선택입니다. 그러나 테스트 라이브러리는 일반적으로 devDependencies에 포함되는 것이 관례입니다. @testing-library/dom은 개발 시에만 필요한 패키지이므로 devDependencies로 이동하는 것이 좋습니다.
또한 자체 패키지인 pesu-hooks에 의존하는 것이 이 패키지의 목적에 맞는지 재고해보면 좋겠습니다. 순환 의존성이 발생할 가능성이 있습니다.
🤖 Prompt for AI Agents (early access)
In hooks/package.json around lines 20 to 21, move the @testing-library/dom
dependency from "dependencies" to "devDependencies" since it is only needed
during development and testing. Also, review the use of the pesu-hooks
dependency to ensure it aligns with the package's purpose and does not create
circular dependencies; remove or refactor it if necessary.
| "@emotion/react": "^11.14.0", | ||
| "@emotion/styled": "^11.14.0", | ||
| "@storybook/addon-viewport": "^8.6.12", | ||
| "pesu-modal": "^0.0.4", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
새 종속성 추가에 대한 고려
Emotion 라이브러리(@emotion/react, @emotion/styled)를 추가한 것은 컴포넌트 스타일링을 위한 좋은 선택입니다. 또한 Storybook 뷰포트 애드온을 추가하여 반응형 테스트를 개선한 것도 적절합니다.
그러나 pesu-modal 패키지에 대한 의존성은 재고해볼 필요가 있습니다. 이 패키지가 현재 개발 중인 "supe-modal" 패키지와 유사한 기능을 제공한다면, 왜 두 패키지가 모두 필요한지 명확히 하는 것이 좋습니다. 불필요한 중복이나 순환 의존성이 발생할 수 있습니다.
🤖 Prompt for AI Agents (early access)
In components/package.json around lines 21 to 24, review the addition of the
"pesu-modal" dependency. Verify if "pesu-modal" overlaps in functionality with
the in-development "supe-modal" package. If they provide similar features,
decide which one to keep to avoid redundancy and potential circular
dependencies. Remove "pesu-modal" if it is unnecessary or consolidate
functionality to a single modal package.
| const { expiryDate, handleExpiryDateChange, errorMessage } = useExpiryDate(); | ||
|
|
||
| console.log(expiryDate, errorMessage); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 메시지 처리 방식 개선이 필요합니다.
현재 에러 메시지를 콘솔에 로깅하고 있지만, 사용자에게 표시하는 부분이 없습니다. 유효성 검사 오류를 사용자에게 보여주는 방식으로 개선하는 것이 좋겠습니다.
에러 메시지를 UI에 표시하도록 변경:
export default function App() {
const { expiryDate, handleExpiryDateChange, errorMessage } = useExpiryDate();
- console.log(expiryDate, errorMessage);
+ // console.log 대신 UI에 표시
return (
<>
<input
type="text"
onChange={handleExpiryDateChange}
data-date-type="year"
/>
<input
type="text"
onChange={handleExpiryDateChange}
data-date-type="month"
/>
+ {errorMessage && <p className="error-message">{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.
| const { expiryDate, handleExpiryDateChange, errorMessage } = useExpiryDate(); | |
| console.log(expiryDate, errorMessage); | |
| export default function App() { | |
| const { expiryDate, handleExpiryDateChange, errorMessage } = useExpiryDate(); | |
| // console.log 대신 UI에 표시 | |
| return ( | |
| <> | |
| <input | |
| type="text" | |
| onChange={handleExpiryDateChange} | |
| data-date-type="year" | |
| /> | |
| <input | |
| type="text" | |
| onChange={handleExpiryDateChange} | |
| data-date-type="month" | |
| /> | |
| {errorMessage && <p className="error-message">{errorMessage}</p>} | |
| </> | |
| ); | |
| } |
🤖 Prompt for AI Agents (early access)
In hooks/src/App.tsx around lines 6 to 8, the errorMessage is currently only
logged to the console and not shown to the user. Modify the component to display
the errorMessage in the UI, for example by rendering it conditionally below the
input or relevant element, so users can see validation errors directly.
| import { useExpiryDate } from "pesu-hooks"; | ||
|
|
||
| export default function App() { | ||
| const { expiryDate, handleExpiryDateChange, errorMessage } = useExpiryDate(); | ||
|
|
||
| console.log(expiryDate, errorMessage); | ||
|
|
There was a problem hiding this comment.
패키지 이름 불일치 문제가 있습니다.
AI 요약에 따르면 패키지 이름이 "supe-hooks"로 변경되었지만, 여기서는 "pesu-hooks"를 가져오고 있습니다. 이는 패키지 명칭 변경 후 업데이트되지 않은 것으로 보입니다.
- import { useExpiryDate } from "pesu-hooks";
+ import { useExpiryDate } from "supe-hooks";📝 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.
| import { useExpiryDate } from "pesu-hooks"; | |
| export default function App() { | |
| const { expiryDate, handleExpiryDateChange, errorMessage } = useExpiryDate(); | |
| console.log(expiryDate, errorMessage); | |
| import { useExpiryDate } from "supe-hooks"; | |
| export default function App() { | |
| const { expiryDate, handleExpiryDateChange, errorMessage } = useExpiryDate(); | |
| console.log(expiryDate, errorMessage); |
🤖 Prompt for AI Agents (early access)
In hooks/src/App.tsx around lines 3 to 9, the import statement uses the outdated
package name "pesu-hooks" instead of the updated "supe-hooks". Update the import
statement to import useExpiryDate from "supe-hooks" to match the new package
name and avoid import errors.
| <input | ||
| type="text" | ||
| onChange={handleExpiryDateChange} | ||
| data-date-type="year" | ||
| /> | ||
| <input | ||
| type="text" | ||
| onChange={handleExpiryDateChange} | ||
| data-date-type="month" | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
입력 필드에 접근성과 사용성 개선이 필요합니다.
현재 입력 필드에 레이블이 없고, placeholder나 value 속성도 설정되어 있지 않아 사용성과 접근성이 부족합니다.
접근성 개선을 위한 레이블과 placeholder 추가:
return (
<>
+ <div className="expiry-date-container">
+ <label htmlFor="year-input">연도:</label>
<input
type="text"
+ id="year-input"
onChange={handleExpiryDateChange}
data-date-type="year"
+ placeholder="YY"
+ value={expiryDate.year}
+ aria-describedby={errorMessage ? "expiry-error" : undefined}
/>
+ <label htmlFor="month-input">월:</label>
<input
type="text"
+ id="month-input"
onChange={handleExpiryDateChange}
data-date-type="month"
+ placeholder="MM"
+ value={expiryDate.month}
+ aria-describedby={errorMessage ? "expiry-error" : undefined}
/>
+ {errorMessage && <p id="expiry-error" className="error-message">{errorMessage}</p>}
+ </div>
</>
);📝 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="text" | |
| onChange={handleExpiryDateChange} | |
| data-date-type="year" | |
| /> | |
| <input | |
| type="text" | |
| onChange={handleExpiryDateChange} | |
| data-date-type="month" | |
| /> | |
| <div className="expiry-date-container"> | |
| <label htmlFor="year-input">연도:</label> | |
| <input | |
| type="text" | |
| id="year-input" | |
| onChange={handleExpiryDateChange} | |
| data-date-type="year" | |
| placeholder="YY" | |
| value={expiryDate.year} | |
| aria-describedby={errorMessage ? "expiry-error" : undefined} | |
| /> | |
| <label htmlFor="month-input">월:</label> | |
| <input | |
| type="text" | |
| id="month-input" | |
| onChange={handleExpiryDateChange} | |
| data-date-type="month" | |
| placeholder="MM" | |
| value={expiryDate.month} | |
| aria-describedby={errorMessage ? "expiry-error" : undefined} | |
| /> | |
| {errorMessage && ( | |
| <p id="expiry-error" className="error-message"> | |
| {errorMessage} | |
| </p> | |
| )} | |
| </div> |
🤖 Prompt for AI Agents (early access)
In hooks/src/App.tsx around lines 12 to 21, the input fields lack labels and
placeholders, which reduces accessibility and usability. Add appropriate <label>
elements linked to each input using the htmlFor attribute, and include
descriptive placeholder attributes in the input elements to guide users on the
expected input format.
| ])( | ||
| "cardNumber가 622126~622925/624~626/6282~6288으로 시작하면 Amex를 반환한다.", | ||
| (cardNumber) => { | ||
| const { result, rerender } = renderHook(() => useCardNumber()); | ||
|
|
||
| changeCardNumber(result, rerender, cardNumber); | ||
|
|
||
| expect(result.current.cardBrand()).toBe("UnionPay"); | ||
| } | ||
| ); |
There was a problem hiding this comment.
테스트 설명 정확성 향상 필요
테스트 설명이 정확하지 않습니다. 카드 번호가 특정 패턴으로 시작하면 "Amex"가 아닌 "UnionPay"를 반환한다고 말해야 합니다.
- "cardNumber가 622126~622925/624~626/6282~6288으로 시작하면 Amex를 반환한다.",
+ "cardNumber가 622126~622925/624~626/6282~6288으로 시작하면 UnionPay를 반환한다.",📝 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.
| ])( | |
| "cardNumber가 622126~622925/624~626/6282~6288으로 시작하면 Amex를 반환한다.", | |
| (cardNumber) => { | |
| const { result, rerender } = renderHook(() => useCardNumber()); | |
| changeCardNumber(result, rerender, cardNumber); | |
| expect(result.current.cardBrand()).toBe("UnionPay"); | |
| } | |
| ); | |
| ])( | |
| "cardNumber가 622126~622925/624~626/6282~6288으로 시작하면 UnionPay를 반환한다.", | |
| (cardNumber) => { | |
| const { result, rerender } = renderHook(() => useCardNumber()); | |
| changeCardNumber(result, rerender, cardNumber); | |
| expect(result.current.cardBrand()).toBe("UnionPay"); | |
| } | |
| ); |
🤖 Prompt for AI Agents (early access)
In hooks/src/lib/useCardNumber.test.ts around lines 110 to 119, the test
description incorrectly states that card numbers starting with certain patterns
return "Amex," but the actual expected return value is "UnionPay." Update the
test description string to accurately reflect that card numbers starting with
622126~622925, 624~626, or 6282~6288 return "UnionPay" instead of "Amex."
| "visa/master 카드는 4자리씩 띄어서 반환한다.", | ||
| (cardNumber, formattedCardNumber) => { | ||
| const { result, rerender } = renderHook(() => useCardNumber()); | ||
|
|
||
| changeCardNumber(result, rerender, cardNumber); | ||
|
|
||
| expect(result.current.formattedCardNumber()).toBe(formattedCardNumber); | ||
| } | ||
| ); |
There was a problem hiding this comment.
테스트 설명 오류 수정 필요
테스트 설명에 오류가 있습니다. "visa/master" 카드가 아닌 "unionPay" 카드에 대한 테스트입니다.
- "visa/master 카드는 4자리씩 띄어서 반환한다.",
+ "unionPay 카드는 4자리씩 띄어서 반환한다.",🤖 Prompt for AI Agents (early access)
In hooks/src/lib/useCardNumber.test.ts around lines 291 to 299, the test
description incorrectly states it is for "visa/master" cards, but the test is
actually for "unionPay" cards. Update the test description string to correctly
reflect that it is testing the formatting for "unionPay" cards instead of
"visa/master" cards.
| "visa/master 카드는 4/6/4자리씩 띄어서 반환한다.", | ||
| (cardNumber, formattedCardNumber) => { | ||
| const { result, rerender } = renderHook(() => useCardNumber()); | ||
|
|
||
| changeCardNumber(result, rerender, cardNumber); | ||
|
|
||
| expect(result.current.formattedCardNumber()).toBe(formattedCardNumber); | ||
| } | ||
| ); |
There was a problem hiding this comment.
테스트 설명 오류 수정 필요
테스트 설명에 오류가 있습니다. "visa/master" 카드가 아닌 "diners" 카드에 대한 테스트입니다.
- "visa/master 카드는 4/6/4자리씩 띄어서 반환한다.",
+ "diners 카드는 4/6/4자리씩 띄어서 반환한다.",📝 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.
| "visa/master 카드는 4/6/4자리씩 띄어서 반환한다.", | |
| (cardNumber, formattedCardNumber) => { | |
| const { result, rerender } = renderHook(() => useCardNumber()); | |
| changeCardNumber(result, rerender, cardNumber); | |
| expect(result.current.formattedCardNumber()).toBe(formattedCardNumber); | |
| } | |
| ); | |
| "diners 카드는 4/6/4자리씩 띄어서 반환한다.", | |
| (cardNumber, formattedCardNumber) => { | |
| const { result, rerender } = renderHook(() => useCardNumber()); | |
| changeCardNumber(result, rerender, cardNumber); | |
| expect(result.current.formattedCardNumber()).toBe(formattedCardNumber); | |
| } | |
| ); |
🤖 Prompt for AI Agents (early access)
In hooks/src/lib/useCardNumber.test.ts around lines 262 to 270, the test
description incorrectly states "visa/master 카드" when the test is actually for
the "diners" card. Update the test description string to correctly reflect that
it is testing the "diners" card formatting, ensuring the description matches the
test logic.
| interface ButtonInterface | ||
| extends React.ButtonHTMLAttributes<HTMLButtonElement> { | ||
| variant?: "confirm" | "cancel"; | ||
| size: "small" | "medium" | "large"; | ||
| tabIndex?: number; | ||
| text?: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Button 컴포넌트의 size prop 기본값 추가 필요
현재 size prop은 필수로 지정되어 있지만 기본값이 없습니다. 이로 인해 사용자가 size를 지정하지 않으면 TypeScript 에러가 발생할 수 있습니다. 기본값을 제공하여 사용성을 개선하는 것이 좋겠습니다.
interface ButtonInterface
extends React.ButtonHTMLAttributes<HTMLButtonElement> {
variant?: "confirm" | "cancel";
- size: "small" | "medium" | "large";
+ size?: "small" | "medium" | "large";
tabIndex?: number;
text?: string;
}
export default function Button({
variant,
- size,
+ size = "medium",
text,
...rest
}: ButtonInterface) {이렇게 하면 size prop을 생략해도 기본적으로 "medium" 크기로 렌더링되어 컴포넌트의 사용성이 향상됩니다.
Also applies to: 45-50
🤖 Prompt for AI Agents (early access)
In components/src/components/common/Button/Button.tsx around lines 37 to 43 and
45 to 50, the size prop in the ButtonInterface is currently required without a
default value, causing TypeScript errors if omitted. To fix this, make the size
prop optional in the interface and set a default value of "medium" for size
within the Button component's default props or function parameter defaults. This
ensures the component renders with a medium size when size is not explicitly
provided.
| export const getCardBrand = (value: string): CardBrandType => { | ||
| for (const brand in CARD_BRAND) { | ||
| const condition = CARD_BRAND[brand as CardBrandType].condition; | ||
| if (condition && condition(value)) { | ||
| return brand as CardBrandType; | ||
| } | ||
| } | ||
| return "none"; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
선택적 체이닝을 사용하여 코드 안전성 향상
getCardBrand 함수에서 condition 속성에 접근할 때 선택적 체이닝을 사용하는 것이 좋습니다. 특히 CARD_BRAND 객체의 일부 브랜드에 condition 속성이 정의되지 않았거나 타입 확장시 누락될 가능성이 있습니다.
export const getCardBrand = (value: string): CardBrandType => {
for (const brand in CARD_BRAND) {
const condition = CARD_BRAND[brand as CardBrandType].condition;
- if (condition && condition(value)) {
+ if (condition?.(value)) {
return brand as CardBrandType;
}
}
return "none";
};🧰 Tools
🪛 Biome (1.9.4)
[error] 8-8: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents (early access)
In hooks/src/utils/card.ts between lines 5 and 13, the getCardBrand function
accesses the condition property directly on CARD_BRAND entries, which may be
undefined for some brands. To improve code safety, update the access to use
optional chaining on the condition property, ensuring that if condition is
undefined or missing, it does not cause runtime errors. This means replacing
direct condition access with condition?.(value) in the if statement.
bassyu
left a comment
bassyu
left a comment
There was a problem hiding this comment.
안녕하세요 수이~!
컴포넌트 구조에 대해 많이 고민해보시고, 느낀점을 잘 전달 해주셨네요 💯 💯
공감가는 포인트가 많았는데요, 그래서 지금 구조를 유지하면서 props만 줄이는 방향성도 코멘트로 남겼습니다~!
코멘트 확인 후 다시 요청 주세요~! 💪 💪
|
|
||
| const modalEl = modalRef.current; | ||
| const focusableEls = modalEl?.querySelectorAll<HTMLElement>( | ||
| 'button, a[href], input, select, textarea, [tabindex]:not([tabindex="-1"])' |
There was a problem hiding this comment.
'button, a[href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
위의 샐랙터에 포함되면서도, 동시에 포커스는 하기 싫은 요소가 있을 수 있는데요!
어떻게 대응하면 좋을까요?
대응해보고, 토끼한테 다시 물어봐도 좋을 것 같아요~! (토끼 의견도 궁금하네요)
There was a problem hiding this comment.
앗 이건 탭만 신경쓰는 거니, tabIndex만으로도 해결이 되겠네요! 이대로도 괜찮아보입니다!
| margin: auto; | ||
| `; | ||
|
|
||
| interface ModalInterface { |
There was a problem hiding this comment.
하지만 리뷰어의 입장에서는 다를 수 있을 것 같아 질문 드립니다! 놀림받을만 한가요,,?🥹
앗 놀림받을 정도는 아니지만, 이것 저것 수이가 직접 시도해보면서 경험하면 좋을 것 같습니다! 🙌
단점의 예시를 들면, 특정 props들은 서로 그룹화 되어있고 서로에게 영향을 주는 경우가 있는데요,
props가 플랫하게 너무 많으면 이걸 눈치채기 어려울 수도 있습니다~!
다른 예시로는.. 프리코스와 1레벨 때 함수를 분리하는 연습을 했었는데요.
컴포넌트도 UI 그리는 함수라고 생각해보세요!
함수 파라미터의 개수를 줄이고 역할을 잘게 분리해서, 테스트 또는 조합과 대응에 유연한 구조를 계속 고민해봐도 좋을 것 같아요~!
| margin: auto; | ||
| `; | ||
|
|
||
| interface ModalInterface { |
There was a problem hiding this comment.
하지만 이번 미션은 요구하는 디자인이 한정적이고 라이브러리인 만큼 편의성에 초점을 두고 싶었습니다.
이것도 맞습니다! 수이의 초점 공감 됩니다 👍
그래도 아쉬우시다면, 경험 관점에서 패턴을 써보셔도 괜찮습니다.
그런데, 꼭 context나 컴파운드 패턴을 쓰지 않더라도 수이가 이미 ModalTop, ModalBottom 으로 잘 분리해 주셨습니다!
이걸 그대로 사용처에서 가져다 쓰면, 비슷한 모양이겠네요~!
| margin: auto; | ||
| `; | ||
|
|
||
| interface ModalInterface { |
There was a problem hiding this comment.
또한 컴포넌트 조합 패턴으로 보이는 구조보다 props로 전달하는 것이 어떤 요소가 포함되는지 더 직관적으로 표현된다고 생각했습니다.
좋습니다! 👍 그래서 다른 방법으로는 props 로 여러 선택지를 주는 대신, 아예 여러 조합의 완성본을 제공해도 좋습니다.
예를 들어, ModalTop, ModalBottom의 props를 각각 다르게 조합한 모달을 만들고,
각각 (Default)Modal, AlertModal, ConfirmModal로 이름을 붙여서 모두 배포하는 식으로요~!
이러면 충분히 중복 제거도 되면서, 서로 관련 없는 props끼리 몰라도 되는 장점도 있을 것 같아요!
| }, | ||
| }; | ||
|
|
||
| export const ConfirmModal: Story = { |
There was a problem hiding this comment.
Confirm 모달은 중요한 정보를 보여주는 케이스가 많습니다!
그래서 Backdrop을 눌렀을 때 모달이 닫히지 않도록 대응해주세요~!
| Visa: { | ||
| length: 16, | ||
| format: [4, 4, 4, 4], | ||
| condition: (value: string) => value.startsWith("4"), | ||
| }, |
There was a problem hiding this comment.
length, format, condition으로 그룹화 해주셔서 관리하기 좋아보이네요 👍
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
components/src/components/Modal/Modal.stories.tsx (2)
20-64: 스토리 이름에 오타가 있고 중복 로직이 많습니다.스토리 이름에 오타가 있습니다.
Positioin을Position으로 수정해주세요.-export const Positioin: Story = { +export const Position: Story = {또한, 모달 열기/닫기 로직이 반복적으로 사용되고 있습니다. 커스텀 훅을 만들어 코드를 더 간결하게 만들 수 있습니다:
function useModal(initialState = false) { const [isOpen, setIsOpen] = useState(initialState); const open = () => setIsOpen(true); const close = () => setIsOpen(false); return { isOpen, open, close }; }이후 다음과 같이 사용할 수 있습니다:
const centerModal = useModal(false); const bottomModal = useModal(false); return ( <> <Button text="center" size="medium" onClick={centerModal.open} /> <Button text="bottom" size="medium" onClick={bottomModal.open} /> <Modal onClose={centerModal.close} isOpen={centerModal.isOpen} position={"center"} size={"large"} > center </Modal> {/* ... */} </> );
66-127: 모달 상태 관리 로직이 반복됩니다.Size 스토리에서도 모달 상태 관리 로직(openModal, closeModal 함수들)이 반복되고 있습니다. 앞서 제안한
useModal커스텀 훅을 활용하면 코드를 더 간결하게 만들 수 있습니다:export const Size: Story = { render: () => { const smallModal = useModal(false); const mediumModal = useModal(false); const largeModal = useModal(false); return ( <> <Button text="small" size="medium" onClick={smallModal.open} /> <Button text="medium" size="medium" onClick={mediumModal.open} /> <Button text="large" size="medium" onClick={largeModal.open} /> <Modal onClose={smallModal.close} isOpen={smallModal.isOpen} position={"center"} size={"small"} closeButton={true} > 작은 모달입니다! </Modal> {/* ... 나머지 모달 컴포넌트 ... */} </> ); }, };
🧹 Nitpick comments (6)
components/src/components/Modal/AlertModal.tsx (1)
32-41: 스타일 속성에 오타가 있습니다.35번 줄의
align-itmes는 오타입니다. 올바른 CSS 속성명은align-items입니다.display: flex; justify-content: flex-end; - align-itmes: center; + align-items: center; gap: 10px;또한, 38-40번 줄의 미디어 쿼리에서
flex-direction: row는 flex 컨테이너의 기본값이므로 불필요합니다. 모바일 환경에서 다른 레이아웃 변경이 필요하다면 의미있는 변경으로 수정하거나, 필요없다면 제거하는 것이 좋습니다.components/src/components/Modal/PromptModal.tsx (1)
33-33: Input 컴포넌트의 접근성 및 유연성 개선현재 Input 컴포넌트는 하드코딩된 높이(
height={"32px"})를 가지고 있으며, placeholder나 label이 없습니다. 이는 접근성(accessibility)과 유연성 측면에서 개선이 필요합니다.다음과 같이 placeholder를 추가하고, size prop에 따라 동적으로 height를 결정하도록 수정하는 것이 좋습니다:
- <Input height={"32px"} value={inputValue} onChange={onInputChange} /> + <Input + height={size === "small" ? "28px" : size === "medium" ? "32px" : "36px"} + value={inputValue} + onChange={onInputChange} + placeholder="여기에 입력하세요" + aria-label="프롬프트 입력" + />혹은 Input 컴포넌트 자체에 size prop을 전달하여 내부적으로 높이를 결정하도록 리팩토링하는 것도 좋은 방법입니다.
components/src/components/Modal/ConfirmModal.tsx (2)
41-50: 스타일 속성에 오타가 있습니다.44번 줄의
align-itmes는 오타입니다. 올바른 CSS 속성명은align-items입니다.display: flex; justify-content: flex-end; - align-itmes: center; + align-items: center; gap: 10px;또한, 47-49번 줄의 미디어 쿼리에서
flex-direction: row는 flex 컨테이너의 기본값이므로 불필요합니다. 모바일 환경에서 버튼 배치를 변경하려면 의미 있는 변경을 적용하거나, 불필요한 경우 미디어 쿼리를 제거하는 것이 좋습니다.
14-37: 좋은 접근성 및 사용자 경험을 위한 모달 구현이
ConfirmModal컴포넌트는 의도적으로closeOnBackdropClick={false}를 설정하여 사용자가 실수로 모달을 닫지 않도록 방지했습니다. 이는 중요한 확인 작업에 적합한 좋은 UX 설계입니다.하지만 키보드 접근성을 개선하기 위해 취소 버튼에 초점을 맞출 수 있도록
ref또는autoFocus속성을 추가하는 것을 고려해 보세요. 이렇게 하면 키보드 사용자가 쉽게 모달을 취소할 수 있습니다.<ModalBottom> <Button variant="confirm" size={size} onClick={onConfirm} /> - <Button variant="cancel" size={size} onClick={onClose} /> + <Button variant="cancel" size={size} onClick={onClose} autoFocus /> </ModalBottom>이러한 개선 사항은 웹 접근성 표준(WCAG)을 준수하는 데 도움이 됩니다.
components/src/components/Modal/Modal.stories.tsx (2)
182-211: Prompt 스토리의 핸들러 간소화 및 초기 상태 개선Prompt 스토리에서
handleSetInputValue함수는 불필요하게 복잡합니다. 또한 초기 상태가 true로 설정되어 있어 스토리북을 열자마자 모달이 표시됩니다. 다음과 같이 개선할 수 있습니다:export const Prompt: Story = { render: () => { - const [isOpened, setIsOpened] = useState(true); + const [isOpened, setIsOpened] = useState(false); const [inputValue, setInputValue] = useState(""); const openModal = () => { setIsOpened(true); }; const closeModal = () => { setIsOpened(false); }; - const handleSetInputValue = (v: string) => { - setInputValue(v); - }; return ( <> <Button size="small" text="열기" onClick={openModal} /> <PromptModal title="쿠폰 번호를 입력해 주세요." onClose={closeModal} isOpen={isOpened} inputValue={inputValue} onConfirm={() => {}} - onInputChange={(e) => handleSetInputValue(e.target.value)} + onInputChange={(e) => setInputValue(e.target.value)} size="medium" ></PromptModal> </> ); }, };또한
onConfirm핸들러에 빈 콜백을 전달하는 대신, 실제 동작을 수행하는 함수(예: 모달 닫기, 알림 표시 등)를 연결하는 것이 더 좋은 예시가 될 것입니다.
129-153: 모달 유형 스토리의 일관성 및 접근성 개선Alert와 Confirm 스토리의 초기 상태가
true로 설정되어 있어, 스토리북을 열자마자 모달이 표시됩니다. 이는 사용자 경험 면에서 일관되지 않을 수 있습니다. 일반적으로 스토리북에서는 기본적으로 모달이 닫혀 있다가 사용자가 버튼을 클릭할 때 열리도록 하는 것이 좋습니다.또한, onConfirm 핸들러가 비어 있어 실제 확인 동작을 보여주지 못합니다. 실제 사용 사례를 더 잘 보여주기 위해 다음과 같이 개선할 수 있습니다:
export const Alert: Story = { render: () => { - const [isOpened, setIsOpened] = useState(true); + const [isOpened, setIsOpened] = useState(false); + const [message, setMessage] = useState(""); const openModal = () => { setIsOpened(true); }; const closeModal = () => { setIsOpened(false); }; return ( <> <Button size="medium" text="열기" onClick={openModal} /> + {message && <div aria-live="polite">{message}</div>} <AlertModal onClose={closeModal} isOpen={isOpened} title="아이디를 입력해 주세요." size="medium" > 아이디는 필수로 입력해야 합니다. </AlertModal> </> ); }, }; export const Confirm: Story = { render: () => { - const [isOpened, setIsOpened] = useState(true); + const [isOpened, setIsOpened] = useState(false); + const [confirmResult, setConfirmResult] = useState<string | null>(null); const openModal = () => { setIsOpened(true); }; const closeModal = () => { setIsOpened(false); + setConfirmResult("취소되었습니다"); }; + const handleConfirm = () => { + setIsOpened(false); + setConfirmResult("확인되었습니다"); + }; return ( <> <Button size="medium" text="열기" onClick={openModal} /> + {confirmResult && <div aria-live="polite">{confirmResult}</div>} <ConfirmModal onClose={closeModal} - onConfirm={() => {}} + onConfirm={handleConfirm} isOpen={isOpened} title="카드를 삭제하시겠습니까?" size="medium" > 삭제하면 복구하실 수 없습니다. </ConfirmModal> </> ); }, };이러한 변경으로 사용자는 모달의 동작을 보다 현실적으로 테스트하고 이해할 수 있습니다.
Also applies to: 155-180
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (5)
components/src/components/Modal/AlertModal.tsx(1 hunks)components/src/components/Modal/ConfirmModal.tsx(1 hunks)components/src/components/Modal/Modal.stories.tsx(1 hunks)components/src/components/Modal/Modal.tsx(3 hunks)components/src/components/Modal/PromptModal.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/src/components/Modal/Modal.tsx
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.tsx`: You MUST follow these guidelines: - Please leave 1–2 key review comments that address recurring mistakes or pain points the code author frequently makes. If possible, r...
**/*.tsx: You MUST follow these guidelines:
- Please leave 1–2 key review comments that address recurring mistakes or pain points the code author frequently makes. If possible, reference official documentation that can help resolve these issues
- Analyze the commit history to infer the cognitive process the developer went through while solving the problem. Summarize their thought progression: how they understood the problem, iterated on solutions, and refined the code. Focus on their reasoning patterns, learning moments, or shifts in approach. If applicable, relate their process to established best practices in the React ecosystem.
- Do not provide complete solutions unless explicitly requested.
- Guide the programmer in problem-solving instead of providing direct answers.
- When asked about programming concepts (e.g., "What is a hook?"), give direct and clear explanations.
- Break problems into smaller, manageable steps to help think through them logically.
- Ask leading questions and provide hints instead of just telling the answer.
- Encourage independent debugging before offering suggestions.
- Refer to relevant documentation instead of providing ready-made solutions.
- Promote modular thinking—breaking problems into reusable components.
- Remind the programmer to reflect on lessons learned after solving an issue.
- If explicitly asked for code (e.g., "Give me the code"), then provide it.
components/src/components/Modal/ConfirmModal.tsxcomponents/src/components/Modal/AlertModal.tsxcomponents/src/components/Modal/PromptModal.tsxcomponents/src/components/Modal/Modal.stories.tsx
🧬 Code Graph Analysis (4)
components/src/components/Modal/ConfirmModal.tsx (2)
components/src/components/Modal/Modal.tsx (1)
Modal(80-109)components/src/components/common/Button/Button.tsx (1)
Button(45-63)
components/src/components/Modal/AlertModal.tsx (2)
components/src/components/Modal/Modal.tsx (1)
Modal(80-109)components/src/components/common/Button/Button.tsx (1)
Button(45-63)
components/src/components/Modal/PromptModal.tsx (1)
components/src/components/common/Input/Input.tsx (1)
Input(12-26)
components/src/components/Modal/Modal.stories.tsx (3)
components/src/components/Modal/Modal.tsx (1)
Modal(80-109)components/src/components/common/Button/Button.tsx (1)
Button(45-63)components/src/components/common/Button/Button.stories.tsx (1)
Confirm(15-23)
| const AlertModal = ({ | ||
| isOpen, | ||
| onClose, | ||
| title, | ||
| children, | ||
| size, | ||
| }: PropsWithChildren<ModalInterface>) => { | ||
| return ( | ||
| <Modal isOpen={isOpen} size={size} onClose={onClose} title={title}> | ||
| {children} | ||
| <ModalBottom> | ||
| <Button variant="confirm" size={size} onClick={onClose} /> | ||
| </ModalBottom> | ||
| </Modal> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
모달 닫기 제어가 부족할 수 있습니다.
현재 AlertModal 컴포넌트는 배경(backdrop)을 클릭했을 때 모달이 닫힐 수 있습니다. 이는 기본 Modal 컴포넌트의 closeOnBackdropClick 기본값이 true이기 때문입니다.
알림 모달(Alert)은 중요한 메시지를 보여주는 경우가 많으므로, 사용자가 실수로 배경을 클릭해 모달이 닫히지 않도록 closeOnBackdropClick={false}를 명시적으로 설정하는 것이 좋습니다. ConfirmModal 컴포넌트처럼 의도적인 동작으로 설정하세요.
<Modal isOpen={isOpen} size={size} onClose={onClose} title={title}
+ closeOnBackdropClick={false}
>
{children}
<ModalBottom>
<Button variant="confirm" size={size} onClick={onClose} />
</ModalBottom>
</Modal>📝 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 AlertModal = ({ | |
| isOpen, | |
| onClose, | |
| title, | |
| children, | |
| size, | |
| }: PropsWithChildren<ModalInterface>) => { | |
| return ( | |
| <Modal isOpen={isOpen} size={size} onClose={onClose} title={title}> | |
| {children} | |
| <ModalBottom> | |
| <Button variant="confirm" size={size} onClick={onClose} /> | |
| </ModalBottom> | |
| </Modal> | |
| ); | |
| }; | |
| const AlertModal = ({ | |
| isOpen, | |
| onClose, | |
| title, | |
| children, | |
| size, | |
| }: PropsWithChildren<ModalInterface>) => { | |
| return ( | |
| <Modal isOpen={isOpen} size={size} onClose={onClose} title={title} | |
| closeOnBackdropClick={false} | |
| > | |
| {children} | |
| <ModalBottom> | |
| <Button variant="confirm" size={size} onClick={onClose} /> | |
| </ModalBottom> | |
| </Modal> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In components/src/components/Modal/AlertModal.tsx around lines 13 to 28, the
AlertModal component currently allows closing the modal by clicking the backdrop
because the Modal's closeOnBackdropClick prop defaults to true. To prevent
accidental closure of important alert messages, explicitly set
closeOnBackdropClick={false} on the Modal component to disable closing the modal
via backdrop clicks, similar to the ConfirmModal behavior.
| interface ModalInterface { | ||
| title?: string; | ||
| onClose: () => void; | ||
| isOpen: boolean; | ||
| onConfirm: () => void; | ||
| inputValue?: string; | ||
| onInputChange?: (e: React.ChangeEvent<HTMLInputElement>) => void; | ||
| size: "small" | "medium" | "large"; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
선택적 Props에 대한 타입 안전성 강화
inputValue와 onInputChange 속성이 옵셔널(?)로 정의되어 있지만, 실제 구현에서 이러한 값들이 제공되지 않을 경우에 대한 처리가 없습니다. 사용자가 이 속성들을 제공하지 않으면 런타임 오류가 발생할 수 있습니다.
다음과 같이 인터페이스를 수정하여 이러한 속성들을 필수로 만들거나, 구현 시 기본값을 제공하는 것이 좋습니다:
interface ModalInterface {
title?: string;
onClose: () => void;
isOpen: boolean;
onConfirm: () => void;
- inputValue?: string;
- onInputChange?: (e: React.ChangeEvent<HTMLInputElement>) => void;
+ inputValue: string;
+ onInputChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
size: "small" | "medium" | "large";
}또는 구현 부분에서 기본값을 설정하거나 조건부 렌더링을 추가하세요.
🤖 Prompt for AI Agents
In components/src/components/Modal/PromptModal.tsx around lines 5 to 13, the
optional props inputValue and onInputChange lack handling when not provided,
risking runtime errors. To fix this, either make these props required in the
ModalInterface or update the component implementation to provide default values
or add conditional checks before using them, ensuring safe usage regardless of
whether they are passed.
|
쵸파 안녕하세요~ |
bassyu
left a comment
bassyu
left a comment
There was a problem hiding this comment.
안녕하세요 수이!
의심도 하고, 새로운 시도도 하며 스스로의 기준을 잘 만들어 가시는 것 같네요~!
이번 미션은 여기서 머지하겠습니다!
다음 미션도 파이팅입니다! 💪 💪
| margin: auto; | ||
| `; | ||
|
|
||
| interface ModalInterface { |
2 participants
쵸파 안녕하세요! 주말 잘 보내셨나요?
요즘 감기가 유행하던데 조심하세용 저는 지금 콧물 땜에 정말 죽겟서요,,😵💫
step2도 리뷰 잘 부탁드립니다!
🎯 페이먼츠 모듈
이번 미션을 통해 다음과 같은 학습 경험들을 쌓는 것을 목표로 합니다.
1단계 - Module
2단계 - Refactoring
🕵️ 셀프 리뷰(Self-Review)
제출 전 체크 리스트
리뷰 요청 & 논의하고 싶은 내용
Prop 개수
수업 시간에 크루들이 Modal 컴포넌트 12개의 props를 보고 놀렸습니다,, 사실 저는 많은 props의 단점이 잘 체감되지 않습니다. 크루들이 단점으로 가독성 저하와 재사용성 감소 등을 꼽아줬는데, Modal을 사용할 때 특별히 가독성이 나쁘다고 느끼지 못했습니다. 기본 값이 존재하거나 옵션으로 제공되는 props가 대다수이기 때문에 스토리북에서 다양한 모달을 테스트할 때도 전달되는 prop은 많지 않았습니다. 다만 Modal 내부는 너무 많은 props로 인해 코드가 지저분합니다. 사실 Modal 컴포넌트 내부에서 컴포넌트를 분리하여 어느 정도 해소했고 직접 작성했기 때문에 저는 큰 불편함을 느끼지 못했습니다. 하지만 리뷰어의 입장에서는 다를 수 있을 것 같아 질문 드립니다! 놀림받을만 한가요,,?🥹
컴포넌트 조합 패턴
많은 크루들이 컴포넌트 합성 패턴을 사용한 것으로 알고 있습니다. 고민이었던 많은 props 문제를 해결할 수 있기도 해서 step2에서는 저도 사용해야 하나 고민했습니다. 확장성과 재사용성을 보장한다는 점이 좋아보였어요! 하지만 이번 미션은 요구하는 디자인이 한정적이고 라이브러리인 만큼 편의성에 초점을 두고 싶었습니다. alert 모달, confirm 모달, prompt 모달과 같이 각 요소의 위치 변동이 거의 없는 경우 매번 조합하여 사용하는 것이 오히려 불편할 것이라고 생각했습니다. 또한 컴포넌트 조합 패턴으로 보이는 구조보다 props로 전달하는 것이 어떤 요소가 포함되는지 더 직관적으로 표현된다고 생각했습니다. 제가 생각한 컴포넌트 합성 패턴의 장단점과 적용하지 않은 이유는 이 정도인데, 사실 컴포넌트 합성 패턴을 알아보기만 했지 직접 구현해보지 않아서 놓치고 있는 부분이 있는지 궁금합니다! 또 미션을 마무리하고(아니묜 쵸파의 의견에 따라 이번 미션에서) 적용해보면서 참고하면 좋을 부분이 있다면 알려주세요!
✅ 리뷰어 체크 포인트
1. 모달 컴포넌트의 유연성
2. 커스텀 훅의 역할과 모듈화
3. 모듈 배포