[1단계 - 페이먼츠 모듈] 마빈(김재영) 미션 제출합니다. #91
Conversation
Co-authored-by: mun-kyeong <[email protected]>
Co-authored-by: mun-kyeong <[email protected]>
Co-authored-by: mun-kyeong <[email protected]>
Co-authored-by: mun-kyeong <[email protected]>
Co-authored-by: mun-kyeong <[email protected]>
…CardNetwork function
…and modular components
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
components/src/App.tsx (1)
6-6: 초기 모달 상태 검토 필요모달의 초기 상태가
true로 설정되어 있어 페이지 로드 시 자동으로 모달이 열립니다. 일반적인 UX 패턴에서는 모달이 초기에 닫혀 있는 것이 더 일반적입니다. 의도적인 설계가 아니라면 초기값을false로 변경하는 것이 좋습니다.- const [isOpen, setIsOpen] = useState(true); + const [isOpen, setIsOpen] = useState(false);
🧹 Nitpick comments (7)
components/src/App.tsx (1)
14-23: 모달 위치와 타이틀의 불일치현재 모달의 위치는
position="bottom"으로 설정되어 있지만, 모달 헤더의 텍스트는 "중앙 모달 제목입니다"로 되어 있습니다. 이는 사용자에게 혼란을 줄 수 있습니다. 모달 위치와 헤더 텍스트를 일치시키는 것이 좋습니다.- <Modal.Header>중앙 모달 제목입니다</Modal.Header> + <Modal.Header>하단 모달 제목입니다</Modal.Header>components/src/lib/Components/Modal/Container.tsx (1)
5-17: 컨텍스트 사용 패턴이 잘 구현되었습니다컨텍스트 사용과 조건부 렌더링이 적절하게 구현되어 있습니다.
e.stopPropagation()을 사용하여 모달 내부 클릭이 외부로 전파되지 않도록 한 것이 좋습니다.다만, 더 나은 사용자 경험을 위해 컨텍스트가 없는 경우 에러 로깅 또는 개발 환경에서의 경고를 추가하는 것을 고려해 볼 수 있습니다:
const ctx = useContext(ModalContext); - if (!ctx) return null; + if (!ctx) { + if (process.env.NODE_ENV === 'development') { + console.warn('Modal.Container must be used within a Modal component'); + } + return null; + }components/src/lib/Components/Modal/Background.tsx (2)
6-7: 불필요한 주석"배경" 이라는 주석은 코드의 가독성을 위해 존재하는 것으로 보이지만, 함수명
Background가 이미 그 역할을 명확하게 설명하고 있어 불필요합니다. 자기 문서화(self-documenting) 코드 원칙에 따라 제거하는 것이 좋습니다.- // 배경 function Background({ children }: { children: React.ReactNode }) {
1-19: 개발 과정 분석커밋 기록을 살펴보면 개발자가 모달 컴포넌트를 구현하는 과정에서 다음과 같은 사고 흐름을 가졌음을 알 수 있습니다:
- 먼저 기본적인 모달 컨텍스트를 설계하여 공유 상태를 관리할 수 있는 기반을 마련
- 컴포넌트를 Background, Container, Header, Content로 분리하여 모듈화 설계
- 각 컴포넌트가 컨텍스트를 통해 상태를 공유하도록 구현
- 모달 위치 옵션(bottom/center)을 추가하여 유연성 부여
이는 컴포넌트 기반 아키텍처와 컨텍스트 API를 잘 활용한 접근 방식입니다. 다만, 컨텍스트의 null 체크와 같은 안전장치에 일관성이 부족한 점이 개선되면 더 견고한 컴포넌트가 될 것입니다.
React 생태계의 모범 사례에 따르면, 컴포넌트가 외부에서 제공된 컨텍스트에 의존할 때는 항상 컨텍스트의 존재 여부를 체크하는 것이 좋습니다. 이 점을 Background 컴포넌트에도 적용하면 좋겠습니다.
components/src/lib/Components/Modal/Modal.stories.tsx (2)
14-72: CenterModal 스토리에 잘 작성된 play 함수가 있습니다만, BottomModal 스토리에는 없습니다.CenterModal 스토리에서는 모달이 열리고 닫히는 세 가지 방법(닫기 버튼, 배경 클릭, ESC 키)을 자동화된 테스트로 잘 구현하셨습니다. 하지만 BottomModal 스토리에는 play 함수가 누락되어 있습니다. 일관성을 위해 BottomModal에도 유사한 play 함수를 추가하는 것이 좋을 것 같습니다.
play: async ({ canvasElement }) => { const triggerButton = canvasElement.querySelector( "#trigger-button-bottom" ) as HTMLButtonElement; await userEvent.click(triggerButton); // 여기에 BottomModal에 대한 테스트 추가... }또한 BottomModal과 CenterModal의 render 함수 간에 중복된 코드가 많습니다. 공통 로직을 추출하여 재사용 가능한 함수로 만들면 코드 중복을 줄일 수 있습니다.
39-71: 테스트 시나리오가 복잡할수록 명확한 주석이 필요합니다.현재 play 함수에서는 주석으로 닫기 방법을 설명하고 있지만, 각 테스트 시나리오의 시작과 기대 결과에 대한 주석이 더 명확하면 좋을 것 같습니다. 또한 테스트 코드에서 재사용 가능한 요소(예: waitFor와 expect 조합)를 함수로 추출하면 가독성이 향상될 것입니다.
다음과 같이 주석을 보강하는 것이 좋겠습니다:
// 시나리오 1: 모달 열기 및 내용 확인 await userEvent.click(triggerButton); await waitFor(() => expect(modalContainer).toHaveTextContent("중앙 모달 제목입니다")); // 시나리오 2: 닫기 버튼으로 모달 닫기 await userEvent.click(triggerButton); // 모달 다시 열기 const closeButton = document.body.querySelector("#modal-close-button") as HTMLImageElement; await userEvent.click(closeButton); await waitFor(() => expect(modalContainer).not.toBeInTheDocument());또한 반복되는 검증 로직을 함수로 추출하는 것도 고려해 보세요.
components/src/lib/Components/Modal/index.tsx (1)
16-25: ESC 키 이벤트 핸들러를 커스텀 훅으로 추출하면 재사용성이 향상됩니다.현재 ESC 키 이벤트 처리 로직이 모달 컴포넌트 내부에 직접 구현되어 있습니다. 이 로직을 별도의 커스텀 훅(
useEscapeKey)으로 추출하면 다른 컴포넌트에서도 재사용할 수 있고, 모달 컴포넌트의 복잡성도 줄일 수 있습니다.// useEscapeKey.ts import { useEffect } from 'react'; export const useEscapeKey = (isActive: boolean, callback: () => void) => { useEffect(() => { if (!isActive) return; const handler = (e: KeyboardEvent) => { if (e.key === "Escape") callback(); }; window.addEventListener("keydown", handler); return () => window.removeEventListener("keydown", handler); }, [isActive, callback]); }; // Modal 컴포넌트에서 사용: // useEscapeKey(isOpen, onClose);이렇게 하면 코드의 가독성과 재사용성이 향상됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (12)
components/src/App.tsx(1 hunks)components/src/lib/Components/Modal/Background.tsx(1 hunks)components/src/lib/Components/Modal/Container.tsx(1 hunks)components/src/lib/Components/Modal/Content.tsx(1 hunks)components/src/lib/Components/Modal/Header.tsx(1 hunks)components/src/lib/Components/Modal/Modal.stories.tsx(1 hunks)components/src/lib/Components/Modal/ModalBackground.module.css(1 hunks)components/src/lib/Components/Modal/ModalContainer.module.css(1 hunks)components/src/lib/Components/Modal/ModalContext.tsx(1 hunks)components/src/lib/Components/Modal/ModalHeader.module.css(1 hunks)components/src/lib/Components/Modal/index.tsx(1 hunks)components/src/lib/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- components/src/lib/Components/Modal/ModalBackground.module.css
- components/src/lib/Components/Modal/Content.tsx
- components/src/lib/Components/Modal/ModalHeader.module.css
- components/src/lib/Components/Modal/Header.tsx
- components/src/lib/Components/Modal/ModalContainer.module.css
🚧 Files skipped from review as they are similar to previous changes (1)
- components/src/lib/index.ts
🧰 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/App.tsxcomponents/src/lib/Components/Modal/ModalContext.tsxcomponents/src/lib/Components/Modal/Background.tsxcomponents/src/lib/Components/Modal/index.tsxcomponents/src/lib/Components/Modal/Container.tsxcomponents/src/lib/Components/Modal/Modal.stories.tsx
🧬 Code Graph Analysis (3)
components/src/lib/Components/Modal/Background.tsx (1)
components/src/lib/Components/Modal/ModalContext.tsx (1)
ModalContext(8-8)
components/src/lib/Components/Modal/index.tsx (1)
components/src/lib/Components/Modal/ModalContext.tsx (1)
ModalContext(8-8)
components/src/lib/Components/Modal/Container.tsx (1)
components/src/lib/Components/Modal/ModalContext.tsx (1)
ModalContext(8-8)
🔇 Additional comments (3)
components/src/lib/Components/Modal/ModalContext.tsx (1)
1-8: Modal 컨텍스트 구현이 깔끔합니다!타입 정의와 컨텍스트 생성이 명확하게 되어 있습니다. 모달의 위치 옵션을 제한된 리터럴 타입으로 정의한 것이 좋습니다.
components/src/lib/Components/Modal/index.tsx (2)
28-33: Modal 컴포넌트의 개발 패턴 분석모달 구현을 위해 React Portal을 사용하고 Context를 통해 하위 컴포넌트에 상태를 전달하는 패턴을 선택하셨네요. 이는 컴포넌트 합성을 통한 유연성을 제공하는 좋은 접근법입니다.
커밋 히스토리를 분석해 보면, 처음에는 기본적인 모달 구조를 구현한 후 점진적으로 기능을 추가하시는 개발 패턴을 보이십니다. 특히 컨텍스트 사용, 키보드 접근성 추가, 그리고 컴파운드 컴포넌트 패턴을 적용하는 과정에서 React의 모범 사례를 잘 따르고 계십니다.
향후 개발에서는 테스트 주도 개발(TDD)을 고려해보시고, 초기 단계에서 더 작은 단위로 기능을 분리하여 개발하면 코드의 유지보수성이 더욱 향상될 것입니다.
36-40: 컴파운드 컴포넌트 패턴이 잘 구현되었습니다.Modal 컴포넌트에 Background, Container, Header, Content 서브컴포넌트를 할당하는 컴파운드 컴포넌트 패턴이 잘 구현되었습니다. 이 패턴은 사용자가 모달의 구조를 직관적으로 이해하고 유연하게 구성할 수 있게 해줍니다.
추가 개선 사항으로, TypeScript의
as const타입 어설션과 함께 서브컴포넌트를 객체로 내보내는 방법도 고려해 볼 수 있습니다:const ModalComponents = { Background, Container, Header, Content, } as const; const Modal = Object.assign( ({ isOpen, onClose, position, children }: ModalProps) => { // 컴포넌트 구현... }, ModalComponents );이 접근 방식은 더 엄격한 타입 안전성을 제공하며, 동시에 서브컴포넌트의 구성을 더 명시적으로 만듭니다.
|
|
||
| // 배경 | ||
| function Background({ children }: { children: React.ReactNode }) { | ||
| const ctx = useContext(ModalContext)!; |
There was a problem hiding this comment.
컨텍스트 null 체크 누락
현재 코드는 non-null assertion operator (!)를 사용하여 컨텍스트가 항상 존재한다고 가정하고 있습니다. 그러나 ModalContext는 초기값이 null로 설정되어 있어, 컴포넌트가 Modal 외부에서 사용될 경우 런타임 오류가 발생할 수 있습니다.
Container.tsx와 같이 컨텍스트의 존재를 먼저 확인하는 패턴을 적용하는 것이 안전합니다.
- const ctx = useContext(ModalContext)!;
+ const ctx = useContext(ModalContext);
+ if (!ctx) return 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 ctx = useContext(ModalContext)!; | |
| const ctx = useContext(ModalContext); | |
| if (!ctx) return null; |
🤖 Prompt for AI Agents (early access)
In components/src/lib/Components/Modal/Background.tsx at line 8, the code uses a non-null assertion on ModalContext which can cause runtime errors if the context is null. Modify the code to first check if the context is null and handle that case safely, similar to the pattern used in Container.tsx, to prevent errors when the component is used outside of a Modal provider.
| export const BottomModal: Story = { | ||
| args: { | ||
| position: "bottom", | ||
| }, | ||
| render: function Render(args) { | ||
| const [isOpen, setIsOpen] = useState(false); | ||
| return ( | ||
| <> | ||
| <button | ||
| onClick={() => setIsOpen((prev) => !prev)} | ||
| id="trigger-button-bottom" | ||
| > | ||
| 모달창 trigger | ||
| </button> | ||
|
|
||
| <Modal {...args} isOpen={isOpen} onClose={() => setIsOpen(false)}> | ||
| <Modal.Background> | ||
| <Modal.Container> | ||
| <Modal.Header>하단 모달 제목입니다</Modal.Header> | ||
| <Modal.Content> | ||
| <p>하단 모달 내용 입니다.</p> | ||
| </Modal.Content> | ||
| </Modal.Container> | ||
| </Modal.Background> | ||
| </Modal> | ||
| </> | ||
| ); | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
BottomModal 스토리에 테스트 시나리오가 누락되었습니다.
CenterModal과 달리 BottomModal에는 play 함수가 없어 자동화된 테스트가 이루어지지 않습니다. 스토리북 테스트의 일관성과 완전성을 위해 BottomModal에도 유사한 테스트 시나리오를 추가하는 것이 좋습니다. 특히 bottom 위치에서의 사용자 경험 테스트는 center 위치와 다른 점이 있을 수 있으므로 중요합니다.
🤖 Prompt for AI Agents (early access)
In components/src/lib/Components/Modal/Modal.stories.tsx around lines 74 to 102, the BottomModal story lacks a play function for automated testing, unlike CenterModal. Add a play function to BottomModal that simulates user interactions such as clicking the trigger button to open and close the modal. This will ensure consistent and complete test coverage for the bottom-positioned modal's user experience.
| export interface ModalProps { | ||
| isOpen: boolean; | ||
| onClose: () => void; | ||
| position: "bottom" | "center"; | ||
| children: React.ReactNode; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
props에 z-index나 커스텀 스타일링을 위한 옵션이 추가되면 좋을 것 같습니다.
현재 Modal 컴포넌트는 isOpen, onClose, position 및 children props만 받고 있습니다. 하지만 실제 애플리케이션에서는 모달의 z-index를 조절하거나 추가적인 스타일링이 필요한 경우가 많습니다. className이나 style props, 또는 zIndex 같은 특정 스타일링 props를 추가하는 것을 고려해보세요.
export interface ModalProps {
isOpen: boolean;
onClose: () => void;
position: "bottom" | "center";
children: React.ReactNode;
zIndex?: number;
className?: string;
style?: React.CSSProperties;
}이렇게 하면 모달 컴포넌트의 유연성이 크게 향상될 것입니다.
🤖 Prompt for AI Agents (early access)
In components/src/lib/Components/Modal/index.tsx around lines 9 to 14, the ModalProps interface currently lacks options for custom styling such as z-index, className, or style. To improve flexibility, add optional props zIndex (number), className (string), and style (React.CSSProperties) to the ModalProps interface. This will allow users to control modal layering and apply custom styles as needed.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
components/README.md (4)
1-3: README 제목에 패키지 이름 반영 제안
현재 상단 제목이 “Modal 컴포넌트”로만 되어 있어, 실제 패키지 이름(@woowacourse/modal-component-marvin)이 드러나지 않습니다. 패키지 설치/검색 시 혼동을 줄이기 위해 제목을 다음과 같이 변경할 것을 제안합니다.- # Modal 컴포넌트 + # woowacourse-modal-component-marvin (Modal 컴포넌트)
5-10: 설치 방법에 Yarn 옵션 및 Peer Dependencies 추가를 권장
npm 외에도 Yarn 사용자들을 위해 설치 명령어를 함께 안내하고, React/React-DOM과 같은 필수 peer dependencies 정보를 명시하면 소비자가 오류를 줄일 수 있습니다.## 설치 방법 ```bash - npm i woowacourse-modal-component-marvin + npm i woowacourse-modal-component-marvin + # 또는 + yarn add woowacourse-modal-component-marvin+## Peer Dependencies
+- react (>=17.x)
+- react-dom (>=17.x)--- `47-50`: **스토리북 링크를 마크다운 링크로 변환 제안** 직접 URL 대신 `[링크 텍스트](URL)` 형태로 제공하면 가독성이 좋아집니다. 또한, 로컬에서 Storybook을 실행하는 명령어(예: `npm run storybook`)를 함께 안내해도 좋습니다. ```diff - https://step1--6812ddd3aa5be0c9ccb7572d.chromatic.com + [스토리북에서 확인하기](https://step1--6812ddd3aa5be0c9ccb7572d.chromatic.com)
59-65: 기능 목록에 접근성(Accessibility) 지원 항목 추가 제안
ESC 키로 닫기 외에도, 포커스 트랩이나 ARIA 속성 지원 등 접근성을 명시하면 더욱 완성도 높은 컴포넌트로 보입니다.- 키보드 지원 (ESC 키로 닫기) +- 접근성: 포커스 트랩(focus trap) 및 ARIA 속성 지원
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
⛔ Files ignored due to path filters (1)
components/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
components/README.md(1 hunks)components/package.json(3 hunks)components/src/lib/Components/Modal/index.tsx(1 hunks)components/src/lib/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/src/lib/index.ts
- components/src/lib/Components/Modal/index.tsx
- components/package.json
| ## 사용 방법 | ||
|
|
||
| ```tsx | ||
| import React, { useState } from "react"; | ||
| import { Modal } from "woowacourse-modal-component-marvin"; | ||
|
|
||
| function App() { | ||
| const [isOpen, setIsOpen] = useState(true); | ||
|
|
||
| const toggleModal = () => setIsOpen((prev) => !prev); | ||
| const closeModal = () => setIsOpen(false); | ||
|
|
||
| return ( | ||
| <> | ||
| <button onClick={toggleModal} id="trigger-button"> | ||
| 모달창 트리거 | ||
| </button> | ||
|
|
||
| <Modal isOpen={isOpen} onClose={closeModal} position="bottom"> | ||
| <Modal.Background> | ||
| <Modal.Container> | ||
| <Modal.Header>하단 모달 제목입니다</Modal.Header> | ||
| <Modal.Content> | ||
| <p>하단 모달 내용입니다.</p> | ||
| </Modal.Content> | ||
| </Modal.Container> | ||
| </Modal.Background> | ||
| </Modal> | ||
| </> | ||
| ); | ||
| } | ||
|
|
||
| export default App; | ||
| ``` |
There was a problem hiding this comment.
**사용 예제 코드 블록 종료() 누락** 코드 블록이 열렸지만 닫는 가 없어 마크다운 렌더링이 깨질 수 있습니다. 맨 마지막에 닫는 마커를 추가해주세요.
@@ export default App;
- export default App;
+ export default App;
+
+ ``` 📝 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.
| ## 사용 방법 | |
| ```tsx | |
| import React, { useState } from "react"; | |
| import { Modal } from "woowacourse-modal-component-marvin"; | |
| function App() { | |
| const [isOpen, setIsOpen] = useState(true); | |
| const toggleModal = () => setIsOpen((prev) => !prev); | |
| const closeModal = () => setIsOpen(false); | |
| return ( | |
| <> | |
| <button onClick={toggleModal} id="trigger-button"> | |
| 모달창 트리거 | |
| </button> | |
| <Modal isOpen={isOpen} onClose={closeModal} position="bottom"> | |
| <Modal.Background> | |
| <Modal.Container> | |
| <Modal.Header>하단 모달 제목입니다</Modal.Header> | |
| <Modal.Content> | |
| <p>하단 모달 내용입니다.</p> | |
| </Modal.Content> | |
| </Modal.Container> | |
| </Modal.Background> | |
| </Modal> | |
| </> | |
| ); | |
| } | |
| export default App; | |
| ``` | |
| ## 사용 방법 | |
🤖 Prompt for AI Agents (early access)
In components/README.md between lines 12 and 45, the example code block starts with triple backticks but is missing the closing triple backticks at the end. Add the closing ``` on a new line after the last line of the example code to properly close the code block and fix markdown rendering.
| ## Props | ||
|
|
||
| | Prop | Type | 필수 여부 | 기본값 | 설명 | | ||
| | -------- | -------- | --------- | -------- | ------------------------------------- | | ||
| | position | string | 아니오 | 'center' | 모달 위치 ('center', 'top', 'bottom') | | ||
| | isOpen | boolean | 예 | - | 모달 표시 여부 제어 | | ||
| | onClose | function | 예 | - | 모달이 닫힐 때 호출되는 콜백 함수 | | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Props 테이블에 children 항목 추가 및 타입 명세 구체화
현재 children prop이 누락되어 있고, onClose 타입도 function보다 () => void로 명세하는 편이 더 깔끔합니다. 아래와 같이 보완을 권장합니다.
| onClose | function | 예 | - | 모달이 닫힐 때 호출되는 콜백 함수 |
+| onClose | () => void | 예 | - | 모달이 닫힐 때 호출되는 콜백 함수 |
+| children | React.ReactNode | 예 | - | 모달 내부에 렌더링되는 콘텐츠 |Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents (early access)
In components/README.md around lines 51 to 58, the Props table is missing the `children` prop and the `onClose` prop type is too generic as `function`. Add a row for the `children` prop specifying its type (e.g., React.ReactNode) and update the `onClose` type to a more precise function signature like `() => void` for clarity and better documentation.
|
@coderabbitai 래빗! 9d176c0 이후의 커밋 내역을 읽고, |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@spoyodevelop 9d176c0 이후의 커밋 내역을 분석하여 카테고리별로 정리한 패치 노트입니다: 모달 컴포넌트기능 개선 (Feat)
리팩토링 (Refactor)
테스트 (Test)
훅 컴포넌트기능 개선 (Feat)
리팩토링 (Refactor)
테스트 (Test)
문서화 (Docs)
이 패치 노트는 9d176c0 이후의 주요 변경사항을 요약한 것으로, 실제 커밋 순서와 정확한 시간대는 다를 수 있습니다. 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 11268 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 54 🏁 Script executed: Length of output: 149 🏁 Script executed: Length of output: 642 @spoyodevelop Git 커맨드에서 9d176c0 이후의 커밋을 가져오지 못했습니다. 하지만 PR 정보와 코드 분석을 바탕으로 패치 노트를 작성해 드리겠습니다: 모달 컴포넌트기능 개선 (Feat)
리팩토링 (Refactor)
테스트 (Test)
훅 컴포넌트기능 개선 (Feat)
리팩토링 (Refactor)
테스트 (Test)
문서화 (Docs)
이 패치 노트는 PR의 전체 내용을 기반으로 작성되었으며, 실제 커밋 단위와 다를 수 있습니다. |
패치 노트모달 컴포넌트 관련 변경사항Refactor
Feat
Docs
훅 컴포넌트 관련 변경사항Refactor
Test
Fix
기타 변경사항Chore
이렇게 정리가 되었네요. 확실히 문서화 같은 잡일은 AI에게 주는것이 생산성에 좋은것 같습니다. 다만, 중요 요점을 다시 정리해볼께요. hook은 코드 베이스 80%를 가는 전반적인 리펙터를 진행했습니다. 상당 부분 많은 추상화를 진행했으며, 더욱더 견고한 테스트를 작성했습니다.
굉장히 좋은 조언 감사합니다! 그런데... 정확히 똑같이 일을 겪고 있는 사람이 이미 있고, 답변이 없는것을 보면... 참 이렇게 파편화된 정보들을 어떻게 가공해야 할지 궁금하긴 합니다. 중요한 점은 '공통되는 패턴'을 찾는것이라고 생각해요. 그것을 핵심적인 키워드로 정리해서 구글에 검색하면 대개 비슷한 문제를 가진사람들이 git에 issue를 올려주거나, stack overflow에 답변을 올려놓은 확률이 매우 크다고 생각합니다. 하지만, 그렇게 **'핵심적인 키워드'**를 뽑는 기량은, 훈련을 조금 더 해야 할것 같습니다.
굉장히 좋은 의견이에요. 우테코 미션자체가 굉장히 잘 설계가 되어있기 때문에, 이것만 따라가도 굉장히 많이 남는것이 많다고 생각합니다. 다만, 저도 요구사항을 무시하는 것은 아니지만, 이 질문의 요지는 '우테코에서는 얼마정도로 요구사항을 벗어나 자신의 해석을 담는것을 허용하는가?'에 대한 질문이라고 생각해보셨으면 해요. 야생에서는 '요구사항'을 정의하고, '의문이 가는 요구사항'에 대해서 소통하고, 의도를 파악하는것도 능력이라고 생각하기 때문이에요. 길이 깁니다! 그리고 두서없이 글을 적으셨다고 이야기를 했지만, 길고 사려 깊은 글은 제가 매우, 매우 좋아하는 소통 방식입니다. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/src/lib/Components/Modal/Background.tsx (1)
11-11: 컨텍스트 null 체크 누락현재 코드는 non-null assertion operator (
!)를 사용하여 컨텍스트가 항상 존재한다고 가정하고 있습니다. 그러나ModalContext는 초기값이null로 설정되어 있어, 컴포넌트가Modal외부에서 사용될 경우 런타임 오류가 발생할 수 있습니다.
ModalContext.tsx에서 초기값이null이기 때문에 안전한 접근 방식을 사용하는 것이 좋습니다.- const ctx = useContext(ModalContext)!; + const ctx = useContext(ModalContext); + if (!ctx) return null;
🧹 Nitpick comments (3)
components/src/lib/Components/Modal/Background.tsx (1)
1-23: 모달 시스템 내 컴포넌트 구조화에 대한 분석개발 과정에서 채택한 컴포넌트 분리 접근법은 효과적입니다. 모달의 각 부분(배경, 컨테이너, 헤더, 콘텐츠)을 별도의 컴포넌트로 분리하고 Context를 통해 상태를 공유하는 방식은 재사용성과 유지보수성을 높입니다.
다만, 개발 패턴에서 반복적으로 나타나는 특징으로:
- 컨텍스트 사용 시 안전성 검사 패턴이 일관되지 않음
- 명시적 타입 정의와 컴포넌트 프롭스 확장 패턴은 좋은 습관임
React 컴포넌트 설계 시, 아래 문서를 참고하시면 좋을 것 같습니다:
- 컨텍스트 사용 패턴: https://react.dev/reference/react/useContext
- 컴포넌트 합성 패턴: https://react.dev/learn/passing-props-to-a-component
이 컴포넌트가 더 안전하고 견고해질 수 있도록 null 체크 로직을 추가하고, 일관된 안전 패턴을 모든 컴포넌트에 적용하시길 권장합니다.
components/src/lib/Components/Modal/Content.tsx (2)
3-3: 빈 인터페이스 간소화 가능현재
ModalContentProps인터페이스는ComponentProps<"div">를 확장하지만 추가적인 속성을 정의하지 않고 있습니다. 코드를 더 간결하게 만들기 위해 직접ComponentProps<"div">타입을 사용하는 것을 고려해 보세요.- interface ModalContentProps extends ComponentProps<"div"> {} + type ModalContentProps = ComponentProps<"div">;하지만 이것은 작은 최적화이며, 미래에 이 인터페이스에 추가 속성을 정의할 계획이 있거나 다른 모달 컴포넌트와의 일관성을 유지하기 위한 의도가 있다면 현재 방식을 유지해도 좋습니다.
5-7: 컴포넌트 설계 접근법 분석개발 과정에서 보이는 사고방식은 "최소 책임 원칙"에 잘 부합합니다. 이
Content컴포넌트는 단순히 자식 요소를 렌더링하고 props를 전달하는 기본적인 역할만 수행합니다. 이러한 접근법은:
- 재사용성 증가: 컴포넌트가 특정 로직에 의존하지 않아 다양한 상황에서 재사용하기 쉬움
- 테스트 용이성: 단순한 컴포넌트는 테스트하기 쉬움
- 유지보수성 향상: 책임이 명확하게 분리되어 향후 수정이 용이함
이러한 패턴은 컴포넌트 라이브러리를 구축할 때 특히 효과적이며, 사용자가 필요에 따라 스타일링과 기능을 확장할 수 있는 유연성을 제공합니다.
한 가지 더 생각해볼 점은, 이 컴포넌트가 의미적으로 모달 내의 "콘텐츠" 역할을 한다는 것을 더 명확히 하기 위해 기본 클래스나 데이터 속성을 추가하는 것입니다. 이는 선택적 개선사항입니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
⛔ Files ignored due to path filters (1)
components/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
components/package.json(3 hunks)components/src/lib/Components/Modal/Background.tsx(1 hunks)components/src/lib/Components/Modal/Container.tsx(1 hunks)components/src/lib/Components/Modal/Content.tsx(1 hunks)components/src/lib/Components/Modal/Header.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/src/lib/Components/Modal/Header.tsx
- components/src/lib/Components/Modal/Container.tsx
- components/package.json
🧰 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/lib/Components/Modal/Content.tsxcomponents/src/lib/Components/Modal/Background.tsx
🧬 Code Graph Analysis (1)
components/src/lib/Components/Modal/Background.tsx (1)
components/src/lib/Components/Modal/ModalContext.tsx (1)
ModalContext(8-8)
|
d15da0b |
|
@coderabbitai TaskFrom this pull request’s code, identify exactly one place where the code is written in an imperative style, and explain how to make it more declarative. How to respondStart your response with the following markdown title: Then respond with:
StyleRespond in bullet points, and include only one clear improvement point. LanguagePlease write your response in Korean. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain1. ✨ 선언형 UI로서 개선할 피드백 포인트
🏁 Script executed: Length of output: 1132 🏁 Script executed: Length of output: 804 🏁 Script executed: Length of output: 713 1. ✨ 선언형 UI로서 개선할 피드백 포인트
|
|
@coderabbitai TaskFrom this pull request’s code, identify exactly one state value that can be derived from other state or props, and explain how removing it would simplify the code. How to respondStart your response with the following markdown title: Then respond with:
StyleRespond in bullet points, and focus on one state-related improvement. LanguagePlease write your response in Korean. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 13654 2. 📉 상태 최소화를 위한 개선 포인트
|
|
@coderabbitai TaskFrom this pull request’s code, identify exactly one component whose responsibility is unclear or mixed, and explain how to refactor it to separate concerns more clearly. How to respondStart your response with the following markdown title: Then respond with:
StyleRespond in bullet points, and provide clear before/after contrast. LanguagePlease write your response in Korean. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLooking at the code, I can identify a component with mixed responsibilities that would benefit from refactoring. I'll create a detailed analysis in Korean as requested: 3. 🧩 컴포넌트 책임 분리를 위한 리팩토링 포인트
🏁 Script executed: Length of output: 6098 3. 🧩 컴포넌트 책임 분리를 위한 리팩토링 포인트
|
|
@coderabbitai TaskFrom this pull request’s code, identify exactly one recurring mistake, anti-pattern, or code smell that appears in multiple places and seems to reflect the author's coding habit. How to respondStart your response with the following markdown title: Then respond with:
StyleRespond in bullet points, focused on identifying a single recurring behavioral pattern. LanguagePlease write your response in Korean. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'm checking the hooks and validation files to identify recurring code patterns. I'll analyze the structure and look for any anti-patterns. Hey @spoyodevelop, I'm currently looking for recurring code patterns in your hooks and validation files. Once I've analyzed the code, I'll provide specific feedback on any anti-patterns I identify. 🏁 Script executed: Length of output: 9297 4. 🔁 반복적으로 나타나는 안티패턴 또는 코드 습관 피드백
|
안녕하세요 온스타! 다시 만나게 되는군요! 이번에도 좋은 고민거리를 가져왔다고 생각해요. 다만, 이번에는 코드리뷰에 적극적으로 커멘트를 달고 조금 더 유연한 의사소통을 진행하려고 합니다. 이번에도 코드리뷰 잘 부탁 드립니다!
🎯 페이먼츠 모듈
이번 미션을 통해 다음과 같은 학습 경험들을 쌓는 것을 목표로 합니다.
1단계 - Module
🕵️ 셀프 리뷰(Self-Review)
제출 전 체크 리스트
리뷰 요청 & 논의하고 싶은 내용
1) 이번 단계에서 가장 많이 고민했던 문제와 해결 과정에서 배운 점
으.. 정말 끔찍한 방식으로
vite.config.js를 작성하는 법을 배웠어요.이번 단계에서 가장 큰 골칫거리는 라이브러리로 배포할 때 CSS 모듈과 에셋(이미지)을 안전하게 포함시키는 설정이었어요. Vite의 기본 빌드 설정만으로는 CSS가 사라지거나, 트리 쉐이킹에 걸리거나, CommonJS 번들에서 제대로 로드되지 않는 문제가 발생했죠.
어떻게든
index.ts에:을 하고, import된
styles가 트리 쉐이킹 과정에서 제거되지 않게 하려고 많은 노력을 했습니다. 그런데 어떻게 하던style줄이 사라지고, CSS import가 깨지는 현상이 발생했습니다.다음과 같은 방식들을 시도했어요:
sideEffects에*.css를package.json에 작성해서, 어떻게든 해당 코드를 트리 쉐이킹에 걸리지 않게 하는 방식계속 되었다가 말았다 해서, 결국
cssInjectedByJs롤업 플러그인을 사용해 JS에 인라인으로 삽입하도록 했습니다.굉장히 많이 고생한 내용이라고 생각해요.
이 부분을 어디서부터 공부하면 좋을지 ChatGPT에 물어보니, 1주 정도 학습하는 플랜을 짜주더라고요.
인터넷에 자료가 webpack과 Rollup의 진짜 '사용자 친화적이지 않은 문서'들 밖에 없어서, 어디서부터 공부를 시작해야 할지 고민이 많습니다. 어떤 학습 루트를 추천해 주실 수 있을까요?
2) 이번 리뷰를 통해 논의하고 싶은 부분
고민거리들이 많이 있는데, 이 뒤부터는 짧게 대답하셔도 무관합니다. 중요도에 따라 순서대로 작성했습니다.
1. 컴포넌트 무게와 유연성
모달을 작성할 때, 얼마나 다양한 사용성을 가정해야 할까요? 버튼의 위치, 크기, 모양을 props로 조정할 수 있게 하는 크루들도 있었고, 폰트 사이즈 같은 세부 스타일을 넘겨주는 크루도 있었어요. 저는 '필수적인 모달 동작'만 보증하면 컴포넌트의 책임을 다했다고 생각했고, 내부 state도 보관하지 않고 하위에서 상태를 내려받는 'dumb component'로 설계했습니다. 이 모달이 적절한 구성이라고 생각하시는지, 아니면 조금 더 모달에 책임을 부여하는 것이 맞다고 보시는지 궁금합니다.
2. CSS Reset 전략
대부분 프로젝트에서 CSS Reset은 필수인데, component 배포 시 Reset은 누가 관리해야 할지 고민이에요. 컴포넌트와 함께 Reset을 포함하여 배포해야 할까요? 아니면 Reset은 소비자가 별도로 적용하도록 안내하는 것이 좋을까요? Reset 때문에 자체 서식이 적용되는 태그(
h2같은 태그)를 사용하는 것을 기피하게 돼, 시맨틱 마크업 활용에도 제약이 생기는데, 어떻게 해결하면 좋을까요?3. 상태 관리 vs. 순수 유틸
useCardNumber훅이 꼭 훅일 필요가 있을까 고민했어요. validator 함수 형태의 순수 유틸로도 충분할 것 같다는 생각이 들었죠. 필요하다면 에러 메시지 등 상태는 소비자가 직접 관리하도록 하고요. 하지만 실제로 사용해 보니 상태가 있으면 확실히 편리해서, 훅 형태도 나쁘지 않다고 느꼈습니다. 이 정체성 혼란을 어떻게 접근하면 좋을까요?4. 테스트 코드 중복
RTL과 jest의 테스트 코드가 중복됩니다. 저는 , validation tool만 jest로 테스트 코드를 작성해도 좋다고 생각하지만, RTL을 쓰는게 요구 사항 이더군요. 혹시, 요구사항중에 '꼭 할필요는 없고 대체제가 있는데?'하는 요구사항이 있으면 어떻게 대처를 하시는 편인가요?
이상, 긴 본문 읽어주셔서 감사합니다. 비가 많이 내리네요. 감기 조심하시고, 즐거운 주말 보내세요, 온스타!
✅ 리뷰어 체크 포인트
1. 모달 컴포넌트의 유연성
2. 커스텀 훅의 역할과 모듈화
3. 모듈 배포