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-240] settingpage 라우터 처리 (1h/2h) #98

Merged
merged 6 commits into from
Nov 23, 2023
Merged

Conversation

Yoon-Hae-Min
Copy link
Collaborator

@Yoon-Hae-Min Yoon-Hae-Min commented Nov 23, 2023

NDD-240 Powered by Pull Request Badge

How

설정페이지에 대한 작업이 모두 끝났습니다 만약에 추가로 제가 놓친 부분이 있거나 추가적인 사안이 있으면 해당 PR에서 모두 코멘트 남겨주시면 감사하겠습니다.

포인트1

성인님이 만들어주신 hook을 그대로 사용하면 페이지가 mount될 시에 카메라가 연결이 되었는데요. 이는 문제 선택 페이지에서 카메라가 연결되는 모습을 볼 수 있었습니다. 이는 세팅 페이지가 가지고 있는 특징 때문인데요. 세팅 안에 각각의 페이지들은 첫 진입시 mount가 되고 display none으로 처리되어서 dom자체는 mount가 되고 있는것을 확인할 수 있습니다. 그래서 카메라 연결 페이지로 가지 않아도 바로 카메라가 연결되는 특징을 가지고 있죠
따라서 hook에 trigger라는 변수를 집어넣어 해당 페이지에 도달 했을때 카메라가 연결이 되도록 재조정 하였습니다.

포인트 2

페이지 라우팅 처리를 할때 어떻게 해야 기존의 상태를 알고 이전으로 넘어가지는 고민을 하였습니다. 기존에는 각 페이지 마다 이전까지(A~C까지)의 state를 각각 확인하면서 만드려 하였지만 그것은 기존에 만들었던 settingpage가 가지고 있는 장점인 확장성이 낮다 라는 특징을 해친다고 생각하였습니다.

그래서 생각해 낸것이 이전페이지만 체크해서 안되어 있으면 이전 페이지로 넘어가자 입니다. 그래서 현제 페이지의 path를 기반으로 이게 data의 몇번째 index에 있는것인지 체크하며 이전 데이터의 state를 확인하는 방식으로 진행하였습니다.

Result

2023-11-23.10.05.48.mov

@Yoon-Hae-Min Yoon-Hae-Min added FE 프론트엔드 코드 변경사항 feature 새로운 기능이 추가 된 경우 labels Nov 23, 2023
@Yoon-Hae-Min Yoon-Hae-Min self-assigned this Nov 23, 2023
Copy link
Collaborator

@milk717 milk717 left a comment

Choose a reason for hiding this comment

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

useMedia 훅이 수정되면 변경될 플로우라서 일단 간단하게 리뷰하겠습니다~

@@ -12,7 +12,7 @@ const InterviewSettingPageLayout: React.FC<InterviewSettingPageLayoutProps> = ({
<Layout
direction="column"
css={css`
padding: 0 1rem 1rem 1rem;
padding: 0rem 1rem 1rem 1rem;
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] 0에는 단위 빼주세요! 웹스톰에서 경고를 표시해줘서 보기 불편합니다 ㅜㅜ

@@ -23,7 +23,7 @@ const useMedia = () => {
}, []);

useEffect(() => {
if (!media) {
if (!media && trigger) {
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] 이 trigger 플래그는 훅이 수정되면 없어질 부분인거죠??

Copy link

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

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: eb7b523
Status: ✅  Deploy successful!
Preview URL: https://d2f3fc13.gomterview.pages.dev
Branch Preview URL: https://feature-ndd-240.gomterview.pages.dev

View logs

Copy link
Collaborator

@adultlee adultlee left a comment

Choose a reason for hiding this comment

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

꼭 수정해야합니다...후후

@Yoon-Hae-Min Yoon-Hae-Min merged commit 5735f4a into dev Nov 23, 2023
1 check passed
@delete-merged-branch delete-merged-branch bot deleted the feature/NDD-240 branch November 23, 2023 04:11
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.

3 participants