[FE] fix: 모아보기 페이지 버그 수정 #858
Conversation
| @@ -114,7 +114,7 @@ const HighlightEditor = ({ questionId, answerList }: HighlightEditorProps) => { | |||
| <EditSwitchButton isEditable={isEditable} handleEditToggleButton={handleEditToggleButton} /> | |||
| </S.SwitchButtonWrapper> | |||
| {[...editorAnswerMap.values()].map(({ answerId, answerIndex, lineList }) => ( | |||
| <div | |||
| <S.LineListWrapper | |||
There was a problem hiding this comment.
AnswerList라는 ul 태그인 스타일 컴포넌트([edtiorAnswerMap.values()].map의 부모)를 만들고, Answer라는 li태그 스타일 컴포넌트를 만드는게
시맨틱 구조로 좋을 것 같습니다
There was a problem hiding this comment.
시맨틱 태그를 사용해서 구조를 수정했어요
| import { GroupedReview } from '@/types'; | ||
| import { substituteString } from '@/utils'; | ||
|
|
||
| import ReviewEmptySection from '../../components/common/ReviewEmptySection'; |
There was a problem hiding this comment.
절대경로 사용 추천드립니다
component/common 의 index.tsx 사용하면 해당 경로 import 들 다 같이 묶을 수 있어요
There was a problem hiding this comment.
common으로 옮기면서 index를 빼먹었네요! 수정했습니다
| @@ -19,6 +25,37 @@ const ReviewCollectionPage = () => { | |||
| const [selectedSection, setSelectedSection] = useState<DropdownItem>(dropdownSectionList[0]); | |||
| const { data: groupedReviews } = useGetGroupedReviews({ sectionId: selectedSection.value as number }); | |||
|
|
|||
| const { revieweeName, projectName, totalReviewCount } = useReviewInfoData(); | |||
There was a problem hiding this comment.
useReviewInfoData가 api요청을 하는 훅이라
여기서 부르면 목록 페이지에서 부른 api가 또 불러져서, 다른 방법을 논의해보는 것이 좋을 것 같습니다
There was a problem hiding this comment.
바다가 남겨준 코멘트에 대한 코멘트를 남기겠습니다ㅋㅋㅋ
There was a problem hiding this comment.
EssentialPropsWithChildren 에서 children이 props를 받을 수 있도록 해보는 거 시도해보면 어떨까요? 이게 될지는 모르겠지만 좋은 시도가 될 것 같아요!
// 특정 컴포넌트 타입을 children으로 받기 위해 제너릭을 활용한 예시
const ReviewDisplayLayout: React.FC<EssentialPropsWithChildren<ReviewDisplayLayoutProps, (props: ChildrenProps) => EmotionJSX.Element>> = ({ isReviewList, children }) => {
return (
<div>
{children({ isReviewList })} {/* children을 함수로 호출 */}
</div>
);
};
There was a problem hiding this comment.
다들 지금은 현행 방법+요 PR대로 가고 나중에 리팩토링하는 건 어떻게 생각하시나용
There was a problem hiding this comment.
받은 답변이 없을 때 UI 처리에 대해 코멘트 남겼어요!~~
그리고 ReviewDisplayLayout 구조에 대해 다소 장문의😌 코멘트도 남깁니다
훅을 사용해 데이터를 가져오는 방식 자체는 좋다고 생각합니다. 최대한 Layout 코드를 건드리지 않으면서 데이터를 받기 위한 최선의 방식을 선택했다고 느껴졌어요!
다만 현재 페이지 구조를 봤을 때 ReviewInfoData의 흐름이 끊긴다는 느낌을 받았어요.
ReviewDisplayLayout에서 API를 호출해 정보를 받아온 뒤 그 데이터를 그 자식인 ReviewInfoSection에 내려주고 있습니다.
하지만 children으로 들어오는 모아보기 페이지에서는 PR 본문대로 데이터를 내려주기 어려운 문제가 있습니다.
children으로 올 수 있는 페이지가 명확히 정해진 상황에서 위와 같은 이유로 데이터를 props로 전달할 수 없다는 게 어색하게 느껴졌습니다.
레이아웃 컴포넌트는 여태까지의 공통 컴포넌트와 달리 특정 페이지 형식과 연관이 깊기 때문에 이 레이아웃이 어떤 페이지를 다루는지, 어떤 데이터를 갖고 있는지를 props 넘겨주기를 통해 드러내주는 게 가독성이 좋다고 생각했습니당
그래서 리팩토링 방향을 크게 두 가지 생각해봤어요 (지금은 넘 바빠서ㅋㅋㅋ리팩토링은 레벨 5에서 차차 하는 것으로 생각했어요. 어차피 제가 만든 레이아웃이기도 해서😂)
ReviewDisplayLayout을 정말 뼈대만 제공하는 컴포넌트로 바꾼다.
각 페이지에서 필요한 데이터는 각자 알아서 호출하거나 훅을 통해 가져온다.
장점
- 목록보기, 모아보기 페이지를 독립적으로 운용할 수 있음
- 각 페이지에서 필요한 데이터를 각자 알아서 불러올 수 있음
단점
- props 전달을 통해 데이터 흐름을 파악하기 어려워짐
ReviewDisplayLayout 안에서 목록 페이지와 모아보기 페이지를 직접 렌더한다.
장점
- 이 레이아웃을 어떤 페이지들이 공유하는지 직관적으로 확인할 수 있음 (부모-자식 관계 강화)
ReviewDisplayLayout이 내려 주는 리뷰 정보(ReviewInfoData)를 사용하는 페이지들에게 props로 바로 전달 가능
단점
- children보다 확장성이 낮아짐
- 위 페이지들을 조건부 렌더링 or 라우팅하는 로직을 별도로 리팩토링해서 써야 함
children으로 오는 페이지들은 어차피 url이 달라지니까outlet을 사용해 라우팅하도록 바꿔 볼 수도 있을 것 같습니다
| content: string; | ||
| } | ||
|
|
||
| const ReviewEmptySection = ({ content }: ReviewEmptySectionProps) => { |
There was a problem hiding this comment.
이 널페이지는 당초 한 페이지에 준하는 면적을 커버하기 위해 만들어져서 그런지 아코디언에서 사용하기에는 자리를 많이 차지한다는 생각이 들었어요. 크기를 줄여볼 수도 있겠지만 개인적으로 널은 빼고 받은 답변이 없다는 텍스트만 심플하게 띄워주는 방식을 건의합니다!
| @@ -19,6 +25,37 @@ const ReviewCollectionPage = () => { | |||
| const [selectedSection, setSelectedSection] = useState<DropdownItem>(dropdownSectionList[0]); | |||
| const { data: groupedReviews } = useGetGroupedReviews({ sectionId: selectedSection.value as number }); | |||
|
|
|||
| const { revieweeName, projectName, totalReviewCount } = useReviewInfoData(); | |||
There was a problem hiding this comment.
바다가 남겨준 코멘트에 대한 코멘트를 남기겠습니다ㅋㅋㅋ
| const { revieweeName, projectName, totalReviewCount } = useReviewInfoData(); | ||
|
|
||
| const renderContent = (review: GroupedReview) => { | ||
| if (review.question.type === 'CHECKBOX') { |
There was a problem hiding this comment.
이렇게 객관식 질문인지를 체크하는 조건문이 프로젝트 전반에 걸쳐 많이 사용되고 있어서, 보다 직관적으로 isObjectiveQuestion같은 유틸을 만들어서 쓰면 좋겠다는 생각이 들었는데... 이제 와서 적용하기에는 이미 늦었을지도 모르겠네요ㅋㅋㅋㅋㅋㅋ
| import { ROUTE } from '@/constants/route'; | ||
| import { useGetReviewList, useSearchParamAndQuery } from '@/hooks'; | ||
|
|
||
| import ReviewEmptySection from '../../../../components/common/ReviewEmptySection'; |
There was a problem hiding this comment.
위에서 바다가 코멘트 남긴 것 처럼 절대 경로를 사용하면 될 것 같습니다~
🚀 어떤 기능을 구현했나요 ?
🔥 어떻게 해결했나요 ?
데이터 내려주는 방식 - 고민의 흔적
리뷰 목록과 모아보기 페이지에서 공통으로 사용하고 있는
ReviewDisplayLayout컴포넌트가 있는데, 해당 컴포넌트에서 리뷰이/프로젝트/총 리뷰 개수 정보를 불러오고 있습니다. 그리고 레이아웃 컴포넌트의 children으로 오는 모아보기 페이지 컴포넌트에서 총 리뷰 개수 정보가 필요한 상황이에요.children으로 데이터를 내려주기 위해 여러 가지 방법을 고민해보았어요.
(totalReviewCount: number) ⇒ React.ReactNode타입의 함수를 렌더링해서 데이터를 넘겨줄 수도 있어요. 하지만 공통으로 사용되는 레이아웃 컴포넌트에서 이 패턴은 적당하지 않다고 판단했습니다.결론: 데이터를 내려주지 않고
useReviewInfoData훅에 데이터가 필요할 때마다 접근합니다.ReviewEmptySection
리뷰가 없는 경우 '널~'을 띄워주는 컴포넌트인데요, 지금은 두 가지로 사용됩니다.
두 가지 경우를 분리하기 위해 '널~'과 함께 보여주는 문구를 props로 뚫어서 선택할 수 있게 했습니다.
▲ 총 리뷰가 0개인 경우
▲ 특정 질문에 대한 답변이 0개인 경우
📝 어떤 부분에 집중해서 리뷰해야 할까요?
📚 참고 자료, 할 말