feat: 내비게이션 바 애니메이션 추가 #435

WaiNaat · 2023-10-14T16:25:16Z

🔥 연관 이슈

close navigation bar에 슬라이드 액션을 추가한다. #425

🚀 작업 내용

페이지 이동 시 내비게이션 바 애니메이션 추가
- 만약 로딩중이라 상단에 로딩 바가 보이는 상황이라면 로딩이 다 끝나고 이동
내비게이션 바 RootTemplate으로 분리
자잘한 코드 리팩터링
- 컴포넌트 렌더링 함수 내에서 다른 컴포넌트를 정의하는 문제
- 불필요한 non-null assertion, useMemo
- exhaustive deps

💬 리뷰 중점사항

css 쉽지 않네요..

대략적인 로직은 아래와 같습니다

grid로 배치
border-top은 div로 분리
resizeObserver로 nav item들의 좌표 계산
pathname을 비교해서 2번의 div 위치 조정

페이지도 같은 방향으로 넘어가게 하는 것도 하고 싶었는데 이건 진짜 쉽지 않더라구요
리코일로 현재 페이지와 이전 페이지 두 개를 기억하는 쪽으로 구현해서 돌아가게는 할 수 있었는데 여러모로 부족한 부분이 많아 그냥 버렸습니다. 다가가는 방향 자체를 잘못 잡은 것 같아요

컴포넌트의 렌더링 함수에서 정의한 컴포넌트라 분리

as prop을 적용해서 Button으로 통일

bassyu

깔끔하네요~!
Navbar가 전역으로 가니 간단하게 해결됐군요 👍
갠적으로는 내려가는 애니메이션도 맘에 드네용

bassyu · 2023-10-17T01:20:05Z

frontend/src/components/@common/Navbar/NavItem.tsx

A

분리 좋습니다 👍

bassyu · 2023-10-17T01:22:03Z

frontend/src/components/@common/Navbar/index.tsx

+          <Button type="button" onClick={askLogin}>
+            <NavItem isActive={isActive(URL_PATH.petList)} iconId="leaf" label="내 식물" />
+          </Button>
+          <Button as={Link} to={URL_PATH.login} state={{ prevPathname: pathname }}>


A

오 as를 쓰니 속성도 생기고 자동완성도 잘 되네요?

hozzijeong

참새 멋있습니다 진짜 감동의 네브바 슬라이드네요...

프레임 드랍만 어느정도 해결되면 좋을 것 같아요!

hozzijeong · 2023-10-17T11:11:11Z

frontend/src/components/@common/Navbar/index.tsx

R

전체적으로 Roof관련된 로직을 훅으로 분리하면 좀 더 깔끔할 것 같아요!

R

아까 말한 프레임 드랍도 같이 해결하면 좋을 것 같아요!

근데 이걸 훅으로 분리한다고 관심사 분리가 잘 될지는 모르겠네요..
roof position 자체가 navbar 내용물 순서에 종속되어 있는데 이 내용들이 jsx에 하드코딩되어 있기 때문에 어디를 분리하는 게 맞는지 잘 모르겠어요

requestAnimationFrame도 찾아보니 화면 주사율에 따라서 호출 횟수가 달라서 이동 시간을 모든 모니터에서 동일하게 맞출 수 있는 방법이 있을까요?

hozzijeong · 2023-10-17T11:32:00Z

frontend/src/hooks/@common/useIntersectionRef.ts

+  const intersectionRef = useCallback(
+    <T extends Element>(instance: T | null) => {
+      if (instance) observer.observe(instance);
+    },
+    [observer]


크~ 멋있ㅅ습니다

hozzijeong · 2023-10-17T11:33:24Z

frontend/src/pages/garden/GardenPostList/index.tsx

+const SKELETONS = new Array(SKELETON_LENGTH)
+  .fill(null)
+  .map((_, index) => <GardenPostItemSkeleton key={index} />);


C

스켈레톤을 반환하는 값은 어차피 한번만 호출하면 되기 때문에 컴포넌트 밖으로 뺀 것인가요??

네 기존의 컴포넌트 내부에서 컴포넌트 정의하는 문제를 해결하려고 밖으로 뺐는데 굳이 스켈레톤 목록이 컴포넌트여야 하는가라는 생각이 들어서 분리했어요

- 자식들 위치 계산 로직 훅으로 분리 - transition offset 계산 로직 분리

github-actions · 2023-10-18T13:10:30Z

Unit Test Results

  80 files   80 suites 26s ⏱️
339 tests 339 ✔️ 0 💤 0 ❌
345 runs 345 ✔️ 0 💤 0 ❌

Results for commit 6ef34e2.

WaiNaat · 2023-10-18T13:14:56Z

클린과 얘기 나눠보니까 확실히 분리하는 게 깔끔하겠더라구요

저도 마음으로는 컴포넌트 안쪽에 로직은 최대한 줄이는 게 좋다고 생각하고 있었지만 절대 재사용할 수 없는 로직을 마구 훅으로 분리하는 건 co-location이 아니라면 리팩터링 비용이 비싸다고 생각해서 망설이고 있었습니다. 근데 다시 생각해보니 너무 미래 중심적인 생각인 것 같았어요 확실히 분리하니까 보기 좋네요ㅋㅋㅋ

추가로 애니메이션이 휙휙 바뀌던 부분은 처리 완료했습니다
범인은 프레임 드랍이 아니고 다른 두 가지였어요

예를 들어 내 식물 -> 마이페이지를 누르고 isLoading 상태에서 roof는 마이페이지이지만 offset은 0이라서 애니메이션 X
예를 들어 리마인더 -> 내 식물 -> 마이페이지로 갈 때 세 요소 사이의 픽셀 간격이 완벽히 똑같다면 동일한 애니메이션이라 재실행하지 않는 문제 (stack에서 겪었던 문제와 똑같습니다)

hozzijeong

멋져요!

hozzijeong · 2023-10-19T01:02:05Z

frontend/src/hooks/@common/useChildrenLeftPositions.ts

* refacor: NavItem 컴포넌트 분리 컴포넌트의 렌더링 함수에서 정의한 컴포넌트라 분리 * refactor: NavLink 제거 as prop을 적용해서 Button으로 통일 * feat: Navbar 애니메이션 추가 * refactor: Navbar RootTemplate으로 이동 * refactor: 페이지 로딩 이후에 Navbar도 바뀜 * refactor: 컴포넌트 안에서 컴포넌트 정의하는 문제 수정 * refactor: 불필요한 non-null assertion, useMemo 제거 * refactor: exhaustive deps 문제 해결 * refactor: 애니메이션 로직 정리 - 자식들 위치 계산 로직 훅으로 분리 - transition offset 계산 로직 분리 * refactor: 애니메이션 계산 로직 분리 * fix: typo

WaiNaat added 8 commits October 14, 2023 15:29

refacor: NavItem 컴포넌트 분리

7e170c4

컴포넌트의 렌더링 함수에서 정의한 컴포넌트라 분리

refactor: NavLink 제거

aa372c5

as prop을 적용해서 Button으로 통일

feat: Navbar 애니메이션 추가

736b3ff

refactor: Navbar RootTemplate으로 이동

6a9d46f

refactor: 페이지 로딩 이후에 Navbar도 바뀜

5ae36f6

refactor: 컴포넌트 안에서 컴포넌트 정의하는 문제 수정

fb36f39

refactor: 불필요한 non-null assertion, useMemo 제거

e1f281c

refactor: exhaustive deps 문제 해결

f29883a

WaiNaat added 🛠️ 리팩터링 리팩터링을 위한 이슈입니다 🍇 프론트엔드 프론트엔드 관련 이슈입니다 labels Oct 14, 2023

WaiNaat added this to the 6차 스프린트 milestone Oct 14, 2023

WaiNaat self-assigned this Oct 14, 2023

WaiNaat added the 🌈 기능 새로운 기능을 개발합니다 label Oct 14, 2023

WaiNaat requested review from bassyu and hozzijeong and removed request for bassyu October 15, 2023 02:15

WaiNaat marked this pull request as ready for review October 15, 2023 02:15

bassyu approved these changes Oct 17, 2023

View reviewed changes

hozzijeong requested changes Oct 17, 2023

View reviewed changes

WaiNaat added 3 commits October 18, 2023 15:57

refactor: 애니메이션 로직 정리

df8f82c

- 자식들 위치 계산 로직 훅으로 분리 - transition offset 계산 로직 분리

refactor: 애니메이션 계산 로직 분리

e1ee3c1

Merge branch 'develop' into feature/425-awesome_nav_bar

6ef34e2

fix: typo

e66cdb0

WaiNaat requested a review from hozzijeong October 18, 2023 13:23

hozzijeong approved these changes Oct 19, 2023

View reviewed changes

frontend/src/hooks/@common/useChildrenLeftPositions.ts

Copy link

Collaborator

hozzijeong Oct 19, 2023

Choose a reason for hiding this comment

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

👍

hozzijeong merged commit c8a4bc7 into develop Oct 19, 2023
2 checks passed

hozzijeong deleted the feature/425-awesome_nav_bar branch October 19, 2023 01:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 내비게이션 바 애니메이션 추가 #435

feat: 내비게이션 바 애니메이션 추가 #435

WaiNaat commented Oct 14, 2023

bassyu left a comment

bassyu Oct 17, 2023

bassyu Oct 17, 2023

hozzijeong left a comment

hozzijeong Oct 17, 2023

hozzijeong Oct 17, 2023

WaiNaat Oct 17, 2023 •

edited

Loading

WaiNaat Oct 17, 2023

hozzijeong Oct 17, 2023

hozzijeong Oct 17, 2023

WaiNaat Oct 17, 2023

github-actions bot commented Oct 18, 2023

WaiNaat commented Oct 18, 2023

hozzijeong left a comment

hozzijeong Oct 19, 2023

feat: 내비게이션 바 애니메이션 추가 #435

feat: 내비게이션 바 애니메이션 추가 #435

Conversation

WaiNaat commented Oct 14, 2023

🔥 연관 이슈

🚀 작업 내용

💬 리뷰 중점사항

bassyu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

A

Choose a reason for hiding this comment

A

hozzijeong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

R

Choose a reason for hiding this comment

R

WaiNaat Oct 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C

Choose a reason for hiding this comment

github-actions bot commented Oct 18, 2023

Unit Test Results

WaiNaat commented Oct 18, 2023

hozzijeong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WaiNaat Oct 17, 2023 •

edited

Loading