[FE] 작성한 리뷰를 확인할 수 있는 반응형 레이아웃 #1038
Conversation
| title: string; | ||
| } | ||
|
|
||
| const WrittenReviewContent = ({ title, children }: EssentialPropsWithChildren<WrittenReviewItemProps>) => { |
There was a problem hiding this comment.
네이밍
WrittenReviewContent 라는 네이밍이 목록, 상세보기 페이지안에서 하나의 리뷰에 대한 내용에 사용 되는 건가라고 헷갈렸어요. 용도를 보니, 리뷰 확인 페이지 내에서 목록,상세보기의 부모 컴포넌트로 레이아웃을 담당하는 것 같네요.
이미 WrittenReviewConfirmPage 폴더에 있으니, ContentLayout 이라고 해도 좋을 것 같네요.
layouts 폴더 위치
layouts 은 레이아웃을 담당하는 컴포넌트들을 모아두는 곳 같은데 왜 components 하위가 아니라 페이지 폴더 바로 밑에 있는지 알 수 있을까요?
There was a problem hiding this comment.
네이밍
저도 하면서 이름을 뭘로 지을까 고민했었는데 역시 혼란의 여지가 있었군요 🥲
레이아웃 컴포넌트임을 드러내면서, 범위는 page임을 보여줌 + 어차피 Written 하위이므로 중복된 뜻을 줄여서 PageContentLayout으로 수정했습니다!
경로
일단 이유는 이 세 가지였습니다.
-
component와 layout은 성격이 조금 다르다고 생각했어요.
-
코드 컨벤션의 예시가 아래와 같았어요.
📂ReviewWritingPage
└ 📂constants
└ 📂form
└ 📂layout
└ 📂modals -
실제 리뷰 작성 페이지의 구성을 참고했는데, 페이지 폴더 아래에 바로 layout이 있었어요. (다만 둘이 따로 있지는 않고, layout 하위로 components가 있긴 합니다)
There was a problem hiding this comment.
경로에 대해
리뷰 작성 페이지 예시를 참고했군요!
리뷰 작성 페이지의 경우, 기능과 그에 따른 컴포넌트와 훅등이 너무 많아서 유지보수 측면에서 기능별로 폴더를 나누었어요. (해당 폴더 하위에 기능에 대한 컴포넌트,훅,유틸이 들어있어요) 레이아웃 폴더는 기능에 해당하지 않는 레이아웃 관련 컴포넌트들이 있어요.
올리가 구현한 경우는 기능별로 폴더를 구별하는 것이 아닌 컴포넌트,훅으로 폴더를 관리하고 있는 방식이라 layout이 컴포넌트 하위에 있는게 자연스러울 것 같아요.
| export const DetailedWrittenReview = styled.div<DetailedWrittenReviewStyleProps>` | ||
| ${media.xSmall} { | ||
| ${({ $isMobile }) => | ||
| $isMobile |
There was a problem hiding this comment.
데스크탑에서 내가 쓴 상세 리뷰 url로 바로 들어오면, 상세보기만 보이겠네요.
회의때 모바일에서 내가 쓴 상세 리뷰를 따로 페이지로 관리하자고 했는데 이러면 url 에 따른 화면 관리가 까다롭겠네요. 상세 보기 페이지에서 뒤로 가기를 하면, 데스크탑에서 목록과 상세보기가 같이 보이는 화면이 나타나는게 이상하기도 하구요. (이전에는 상세보기만 보여줬다가 이제는 목록과 상세보기가 같이 보여진다?)
기존 내가 받은 리뷰 목록, 상세처럼 아예 페이지를 분리하는게 나을 것 같아요. 노트북으로 볼 때 지금 목록 크기 대비 두 개가 같이 있을 때 쓴 목록과 상세보기가 생각보다 작게 보일 수 도 있을 것 같네요.
이 부분은 프론트 전체 회의가 필요할 것 같네요
| /** | ||
| 현재 미디어 쿼리 상태와 디바이스 종류(boolean)를 리턴하는 훅 | ||
| */ | ||
| const useCurrentMediaType = () => { |
There was a problem hiding this comment.
👍🏻 현재 디바이스에 따라 다르게 동작해야 하는 경우 사용하기 좋겠네요
There was a problem hiding this comment.
올리~ 작성한 리뷰 확인 페이지 만드느라 수고했어요! 해당 페이지에서 고려해야 할 부분들이 꽤나 많네요...
작성한 리뷰 확인 페이지에서 목록과 상세 정보를 한 번에 보여주는 방안을 제시했던 이유가 여러 리뷰가 있을 때 사용자가 특정 리뷰의 상세보기를 보려면 매번 목록에서 상세로 넘어가고, 다른 리뷰의 상세보기를 보려면 다시 뒤로 가기를 눌러서 목록으로 돌아가야 하는 번거로움을 줄이기 위함이었어요. 그래서 한 화면에 모든 정보를 확인할 수 있으면 훨씬 편리할 것 같다는 생각이 들었고, 디자인면에서도 꽉 차는 느낌이 괜찮다고 생각했어요. 다만, 이 방식의 도입으로 인해 고려해야 할 부분이 더 늘어났네요...😅
스크롤, 모바일 상세 페이지 url 공유 등 프론트엔드 회의를 거쳐서 해당 방식들이 결정된다면 수정사항이 많아질 것 같아요! 그래서 추후에 코드 리뷰 진행할게요😉
👩💻 쑤쑤의 opinion
이 방식 대신 두 영역에 모두 스크롤을 적용하거나, 단독 스크롤을 없애고 페이지 전역 스크롤 하나만 두는 것도 생각보다 괜찮아 보입니다!
좀 더 고민을 해봐야 겠지만.... 흐음🤔 목록과 상세 영역의 height을 상세보기 height에 맞추고, 목록에 스크롤이 생기게끔 하는 건 어떨까요?? 아래 깃허브처럼요.
건의: xSamll (모바일 진입점) 커트라인 높이기
아이폰 14 프로 맥스의 너비가 430이였군요! 커트라인 높여야 겠네요👍
| export const PageContentLayout = styled.article` | ||
| display: flex; | ||
| flex-direction: column; | ||
| height: 100%; | ||
|
|
||
| ${media.xSmall} { | ||
| margin: 0 auto; | ||
| } | ||
| `; |
There was a problem hiding this comment.
지금 구현 방식대로라면 디바이스 크기가 커지면 커질수록 width가 같이 늘어나요. 큰 모니터에서 볼 경우, ReviewListItem이 너무 길어지는 문제가 있을 것 같아요. max-width는 정하는 게 좋을 것 같아요:)
There was a problem hiding this comment.
두 영역을 같이 보여주는 것 덕분에 고려해야 할 게 늘었지만, 덕분에 UI도 예뻐졌고 페이지 이동이 줄어 UX도 좋아질 것 같아요. 고민하는 과정도 재밌기도 했고요~! 또 어차피 리뷰미는 반응형 지원이니까요 🤣
스크롤 기준
어떤 느낌인지 조금 더 자세한 설명을 해 주실 수 있나요?! 또 다른 뷰어 모드가 있는건지 모르겠네요...
제가 확인해봤을 때 깃허브 files changed에서는 (특수한 상황을 제외하면) 변경사항은 처음부터 끝까지 다 보여주는 것 같고(= 긴 스크롤) 목록은 max-height를 100vh정도로 맞추는 것 같아요.
만약 목록을 상세에 맞춘다면 ref를 사용해 동적으로 높이값을 가져와서 계산해줘야 할 것 같고, 무엇보다 자세히 볼 리뷰를 변경할 때마다 목록에서 layout shift가 일어난다는 우려가 큽니다.
max-width
무의식적으로 전체 페이지 레이아웃에 값이 있을 거라고 생각했는데 최댓값이 없었군요... 추가할게요!
There was a problem hiding this comment.
반응형 작업을 용이하게 하기 위해 width 단위를 vw로 잡았더니 모니터 크기가 커짐에 따라 그대로 증가하는 문제가 있었어요.
간단하게 부모 컴포넌트(PageContainer, ItemLayout 등)에 large 이상의 크기일 때 max-width를 지정해주려 했는데 flex 레이아웃이라 자식들이 뚫고 나와서 의미가 없더라구요.
그래서 일단은 최하위 자식 컴포넌트로 들어가는 ReviewCard와 DetailedReview(레이아웃 말고 데이터 들어가는 컴포넌트)에 아래 속성을 걸어주는 걸로 생각하고 있습니다.
${media.large} {
min-width: ${({ theme }) => theme.writtenReviewLayoutSize.largeWidth}; // largeWidth : 65rem
max-width: ${({ theme }) => theme.writtenReviewLayoutSize.largeWidth};
}
로그인 후 작성한 리뷰 확인 - 쿼리 스트리밍 사용 제안로그인 후 작성한 리뷰 확인 시, 웹과 모바일 디자인이 달라서 생기는 오류가 있었습니다. 쿼리 스트리밍을 사용해 동일한 url로 반응형에 대응하는 방법을 제안합니다. 방법리뷰 확인 목록을 통해 열고자하는 상세 리뷰의 리뷰 아이디를 쿼리스크링으로 사용하는 방법입니다 로그인 시 작성한 리뷰 확인 페이지 - 상세 리뷰를 열지 않은 경우 : 쿼리 스트리밍이 없음 로그인 시 작성한 리뷰 확인 페이지 - 상세 리뷰를 연 경우 : 쿼리 스트링(예시 : id) 존재 이렇게 하면 쿼리 스트링 값으로 동일한 url로 웹과 모바일 모두 대응가능할 것 같아요.
파라미터로 변경할 경우
|
반응형 레이아웃을 위한 QueryString 도입 리팩토링을 하기까지 나눴던 이야기는 추가 코멘트에 남길게요! QueryString 도입에 따른 레이아웃 동작 설명
그 외
건의미디어 쿼리에 large 이상의 크기가 필요할 것 같습니다. 주의사항기본적인 반응형은 구현했으나 세세한 부분은 추후에 별도로 조정해야 합니다. (예: 모바일 크기에서의 상세 페이지 - 아직 실제 컴포넌트를 사용하지 않고 화살표도 달아야 해서 따로 마진을 주지 않았습니다) |
| import { useState, useLayoutEffect } from 'react'; | ||
|
|
||
| import { breakpoint } from '@/styles/theme'; | ||
| import { Breakpoints } from '@/utils/media'; |
There was a problem hiding this comment.
Breakpoints를 해당 유틸함수 이외에서도 사용한다면 types 폴더로 옮겨야 겠네요.
| const handleResize = () => { | ||
| const currentWidth = window.innerWidth; | ||
| const matchedBreakpoint = breakpointsArray.find(([, width]) => currentWidth <= width); | ||
|
|
||
| setCurrentMediaType((matchedBreakpoint?.[0] as Breakpoints) ?? null); | ||
| }; | ||
|
|
There was a problem hiding this comment.
가독성 부분, useLayoutEffect 실행 시 함수가 재생성되어서 저는 effect 안에 함수를 선언하지 않는 편이에요. effect안에 함수를 선언한 이유가 있을까요?
There was a problem hiding this comment.
구현하다가 안 뺐던 것 같네요..ㅋㅋㅋㅋ
저는 함수가 길지 않고 훅과 밀접한 동작이라면 내부에 넣는 것도 선호하는 편이지만 빼는 것도 좋습니다!
|
|
||
| handleResize(); | ||
|
|
||
| window.addEventListener('resize', handleResize); |
There was a problem hiding this comment.
resize시 마다 이벤트가 일어나서, 디바운스나 쓰롤링으로 마지막 이벤트 또는 일정 주기로 handleResize가 실행되게하는게 성능 상 좋을 것 같아요
There was a problem hiding this comment.
스크롤 대비 얌전한(?) 이벤트이고 리뷰미 특성상 리사이징이 빈번하게 일어나지 않는다고 생각해서 전혀 떠올리지 못했는데, 어찌됐든 최소한의 안전장치라도 걸어두는 게 좋겠네요.
비슷한 맥락으로 아마 디바운스를 적용할 것 같아요. 리사이징이 파바박 일어날 때마다 그 중간 상태를 꼭 보여줄 필요는 없고 마지막 상태만 보여줘도 될 것 같아 디바운스로도 충분하다고 생각하는데 다른 팀원들의 생각도 궁금하네요~!
| 현재 미디어 쿼리 상태와 디바이스 종류(boolean)를 리턴하는 훅 | ||
| */ | ||
| const useCurrentMediaType = () => { | ||
| const [currentMediaType, setCurrentMediaType] = useState<Breakpoints | null>(null); |
There was a problem hiding this comment.
useCurrentMediaType훅을 사용하는 곳을 봤더니, currentMediaType를 사용하고 있지않고 currentDeviceType을 사용하고 있네요.
디바이스 사이즈를 기준으로 currentDeviceType을 계산하는게 주 역할 같은데 맞나요?
맞다면 currentDevice를 상태 관리하고 훅명을 currentDevice를 사용하는게 어떨까요? 이게 올리의 구현 의도를 더 잘 들어낼 것 같아요
참고로 미디어 타입이 CSS 미디어 쿼리의 미디어 타입과 헷갈리네요.. 😅
There was a problem hiding this comment.
ㅋㅋㅋㅋ오랜만에 보니 저도 헷갈리는 것 같기도... 훅 이름을 바꾸긴 해야겠는데 뭘로 해야 할지 고민이네요.
실시간으로 계산하는 훅이라 current를 썼는데 사용하는 사람은 사이즈는 당연히 실시간으로 계산해준다고 생각할 것 같아서 빼는 방향으로 생각했어요.
current를 뺀 대신 훅 동작을 명확하게 드러내기 위해 useDeviceBreakpoints 정도가 생각나네요!
There was a problem hiding this comment.
어라? 이 코멘트 삭제하고 다시 썼는데 어디갔지? ㅎㅎㅎㅎ
currentMediaType보다 currentDeviceType을 상태로 관리하는 방법은 어떤가요?
currentMediaType 상태 업데이트하고, currentDeviceType을 계산하기 보다 바로 currentDeviceType 상태를 업데이트해주는 게 올리의 구현의도를 더 잘 들어낼 것 같아요.
| const { currentDeviceType } = useCurrentMediaType(); | ||
|
|
||
| const { queryString: reviewIdString } = useSearchParamAndQuery({ | ||
| paramKey: '', |
There was a problem hiding this comment.
paramKey가 없다면, useSearchParamAndQuery에서 parmaKey를 옵셔널로 변경해도 좋을 것 같아요.
There was a problem hiding this comment.
그렇게 해보려다가 뭔가 훅의 역할이 애매해지는 느낌이라 (예전에는 url에서 reviewRequestCode를 추출하는 용도로 사용해서 paramKey가 필수였던 것 같아) 일단은 기본값을 주는 방식으로 구현했어요. 매개변수가 둘 다 optional인 것이 어색하기도 했고요.
그런데 지금 생각해보니 훅 이름이 -ParamAndQuery긴 하지만, 둘 중 하나만 쓴다 해도 결국 URLSearchParams 객체를 사용해야 해서 훅의 존재의의가 충분하네요. 왜 이런 생각은 구현을 다 하고 드는 걸까요..ㅋㅋㅋㅋ
| ) : ( | ||
| <WrittenReviewList handleClick={handleReviewItemClick} /> | ||
| ); | ||
| } else { |
There was a problem hiding this comment.
early return 을 사용해서 else 사용하지 않아도 될 것 같아요.
There was a problem hiding this comment.
추가로 리팩토링 시 참고할 만한 코드 두고 가요.
const renderContent = () => {
const isMobile = currentDeviceType.isMobile;
const renderDetailedReview = (
<DetailedWrittenReview
$isMobile={isMobile}
selectedReviewId={selectedReviewId}
/>
);
const renderReviewList = (
<WrittenReviewList handleClick={handleReviewItemClick} />
);
if (isMobile) {
// 모바일: queryString 없으면 목록, 있으면 상세보기
return selectedReviewId ? renderDetailedReview : renderReviewList;
}
// 태블릿 이상: 목록 + 상세보기
return (
<S.PageContainer>
{renderReviewList}
{renderDetailedReview}
</S.PageContainer>
);
};
디자인 오류 관련
iphone 14 pro max (width:430px) 일때
### ipad air
- 1024px이하에서도 목록/상세 둘 중 하나만 보여주거나 페이지 좌우 간격 조정이 필요할 것 같아요.
리뷰 상세 페이지
|
디자인 코멘트
|
탭이 페이지 컴포넌트 상위에 온다고 생각하고 만들었습니다~
List와 Detail이 스타일을 공유하기 때문에 레이아웃용 Wrapper 컴포넌트를 만들었습니다.
🚀 어떤 기능을 구현했나요 ?
🔥 어떻게 해결했나요 ?
반응형 레이아웃
라우팅을 할 때는
DetailedWrittenReview의isMobile을 무조건 true로 넘겨서 모바일 화면이 보이게 했고, 부모 컴포넌트인WrittenReviewConfirmPage에서는 현재 페이지 사이즈에 따라 동적으로 값을 넘겨줍니다.useCurrentMediaType훅을 사용합니다.현재 크기에 따른 디바이스 종류, 현재 적용된 미디어 쿼리 범위를 리턴합니다.
두 영역의 높이
-> 리뷰 상세보기에는
min-height만 적용하고 리뷰 목록에는max-height만 적용했습니다. 같은 똑같은 값을 사용하기에 상수화했습니다.부모의 flex 옵션을
stretch로 설정했기 때문에 상세보기의 높이가 늘어난 경우 페이지 전체 스크롤을 이용합니다.그런데 Detail 부분의 높이가 리뷰 길이에 따라 달라지기에 두 영역의 높이를 항상 맞추는 데 어려움이 있습니다. 또 맞추더라도 선택한 리뷰의 길이에 따라 목록 길이가 계속 변해 UX에도 좋지 않을 것 같다는 생각이 들었습니다.
상세보기가 길어지면 아래와 같은 느낌이 됩니다.
이 방식 대신 두 영역에 모두 스크롤을 적용하거나, 단독 스크롤을 없애고 페이지 전역 스크롤 하나만 두는 것도 생각보다 괜찮아 보입니다! (프룬이 참고용으로 알려준 사이트)
📝 어떤 부분에 집중해서 리뷰해야 할까요?
ㄴ 요거
📚 참고 자료, 할 말
내려받아 실행 시http://localhost:3000/user/written-review-confirm/ABCD1234/로 접근하시면 됩니다http://localhost:3000/user/written-review로 변경했습니다.건의: xSamll (모바일 진입점) 커트라인 높이기
개발자 도구에서 아이폰 14 맥스 프로를 선택하면 너비가 430인데, 현행 미디어 쿼리에서는 425부터 모바일이라고 간주합니다.
-> 모바일임에도 모바일로 넘어가기 직전의 태블릿 사이즈가 적용됩니다.
-> (이 페이지 한정) 태블릿 사이즈는 500 중후반부터 안정적이라 430대에서는 어색하게 보입니다.
=> 모바일 커트라인을 430~440정도로 더 높게 잡는 것을 제안합니다.
또 500대 사이즈에 대한 디자인을 덜 신경쓰는 것(ㅎㅎ)도 제안해봅니다.
작은 태블릿 사이즈는 600 전후 정도라 그 이하의 태블릿 사이즈에는 크게 신경쓰지 않아도 될 것 같습니다!