Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NDD-149] 마이페이지 영상 개별보기 페이지 완료 (8h/3h) #81

Merged
merged 35 commits into from
Nov 22, 2023

Conversation

milk717
Copy link
Collaborator

@milk717 milk717 commented Nov 21, 2023

NDD-149 Powered by Pull Request Badge

💣폭탄💣 투척 죄송합니다!!

Why

아직 디자인이 정해지지 않은 부분에 대해서 못생긴 회색 버튼으로 휘뚜루 마뚜루 만들려고 마음먹고 시작했지만.... 도저히 참을 수 없어서 모달 디자인을 하다가 작업시간이 길어진 것 같습니다.
하지만 이제 마이페이지에서 구현해야할 부분은 대부분 끝났으니 1.0 버전은 걱정마세요~

How

myPageLoader 문제 수정

기존 router loader 작동방식

2023.11.18 (토)에 함께 작업했던 rootLoader에 문제가 발생해서 이를 해결했습니다.
기존 로직은 rootLoader에서 쿠키를 기반으로 유저 데이터를 요청하고, 마이페이지 로더에서는 캐싱된 데이터만 확인해서 리다이랙션 처리를 해줬습니다.

발생한 문제

랜딩 화면에서 Link 태그를 통해 마이페이지로 이동할 때는 문제가 없었지만, url을 통해 바로 마이페이지로 이동할 때는 로더가 의도한대로 동작하지 않았습니다.
원인을 분석해보니 rootLoader의 함수는 api 요청이라서 비동기로 동작하지만, myPageLoader는 동기적으로 동작하기 때문에 url을 통해 이동시 myPageLoader가 rootLoader보다 먼저 실행된다는 것이었습니다.

해결 방법

const myPageLoader = async ({ queryClient }: { queryClient: QueryClient }) => {
  await queryClient.ensureQueryData({
    queryKey: QUERY_KEY.MEMBER,
    queryFn: getMemberInfo,
  });
  const queryState = queryClient.getQueryState(QUERY_KEY.MEMBER);
  return queryState?.data ? null : redirect(PATH.ROOT);
};

마이페이지 로더에서도 api 요청을 수행하도록 구현했습니다.
단, ensureQueryData 는 캐싱된 쿼리가 없을 경우에만 api 요청을 수행하기 때문에 rootLoader와 mypageLoader의 api 요청이 중복될 일은 걱정하지 않아도 됩니다.
이 방식의 아쉬운 점은 rootLoader의 의미가 더이상 크게 없다는 것인데
이 부분은 추후에 rootLoader에서 처리해야할 작업이 늘어난다면 문제가 없을 것이라고 생각했습니다.

영상 공개 페이지와 비공개 페이지 분리

image
공개 페이지와 비공개 페이지를 분리하기 전에는 바보같이 위와 같은 고민을 하고 있었습니다.
영상 플레이어 페이지 진입 시에 url param을 통해 어떤 api 요청을 보낼지 판단하는 로직을 고민하던 중에 해민님의 아이디어로 공개 페이지와 비공개 페이지를 분리하게 되었습니다.
이렇게 구현하니 영상 공개 여부에 따른 api 요청도 문제없이 처리할 수 있었고, 공개 플레이어와 비공개 플레이어의 ui도 다르게 구성할 수 있었습니다.

Toggle foundation 추가

image image 토글 foundtaion을 추가했습니다.

getAPIResponseData 유틸함수 수정

getAPIResponseData 유틸을 통해서 react query에 들어갈 api 요청 함수를 구성하고 있는데 이 코드의 문제점이 있었습니다. 바로 유틸함수 내에서 에러를 catch 하기 때문에 상위로 에러가 전달되지 않아 react query에서 에러를 감지할 수 없다는 것인데요.
이 문제를 해결하기 위해 catch 문에서 상위로 에러를 던지도록 구현했습니다.
try-catch를 없애지 않은 이유는 기존 로깅을 그대로 유지하기 위함입니다.

interviewVideoPublicLoader

interviewVideoPublicLoader 를 통해 공개용 비디오 링크일 때 해시가 올바르지 않다면 404페이지로 리다이랙션 되도록 구성했습니다.
현재 loader에 비디오 api의 응답 값 여부에 따라 리다이랙션 처리를 해주는데 네트워크 에러 등으로 인해 데이터가 없을 경우에도 404로 리다이랙션 됩니다.
만약 페이지 내부에서 api를 호출했다면 에러 응답에 대해 재시도 하는 로직을 설정할 수 있는데 로더를 사용한 방법에서는 이런 경우에 대해 처리하는 것을 아직 고려하지 않았습니다.
이 부분은 Jira에 남겨놓고 추후 개선해볼 예정입니다.

truncateText 유틸함수 생성

아래 모달의 제목 부분을 구현하기 위해 텍스트와 길이를 입력받아 텍스트를 일정 길이로 자르고 나머지는 ...으로 처리하는 유틸함수를 만들었습니다.
image
전체 텍스트를 자르는 것이라면 Typography의 noWrap 속성을 사용할 수도 있었지만, 텍스트의 중간을 잘라야 했기 때문에 유틸을 만들었습니다.

Result

Kapture 2023-11-21 at 17 06 51

개선할 부분

비디오의 공개, 비공개 상태를 바꾸면 비디오 단건 조회 api를 다시 요청하고, 이 데이터를 사용하고 있는 컴포넌트가 모두 리랜더링 됩니다.
하지만 비디오 patch 동작을 수행하더라도 공개 여부를 제외하고는 변경되는 부분이 없어서 다른 컴포넌트를 리랜더링 시키지 않아도 됩니다.
일단 목요일까지 기능 구현이 우선이므로 이 부분도 Jira에 추가해놓고 추후 개선하도록 하겠습니다.

@milk717 milk717 self-assigned this Nov 21, 2023
@milk717 milk717 changed the title [NDD-149] 마이페이지 영상 개별보기 페이지 완료 (8h/3h) [NDD-149] 🍚밥만 먹고 마저 쓸게요!🍚 마이페이지 영상 개별보기 페이지 완료 (8h/3h) Nov 21, 2023
@milk717 milk717 added FE 프론트엔드 코드 변경사항 feature 새로운 기능이 추가 된 경우 labels Nov 21, 2023
FE/src/routes/myPageLoader.ts Show resolved Hide resolved
FE/src/constants/path.ts Show resolved Hide resolved
FE/src/routes/interviewVideoPublicLoader.ts Show resolved Hide resolved
FE/src/components/landingPage/StartButton.tsx Outdated Show resolved Hide resolved
FE/src/GlobalSvgProvider.tsx Show resolved Hide resolved
<PrivateVideoPlayer {...data!} />
<>
<PrivateVideoPlayer {...data!} />
<VideoShareModal
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아마 VideoShareModal은 최외각에 있어도 괜찮을겁니다 fragment를 지울수 있을것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isFetching을 통해 로딩상태와 데이터 로드가 완료된 상태를 구분하고 있기 때문에 data가 있다는 것을 보장하기 위해 프래그먼트 안에 넣었습니다.
다만 현재 isFetching으로 구분해서 data가 undefind이 아닌 것이 추론되지 않는다는 점이 아쉬웠는데요. 그래서 아래와 같이 변경했습니다.!

 {!data ? (
        //TODO 로딩화면 일단 임시로 처리
        <CenterLayout>
          <LoadingBounce />
        </CenterLayout>
      ) : (
        <>
          <PrivateVideoPlayer {...data} />
          <VideoShareModal
            videoId={Number(data.id)}
            videoName={data.videoName}
            isPublic={!!data?.hash}
            isOpen={isOpen}
            closeModal={() => setIsOpen(false)}
          />
        </>
      )}

f40c7a7b

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p5: 리액트쿼리v5부터는 useSuspenseQuery 처럼 suspense 상황이라면 명확하게 data 존재 여부를 추론해줄 수 있게 되었습니다만, 요게 또 사용하다보면 고민이 생기는 부분들이 있을것이라 살포시 놓고만 갑니다~

