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

fix(view): #540 Total Commit 그래프에 마우스 over, move 시 깜빡임 #550

Merged
merged 2 commits into from
Jul 23, 2024

Conversation

seungineer
Copy link
Member

@seungineer seungineer commented Jul 22, 2024

Related issue

#540 [view] Total Commit 그래프에 마우스 호버 시 깜빡임 현상 발생

원인

githru_mouseover_bug

  • 마우스 move 시 .style {display: none}.style {display: inline-block}이 반복되는 것을 확인할 수 있습니다.

Result

  • Total Commit 그래프(Bar Chart)에 마우스 over 시 tooltip이 렌더링 되는 위치를 수정하였습니다.({e.pageY - 70}px{e.pageY - 90}px 으로)
  • 마우스 Over 상태에서 마우스 Move 때마다 렌더링 되던 tooltip을 마우스 Over 때 tooltip이 렌더링 되고, Move 때마다 tooltip의 위치를 조정하도록 수정하였습니다. (이미지2 참고)

Work list

결과(이미지1)

author_bar_깜빡임해결

  • tooltip의 위치가 항상 마우스보다 위에 있습니다.

tooltip 렌더링 순서 변경 사유(이미지2)

간헐적으로 씹히는 버그

  • 마우스를 빠르게 움직일 경우, 움직이기 전 마우스 위치에서 tooltip이 렌더링 되고, tooltip이 렌더링 된 위치에 움직인 후의 마우스 커서가 존재할 경우 깜빡임이 발생합니다.
  • 위 '깜빡임'의 원인은 렌더링 된 tooltip 위에 마우스가 위치할 경우 Bar Chart에서는 handleMouseOut되고, 이에 따라 tooltip이 사라지게 됩니다. Tooltip이 사라짐으로써 Bar Chart에 handleMouseOver가 호출됩니다. 이후 마우스가 움직이며 handleMouseMove가 호출됩니다. 위 과정이 반복되며 '깜빡'이는 것처럼 보이게 됩니다.

Discussion

Bar Chart 외 handleMouseOver, handleMouseMove 이 호출되는 곳이 있는지 확인한 결과 Bar Chart 외 없어서 함수를 수정하였습니다. handleMouseOver, handleMouseOut 함수를 수정하지 않기 위해서는 Bar Chart 전용 마우스 Over, Move 함수를 만들어야 할 것 같습니다. (예: handleBarChartMouseOver, handleBarChartMouseMove)

@seungineer seungineer requested review from a team as code owners July 22, 2024 08:14
@seungineer seungineer self-assigned this Jul 22, 2024
Copy link
Contributor

@HIITMEMARIO HIITMEMARIO left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@mdgarden mdgarden left a comment

Choose a reason for hiding this comment

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

LGTM👍👍👍

Copy link
Contributor

@lxxmnmn lxxmnmn left a comment

Choose a reason for hiding this comment

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

오 좋은 해결책이네요 👍👍👍👍👍

툴팁 스타일에 pointer-events: none; 속성을 추가하면
[이미지2]처럼 커서가 툴팁 위에 위치하더라도 mouseout 이벤트가 일어나지 않아서
깜빡이는 것처럼 보이는 현상이 사라질 것 같습니다!

Reference

pointer-events

@seungineer
Copy link
Member Author

툴팁 스타일에 pointer-events: none; 속성을 추가하면 [이미지2]처럼 커서가 툴팁 위에 위치하더라도 mouseout 이벤트가 일어나지 않아서 깜빡이는 것처럼 보이는 현상이 사라질 것 같습니다!

속성 추가해보니 아래와 같이 잘 동작하네요! 감사합니다. pointer-events와 같은 속성이 있는 것에 대해 알게 되었어요😄

pointer-events-none

기존 코드를 적게 수정하고, 해결할 수 있어 속성을 추가하는 위 방법이 더 나아 보이는데 어떻게 생각하시는지 궁금합니다! 🤔🤔🤔

@lxxmnmn
Copy link
Contributor

lxxmnmn commented Jul 23, 2024

기존 코드를 적게 수정하고, 해결할 수 있어 속성을 추가하는 위 방법이 더 나아 보이는데 어떻게 생각하시는지 궁금합니다! 🤔🤔🤔

잘 동작한다니 다행입니다! 👏👏👏

mouseover 이벤트는 마우스 커서가 특정 요소에 처음 진입할 때 1회 발생하고
mousemove 이벤트는 마우스 커서가 특정 요소 위에서 움직일 때마다 지속적으로 발생하기 때문에

제 개인적인 생각에는, 승우님이 작업해주신 것처럼
handleMouseOver에서 툴팁을 렌더링하고 handleMouseMove에서는 위치만 변경하는 방식이 좋아보입니다!!

지금까지의 커밋 내용은 유지하면서 pointer-events 속성을 추가하는 건 어떨까요? 🤔🤔

@ytaek
Copy link
Contributor

ytaek commented Jul 23, 2024

기존 코드를 적게 수정하고, 해결할 수 있어 속성을 추가하는 위 방법이 더 나아 보이는데 어떻게 생각하시는지 궁금합니다! 🤔🤔🤔

잘 동작한다니 다행입니다! 👏👏👏

mouseover 이벤트는 마우스 커서가 특정 요소에 처음 진입할 때 1회 발생하고 mousemove 이벤트는 마우스 커서가 특정 요소 위에서 움직일 때마다 지속적으로 발생하기 때문에

제 개인적인 생각에는, 승우님이 작업해주신 것처럼 handleMouseOver에서 툴팁을 렌더링하고 handleMouseMove에서는 위치만 변경하는 방식이 좋아보입니다!!

지금까지의 커밋 내용은 유지하면서 pointer-events 속성을 추가하는 건 어떨까요? 🤔🤔

저도 이렇게 하면 좋을 것 같아요!!

@ytaek
Copy link
Contributor

ytaek commented Jul 23, 2024

PR을 너무 자세하게 잘 써주셔서 좋은 피드백들이 오가는 것 같습니다!! 👍👍👍👍👍👍

@seungineer
Copy link
Member Author

seungineer commented Jul 23, 2024

지금까지의 커밋 내용은 유지하면서 pointer-events 속성을 추가하는 건 어떨까요? 🤔🤔

저도 이렇게 하면 좋을 것 같아요!!

피드백 감사합니다 😆 Approve 되었기에, 추후 'tooltip 속성 추가'하는 PR 다시 올리도록 하겠습니다.
리뷰 해주신 분들 모두 감사합니다👍🌟

@seungineer seungineer merged commit 18d2a9f into githru:main Jul 23, 2024
4 checks passed
@ytaek ytaek added this to the v0.7.0 milestone Aug 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants