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

[Fix] 확성기 스크롤 오류 수정 #1584

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[Fix] 확성기 스크롤 오류 수정 #1584

wants to merge 4 commits into from

Conversation

yoouyeon
Copy link
Member

📌 개요

  • 확성기 스크롤 위치 오류 수정

💻 작업사항

  • 확성기 스크롤 위치가 맞지 않고 빈 메시지가 들어가는 문제를 수정했습니다.
  • 문제 원인으로 추정되는 것은 유저가 임의로 스크롤하지 못하도록 추가했던 overflow-y: hidden !important
  • 스크롤은 정상적으로 동작하도록 하기 위해서 라이브러리 설정 그대로 overflow-y: auto 가 적용되도록 하고
  • 유저가 스크롤을 시도할 때 발생하는 wheel, touchmove 이벤트에서 스크롤 동작이 발생하지 않도록 설정했습니다.

참고

수정 결과 스크린샷

Screen_Shot 2024-12-22 23 15 07

💡관련 Issue

@yoouyeon yoouyeon added the fix bug fix label Dec 22, 2024
@yoouyeon yoouyeon self-assigned this Dec 22, 2024
@nerdchanii
Copy link

@yoouyeon 고생하셨습니다!:) 좋아보여요!👍🏻 추가적으로 궁금한 부분이 하나 있는데, 기술스택을 고르시면서 swiperjs같은 라이브러리들도 고민하시고 고르셨을 것 같은데, virtuoso를 사용하셨던 이유가 조금 궁금해요:)

action의 runner가 환경문제로 빌드에 실패하고 있었고 #1587에서 해결되었습니다. 리베이스나 머지를 이용해 브랜치를 업데이트 해주세요

@yoouyeon
Chore: 불필요한 주석 정리
e2cc7a5 
- f316184
- 5d59ee5
- df4733f
@yoouyeon
Fix: 스크롤 위치 오류 수정
e75de5e 
유저의 임의 스크롤을 막기 위해서 조치한 것이 정상적인 스크롤 동작을 방해함.
@yoouyeon
Feat: 스크롤 이벤트를 무시하는 MegaphoneScroller 컴포넌트 추가
4b6c2ee 
- https://virtuoso.dev/customize-structure/
- https://virtuoso.dev/custom-scroll-container/
- passive 기본값 설정 (true)를 바꾸기 위해 직접 이벤트 리스너를 등록함.
@yoouyeon yoouyeon force-pushed the Fix/#1582-megaphone-scroll-empty branch from 493c543 to 4b6c2ee Compare December 23, 2024 08:55
@yoouyeon
Copy link
Member Author

@nerdchanii

추가적으로 궁금한 부분이 하나 있는데, 기술스택을 고르시면서 swiperjs같은 라이브러리들도 고민하시고 고르셨을 것 같은데, virtuoso를 사용하셨던 이유가 조금 궁금해요:)

우선 virtuoso 같은 가상 리스트 라이브러리를 사용하기로 했던 이유는 하루에 사용될 수 있는 확성기 아이템 개수에 제한을 두지 않았기 때문에 (이유는 기록을 안해둔 것 같네요...) 확성기 아이템 컴포넌트가 무한히 많아지는 상황을 걱정해서 사용하기로 했던 것입니다. 상점 기능을 추가할 당시에는 사용량이 많을 것을 걱정해서 가상리스트를 적용했던 것인데 지금 보면 예상보다는 사용량이 많지 않아서 좀 과한 느낌이 들기도 하네요 😂

말씀해주신 swiperjs는 지금 gg에 적용되어 있는 라이브러리인거죠? 저희 활동 기수에서 추가했던 라이브러리는 아니라 조금 찾아봤는데 자동 스크롤 기능을 지원하고 있더라구요. (https://swiperjs.com/swiper-api#autoplay-parameters)
이런 라이브러리는 고려 대상이 아니었어서 전혀 생각하지 못했던 부분이긴 한데, 지금 생각해보면 가상리스트는 확성기보다는 오히려 게임 기록 페이지에 적용하는게 좀 더 알맞아 보이고 확성기 쪽에는 (만약에 확성기 아이템 사용 개수 제한을 둘 수 있다면) swiperjs를 써서 좀 더 안정적으로 구현했어도 좋았을 것 같기도 합니다...ㅎㅎ

@nerdchanii
Copy link

nerdchanii commented Dec 23, 2024 •
edited
Loading

virtuoso를 이용하신 이유가 메시지가 많이쌓이고 확성기를 사용하는게 유저가 많을걸 고려하셔서 구현하신 것이었군요!

말씀처럼 저도 PR을 보면서 autoplay가 되는 라이브러리가 있는지 찾아보다 swiperjs의 autoplay를 발견했는데 단순히 이 기능이 필요한거면 swiperjs도 나쁘지 않았겠다 싶었고 과거에 어떤 고민이 있었는지 궁금하기도해서 여쭤보았었어요:) 작업하시느라 고생하셨습니다!

@nerdchanii nerdchanii self-requested a review December 24, 2024 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nerdchanii nerdchanii nerdchanii approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

@yoouyeon yoouyeon

Labels
fix bug fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] 확성기 스크롤과 빈 내용 오류
3 participants
@yoouyeon @nerdchanii @JeongRon