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

MyTemplatePage 초기 랜더링 Layoutshift 개선 #706

Merged
merged 10 commits into from
Sep 27, 2024

Conversation

Hain-tain
Copy link
Contributor

@Hain-tain Hain-tain commented Sep 26, 2024

⚡️ 관련 이슈

🪧 리펙토링 결과

  • MyTemplatePage 초기 랜더링 Layoutshift 개선
    - GLS 0.837 => 0.012 로 개선
Before After
image image

📍주요 변경 사항

1. 템플릿 목록, 카테고리 목록, 태그 목록을 한 번에 불러오도록 하고, 서스펜스를 통해 감싸주어 Layoutshift를 개선

  • useTemplateCategoryTagQueries 에서 3개의 요청이 모두 성공할 때 까지 기다림

2. 템플릿 목록 요청시 keyword 있는 경우에만 params 에 추가하도록 변경

  • 기존에는 키워드 없어도 '' 으로 보내줬으나, 백엔드 측에서 사용하지 않는 params 보내지 않도록 변경 요청하여 반영함.

3. -List 로 용어 통일

  • templates, categories, tags => templateList, categoryList, tagList로 이름 변경
  • 컨벤션으로 복수형보다는 List 와 같은 명시적인 단어로 표현하기로 했기 때문.

🎸기타

1. 현재 서스펜스가 MyTemplatePage 자체를 감싸고 있기 때문에, 새로운 요청을 보낼때마다 페이지 전체가 fallback 이 보이는 현상이 있습니다. 개인적으로 이 부분은 사용자 경험이 좋지 않아 반드시 개선되어야 한다고 생각합니다.

=> 추후 컴포넌트 계층 분리를 다시 한 후 서스펜스를 각각 감싸보면 어떨까 싶습니다.

@Hain-tain Hain-tain added FE 프론트엔드 refactor 요구사항이 바뀌지 않은 변경사항 zap 리뷰 우선순위가 높은 사항 labels Sep 26, 2024
@Hain-tain Hain-tain added this to the 5차 스프린트🍗 milestone Sep 26, 2024
@Hain-tain Hain-tain self-assigned this Sep 26, 2024
Jaymyong66
Jaymyong66 previously approved these changes Sep 26, 2024
Comment on lines 61 to 64
if (keyword) {
queryParams.append('keyword', keyword);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

놓친 부분이 있었네요~!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

놓친부분이라기보다는 백엔드에서 요구사항이 변경에 따른 변화입니다..!ㅎㅎ
이전에는 키워드가 없어도 반드시 쿼리에 추가해서 보내달라고했는데,
이번에 리팩토링하면서 키워드가 없을때 주지 않도록 해달라고 하여 변경하게 되었습니다!ㅎㅎ

Comment on lines +7 to +11
const { isChecking } = useAuth();

if (isChecking) {
return null;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 suspense 처리만 해주어도 footer 관련 Layout Shift가 해결이 되나요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이 부분은 Header와 동일한 로직으로 변경한 것인데요, 로그인한 유저인지 아닌지 검사하는동안 footer를 null 로 처리했다가 나중에 return 해주었습니다.
(footer는 언제나 맨 하단에 위치하기 때문에 나중에 떠도 Layout Shift가 발생하지 않을 것이라 생각하였습니다.)

이렇게 하면 Layout Shift을 완벽하게 해결할 순 없지만 처음에 너무 빨리 렌더링 되는 것을 막아 깜빡이는 현상을 없앨 수 있었습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

괜찮은 방법이라고 생각되네요! 더 좋은 방법이 떠오르지 않습니당

handleDelete: () => void;
}

const ConfirmDeleteModal = ({ isDeleteModalOpen, toggleDeleteModal, handleDelete }: ConfirmDeleteModalProps) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

요 모달은 따로 서브 컴포넌트로 파일 분리 안해도 괜찮을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗, 서브컨포넌트 분리하던거 다 reset 한줄 알았는데 아래 남아있었군요!
이 부분은 추후 MyTemplatesPage 리펙토링 할 때 함께 하겠습니다 👍

<ErrorBoundary fallback={<NotFoundPage />}>
<Suspense
fallback={
<Flex align='center' justify='center' css={{ width: '100vw', height: '100vh' }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

요게 Flex에 width, height 속성이 사실 있을텐데..! Flex 컴포넌트 리팩토링이 시급해보이긴하네요

Copy link
Contributor Author

@Hain-tain Hain-tain Sep 26, 2024

Choose a reason for hiding this comment

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

오랜만에 Flex 를 사용하다보니...ㅎㅎ
추후에는 저희 함께 논의했던 것처럼 Flex 컴포넌트를 사용하지 않는 방향으로 리팩토링할 예정입니다!👍

(현재 이 컴포넌트도 LoadingFallback 컴포넌트를 새로 만들어서 대체하였습니다!!)

vi-wolhwa
vi-wolhwa previously approved these changes Sep 26, 2024
Copy link
Contributor

@vi-wolhwa vi-wolhwa left a comment

Choose a reason for hiding this comment

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

👌🏻👍🏻

Comment on lines +7 to +11
const { isChecking } = useAuth();

if (isChecking) {
return null;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

괜찮은 방법이라고 생각되네요! 더 좋은 방법이 떠오르지 않습니당

@vi-wolhwa vi-wolhwa merged commit a21210a into dev/fe Sep 27, 2024
3 checks passed
@vi-wolhwa vi-wolhwa deleted the refactor/695-perf-optimization branch September 27, 2024 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE 프론트엔드 refactor 요구사항이 바뀌지 않은 변경사항 zap 리뷰 우선순위가 높은 사항
Projects
Status: Weekend Done
Development

Successfully merging this pull request may close these issues.

3 participants