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

DNA 페이지 이미지 로직 변경 #451

Merged
merged 10 commits into from Nov 2, 2023
Merged

Conversation

hyesungoh
Copy link
Member

@hyesungoh hyesungoh commented Oct 30, 2023

🤔 해결하려는 문제가 무엇인가요?

  • DNA 페이지의 TTFB가 느려요

Note
문제는 서버 사이드에서 이미지를 만들고 나서야 반환하기 떄문이에요

🎉 변경 사항

  • API route를 이용해서 필요할 때 이미지를 동적으로 만들도록 했어요

  • Modal을 lazy load 해 초기 이미지 요청을 지연시켜요

@hyesungoh hyesungoh self-assigned this Oct 30, 2023
@github-actions
Copy link

github-actions bot commented Oct 30, 2023

Bundle Sizes

Compared against 1077694

Route: No significant changes found

Dynamic import Size (gzipped) Diff
dna/LoadedDna.tsx -> ./DNAImageDownloadModal 1.16 KB added

@hyesungoh hyesungoh changed the title WIP: DNA 페이지 이미지 로직 변경 DNA 페이지 이미지 로직 변경 Nov 2, 2023
@hyesungoh hyesungoh added feat New feature or request fix Fix something was not working labels Nov 2, 2023
@hyesungoh hyesungoh marked this pull request as ready for review November 2, 2023 14:17
@codecov-commenter
Copy link

codecov-commenter commented Nov 2, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1077694) 89.34% compared to head (6f21803) 89.34%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #451   +/-   ##
=======================================
  Coverage   89.34%   89.34%           
=======================================
  Files          44       44           
  Lines         338      338           
  Branches       61       61           
=======================================
  Hits          302      302           
  Misses         36       36           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions github-actions bot added the docs Improvements or additions to documentation label Nov 2, 2023
Copy link
Member

@sumi-0011 sumi-0011 left a comment

Choose a reason for hiding this comment

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

그당시에는 next.js를 쓰니까 뭔가 페이지 보이기전 server-side에서 이미지를 만들어야 한다는 생각에 갖혀있었던것 같아요!!

next의 api router를 이용해서 필요할 때 이미지를 만드니까, 페이지 로드에 들어가는 시간이 확실이 많이 단축된 것 같네요! 감사합니다 😀

Comment on lines +47 to +56
style={{
width: '375px',
height: '666px',
}}
src={HOISTING_IMAGE_BY_GROUP[group as Group]}
alt={'dna_' + group}
width={375}
height={666}
/>
<span
Copy link
Member

Choose a reason for hiding this comment

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

다운받는 사진의 화질이 안좋은 문제는 이부분에서 사진 비율에 맞게 이미지 자체 크기를 키우면 되지 않을까 싶어요!

Copy link
Member

Choose a reason for hiding this comment

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

 width={375}
 height={666}

이부분을 2, 3배수로 바꾸서 확인해보면 좋을 것 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

이미지 안의 요소들이 절대 주소로 되어 있어서, 다음 PR에 나눠서 해보겠읍니다 ~~ 의견 감사드려용 👍

@hyesungoh hyesungoh merged commit 37e5b81 into main Nov 2, 2023
10 checks passed
@hyesungoh hyesungoh deleted the refactor/slow-ttfb-at-dna branch November 2, 2023 14:29
Copy link
Member Author

Choose a reason for hiding this comment

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

dynamic load를 위해 파일을 분리했어요

Comment on lines +97 to +100
imageDownloadPC(
`/api/dna-image?group=${group}&nickname=${userInfo?.nickname}&position=${userInfo?.position}`,
'dna',
);
Copy link
Member Author

Choose a reason for hiding this comment

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

api route를 이용해 이미지를 저장해요

Comment on lines 181 to 185
<DNAImageDownloadModal
downloadableBase64={downloadableImage.base64}
imageSrc={`/api/dna-image?group=${group}&nickname=${userInfo?.nickname}&position=${userInfo?.position}`}
isShowing={isImageModalShowing}
onClose={() => setIsImageModalShowing(false)}
/>
Copy link
Member Author

Choose a reason for hiding this comment

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

api route를 이용해 이미지를 렌더링해요

@hyesungoh
Copy link
Member Author

개선 이전

나랩 dna 개선 이전

개선 이후

나랩 dna 개선 이후

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation feat New feature or request fix Fix something was not working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants