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

feat: [LINKER-110] 프로필 헤더 수정 #52

Merged
merged 25 commits into from
Feb 11, 2024
Merged

Conversation

useonglee
Copy link
Member

@useonglee useonglee commented Feb 7, 2024

작업 내용

  • 스크롤했을 경우 프로필 헤더가 덜컹거리는 느낌을 주지않기 위해 스크롤을 하면 헤더 위에 헤더를 덮어 씌우는 식으로 구현했습니다.
  • @danivelop 님이 features 폴더를 만들어 주셨었는데, 폴더 아키텍쳐 레퍼런스 보고 참고해서 폴더 구조 수정을 했어요.
features
  |- my
  |- friend 
 
  • Tabs 컴포넌트 수정은 다음 PR에서 진행하겠습니다 🙏
  • useIsScrollOver hook 추가
    스크린샷 2024-02-09 오전 2 58 09

반영 화면

비 로그인 로그인
2024-02-08.6.06.42.mov
2024-02-08.6.07.00.mov

기타

  • n/a

@useonglee useonglee self-assigned this Feb 7, 2024
Copy link

github-actions bot commented Feb 7, 2024

Copy link

github-actions bot commented Feb 7, 2024

Copy link

github-actions bot commented Feb 7, 2024

Copy link

github-actions bot commented Feb 7, 2024

@@ -0,0 +1,27 @@
import { useEffect, useState } from 'react';

const useIsScrolling = (y = 0) => {
Copy link
Member

Choose a reason for hiding this comment

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

hook이 하는 역할이 스크롤 위치가 인자로 받은 값을 넘었는지 여부를 나타내는것 같아서 useIsScorllOver, useScrollBoundary 같은 네이밍은 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

useIsScrollOver로 수정했습니다

f6b052e

Comment on lines 7 to 21
const handleScroll = () => {
if (y < window.scrollY) {
setIsScrolling(true);

return;
}

setIsScrolling(false);
};

window.addEventListener('scroll', handleScroll);

return () => {
window.removeEventListener('scroll', handleScroll);
};
Copy link
Member

Choose a reason for hiding this comment

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

scroll이벤트시마다 리렌더링이 발생하며 성능상 저하가 발생할 수 있을것 같은데 실제로 scrollY가 인자값을 통과하는 시점에만 setIsScrolling을 호출하도록 개선해봐도 좋을것 같습니다!

현재 isScrolling에 대한 ref값만 하나 추가하면 구현 가능하지 않을까 싶네요

Copy link
Member Author

@useonglee useonglee Feb 8, 2024

Choose a reason for hiding this comment

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

useEffect dependency array로 인해서 scroll 이벤트마다 리렌더링이 발생하지 않습니닷
unmount 이후 인수값보다 낮아지지 않으면 scroll 이벤트가 발생하지 않는다는 테스트 코드 추가해두었어용

9681f8a

스크린샷 2024-02-09 오전 2 58 09

Copy link

github-actions bot commented Feb 8, 2024

return (
<Image
src={`${bucket}/icons/${name}.svg`}
alt={`${name} icon`}
width={size}
height={size}
height={height ?? size}
Copy link
Collaborator

Choose a reason for hiding this comment

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

height와 size의 차이가 무엇인가요?!

Copy link
Member Author

Choose a reason for hiding this comment

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

icon은 너비와 높이가 항상 일정해서 size 값 하나로 width, height를 적용하려고 했었어요. 그러나 로고만 유일하게 width, height가 달라서 icon 컴포넌트가 height도 따로 받을 수 있도록 추가했었습니다.

그러나 생각해보니 logo가 아니면 height가 필요하지 않을 것 같아서 제거했습니다
8e77c1a

그리고 Logo 컴포넌트를 따로 추가했습니닷
8e77c1a

Copy link
Collaborator

Choose a reason for hiding this comment

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

감사합니당! 이해완료했습니닷!

@useonglee useonglee merged commit 35a053d into develop Feb 11, 2024
5 checks passed
@useonglee useonglee deleted the feat/LINKER-110 branch February 11, 2024 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants