[Step3] cactus - 조건부 렌더링 활용 - #8
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: Organization UI 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 |
| export default function AddRestaurantModal() { | ||
| return ( | ||
| <div className={`${modalStyles.modal} ${modalStyles["modal--open"]}`}> | ||
| <div className={modalStyles.modal}> |
There was a problem hiding this comment.
step3에서 AddRestaurantModal은 아직 작업 범위가 아닌데 modal--open이 달려있어 화면에 계속 보여서 제거했습니다. step4에서 유성님 README를 보고 조건부 렌더링에서는 CSS 토글이 불필요하다는 걸 인지하고, CSS 토글 패턴 자체를 제거하면서 함께 수정했습니다!
| <li | ||
| className={styles.restaurant} | ||
| key={restaurant.id} | ||
| role="button" | ||
| tabIndex={0} | ||
| onClick={() => onRestaurantClick(restaurant)} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { | ||
| onRestaurantClick(restaurant); | ||
| } | ||
| }} | ||
| > |
There was a problem hiding this comment.
[배움]
클릭 가능한 <li>에 웹 접근성 속성 추가하신 부분이 인상 깊었습니다. 제 코드에도 적용해볼게요!
There was a problem hiding this comment.
저는 <li>에 직접 웹 접근성 속성을 추가해서 해결했는데, 유성님 코드 보니까 <button>으로 감싸는 방식으로 수정하셨더라고요! <button>이 키보드 접근성을 기본으로 제공해서 더 시맨틱하다고 생각해서 오히려 저도 배웠어요. 저도 적용해볼게요!
meteorqz6
left a comment
There was a problem hiding this comment.
React 공식 문서를 학습할 때, "동기화를 유지하지 않아도 되도록 불필요하고 중복된 state를 피하세요" 이 문장이 기억에 남습니다. 이번 미션을 구현할 때, 저는 이 부분에 집중했습니다! 기존 state 변수에서 일부 정보를 계산할 수 있다면 컴포넌트의 state에 해당 정보를 넣을 필요가 없다는 점에서 모달 열림/닫힘 상태를 clickedRestaurant 하나로 관리하는 선택은 옳은 선택이라고 생각합니다.
meteorqz6
left a comment
There was a problem hiding this comment.
W3C의 ARIA 디자인 패턴에 대해 찾아보았습니다. 첫 번째 원칙은 "가능하다면 ARIA 속성보다 네이티브 HTML 요소를 사용하라" 입니다. 이 원칙에 따르면 <button> 태그를 활용하는 방식으로 마크업을 변경하는 것이 더 적절한 선택이 될 것 같아요. <li> 태그 내부에 <button>을 꽉 차게 렌더링하는 방식으로 스터디 세션 때 리팩토링해보면 좋을 것 같습니다!
| setClickedRestaurant(restaurant); | ||
| }; | ||
|
|
||
| const handleModalClose = () => { |
There was a problem hiding this comment.
[제안]
handleModalClose -> handleDetailModalClose
개인 목표 달성 여부
&&연산자로clickedRestaurant가 있을 때만 모달을 렌더링했다.clickedRestaurant의 null 여부로 표현했다.리뷰어에게
<li>에 onClick을 달면서 웹 접근성을 위해role="button",tabIndex={0},onKeyDown을 추가했습니다. 이런 경우<li>대신<button>으로 마크업을 바꾸는 게 더 나은 선택일지 의견이 궁금합니다.clickedRestaurant하나로 관리했는데, 이 방식에 대해 피드백 부탁드립니다.