[Step1] cactus: styled-components 적용하기 - #2
Open
ehlung wants to merge 16 commits into
Open
Conversation
📝 WalkthroughWalkthroughCSS 모듈 기반의 스타일링을 styled-components로 전환하는 리팩토링이다. Changesstyled-components 마이그레이션
추정 코드 리뷰 난이도🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
|
@CodeRabbit review |
✅ Action performedReview finished.
|
meteorqz6
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/components/RestaurantList/RestaurantList.jsx (1)
13-13: ⚡ Quick win불필요한 빈 styled 컴포넌트 검토
RestaurantUl은 CSS 규칙이 정의되지 않은 빈 styled 컴포넌트입니다. 이 컴포넌트가 스타일링이 필요 없다면,<ul>태그를 직접 사용하거나ListContainer에 직접 포함시키는 것이 더 간결합니다. 원본 CSS 모듈(RestaurantList.module.css)에.ul규칙이 있었는지 확인하세요.♻️ 제안: RestaurantUl 제거 및 직접 ul 사용
const ListContainer = styled.section` display: flex; flex-direction: column; padding: 0 16px; margin: 16px 0; `; -const RestaurantUl = styled.ul``; +const RestaurantUl = styled.ul` + padding: 0; + margin: 0; + list-style: none; +`;또는 CSS reset이 별도로 처리되고 있다면 완전히 제거하고
<ul>태그 직접 사용도 고려하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/RestaurantList/RestaurantList.jsx` at line 13, The RestaurantUl styled component is empty with no CSS rules defined, which means it serves no styling purpose. Remove the RestaurantUl styled component declaration entirely and replace all instances where RestaurantUl is used as a component with standard HTML ul elements. This simplifies the code by eliminating unnecessary abstraction when no styling is being applied through the styled component.01-styled-components/README.md (1)
47-76: ⚡ Quick win구현 예시에 transient props 패턴 추가 고려
미션 요구사항(7번 라인)에서 styled-components 왜 사용하는지를 설명하도록 명시했지만, 제공되는 구현 예시는 기본
styled.button문법만 보여줍니다. 조건부 스타일링에서 props를 활용하는 패턴(예:$primarytransient props)을 추가하면 학습자가 더 현실적인 사용 사례를 이해할 수 있을 것입니다.예를 들어, 기존 CSS Module 방식과 styled-components 방식의 차이를 시각화할 수 있습니다:
// CSS Module 방식과의 비교 추가 const PrimaryButton = styled.button` background-color: ${props => props.$primary ? '`#ec4a0a`' : '`#ffffff`'}; color: ${props => props.$primary ? 'white' : '`#000000`'}; /* ... 기타 스타일 ... */ `; // 사용: props로 스타일 제어 <PrimaryButton $primary>Primary</PrimaryButton> <PrimaryButton>Secondary</PrimaryButton>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@01-styled-components/README.md` around lines 47 - 76, The current Button component example in the code snippet only demonstrates basic styled-components syntax without showing dynamic/conditional styling capabilities. Enhance the Button styled component to use the transient props pattern (like `$primary`) to conditionally apply different styles based on props passed to the component. Update the styled.button template literal to use `${props => props.$primary ? ... : ...}` expressions for background-color and color properties, and then show both usage examples (`<Button $primary>Click Me</Button>` and `<Button>Click Me</Button>`) to illustrate how this prop-based approach handles conditional styling more elegantly than CSS Modules, which directly addresses the mission requirement of explaining why styled-components is used.src/components/Modal/AddRestaurantModal.jsx (1)
127-127: ⚡ Quick win제출 버튼의
type을 명시해 주세요.공유
Button재사용 구조에서는 기본 submit 동작에 암묵적으로 의존하기보다 의도를 명시하는 편이 안전합니다.✏️ 제안 수정안
- <Button $primary>추가하기</Button> + <Button $primary type="submit">추가하기</Button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Modal/AddRestaurantModal.jsx` at line 127, The Button component in AddRestaurantModal does not have an explicit type attribute specified. Add the type prop to the Button component with value "submit" to explicitly declare its intention rather than relying on implicit default behavior. This makes the form submission behavior clear and safer when using shared reusable Button components.src/components/Modal/Modal.jsx (1)
27-51: 공유Button/ModalButtonContainer추출과$primarytransient prop 선택은 적절합니다.스타일 전용 prop이 DOM으로 전달되지 않아서 API 경계가 깔끔합니다. 현재 방향 유지해도 좋습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Modal/Modal.jsx` around lines 27 - 51, This is a positive review comment approving the current implementation. The extraction of the shared Button and ModalButtonContainer components is appropriate, and the use of the $primary transient prop (prefixed with $) is correct because it prevents style-only props from being passed to the DOM, maintaining a clean API boundary. No changes are required; the current direction is good to maintain.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 141: The README.md contains an inaccurate description of the refactoring
status. Currently it states that refactoring using the css helper is "planned"
(예정이다), but according to the PR context, the work has already been completed:
the src/styles/typography.js file exists with extracted style snippets like
textTitle, textSubtitle, textBody, and textCaption that components are already
importing and using. Update the statement in the README to reflect that this is
completed refactoring work rather than planned work, ensuring the documentation
accurately describes the current state of the codebase.
- Line 25: Line 25 of README.md incorrectly states that styled-components
generates unique class names at "compile time" (컴파일 시). This contradicts the
correct explanation on line 28 which states that styled-components generates
unique class names at "runtime" (런타임에 컴포넌트마다 고유 클래스명을 생성). Replace the phrase
"컴파일 시" with "런타임" in line 25 to accurately describe how styled-components works
and to maintain consistency with the Scoped Styling explanation on line 28. This
change is important because styled-components' key advantage is its runtime
flexibility, which differs from the compile-time approach of CSS Modules.
In `@src/components/Header/Header.jsx`:
- Around line 39-40: The GnbButtonImg component on line 40 is missing an alt
attribute, which can cause accessibility warnings. Since the button's accessible
name is already provided by the aria-label on line 39, add alt="" to the
GnbButtonImg component to mark the image as decorative and prevent unnecessary
exposure to assistive devices.
In `@src/components/Modal/Modal.jsx`:
- Around line 53-61: The Modal component is missing core accessibility features
required for proper screen reader support and keyboard navigation. Add the
role="dialog" and aria-modal="true" attributes to the ModalContainer element.
Generate a unique ID for the ModalTitle element and connect it to ModalContainer
using aria-labelledby to establish the accessible name relationship. Implement a
useEffect hook to listen for the Escape key press and call onClose when the user
presses Escape, ensuring the effect cleans up its event listener on unmount.
These changes will ensure the Modal component provides proper semantic structure
and keyboard interaction support for all users.
---
Nitpick comments:
In `@01-styled-components/README.md`:
- Around line 47-76: The current Button component example in the code snippet
only demonstrates basic styled-components syntax without showing
dynamic/conditional styling capabilities. Enhance the Button styled component to
use the transient props pattern (like `$primary`) to conditionally apply
different styles based on props passed to the component. Update the
styled.button template literal to use `${props => props.$primary ? ... : ...}`
expressions for background-color and color properties, and then show both usage
examples (`<Button $primary>Click Me</Button>` and `<Button>Click Me</Button>`)
to illustrate how this prop-based approach handles conditional styling more
elegantly than CSS Modules, which directly addresses the mission requirement of
explaining why styled-components is used.
In `@src/components/Modal/AddRestaurantModal.jsx`:
- Line 127: The Button component in AddRestaurantModal does not have an explicit
type attribute specified. Add the type prop to the Button component with value
"submit" to explicitly declare its intention rather than relying on implicit
default behavior. This makes the form submission behavior clear and safer when
using shared reusable Button components.
In `@src/components/Modal/Modal.jsx`:
- Around line 27-51: This is a positive review comment approving the current
implementation. The extraction of the shared Button and ModalButtonContainer
components is appropriate, and the use of the $primary transient prop (prefixed
with $) is correct because it prevents style-only props from being passed to the
DOM, maintaining a clean API boundary. No changes are required; the current
direction is good to maintain.
In `@src/components/RestaurantList/RestaurantList.jsx`:
- Line 13: The RestaurantUl styled component is empty with no CSS rules defined,
which means it serves no styling purpose. Remove the RestaurantUl styled
component declaration entirely and replace all instances where RestaurantUl is
used as a component with standard HTML ul elements. This simplifies the code by
eliminating unnecessary abstraction when no styling is being applied through the
styled component.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c24a35b6-e8bf-4d47-9edf-5d4471d9ffa5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
01-styled-components/README.md05-effects/README.mdREADME.mdpackage.jsonsrc/App.csssrc/components/CategoryFilter/CategoryFilter.jsxsrc/components/CategoryFilter/CategoryFilter.module.csssrc/components/Header/Header.jsxsrc/components/Header/Header.module.csssrc/components/Modal/AddRestaurantModal.jsxsrc/components/Modal/AddRestaurantModal.module.csssrc/components/Modal/Modal.jsxsrc/components/Modal/Modal.module.csssrc/components/Modal/RestaurantDetailModal.jsxsrc/components/RestaurantList/RestaurantList.jsxsrc/components/RestaurantList/RestaurantList.module.csssrc/styles/typography.js
💤 Files with no reviewable changes (7)
- 05-effects/README.md
- src/components/Modal/AddRestaurantModal.module.css
- src/components/CategoryFilter/CategoryFilter.module.css
- src/components/Header/Header.module.css
- src/App.css
- src/components/Modal/Modal.module.css
- src/components/RestaurantList/RestaurantList.module.css
aria-label이 버튼의 접근 가능한 이름을 이미 제공하므로, 장식용 아이콘 이미지는 스크린리더가 건너뛰도록 alt를 빈 문자열로 명시.
backdrop 클릭으로만 닫히던 모달에 키보드 사용자를 위한 Escape 닫기를 추가하고, role="dialog", aria-modal, aria-labelledby로 스크린리더에 모달 정보를 전달.
장식용 이미지 alt 처리 개념을 학습 내용에 추가.
meteorqz6
reviewed
Jun 16, 2026
Comment on lines
+28
to
+52
| export const ModalButtonContainer = styled.div` | ||
| display: flex; | ||
| `; | ||
|
|
||
| export const Button = styled.button` | ||
| width: 100%; | ||
| height: 44px; | ||
| margin-right: 16px; | ||
| border: none; | ||
| border-radius: 8px; | ||
| cursor: pointer; | ||
|
|
||
| ${(props) => | ||
| props.$primary && | ||
| ` | ||
| background: var(--primary-color); | ||
| color: var(--grey-100); | ||
| `} | ||
|
|
||
| &:last-child { | ||
| margin-right: 0; | ||
| } | ||
|
|
||
| ${textCaption} | ||
| `; |
Contributor
There was a problem hiding this comment.
[질문]
Modal.jsx가 자신의 렌더링에서 사용하지 않는 스타일을 export하는 구조인데 어떤 의도로 이 구조를 선택했는지 궁금합니다!
Contributor
Author
There was a problem hiding this comment.
AddRestaurantModal과 RestaurantDetailModal에 Button, ModalButtonContainer가 거의 동일하게 중복 정의되어 있어서 공통화하고 싶었습니다. 둘 다 Modal을 감싸서 사용하는 컴포넌트라 Modal.jsx가 두 모달의 공통 상위 개념이라고 보고, 거기서 export해서 양쪽에서 import하는 방식을 택했습니다.
|
|
||
| const GnbTitle = styled.h1` | ||
| ${textTitle} | ||
| color: #fcfcfd; |
Contributor
There was a problem hiding this comment.
[제안]
하드코딩된 색상을 App.css :root에 정의를 하면 의미 전달, 단일 진실 공급원 관점에서 더 좋을 것 같습니다.
Contributor
Author
There was a problem hiding this comment.
좋은 제안 감사합니다. 하드코딩된 색상값을 App.css :root에 --grey-50, --grey-150로 추가하고, var()로 참조하도록 수정했습니다. CategoryFilter의 #d0d5dd도 기존 --grey-200과 동일한 값이라 함께 교체했습니다.
meteorqz6
reviewed
Jun 16, 2026
Comment on lines
+59
to
+61
| const TextCaption = styled.label` | ||
| ${textCaption} | ||
| `; |
Contributor
There was a problem hiding this comment.
[제안]
label {
color: var(--grey-400);
${textCaption}
}
스타일이 중복돼서 하나를 삭제하는 방향으로 개선이 필요할 것 같아요!
Contributor
Author
There was a problem hiding this comment.
확인했습니다. FormItem의 label { } 선택자가 이미 모든 자식 label에 스타일을 적용하고 있어서, TextCaption을 별도 styled component로 만들 필요가 없었네요! TextCaption을 제거하고 일반 <label> 태그로 교체했습니다.
Comment on lines
+35
to
+41
| select { | ||
| height: 44px; | ||
| padding: 8px; | ||
| border: 1px solid var(--grey-200); | ||
| border-radius: 8px; | ||
| color: var(--grey-300); | ||
| } |
Contributor
There was a problem hiding this comment.
[제안]
input,
select {
padding: 8px;
margin: 6px 0;
border: 1px solid var(--grey-200);
border-radius: 8px;
font-size: 16px;
}
select에 중복되는 스타일이 있어서 확인 부탁드려요.
Contributor
Author
There was a problem hiding this comment.
input, select, textarea를 하나의 selector로 묶어 공통 속성을 정리하고, textarea와 select에는 고유한 속성만 남기도록 수정했습니다. 확인해주셔서 감사합니다!
meteorqz6
reviewed
Jun 16, 2026
Contributor
There was a problem hiding this comment.
[배움]
저는 font-size, line-height, font-weight 값을 각 컴포넌트에 직접 작성했는데, css 헬퍼로 공통 타이포그래피를 추출하면 한 곳에서 관리할 수 있다는 이점이 있네요!
Contributor
|
저는 같은 고민을 하다가 현재 규모에서는 각 모달이 자신의 Button을 직접 정의하는 방향으로 결론 냈어요. 두 모달에 걸쳐 10줄 정도의 중복이 생기지만, 각 파일이 자신에게 필요한 것만 갖는 구조가 더 명확하다고 판단했습니다. 나중에 버튼 변형이 늘어나거나 모달 외 다른 곳에서도 쓰인다면 그때 |
Contributor
Author
|
말씀해주신 대로 각 모달이 자신의 |
Modal.jsx가 자신이 렌더링하지 않는 스타일을 export하던 구조가 파일 책임을 불명확하게 만든다는 리뷰 피드백을 반영해 되돌림. AddRestaurantModal의 select/textarea 중복 스타일과 TextCaption 중복 선언도 함께 정리.
App.css :root에 --grey-50, --grey-150을 추가하고 Header, RestaurantList의 하드코딩된 색상값을 var()로 교체. CategoryFilter의 #d0d5dd는 기존 --grey-200과 동일해 함께 교체.
Button/ModalButtonContainer 위치 재조정, Modal 접근성 보강, 색상 디자인 토큰화, 중복 스타일 정리 과정을 리팩토링 섹션에 추가.
meteorqz6
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
개인 목표 달성 여부
styled-components를 사용하는 이유와 CSS 파일 방식, CSS Module과의 trade-off는 README 학습 내용 섹션에 정리했습니다.
리뷰어에게
css헬퍼로 공통 typography를 추출하고,Button/ModalButtonContainer는 styled component를 export하는 방식으로 중복을 제거했습니다. 더 나은 구조가 있다면 피드백 부탁드립니다.$required,$primary같은 transient props 방식이 적절한지 봐주세요.Summary by CodeRabbit
Release Notes
Refactor
Dependencies