-
Notifications
You must be signed in to change notification settings - Fork 532
fix: simplebar overlaps header #1743
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
base: develop
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
4603f65 to
59d2be4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메인페이지에서 밑으로 스크롤하고 위로 스크롤하면 아래에서 refresh button이 나타나야 하는데 이 기능이 작동이 안하게 되었어요. 다시 살펴봐주세용 -> 이 문제는 제가 수정했습니다. 추가 커밋 봐주세욤
그리고 select wallet page에서 Add Wallet 버튼의 위치가 이상해졌어요. 해당 페이지에서 월렛이 여러개있을때 왼쪽의 작대기 두개를 드래그 앤 드랍해서 월렛 순서를 바꾸는 기능이 있는데 그게 제대로 동작하는지도 확인 필요 -> 수정함
Thunnini
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixedHeight prop등이 사라진건 별 문제없는 것 같아용
| scrollElement.removeEventListener("scroll", handleScroll); | ||
| }; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| const handleShowBorderBottomChange = useCallback((show: boolean) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이걸 useCallback으로 처리하는건 솔직히 의미없어보입니당
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지우겠습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| }, | ||
| ]} | ||
| > | ||
| <PageChangeScrollTop /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PageChangeScrollTop이 삭제된건 simplebar가 각 layout 밑으로 옮겨졌기 때문에 쓸모가 없어진건가요. 대체하는 코드가 있는데 제가 못 찾은걸까용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이제는 Route가 바뀌면 스크롤 위치가 초기화되는데, Route가 대부분의 pathname을 커버하고 있는듯하여 불필요하다 생각해서 지웠슴니다. 파일도 같이 지워줘야겠네용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| value={useMemo(() => { | ||
| return { | ||
| ref, | ||
| refChangeHandler: (handler) => { | ||
| refHandlers.current.push(handler); | ||
|
|
||
| if (ref.current) { | ||
| handler(ref.current); | ||
| } | ||
|
|
||
| return () => { | ||
| refHandlers.current = refHandlers.current.filter( | ||
| (h) => h !== handler | ||
| ); | ||
| }; | ||
| }, | ||
| }; | ||
| }, [])} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
작업한거 한번 날려먹어서 빠뜨렸는데..
value에 memo 안하고 ref만 넣어주면 잘 됐던 거 같은데 낼 함 확인해볼게용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
provider value에 (memo하지 않은) ref를 넣고, 사용처에서 useEffect에 의존성 배열에 pageSimpleBar를 넣어주면 잘 동작하네요. 다만, Simplebar 컨텍스트가 HeaderLayout에 있기 때문에 HeaderLayout이 리렌더 되는 횟수만큼 effect가 실행됨니다.
저는 요렇게 가는게 좀더 간단해서 좋을 것 같은데 어떠신가요 @Thunnini
<PageSimpleBarContext.Provider value={{ ref }}>
<SimpleBar style={style} ref={ref}>
{children}
</SimpleBar>
</PageSimpleBarContext.Provider> useEffect(() => {
const scrollElement = pageSimpleBar.ref.current?.getScrollElement();
...
}
}, [pageSimpleBar]);There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
구두 논의) effect에서 불필요한 계산 반복을 막으려면
PageSimpleBar에 refChangeHandler 기능 추가 이 방식이 나을 것 같음
|
지금 변경사항이 많이 쌓여있는데 이 PR은 QA에서 모든 페이지를 확인해봐야하므로 다다음 배포 버전으로 넘기겠습니다. |
문제
해결 및 변경 사항
Simplebar를 전역 컨텍스트에서 layout component(
HeaderLayout) 내부로 이동HeaderLayout이 존재하는 페이지에 컨텍스트가 생성되고, 해당 컨텍스트에서만 simplebar가 유효함. 이제GlobalSimpleBar는 더이상 글로벌이 아니게 되었으므로,PageSimpleBar로 이름 변경.HeaderLayout또는MainHeaderLayout을 사용하기 때문에 대부분의 페이지에 올바르게 스크롤이 동작함. (register, blocklist는 별도 페이지라 브라우저 스크롤이 사용되고, tx-result, unlock는 extension 화면 크기에 맞게 설계됨)본문 영역의 높이를 계산하는 로직을 제거하고
height: 100%로 단순화. 2c6b58dfixedHeight,fixedMinHeight,fixedTop,fixedTopHeight는 더이상 안쓰거나, 불필요한 듯하여 제거했습니다. resize event에 반응해서 height를 계산하던 로직도 마찬가지로 제거했습니다.AS-IS
2026-01-05.6.57.13.mov
TO-BE
2026-01-05.6.56.35.mov