@milk717 milk717 marked this pull request as ready for review November 21, 2023 09:28
@milk717 milk717 changed the title [NDD-149] 🍚밥만 먹고 마저 쓸게요!🍚 마이페이지 영상 개별보기 페이지 완료 (8h/3h) [NDD-149] 마이페이지 영상 개별보기 페이지 완료 (8h/3h) Nov 21, 2023
Copy link

cloudflare-workers-and-pages bot commented Nov 21, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5847b13
Status:⚡️  Build in progress...

View logs

Copy link

@teihong93 teihong93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

지나갑니다..
이미 보셨겠지만 리액트 쿼리를 사용하실때에는 tk 블로그 한번 정독후 시작하시면 좋습니다!

<PrivateVideoPlayer {...data!} />
<>
<PrivateVideoPlayer {...data!} />
<VideoShareModal

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p5: 리액트쿼리v5부터는 useSuspenseQuery 처럼 suspense 상황이라면 명확하게 data 존재 여부를 추론해줄 수 있게 되었습니다만, 요게 또 사용하다보면 고민이 생기는 부분들이 있을것이라 살포시 놓고만 갑니다~

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[p-3] 폴더명과 컴포넌트 명을 맞추는건 어떤가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추후에 로딩과 관련된 컴포넌트가 더 다양해질 것 같아서 이렇게 네이밍을 분리했습니다!

2023-11-22.12.30.41.mov

요런 로딩도 만들고싶어요 ㅎㅎ

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 좋아요!!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[p-5] 요것도 뭔가 Button과 통일하면 좋을것 같아요 아직 디자인이 확립된게 없어서 어떻게 처리해야 할지는 모르겠지만 jira에 추가해 놓겠습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인정합니다! 저 부분도 만들면서 기본 버튼을 primary, light 이렇게 두가지 테마로 분리하면 좋겠다고 생각했습니다!


&:not(:disabled):hover {
background-color: ${theme.colors.surface.weak};
`}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[p-1] 괄호 짝이 안맞는것 같은데요???

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[p-100] 괄호도 짝이 없군요... 저도 짝이 없습니다🤣

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ㄷㄷ 매의 눈을 가지셨군요!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d3efeb00 반영완료!!

FE/src/utils/getAPIResponseData.ts Show resolved Hide resolved
FE/src/utils/textUtils.ts Show resolved Hide resolved
@milk717
Copy link
Collaborator Author

milk717 commented Nov 22, 2023

지나갑니다.. 이미 보셨겠지만 리액트 쿼리를 사용하실때에는 tk 블로그 한번 정독후 시작하시면 좋습니다!

리액트 쿼리를 처음 써봐서 아직 모르는 것이 많은데 useSuspenseQuery라는게 있군요! 조언 감사합니다😊

Copy link
Collaborator

@Yoon-Hae-Min Yoon-Hae-Min left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제 짝도 찾아주실분..?

- url을 통해 mypage에 접근할 때 로더가 제대로 동작하지 않는 문제 발생
- rootLoader가 비동기라서 myPageloader보다 나중에 실행되는 문제였음
- mypageLoader에서도 ensureQueryData를 호출하는 방식으로 해결
- ensureQueryData는 캐시가 없을 때만 요청을 하기 때문에 중복 api 호출은 걱정하지 않아도 됨
- 단, 이렇게하면 rootLoader의 의미가 크게 없어지기 때문에 추후 다시 고민해볼것
- 나중에 모바일 지원 시 세로 영상이 들어올 수도 있어서 동영상 가로, 세로 길이를 고정해야함
동영상 화면에서도 사용하기 위함
@milk717 milk717 merged commit f87d9c6 into dev Nov 22, 2023
1 check was pending
@Yoon-Hae-Min Yoon-Hae-Min deleted the feature/NDD-149 branch November 22, 2023 04:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE 프론트엔드 코드 변경사항 feature 새로운 기능이 추가 된 경우
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants