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.

Sign up for GitHub

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

Jump to bottom

[FE] FIX: 새로고침 버튼 누를 때 나타나는 스크롤바 제거 #1693

Merged
merged 3 commits into from
Oct 18, 2024
Merged

[FE] FIX: 새로고침 버튼 누를 때 나타나는 스크롤바 제거 #1693

merged 3 commits into from
Oct 18, 2024

Conversation

jihyunk03
Copy link
Collaborator

@jihyunk03 jihyunk03 commented Oct 8, 2024 •
edited
Loading

해당 사항 (중복 선택)

  • FEAT : 새로운 기능 추가 및 개선
  • FIX : 기존 기능 수정 및 정상 동작을 위한 간단한 추가, 수정사항
  • BUG : 버그 수정
  • REFACTOR : 결과의 변경 없이 코드의 구조를 재조정
  • TEST : 테스트 코드 추가
  • DOCS : 코드가 아닌 문서를 수정한 경우
  • REMOVE : 파일을 삭제하는 작업만 수행
  • RENAME : 파일 또는 폴더명을 수정하거나 위치(경로)를 변경
  • ETC : 이외에 다른 경우 - 어떠한 사항인지 작성해주세요.

설명

  • MainPage에서 isLoading의 값에 따라 보이는 컴포넌트 변경
  • LoadingAnimation에 스크롤 기능 제거하기 위해 hidden 추가

#1669

@jihyunk03 jihyunk03 linked an issue Oct 8, 2024 that may be closed by this pull request
[FE] 층 Section 에서 새로고침 버튼 누를 시 스크롤 바가 나타나는 현상 #1669
Closed
@jihyunk03
Copy link
Collaborator Author

컨벤션에 삼항연산자를 지양한다고 되어있는데, if else로 return 하는 것보다 가독성이 좋다고 생각했습니다..
그리고 해당 컴포넌트에서 return되는 최상위 컴포넌트가 하나여서 <></> 부분을 제거하였더니, 변경된 부분이 많아졌습니다.. (실제 변경된 부분은 삼항 연산자 부분 뿐입니다)

wet6123
wet6123 reviewed Oct 9, 2024
View reviewed changes
Copy link
Collaborator

@wet6123 wet6123 left a comment

Choose a reason for hiding this comment

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

첫번째 이슈 해결하시느라 고생 많으셨습니다!
로딩 컴포넌트가 기존 컴포넌트를 밀어내서 스크롤이 생기는 것이었군요!

이번 이슈가 로딩창을 확인하는 것이었는데 로딩이 엄청 빨라서 확인하기 힘들던데 어떤 방식으로 확인해보셨나요?
저는 개발자도구 network탭에서 throttling을 걸어서 로딩이 오래 걸리게 만들었는데 또 좋은 방법이 있는지 궁금합니다.

추가적으로 궁금한 부분을 코멘트로 달아놨는데 알려주시면 정말 감사할것 같습니다.

