Conversation
There was a problem hiding this comment.
Code Review
This pull request implements a restaurant listing application featuring category filtering, a detailed view modal, and a form for adding new restaurants. The feedback identifies several missing functional elements, such as the lack of click handlers for the modal backdrop and the header's add button, as well as a missing form submission handler to prevent page reloads. Additionally, there are suggestions to improve code maintainability by extracting hardcoded category lists into a shared constants file and replacing temporary comments with dynamic state logic.
| export default function RestaurantDetailModal({ restaurant, onClose }) { | ||
| return ( | ||
| <div className="modal modal--open"> | ||
| <div className="modal-backdrop"></div> |
There was a problem hiding this comment.
합당한 코멘트라 생각해요, PR에서 말씀하신 의도와는 다른 상황이니까요
그런데 꼭 backdrop을 눌렀을 때 모달의 창이 닫히도록 컴포넌트를 설계할지는 또 다른 이야기가 될 수 있어보이는데, 이 모달에서 backdrop을 눌렀을 때 창이 닫히도록 처리를 하려고 하시는 이유를 들어보고 싶어요.
그리고 뒤에 올 4, 5단계에서는 음식점 추가 모달도 추가로 관리하셔야 할 텐데, 이 모달은 backdrop을 누르면 닫히도록 구현하실 건지도 궁금해요
There was a problem hiding this comment.
templates/index.html의 modal-backdrop 마크업을 그대로 컴포넌트로 옮기다 보니 핸들러 추가를 놓쳤습니다. HTML 템플릿에는 이벤트 속성이 없으니 JSX로 옮길 때 따로 챙겨야 한다는 걸 빠뜨린 것 같습니다.
두 모달 모두 backdrop 클릭으로 닫도록 구현할 예정입니다. backdrop 영역을 클릭했을 때 모달이 닫히는 건 사용자가 자연스럽게 기대하는 동작이라 상세 모달과 추가 모달 모두 동일하게 가져가는 게 일관성 있다고 생각했습니다.
폼 입력 중 실수 클릭이 걱정될 수 있는데, 그건 backdrop을 막는 것보다 확인 다이얼로그 같은 방식으로 대응하는 게 더 나을 것 같습니다. (그만 입력할까요? 이런식으로)
| // <div className="modal modal--open"> --- IGNORE --- | ||
| // 개발 단계에서는 모달이 닫흰 상태이므로 modal--open 클래스를 임시제거 |
| <div className="modal-backdrop"></div> | ||
| <div className="modal-container"> | ||
| <h2 className="modal-title text-title">새로운 음식점</h2> | ||
| <form> |
| <option value="전체">전체</option> | ||
| <option value="한식">한식</option> | ||
| <option value="중식">중식</option> | ||
| <option value="일식">일식</option> | ||
| <option value="양식">양식</option> | ||
| <option value="아시안">아시안</option> | ||
| <option value="기타">기타</option> |
| return ( | ||
| <header className="gnb"> | ||
| <h1 className="gnb__title text-title">점심 뭐 먹지</h1> | ||
| <button type="button" className="gnb__button" aria-label="음식점 추가"> |
|
커밋 내역을 보았는데요! 만약 커밋할 때 AngularJS Git Commit Message Conventions 를 따르고 계신거라면, 문서에서 특히 하신 작업 중에는 코드의 로직은 그대로이면서 코드 품질 향상을 위해 가하신 수정, 일명 리팩터링 성격을 강하게 띄는 작업들이 많이 보였던 것 같아요, 이 경우에는 물론 의도이시면 괜찮다고 생각해요, 어쨌든 이건 컨벤션이고, 팀 혹은 개인마다 합의 하에 일부 원칙은 바꾸기도 하니까요 |
There was a problem hiding this comment.
@EM-H20 👋🏻
안녕하세요 의민님, 그리디에서 외부 리뷰어를 맡고 있는 김의천이라고 합니다. 반갑습니다!
3주차에서 뜻깊은 경험 하고 React 미션을 마주하게 되셨군요. React를 이전에 사용해 보셨다고 말씀하셨는데, 사실 그냥 JavaScript를 쓰는 거랑 사용법이 많이 다르기도 해서 전 완전 별개라고 생각했었거든요. 근데 의민님은 MVVM 패턴을 구현하시면서 그걸 자연스럽게 React를 비롯한 여러 라이브러리들이 사용하는 "상태" 와 연결 지으셨네요? 이렇게 이어지다니 뭔가 신기하군요 💪🏻
코드 전반적으로 살펴보았는데 굉장히 깔끔하게 잘 구현해 주신 것 같아서 제가 개선을 요청할 부분이 거의 없어보였어요, 어떤 점이 좋았냐면요:
- 아직 이번 단계에서 다루지 않는 음식점 추가 모달 컴포넌트를 제외했을 때 대부분의 컴포넌트들이 균형잡힌 분량으로 잘 분리되어 있었다고 생각해요(불필요한 중첩이나 과분리가 없다고 생각)
- 일관되고 정확한 의미의 변수명/함수명/컴포넌트명/핸들러가 사용되어 의미 및 기능 예측을 하기가 쉬웠다고 생각해요
- React로의 마이그레이션 중 잘못된 HTML 구조도 찾으시고 무작정 도입하는 것보다 검증 후 도입하신 점이 좋았어요
- 미션 요구사항에 없더라도 UX적인 부분을 생각하시고 리스트 항목에
cursor: pointer를 도입하신 점이 좋았어요- 사소한 변경이라고 간과하기 쉬운데 클릭하기 전의 사용자는 모달이 열릴 거란 걸 예상하기 어렵다고 보거든요, 그런데 마우스 포인터가 바뀌면 일단 리스트 항목에 마우스를 올린 사용자는 '클릭할 수 있다'는 메시지를 확실하게 받게 돼요
그래서 이번에 구현을 하신 의민님의 의도를 더 파헤치기 위해 대부분의 코멘트는 질문으로 적어보았어요. "왜" 를 탐구하는 것은 항상 중요하다고 생각해요.
추가 미션: css modules 조사 요청
수월하게 미션을 수행하신 것 같아서 하나 드려보고 싶은 요청이 있는데, css modules 도입에 대해서 고려해 보시는 것은 어떤가요? 미션에서는 선택사항이었는데 제 생각에는 의민님이 한 번 접해보셨으면 좋겠어서요.
JavaScript 코드와 HTML 요소들을 재사용에 용이하기 위해 컴포넌트로 분리하는 구조를 가져 보았잖아요. 근데 사실 분리하기 껄끄러운 게 하나 더 있는데 바로 CSS 스타일이에요. 그래서 CSS 스타일을 더 체계적으로 다루기 위한 방법들이 여러 가지 있는데 그 중 하나가 css modules거든요.
css modules에 대해 조사해 보시고, 필요성을 탐구해보셨으면 좋겠어요. 도입이 좋다고 생각되시면 도입해 보시는 것도 해 볼 수 있어 보이고, 도입하지 않기로 결정하셨다면 어떠한 이유로 도입을 결정하지 않으셨는지 알고 싶어요.
다음 리뷰 요청 시 css modules에 대한 이야기도 있으면 좋을 것 같아요!
그럼, 의민님의 의견을 들려주세요!
| const [category, setCategory] = useState("전체"); | ||
| const [isModalOpen, setIsModalOpen] = useState(false); | ||
| const [selectedRestaurant, setSelectedRestaurant] = useState(null); | ||
| const filteredRestaurants = | ||
| category === "전체" | ||
| ? restaurants | ||
| : restaurants.filter((restaurant) => restaurant.category === category); |
There was a problem hiding this comment.
전체적으로 굉장히 네이밍이 일관성 있고 좋네요 👍🏻
- 음식점 관련 변수들이 많음에도 의미를 적절히 더해주고 있고, 단수/복수형을 엄격하게 구별하여 적으셔서 이 변수들의 사용처를 보지 않아도 예측이 쉬웠던 것 같아요 (
restaurants,restaurant,selectedRestaurant,filteredRestaurants useState의 상태와set함수도 구별이 매우 잘 되고요isModalOpen의 경우is-접두사를 통해 boolean 변수라는 사실을 명확하게 드러내네요
빈틈없는 노련한 네이밍이라 봅니다
There was a problem hiding this comment.
ㅎㅎ 감사합니다. 전에는 잘 몰랐는데 미션을 거듭하고 리뷰를 받을 수록 신경쓰게 되는 것 같고 많은 것들을 배워가는 거 같습니다!
| @@ -0,0 +1,22 @@ | |||
| export default function CategoryFilter({ category, onChangeCategory }) { | |||
There was a problem hiding this comment.
여기에서는 핸들러 함수가 오는 prop으로 onChangeCategory라는 이름을 사용해주셨는데, onChange라는 이름을 사용했을 때와 비교했을 때 전하는 의미가 어떻게 달라진다고 생각하시나요?
이 사용처 외에도 어떤 사용처에는 순수하게 행위만 나타내는 이름을 사용해 주셨고, 또다른 사용처에는 행위 외에도 무엇이 대상인지도 나타내는 이름을 사용해 주셨더라고요. 의민님만의 기준이 있으신가요?
순수하게 행위만 나타내는 이름의 예: onClick, onChange
행위 외에 무엇이 대상인지도 나타내는 이름의 예: onChangeCategory, onRestaurantClick
There was a problem hiding this comment.
onChange는 뭐가 바뀐다는 행위만 담고, onChangeCategory는 카테고리가 바뀐다는 대상까지 같이 담고 있습니다.
제가 생각한 기준은, prop을 받아서 쓰는 쪽(App)의 시점에서 헷갈릴 여지가 있을 때 대상을 붙이는 방식이었습니다.
CategoryFilter 내부에서만 보면 그냥 onChange여도 충분하지만, App에서는 setCategory, setIsModalOpen, setSelectedRestaurant 같은 핸들러가 여러 개 있어서 구분이 되게 onChangeCategory로 썼습니다.
onRestaurantClick도 같은 이유입니다. 그냥 onClick이면 뭘 클릭하는 건지 App 입장에서 바로 안 보이니까요!
There was a problem hiding this comment.
오, 이렇게 의민님만의 일관된 기준을 가지고 컴포넌트들의 prop들을 네이밍하려고 하신 것은 좋은 접근이라고 생각해요. 코드를 다시 살펴봤는데 그 기준대로 잘 반영이 되어 있네요
다만 여기에서 한계점도 떠올라서 말씀드려 보고 싶은데, 바로 자식 컴포넌트의 prop은 이름 변경이 자유롭지가 않다, 그리고 컴포넌트의 사용처에 따라 어떤 부모가 올지 모를 수도 있다 같아요. 자식의 입장에서는 지금처럼 어떤 부모 컴포넌트가 올 지 정해지는 경우도 있지만, 나중에 요구사항의 변화에 따라 오는 부모가 달라지기도 하고, 심지어 여러 부모에서 자식 컴포넌트를 사용하기도 해요. 이 상황에서 자식 컴포넌트가 부모의 사용처에 따라 자신의 prop명을 바꾸는 것은 아무래도 구조적으로 어려운 상황이라 보아요.
그래서, 현재 React 컴포넌트 설계 시 많이 채용되고 있는 방식은
- 자식 컴포넌트는 사용처의 다양성에 대해 가능성을 열어두고 자신의 역할만 잘 표현하고
- 자식을 어떻게 쓸 지는 사용처인 부모 컴포넌트가 결정한다와 같은 형태
에요. 데이터의 흐름도 부모에서 자식으로 흐르는 형태라 이에 부합하는 경우가 많고요.
그래서 의민님이 우려하시는 자식 컴포넌트의 핸들러 prop 이름이 구체적이지 않아 식별이 어려운 문제는, 부모가 서로 다른 의도를 지니는 핸들러 함수를 주입해주는 식으로 해결하는 경우가 많아요. 아래의 경우 자식 컴포넌트인 <Button>은 onClick이라는 간단한 이름을 통해 "자기 자신을 클릭" 하는 행동에 반응하는 것임을 자식의 입장에서 간략하게 나타내고, 어떤 역할을 실질적으로 하는지는 부모가 제공하는 핸들러 함수에 의해 구분되고 있어요.
const Parent = () => {
/* ... */
return (
<>
<Button onClick={handleDeleteButtonClick}>삭제</Button>
<Button onClick={handleAddButtonClick}>추가</Button>
</>
);
};구체적으로:
- 자식 컴포넌트에 있는 핸들러의 prop명은 그 자식 컴포넌트를 기준으로 해당 핸들러가 자식 컴포넌트 내의 어떤 구성요소가 대상인지, 어떤 행동을 가리키는지 나타내는 데 사용하고
- 어떤 자식 컴포넌트와 상호작용하는지는, 부모 컴포넌트가 제공하는 핸들러 함수들의 기능 및 네이밍으로 식별해 보는 것
으로 접근을 많이 하고 있는 추세에요. 이 방법은 또 어떻게 생각하실지 궁금하네요, 알쏭달쏭한 부분이 많은 것 같아요 😄
| : restaurants.filter((restaurant) => restaurant.category === category); | ||
|
|
||
| return ( | ||
| <> |
There was a problem hiding this comment.
지금 사용하신 <>...</>과 <Fragment>...</Fragment>는 서로 어떤 관계가 있고, 차이점은 무엇일까요?
There was a problem hiding this comment.
There was a problem hiding this comment.
네! 둘 다 Fragment입니다. 유일한 차이점은 말씀하신 대로 key값을 명시할 수 있는가입니다. 우연히(?) 선택해주시긴 했지만 사용해 주신 의도가 JSX를 하나로 묶기 위한 것이 전부이니, <>...</>만으로 충분해 보이네요
| {isModalOpen && ( | ||
| <RestaurantDetailModal | ||
| restaurant={selectedRestaurant} | ||
| onClose={() => setIsModalOpen(false)} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
React에서 컴포넌트를 조건부 렌더링할 때 자주 쓰이는 conditional rendering이네요!
근데 겉으로만 보았을 때는 그냥 조건 && 컴포넌트가 전부인데, 조건이 true이면 컴포넌트가 반환되고, 조건이 false라면 컴포넌트가 반환되지 않는 이유가 무엇일까요? 🤔
if-else문과 같이 분기를 쳐서 컴포넌트를 반환하는 형태이면 조건을 통과하면 실행되는 '문'이 되니 이해가 되지만 조건 && 컴포넌트 와 같은 케이스는 알쏭달쏭해보여요
There was a problem hiding this comment.
[React.js] 리액트에서 자주 쓰는 if문 작성 패턴 5개
자주 쓰던 if문은 JSX의 return () 안에서는 사용이 불가능합니다. return 안에는 값을 반환하는 표현식(expression)만 올 수 있는데, if문은 문(statement)이라 값을 반환하지 않기 때문입니다.
return (
<div>
{if (isOpen) { <Modal /> }} {/* ❌ 문법 오류 */}
</div>
);&&는 문이 아니라 표현식입니다. A && B는 A가 falsy면 A를, truthy면 B를 반환합니다.
false && <Modal /> // → false 반환
true && <Modal /> // → <Modal /> 반환isModalOpen && <Modal />도 마찬가지로, 결국 false 또는 <Modal /> 중 하나의 값으로 평가됩니다. React는 false를 화면에 그리지 않으니 조건이 맞을 때만 컴포넌트가 나타나는 것입니다.
There was a problem hiding this comment.
표현식의 평가 결과에 따라 A, B를 반환한다라는 표현이 정말 정확하게 와닿네요.
지금 예시는 true / false인데, 여기에 null이나 undefined가 오기도 하거든요. 표현식이다 보니 이 상황에도 false를 반환하는 건가 싶을 수 있는데, 말씀하신 설명대로면 정확하게 표현식이 null, undefined를 반환한다는 것이니, 명확한 설명이라 생각해요. 물론 null, undefined 또한 화면에 그려지지 않으니 결과는 같고요.
여기에 더해 하나의 불편한 진실이 있는데, 0 은 falsy하기 때문에 0 && <Component />의 평가 결과는 0이 되는데, 0은 화면에 그려지지 않는 값이 아니기 때문에 의도와 다르게 화면에 0이 나타나는 불상사가 있어요.
그러니 만약 이후 개발에 conditional rendering을 사용하신다면 count && <Component />와 같은 예시는 특히 조심하시는 것을 추천드려요 😓 저도 당해본 적이 있어요
|
|
||
| function App() { | ||
| return <h1>Self-Paced React</h1>; | ||
| const [category, setCategory] = useState("전체"); |
There was a problem hiding this comment.
isModalOpen의 값을 변경할 때 isModalOpen = true와 같이 직접 대입하지 않고, 굳이 useState의 set함수(setIsModalOpen) 을 통해 변경하시는 이유가 무엇인가요?
어쩌면 3주차에서 MVVM 패턴을 채택하여 애플리케이션을 관리하시면서 상태 관리 로직을 설계하셨으니, 해당 미션에서의 경험과 관련지어 이야기해보실 수 있을 것 같아요. 저는 어떻게 체감하셨는지 궁금합니다.
There was a problem hiding this comment.
3주차 좀비 서바이벌에서 MVVM 패턴을 구현할 때, ViewModel에 notify()를 만들어서 상태가 바뀔 때마다 호출했습니다.
notify() {
this.listeners.forEach((callback) => callback());
}handleDrawCard, handleChoice, handleRestart 등 상태를 바꾸는 모든 함수 마지막에 this.notify()를 붙였고, 이게 없으면 모델 값이 바뀌어도 화면이 그대로였습니다.
setIsModalOpen이 하는 역할이 이겁니다. isModalOpen = true로 직접 바꾸는 건 notify() 없이 모델 값만 바꾸는 것과 같아서, React가 변경된 걸 모르고 화면이 안 바뀝니다. 그렇게기에 저는 useState를 사용했습니다!
There was a problem hiding this comment.
MVVM 때 원하셨던, 상태가 바뀌면 UI는 자동으로 변하게 둔다는 것을 이번 미션에도 잘 녹이신 것 같아요 👍🏻 상태 기반이 되려면 라이브러리에 자연스레 알리는 과정이 동반될 테고요.
저도 이 질문 고민해 보았는데, 직접 값을 할당하지 않는 이유로 하나 더 떠오른 건 React의 특징인, 상태 업데이트 시 "이전 상태와 새 상태를 비교" 하며 리렌더 여부를 판단하는 것이었어요. 직접 할당하지 않고 적절한 창구를 거치면 이전 값도 그대로 유지되어 있으니 비교에 관여할 수 있고요 ✔️
| export default function RestaurantDetailModal({ restaurant, onClose }) { | ||
| return ( | ||
| <div className="modal modal--open"> | ||
| <div className="modal-backdrop"></div> |
There was a problem hiding this comment.
합당한 코멘트라 생각해요, PR에서 말씀하신 의도와는 다른 상황이니까요
그런데 꼭 backdrop을 눌렀을 때 모달의 창이 닫히도록 컴포넌트를 설계할지는 또 다른 이야기가 될 수 있어보이는데, 이 모달에서 backdrop을 눌렀을 때 창이 닫히도록 처리를 하려고 하시는 이유를 들어보고 싶어요.
그리고 뒤에 올 4, 5단계에서는 음식점 추가 모달도 추가로 관리하셔야 할 텐데, 이 모달은 backdrop을 누르면 닫히도록 구현하실 건지도 궁금해요
| </div> | ||
|
|
||
| <div className="form-item form-item--required"> | ||
| <label htmlFor="name" className="text-caption"> |
There was a problem hiding this comment.
미션에서 제공한 탬플릿은 아래와 같이 for이 잘못 사용되고 있었는데, 잘 캐치해서 올바르게 변경하셨네요 👍🏻
<label for="name text-caption">이름</label>단순히 옮긴 것을 넘어 검증을 거치셨다는 것이 느껴집니다
There was a problem hiding this comment.
초록스터디
JSX
class -> className
for -> htmlFor
self closing tag
Fragment
{} 내에 쓸 수 있는 JS 식
React Component
기본 구조
export / import
에 잘 나와있어서 잘 골라 사용했습니다!
There was a problem hiding this comment.
말씀해주신 요구사항들은 일반 HTML 코드를 React로 옮길 때 기본적으로 지키실 사항이고요, 디테일하게 잡으신 부분은 for을 단순히 htmlFor로 바꾸신 것이 아닌 마이그레이션 전 for이 "name text-caption"으로 잘못된 값이 있었다는 걸 캐치하신 거라고 보아요.
for에는 id만 올 수 있는데, 마이그레이션 전의 코드는
<label for="name text-caption">이름</label>와 같이 클래스인 text-caption이 for에 같이 와 있어 무언가 클래스를 잘못해서 쓴 상황이 된 것이고, 이거를 의도에 맞게 올바른 구문으로 유추해보면,
<label for="name" class="text-caption">이름</label>을 떠올릴 수 있고, 이제 이걸 React로 마이그레이션하면
<label htmlFor="name" className="text-caption">이름</label>과 같이, 최종적으로 수정하신 사항이 되는 거죠.
for에 잘못된 값이 들어간 것을 눈치채시고 해결 방법을 유추해 마이그레이션으로 이끌어내신 점을 칭찬하고 싶었던 것입니다.
|
|
||
| return ( | ||
| <> | ||
| <Header /> |
There was a problem hiding this comment.
이 컴포넌트는 어쩌면 제목을 prop으로 받아 <Header title="점심 뭐먹지" />로 구현하셨을 수도 있어 보이는데, 닫혀 있는 구조로 구현하셨군요!
미션 예시에서는 <Header /> 이긴 한데, 만약 다시 prop 도입 여부에 대해 결정할 수 있다면 prop을 추가해 보실 건가요? 이유도 궁금해요
There was a problem hiding this comment.
추후에는 prop을 추가할 것 같습니다! 카테고리 필터에서 "전체"가 아닌 특정 카테고리를 선택했을 때, category state를 Header에 prop으로 넘겨서 선택한 카테고리를 헤더에 보여주는 방식도 괜찮을 것 같다는 생각이 듭니다!
There was a problem hiding this comment.
이후에는 확장하고 싶은 마음이 크시군요
사실, 이것도 정답이 없는 질문이긴 했습니다.
추후에 <Header /> 컴포넌트가 제목이 얼마나 자주 바뀔 거라 생각하시나요? 생각하기 나름이라고 생각합니다. 말씀하신 케이스처럼 특정 컴포넌트가 아예 동적으로 헤더 값을 바꾸고 있는 수준이라면, <Header />는 확장에 열려 있는 구조가 적합해 보일 것이고, 단순히 서비스명만 간간이 바꾸는 수준이면 <Header />에서 prop을 굳이 열어두지 않고 필요할 때마다 개발자가 직접 해당 컴포넌트에 들어가 제목을 변경하는 것으로도 충분할 것 같습니다.
전하고 싶었던 것은 어느 쪽도 나쁜 구조는 아니라는 것, 그 대신 컴포넌트의 사용 및 응용 범위에 따라 title을 변경할 수 있도록 열려 있는 구조로 둘지 아니면 그걸 막을지는 -- 개발자의 의도에 달렸다고 생각합니다
앞으로도 컴포넌트를 많이 설계하실 텐데, 의도를 생각해보시면 목적에 부합하는 좋은 컴포넌트들을 설계하실 수 있을 거라 생각합니다
| const categoryImages = { | ||
| 한식: categoryKorean, | ||
| 중식: categoryChinese, | ||
| 일식: categoryJapanese, | ||
| 양식: categoryWestern, | ||
| 아시안: categoryAsian, | ||
| 기타: categoryEtc, | ||
| }; |
There was a problem hiding this comment.
categoryImages 매핑 객체가 컴포넌트 함수 바깥에 선언되어 있는데, 만약 이걸 컴포넌트 함수 안에 넣었다면 어떤 차이가 있었을까요?
There was a problem hiding this comment.
함수 안에 넣으면 렌더링마다 새로운 객체가 생성됩니다. categoryImages는 렌더링과 상관없이 항상 같은 내용이라 함수 바깥에 두는 게 맞다고 생각했습니다.
There was a problem hiding this comment.
오 그렇습니다. 앞으로도 props에 영향을 안 받는, 즉 렌더링 여부와 상관없는 값을 다룬다면, 조심스레 함수 밖으로 뺄 수 있어보이는군요
|
@wzrabbit CSS Module에 대해서 찾아봤는데 저 블로그를 봤습니다 장점Css module 사용 시, 클래스 명 충돌이 X Css module은 Component 단위로 스타일을 적용할 때 유용하다. 현재 프로젝트에서 그리고 이런 식으로 컴포넌트별로 스타일을 관리하는 연습이 나중에 Tailwind CSS를 적용할 때도 도움이 될지 궁금하기도 합니다! |
There was a problem hiding this comment.
수고하셨습니다!
역시 질문 위주로 진행하다 보니 의견을 많이 나눌 수 있었던 것 같아요 ✅ 코멘트를 보니 의민님만의 기준도 있었던 것 같고, 이러이러한 작업을 하게 된 배경지식도 볼 수 있었던 것 같아요.
말씀해주신 대로, css modules라는 친구는 전역으로 적용되는 CSS의 한계를 개선해 나가고자 등장했어요. 아래 특징을 말씀해주셨는데
Css module은 Component 단위로 스타일을 적용할 때 유용하다.
=> 이에 따라 실수로 Css 클래스 이름이 다른 곳에서 중첩으로 사용될 걱정을 할 필요가 없다.
요 특징이 그대로 컴포넌트를 재사용하기 좋은가에 관여를 많이 하게 된다고 봅니다
CSS 클래스 이름이 실수로 겹칠 일이 많다는 것은 그만큼 컴포넌트를 사용함에 있어 부작용이 크다는 이야기도 되거든요
JavaScript 로직은 말끔하게 분리했는데 정작 스타일이 분리가 안 되면 반쪽만 분리된 컴포넌트가 될 수 있어보여요
css modules를 도입하게 되면 CSS 스타일을 한정된 영역에서만 사용할 수 있도록 scoping하니 재사용이 용이한 것이고요.
그리고 Tailwind 이야기도 하셨더라고요.
그리고 이런 식으로 컴포넌트별로 스타일을 관리하는 연습이 나중에 Tailwind CSS를 적용할 때도 도움이 될지 궁금하기도 합니다!
솔직히 접근 방식은 좀 다르다고 생각해요.
- css modules: 이미 위에서 이야기했지만, 기존 CSS를 컴포넌트 단위로 scoping하는 것이 이점이에요.
- tailwindcss: 이쪽은 재사용 가능한 유틸리티 클래스들을 마음껏 쓸 수 있는 것이 장점이에요. 미리 사전 정의된 유틸리티 클래스들을 여러 광범위한 컴포넌트에 사용할 수 있어요.
아래는 tailwindcss 사용 예시에요. 여러 유틸리티 클래스들을 나열해 빠르게 결과물을 만들 수 있어요.
<button className="bg-blue-500 text-white px-4 py-2 rounded">확인</button>그래서 "컴포넌트별로 스타일을 나누는 습관" 자체는 범용적으로 좋은 습관인데요, css modules와 tailwindcss와 목적을 가지고 연관성을 보자면 많이 높지는 않아요. 연결고리는 약한 것 같아요.
오히려 CSS-in-JS이라고, CSS 스타일들을 JavaScript 로직 안에서 정의하고, 런타임이나 빌드 타임에 고유한 클래스명을 자동 생성해주는 방법이 있는데, 이게 오히려 css modules랑 연관성이 커 보여요. css modules랑 동일하게 scoping 개념이기도 하고요. 아래는 대표적인 CSS-in-JS 라이브러리인 styled-components에서 버튼 컴포넌트를 만들어 본 예시에요. StyledButton에서 선언된 스타일들은 해당 버튼 내에만 영향을 주기에 재사용이 한결 편해요.
const StyledButton = styled.button`
background: blue;
color: white;
padding: 8px 16px;
`;
<StyledButton>확인</StyledButton>이렇듯 CSS 스타일들도 사용이 용이하도록 하기 위한 여러 방법이 있으니, 다양하게 탐구해 보시는 것도 좋을 것 같아요
| const categoryImages = { | ||
| 한식: categoryKorean, | ||
| 중식: categoryChinese, | ||
| 일식: categoryJapanese, | ||
| 양식: categoryWestern, | ||
| 아시안: categoryAsian, | ||
| 기타: categoryEtc, | ||
| }; |
There was a problem hiding this comment.
오 그렇습니다. 앞으로도 props에 영향을 안 받는, 즉 렌더링 여부와 상관없는 값을 다룬다면, 조심스레 함수 밖으로 뺄 수 있어보이는군요
| : restaurants.filter((restaurant) => restaurant.category === category); | ||
|
|
||
| return ( | ||
| <> |
There was a problem hiding this comment.
네! 둘 다 Fragment입니다. 유일한 차이점은 말씀하신 대로 key값을 명시할 수 있는가입니다. 우연히(?) 선택해주시긴 했지만 사용해 주신 의도가 JSX를 하나로 묶기 위한 것이 전부이니, <>...</>만으로 충분해 보이네요
| </div> | ||
|
|
||
| <div className="form-item form-item--required"> | ||
| <label htmlFor="name" className="text-caption"> |
There was a problem hiding this comment.
말씀해주신 요구사항들은 일반 HTML 코드를 React로 옮길 때 기본적으로 지키실 사항이고요, 디테일하게 잡으신 부분은 for을 단순히 htmlFor로 바꾸신 것이 아닌 마이그레이션 전 for이 "name text-caption"으로 잘못된 값이 있었다는 걸 캐치하신 거라고 보아요.
for에는 id만 올 수 있는데, 마이그레이션 전의 코드는
<label for="name text-caption">이름</label>와 같이 클래스인 text-caption이 for에 같이 와 있어 무언가 클래스를 잘못해서 쓴 상황이 된 것이고, 이거를 의도에 맞게 올바른 구문으로 유추해보면,
<label for="name" class="text-caption">이름</label>을 떠올릴 수 있고, 이제 이걸 React로 마이그레이션하면
<label htmlFor="name" className="text-caption">이름</label>과 같이, 최종적으로 수정하신 사항이 되는 거죠.
for에 잘못된 값이 들어간 것을 눈치채시고 해결 방법을 유추해 마이그레이션으로 이끌어내신 점을 칭찬하고 싶었던 것입니다.
| {isModalOpen && ( | ||
| <RestaurantDetailModal | ||
| restaurant={selectedRestaurant} | ||
| onClose={() => setIsModalOpen(false)} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
표현식의 평가 결과에 따라 A, B를 반환한다라는 표현이 정말 정확하게 와닿네요.
지금 예시는 true / false인데, 여기에 null이나 undefined가 오기도 하거든요. 표현식이다 보니 이 상황에도 false를 반환하는 건가 싶을 수 있는데, 말씀하신 설명대로면 정확하게 표현식이 null, undefined를 반환한다는 것이니, 명확한 설명이라 생각해요. 물론 null, undefined 또한 화면에 그려지지 않으니 결과는 같고요.
여기에 더해 하나의 불편한 진실이 있는데, 0 은 falsy하기 때문에 0 && <Component />의 평가 결과는 0이 되는데, 0은 화면에 그려지지 않는 값이 아니기 때문에 의도와 다르게 화면에 0이 나타나는 불상사가 있어요.
그러니 만약 이후 개발에 conditional rendering을 사용하신다면 count && <Component />와 같은 예시는 특히 조심하시는 것을 추천드려요 😓 저도 당해본 적이 있어요
|
|
||
| function App() { | ||
| return <h1>Self-Paced React</h1>; | ||
| const [category, setCategory] = useState("전체"); |
There was a problem hiding this comment.
MVVM 때 원하셨던, 상태가 바뀌면 UI는 자동으로 변하게 둔다는 것을 이번 미션에도 잘 녹이신 것 같아요 👍🏻 상태 기반이 되려면 라이브러리에 자연스레 알리는 과정이 동반될 테고요.
저도 이 질문 고민해 보았는데, 직접 값을 할당하지 않는 이유로 하나 더 떠오른 건 React의 특징인, 상태 업데이트 시 "이전 상태와 새 상태를 비교" 하며 리렌더 여부를 판단하는 것이었어요. 직접 할당하지 않고 적절한 창구를 거치면 이전 값도 그대로 유지되어 있으니 비교에 관여할 수 있고요 ✔️
|
|
||
| return ( | ||
| <> | ||
| <Header /> |
There was a problem hiding this comment.
이후에는 확장하고 싶은 마음이 크시군요
사실, 이것도 정답이 없는 질문이긴 했습니다.
추후에 <Header /> 컴포넌트가 제목이 얼마나 자주 바뀔 거라 생각하시나요? 생각하기 나름이라고 생각합니다. 말씀하신 케이스처럼 특정 컴포넌트가 아예 동적으로 헤더 값을 바꾸고 있는 수준이라면, <Header />는 확장에 열려 있는 구조가 적합해 보일 것이고, 단순히 서비스명만 간간이 바꾸는 수준이면 <Header />에서 prop을 굳이 열어두지 않고 필요할 때마다 개발자가 직접 해당 컴포넌트에 들어가 제목을 변경하는 것으로도 충분할 것 같습니다.
전하고 싶었던 것은 어느 쪽도 나쁜 구조는 아니라는 것, 그 대신 컴포넌트의 사용 및 응용 범위에 따라 title을 변경할 수 있도록 열려 있는 구조로 둘지 아니면 그걸 막을지는 -- 개발자의 의도에 달렸다고 생각합니다
앞으로도 컴포넌트를 많이 설계하실 텐데, 의도를 생각해보시면 목적에 부합하는 좋은 컴포넌트들을 설계하실 수 있을 거라 생각합니다
| @@ -0,0 +1,22 @@ | |||
| export default function CategoryFilter({ category, onChangeCategory }) { | |||
There was a problem hiding this comment.
오, 이렇게 의민님만의 일관된 기준을 가지고 컴포넌트들의 prop들을 네이밍하려고 하신 것은 좋은 접근이라고 생각해요. 코드를 다시 살펴봤는데 그 기준대로 잘 반영이 되어 있네요
다만 여기에서 한계점도 떠올라서 말씀드려 보고 싶은데, 바로 자식 컴포넌트의 prop은 이름 변경이 자유롭지가 않다, 그리고 컴포넌트의 사용처에 따라 어떤 부모가 올지 모를 수도 있다 같아요. 자식의 입장에서는 지금처럼 어떤 부모 컴포넌트가 올 지 정해지는 경우도 있지만, 나중에 요구사항의 변화에 따라 오는 부모가 달라지기도 하고, 심지어 여러 부모에서 자식 컴포넌트를 사용하기도 해요. 이 상황에서 자식 컴포넌트가 부모의 사용처에 따라 자신의 prop명을 바꾸는 것은 아무래도 구조적으로 어려운 상황이라 보아요.
그래서, 현재 React 컴포넌트 설계 시 많이 채용되고 있는 방식은
- 자식 컴포넌트는 사용처의 다양성에 대해 가능성을 열어두고 자신의 역할만 잘 표현하고
- 자식을 어떻게 쓸 지는 사용처인 부모 컴포넌트가 결정한다와 같은 형태
에요. 데이터의 흐름도 부모에서 자식으로 흐르는 형태라 이에 부합하는 경우가 많고요.
그래서 의민님이 우려하시는 자식 컴포넌트의 핸들러 prop 이름이 구체적이지 않아 식별이 어려운 문제는, 부모가 서로 다른 의도를 지니는 핸들러 함수를 주입해주는 식으로 해결하는 경우가 많아요. 아래의 경우 자식 컴포넌트인 <Button>은 onClick이라는 간단한 이름을 통해 "자기 자신을 클릭" 하는 행동에 반응하는 것임을 자식의 입장에서 간략하게 나타내고, 어떤 역할을 실질적으로 하는지는 부모가 제공하는 핸들러 함수에 의해 구분되고 있어요.
const Parent = () => {
/* ... */
return (
<>
<Button onClick={handleDeleteButtonClick}>삭제</Button>
<Button onClick={handleAddButtonClick}>추가</Button>
</>
);
};구체적으로:
- 자식 컴포넌트에 있는 핸들러의 prop명은 그 자식 컴포넌트를 기준으로 해당 핸들러가 자식 컴포넌트 내의 어떤 구성요소가 대상인지, 어떤 행동을 가리키는지 나타내는 데 사용하고
- 어떤 자식 컴포넌트와 상호작용하는지는, 부모 컴포넌트가 제공하는 핸들러 함수들의 기능 및 네이밍으로 식별해 보는 것
으로 접근을 많이 하고 있는 추세에요. 이 방법은 또 어떻게 생각하실지 궁금하네요, 알쏭달쏭한 부분이 많은 것 같아요 😄



안녕하세요, 김의천 리뷰어님! 프론트엔드 4기 홍의민입니다.
이전에 React를 잠깐 다뤄본 적이 있지만, 당시에는 useState가 정확히 어떤 역할을 하는지 제대로 이해하지 못한 채 클론 코딩 수준에 머물렀습니다. 그런데 3주차 미션인 좀비 서바이벌을 진행하면서 render, state, notify, listener를 활용해 MVVM 패턴을 직접 구현해 보았고, 상태 변화에 따라 화면이 갱신되는 구조를 손으로 만들어 본 경험이 이번 미션에서 큰 도움이 되었습니다.
그때 직접 설계했던 상태 관리 로직이 결국 React의 useState와 같은 맥락이라는 걸 체감하니, 이전에는 모호하게 느껴졌던 단방향 데이터 흐름이나 선언적 UI 구성 같은 개념들이 훨씬 자연스럽게 와닿았습니다. 이해 없이 따라 치는 느낌에서 논리를 알고 시작하는 느낌으로 바뀌어서 작업하는 내내 재밌었습니다.
이번 미션에서는 3단계 Event Handler까지 구현하였습니다.
1단계: /templates의 HTML을 기반으로 Header, CategoryFilter, RestaurantList, RestaurantItem, RestaurantDetailModal, AddRestaurantModal 컴포넌트로 분리하고, class → className, for → htmlFor 등 JSX 문법에 맞게 변환했습니다.
2단계: 음식점 데이터를 constants/restaurants.js로 분리하고, App에서 useState로 카테고리 상태를 관리하여 필터링된 목록을 RestaurantList에 props로 전달하도록 구현했습니다.
3단계: 음식점 아이템 클릭 시 isModalOpen과 selectedRestaurant 상태를 변경하여 RestaurantDetailModal을 조건부 렌더링하고, 닫기 버튼과 backdrop 클릭으로 모달을 닫도록 구현했습니다.
기타 변경사항
UX 개선을 위해 App.css:131에 cursor: pointer를 추가하여 클릭 가능한 요소에 포인터 커서가 표시되도록 했습니다.
AddRestaurantModal.jsx에서 1~3단계 미션 구현을 위해
<div className="modal modal--open">을
<div className="modal">로 임시 변경했습니다. CSS에서
.modal { display: none } / .modal--open { display: block }으로 제어하고 있어, 모달이 기본적으로 닫힌 상태를 유지하도록 한 조치입니다.
리뷰를 통해 부족한 부분을 개선하고 React에 대한 이해를 더 깊이 하고 싶습니다. 잘 부탁드립니다!
Directory Structure

view image



video
2026-04-07.2.29.27.mov