[FE] feature: 리뷰 연결 페이지 제작 #370
Conversation
…eview-me into fe/feature/355-review-dashboard-page
…eview-me into fe/feature/355-review-dashboard-page
| import PasswordModal from './components/PasswordModal'; | ||
| import * as S from './styles'; | ||
|
|
||
| interface ReviewDashboardPageProps { |
There was a problem hiding this comment.
리뷰방이 생성되면 복사하는 url이기존의 리뷰 작성 페이지에서 리뷰 연결페이지 (=ReviewDashboardPage)로 변경 될거에요.
그렇다면 ReviewDashboardPage 안에서 리뷰 그룹 정보 조회 요청을 통해 리뷰이, 프로젝트명을 받아와서 사용할 거고, ErrorBoundary, Suspense를 적용한다면 현재 ReviewDashboard안의 있는 코드들은 리뷰 상세정보 페이지등과 같이 다른 컴포넌트로 빼고 ReivewDashboardPage안에서 불러와서 사용해야될 것 같네요.
| interface ReviewDashboardPageProps { | |
| const ReviewDashboardPage () =>{ | |
| return ( | |
| <ErrorSuspenseContainer> | |
| <RevuewDasgbord/> | |
| </ErrorSuspenseContainer> | |
| ) | |
| } |
위의 구조처럼 한다면, ReviewDashboard 컴포넌트안에서 리뷰 그룹 정보 조회 요청을 할거고, 요청 하는 동안에 suspense의 fallback이 오류 시에는 errorboundary의 fallback이 작동하겠네요.
There was a problem hiding this comment.
오 이건 전혀 생각하지 못했네요 😮
URL이 보여주는 페이지가 달라진만큼 그에 따른 새로운 Suspense와 에러 바운더리도 챙겼어야 했는데... 몰랐읍니다
바다의 제안대로 고칠 것 같은데 페이지 이름들을 어떻게 하면 좋을지 고민이네요 🤪
There was a problem hiding this comment.
suspense 및 error boundary 처리를 위한 Wrapper 컴포넌트를 새로 만드는 것보다는 라우터에서 ReviewDashboard 컴포넌트를 ErrorSuspenseContainer로 감싸는 게 나을 것 같다는 생각이 들어서 라우터 페이지에서 감싸주는 방식으로 고쳐봤습니당
| // NOTE: 추후 이곳에 API 호출 함수 추가 | ||
|
|
||
| //closeModal(); | ||
| // navigate(`/${ROUTES.reviewWriting}/5`); // NOTE: 유효하지 않은 경로 |
There was a problem hiding this comment.
주석 처리가 되어있지만, 리뷰 작성 페이지로 이동하는 navigate이네요.
올바른 비밀번호면 리뷰 목록 페이지로 이동하지 않고, 작성 페이지 경로로 이동하는 이유가 있을까요?
There was a problem hiding this comment.
물론 이유는 제 정신상태 이슈입니다.
다음주에는 정신차릴게요... 🙃
| <ContentModal handleClose={closeModal}> | ||
| <S.PasswordModal> | ||
| <S.ModalTitle>리뷰 확인을 위해 설정한 비밀 번호를 입력해주세요</S.ModalTitle> | ||
| {/* <S.InputContainer> |
There was a problem hiding this comment.
Input 부분이 주석 처리된 이유가 있을까요?
There was a problem hiding this comment.
주석에 나와있는대로, Input 쪽에 EyeButton 컴포넌트가 적용되어 있는데 아직 해당 컴포넌트를 만든 브랜치가 머지되지 않아서 있는 셈치고 코드를 작성한 뒤 주석 처리했기 때문입니다!
|
|
||
| return ( | ||
| <S.ReviewDashboardPage> | ||
| <S.DashboardMainImg src={DashboardIcon} alt="" /> |
There was a problem hiding this comment.
alt을 빈문자열로 둔 올리의 pr 내용을 읽었어요.
alt는 이미지가 로딩 중이거나,깨졌을때 화면에 표시되죠.
아마 우리는 스크린 리더기를 사용하지 않아서 우리에게는 alt의 용도가 이렇게 국한되게 느껴질거에요.
alt는 스크린 리더기를 사용하는 사람에게 해당 이미지가 어떤 것인지 설명하는 역할을 해요.
그래서 어떤 이미지인지 가능한 상세해야한다고 들었어요.
스크린 리더기를 사용하는 사용자 입자에서 빈문자열로 두는 것이 괜찮을까 하는 걱정에 검색을 해봤어요.
중요하지 않은 이미지라서 불필요하게 전다하지 않아도 된다면 alt을 빈문자열로 두어도 된다고 하네요.😊
해당 이미지들이 디자인적인 요소일뿐이라 alt이 빈문자열이여도 된다는 것에 동의해요.
프론트는 사용자와 가장 맞닿는 분야라서, 화면이 보이지 않는 유저의 입장에서도 생각해봐야할 문제라 이 내용을 공유하고자 코멘트로 달아요.
There was a problem hiding this comment.
바다의 생각을 공유해줘서 고마워요!~~
미션할 때 alt를 몇 번 빼먹고 챙겨달라는 지적을 받은 뒤 모든 이미지에 alt를 달았는데, 오히려 alt를 모든 이미지에 사용하면 해당 페이지에서 정말 중요한 게 무엇인지 잘 드러나지 않아서 사용자 플로우와 직접적으로 관련된 이미지 위주로만 추가하라는 리뷰를 받은 적이 있었습니다.
그치만 직접적으로 연관됐다는 게 어느 정도는 주관적일 수도 있어서 팀원들의 의견을 구해봤습니다!
| { | ||
| path: `/reviewDashboard`, // NOTE: 임시 경로, 추후 논의 및 상수화 필요 | ||
| element: ( | ||
| <ErrorSuspenseContainer> |
There was a problem hiding this comment.
ErrorSuspenseContainer를 사용하는 기존의 페이지들을 페이지 컴포넌트 안에서 사용하고 있어서, 이런 것도 코드의 일관성,통일성 부분에 맞추어햐하는 건지 잘 모르겠네요. 이것까지 맞춘다면 담당자의 코드 스타일을 너무 제하는 느낌이 들기도 하구요. 흠....
이미 ErrorSuspenseContainer안에서 error fallback, suspense fallback 을 정의하고 있으니 이대로 가도 될 것 같아요.
| const handleConfirmButtonClick = () => { | ||
| // NOTE: 추후 이곳에 API 호출 함수 추가 | ||
| //closeModal(); | ||
| // navigate(`/${ROUTES.reviewList}/${}`); // NOTE: 추후 뒤에 groupAccessCode 추가하기 |
There was a problem hiding this comment.
지금은 리뷰 목록 페이지에서 url 뒤에 추가해서 보내지 않고, 헤더로 groupAccessCode 를 보내주고 있어요. 추후에 api가 변경되면 어떻게 될지는 모르겠지만요🥹
🚀 어떤 기능을 구현했나요 ?
🔥 어떻게 해결했나요 ?
ContentModal을 사용했습니다.📝 어떤 부분에 집중해서 리뷰해야 할까요?
HomePage에서 제작했던EyeButton(현재 머지되지 않음)을 이 브랜치에서 복사해 사용하는 대신 이미 있다고 가정하고 코드를 작성했습니다. 해당 코드가 있는 부분은 주석 처리를 해 둔 상태로 추후 HomePage가 머지되면 주석 해제할 예정입니다.DashboardPage에서props로revieweeName과projectName을 받고 있는데 추후 이 페이지로navigate해올 때 관련 정보를 전달하는 방식으로 리팩토링할 예정입니다.📚 참고 자료, 할 말
이번에 작업한 페이지들에 이미지가 많이 들어갔는데, 별도의
alt설명을 하지는 않았습니다. 디자인을 위한 보조적인 이미지라 이미지가 깨졌을 때 별도의 설명을 제공할 필요가 없다고 생각했습니다.현재 svg 이미지의 사이즈 때문에
width를 늘려도 더 이상 가로로 늘어나지 않아서 피그마보다width가 짧게 제작된 상태입니다.디코의 경로 이슈는 경로 문제가 아니라 이 페이지를 Outlet 안에 넣지 않아서(=Router로 감싼 컴포넌트 안에 들어가지 않아서)였습니다. 새 페이지 작업할 때 조심하세요... 😓😓