frontend/src/Cabinet/components/Common/LoadingAnimation.tsx Outdated
@@ -13,6 +13,7 @@ const LoadingAnimationWrapperStyled = styled.div`
width: 100%;
height: 100%;
display: flex;
overflow-y: hidden;
Copy link
Collaborator

Choose a reason for hiding this comment

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

overflow: hidden은 하위 요소에 영향을 미치는걸로 알고있는데 이 부분에서 사용한 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오 지금 다시 생각해보니 해당 속성이 필요가 없네요!
이전에는 컴포넌트 교체를 하지 않기 위해 상위 컴포넌트에 hidden 속성을 사용하려다가 문제가 되어서 하위에 적용 해봤다가, 교체하는 형식으로 해결한 후 hidden 속성으로 스크롤이 생기지 않는다고 생각했는데, 잘못 생각했던 것 같습니다..
삭제하겠습니다 ㅎㅎ 알려주셔서 감사합니다!!

Copy link
Collaborator

Choose a reason for hiding this comment

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

hidden 설정 부분 삭제했으니, pr 설명해서도 취소선이나 삭제 부탁드려욤

frontend/src/Cabinet/pages/MainPage.tsx
Comment on lines +126 to +129
return isLoading ? (
<LoadingAnimation />
) : (
<WrapperStyled
Copy link
Collaborator

@wet6123 wet6123 Oct 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

이렇게 컴포넌트를 교체하는 방식과 기존의 오버레이를 띄우는 방식의 장단점에 대해서 생각해보는 것도 재밌을 것 같습니다.
저는 캐비닛 컴포넌트가 복잡하니 이걸 다시 마운트하는 것보다 기존의 오버레이 방식으로 처리하고 css로 스크롤을 숨기는게 더 좋을것 같다고 생각합니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

먼저, 저도 이에 대해 고민이 많았는데, 크롬의 익스텐션에서 랜더링하는 부분에 대해 녹화를 진행해서 전후를 비교했을 때, 교체하더라도 많은 비용이 발생하지 않아서 이와 같이 진행하게 되었습니다.
기존의 오버레이 방식으로 진행하고 싶었으나, 해당 컴포넌트의 상위 컴포넌트(MainStyled)의 css를 변경하게 되면 교체되는 다른 컴포넌트들에 대해서도 모두 적용이 되어 스크롤이 필요한 부분에서 숨겨지는 문제가 있어 위와 같은 방식으로 진행했습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

먼저, 저도 이에 대해 고민이 많았는데, 크롬의 익스텐션에서 랜더링하는 부분에 대해 녹화를 진행해서 전후를 비교했을 때, 교체하더라도 많은 비용이 발생하지 않아서 이와 같이 진행하게 되었습니다.

오 이건 어떻게 보는건가요 나중에 만나면 알려주세요!!

맞아요 저도 코드 읽어보니 MainStyled에 overflow를 걸기가 어렵긴 하더라구요. 오늘 다시 생각해봤는데 z-index를 이용해서 로딩 화면을 기존 컴포넌트랑 겹쳐서 그 위에 띄우면 오버레이 방식으로도 해볼만한것 같습니다. 스크롤이 생기지 않으니 overflow도 신경 쓰지 않아도 되겠네요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 ㅎㅎ :)
오 제가 z-index를 잘 몰랐는데, 찾아보니 좋은 방법인것 같습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

지현님이 고민하실 때 저도 옆에서 같이 궁금했던 부분인데, z-index를 이용하는 방법도 있었군요! 새롭게 알아갑니다~~

@jihyunk03
Copy link
Collaborator Author

이번 이슈가 로딩창을 확인하는 것이었는데 로딩이 엄청 빨라서 확인하기 힘들던데 어떤 방식으로 확인해보셨나요? 저는 개발자도구 network탭에서 throttling을 걸어서 로딩이 오래 걸리게 만들었는데 또 좋은 방법이 있는지 궁금합니다.

먼저 크롬에서는 로딩 이후에 스크롤을 내려도 별 문제가 생기진 않지만, 사파리에서는 로딩 중 스크롤을 내리면 해당 컴포넌트가 밀려서 사라지는 현상이 생겼습니다.
이를 실제로 확인하기 위해 onclick에서 진행되는 refreshCabinetList 함수 내부에서 setTimeout의 값을 늘렸습니다!

@wet6123
Copy link
Collaborator

wet6123 commented Oct 10, 2024

@jihyunk03

먼저 크롬에서는 로딩 이후에 스크롤을 내려도 별 문제가 생기진 않지만, 사파리에서는 로딩 중 스크롤을 내리면 해당 컴포넌트가 밀려서 사라지는 현상이 생겼습니다.

오 이건 원인이 뭘까요? 사파리에 쓰로틀링 기능이 사라진것 같아서 사파리로 테스트를 해보지는 못했는데 신기하네요.

@jihyunk03
Copy link
Collaborator Author

@wet6123

오 이건 원인이 뭘까요? 사파리에 쓰로틀링 기능이 사라진것 같아서 사파리로 테스트를 해보지는 못했는데 신기하네요.

실은 저도 원인은 모르고, 사파리를 자주 사용하다보니 알게된것 같아요 ㅎㅎ 원인을 알게되면 말씀드릴게요!

@jnkeniaem
Copy link
Collaborator

첫번째 이슈 해결 🎊 추카추카

저두 return 문에선 삼항연산자나 논리 연산자 사용하는 편입니당 ㅎㅎ

@jnkeniaem
Copy link
Collaborator

assignees, labels 선택해주세요~

@jihyunk03 jihyunk03 self-assigned this Oct 15, 2024
@jihyunk03 jihyunk03 added FE Frontend tasks FIX Fix former works better v5 Version 5 labels Oct 15, 2024
seonmiki
seonmiki approved these changes Oct 15, 2024
View reviewed changes
Copy link
Collaborator

@seonmiki seonmiki left a comment

Choose a reason for hiding this comment

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

첫 이슈 해결! 고생하셨습니다~ 준성님 덕분에 이런저런 방법에 대해서 좀 더 고민해볼 수 있었네요 감사합니다:)

jimchoi9
jimchoi9 approved these changes Oct 15, 2024
View reviewed changes
Copy link
Collaborator

@jimchoi9 jimchoi9 left a comment

Choose a reason for hiding this comment

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

첫 이슈 고생하셨습니다~~

@jihyunk03 jihyunk03 merged commit 01136bf into dev Oct 18, 2024
1 check passed
@jihyunk03 jihyunk03 deleted the fe/dev/bug_scrollbar_error/#1669 branch October 18, 2024 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wet6123 wet6123 wet6123 left review comments

@jnkeniaem jnkeniaem jnkeniaem approved these changes

@seonmiki seonmiki seonmiki approved these changes

@jimchoi9 jimchoi9 jimchoi9 approved these changes

Assignees

@jihyunk03 jihyunk03

Labels
FE Frontend tasks FIX Fix former works better v5 Version 5
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FE] 층 Section 에서 새로고침 버튼 누를 시 스크롤 바가 나타나는 현상
5 participants
@jihyunk03 @wet6123 @jnkeniaem @seonmiki @jimchoi9