[Step3] hippo - 조건부 렌더링 활용 - #5
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/components/RestaurantDetailModal/RestaurantDetailModal.jsx`:
- Around line 5-10: Add proper dialog semantics to the modal container: give the
element rendered with className={styles.modal__container} a role="dialog" and
aria-modal="true", add an id on the title element (the h2 that renders
{restaurant.name}) and set aria-labelledby on the container to that title id
(e.g., "restaurant-detail-title-<unique>"). Ensure the id is unique per modal
(use restaurant.id or similar) and do not remove the existing backdrop onClick
handler (onClose) or visual classes.
In `@src/components/RestaurantList/RestaurantList.jsx`:
- Around line 10-14: 리스트 항목의 클릭 인터랙션(<li key={restaurant.id}
className={styles.restaurant} onClick={() => onRestaurantClick(restaurant)}>)이
마우스 전용이라 키보드 접근성이 없으니, 해당 <li>에 role="button"과 tabIndex={0}을 추가하고 onKeyDown 핸들러를
구현해 Enter 또는 Space 키를 누르면 onRestaurantClick(restaurant)을 호출(스페이스의 경우
preventDefault 포함)하도록 변경하세요; 필요하면 aria-label 또는 aria-haspopup 등 적절한 ARIA 속성으로 버튼
목적(모달 오픈)을 보강하세요.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b2f493c-8b06-436c-a11e-6e0948efa285
📒 Files selected for processing (6)
.coderabbit.yaml03-modal/README.mdREADME.mdsrc/App.jsxsrc/components/RestaurantDetailModal/RestaurantDetailModal.jsxsrc/components/RestaurantList/RestaurantList.jsx
| <div className={`${styles.modal} ${styles["modal--open"]}`}> | ||
| <div className={styles.modal__backdrop}></div> | ||
| <div className={styles.modal__backdrop} onClick={onClose}></div> | ||
| <div className={styles.modal__container}> | ||
| <h2 className={`${styles.modal__title} text-title`}>음식점 이름</h2> | ||
| <h2 className={`${styles.modal__title} text-title`}> | ||
| {restaurant.name} | ||
| </h2> |
There was a problem hiding this comment.
모달에 dialog 시맨틱을 추가해주세요.
현재는 스크린리더에 모달로 인식되지 않아 컨텍스트 전달이 약합니다. role="dialog", aria-modal, aria-labelledby를 연결해 주세요.
수정 예시
- <div className={`${styles.modal} ${styles["modal--open"]}`}>
+ <div
+ className={`${styles.modal} ${styles["modal--open"]}`}
+ role="dialog"
+ aria-modal="true"
+ aria-labelledby="restaurant-detail-title"
+ >
@@
- <h2 className={`${styles.modal__title} text-title`}>
+ <h2
+ id="restaurant-detail-title"
+ className={`${styles.modal__title} text-title`}
+ >
{restaurant.name}
</h2>🤖 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/RestaurantDetailModal/RestaurantDetailModal.jsx` around lines
5 - 10, Add proper dialog semantics to the modal container: give the element
rendered with className={styles.modal__container} a role="dialog" and
aria-modal="true", add an id on the title element (the h2 that renders
{restaurant.name}) and set aria-labelledby on the container to that title id
(e.g., "restaurant-detail-title-<unique>"). Ensure the id is unique per modal
(use restaurant.id or similar) and do not remove the existing backdrop onClick
handler (onClose) or visual classes.
| <li | ||
| key={restaurant.id} | ||
| className={styles.restaurant} | ||
| onClick={() => onRestaurantClick(restaurant)} | ||
| > |
There was a problem hiding this comment.
키보드 접근 가능한 클릭 인터랙션으로 바꿔주세요.
현재는 마우스 클릭만 처리되어 키보드 사용자(Enter/Space)가 식당 상세 모달을 열 수 없습니다.
수정 예시
<li
key={restaurant.id}
className={styles.restaurant}
+ role="button"
+ tabIndex={0}
onClick={() => onRestaurantClick(restaurant)}
+ onKeyDown={(e) => {
+ if (e.key === "Enter" || e.key === " ") {
+ e.preventDefault();
+ onRestaurantClick(restaurant);
+ }
+ }}
>📝 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.
| <li | |
| key={restaurant.id} | |
| className={styles.restaurant} | |
| onClick={() => onRestaurantClick(restaurant)} | |
| > | |
| <li | |
| key={restaurant.id} | |
| className={styles.restaurant} | |
| role="button" | |
| tabIndex={0} | |
| onClick={() => onRestaurantClick(restaurant)} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); | |
| onRestaurantClick(restaurant); | |
| } | |
| }} | |
| > |
🤖 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` around lines 10 - 14, 리스트
항목의 클릭 인터랙션(<li key={restaurant.id} className={styles.restaurant} onClick={() =>
onRestaurantClick(restaurant)}>)이 마우스 전용이라 키보드 접근성이 없으니, 해당 <li>에 role="button"과
tabIndex={0}을 추가하고 onKeyDown 핸들러를 구현해 Enter 또는 Space 키를 누르면
onRestaurantClick(restaurant)을 호출(스페이스의 경우 preventDefault 포함)하도록 변경하세요; 필요하면
aria-label 또는 aria-haspopup 등 적절한 ARIA 속성으로 버튼 목적(모달 오픈)을 보강하세요.
| function App() { | ||
| const [category, setCategory] = useState("전체"); | ||
| const [clickedRestaurant, setClickedRestaurant] = useState(null); | ||
| const isRestaurantDetailModalOpen = !!clickedRestaurant; |
There was a problem hiding this comment.
[배움]
clickedRestaurant가 있으면 모달을 열어야 한다는 의도가 JSX에서 바로 읽히네요! 파생 상태를 변수로 명시하는 패턴 저도 적용해보고 싶어요. !!로 boolean으로 명시적 변환하는 것도 처음 알았는데 깔끔한 것 같아요!
| <li | ||
| key={restaurant.id} | ||
| className={styles.restaurant} | ||
| onClick={() => onRestaurantClick(restaurant)} | ||
| > |
There was a problem hiding this comment.
[제안]
<li>에 onClick만 달면 키보드 사용자는 Tab으로 접근이 안 돼요. role="button", tabIndex={0}, onKeyDown(Enter/Space 처리)을 추가하면 키보드로도 동작해요. 저는 과거 코드 리뷰 피드백을 보고 이번에 추가해 봤는데, 유성님도 반영해보는 건 어떠신가요?
There was a problem hiding this comment.
Step4에서 <li>에 onClick 다는 방식에서 <li> 내부에 <button>을 추가하고 버튼에 onClick을 다는 방식으로 수정했습니다! 확인 부탁드려요.
|
|
||
| 파생 변수는 state와 달리 setter가 없어 동기화 버그가 생기지 않고, 렌더링마다 자동으로 최신값을 계산한다. | ||
|
|
||
| ### `&&` 조건부 렌더링 주의사항 |
There was a problem hiding this comment.
[배움]
저는 count && <Modal />이 0을 그대로 렌더링하는 케이스를 생각해보지 못했는데, !!로 명시적 boolean 변환을 하면 이런 오류를 방지할 수 있겠네요!
| 가장 고민이 된 부분은 `restaurant`를 state로 만들어야 하는가였다. `filteredRestaurants`로 계산할 수 있을 것 같아 state가 불필요하다고 생각했는데, 실제로는 두 값의 역할이 다르다. | ||
|
|
||
| - 개선: State를 부모 App 컴포넌트로 올렸다(state lifting). CategoryFilter는 선택된 카테고리를 props로 받아 표시만 하고, 변경 시 콜백 함수를 통해 부모에 알린다. 이제 App이 중앙에서 상태를 관리하고, RestaurantList에 필터된 데이터를 props로 전달한다. | ||
| - `filteredRestaurants` — 화면에 보여줄 **목록** → `category`에서 파생, state 불필요 | ||
| - `clickedRestaurant` — 모달에 보여줄 **선택된 하나** → 클릭 전까지 알 수 없으므로 state 필요 |
There was a problem hiding this comment.
[배움]
저도 같은 부분에서 헷갈렸는데 명확한 기준 없이 나눈 것 같아서 확신이 없었거든요. filteredRestaurants는 category가 정해지면 계산되는 반면, clickedRestaurant는 클릭 전엔 파생할 근거 자체가 없다는 정리를 읽고 확실히 이해가 됐습니다!
2 participants
개인 목표 달성 여부
리뷰어에게
Summary by CodeRabbit
릴리스 노트
새로운 기능
문서