-
Notifications
You must be signed in to change notification settings - Fork 75
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
Zotistics graph pops up on hover #727
Conversation
@MinhxNguyen7 do you still think we should redirect to Zotistics when the button is clicked? If we do that, mobile users would not be able to see the graph popup, which they might prefer to see instead of having to go to Zotistics. Feel free to give other suggestions too. |
Never noticed this before but the x axis on the bar graph is unlabeled. Probably a good idea to do that at some point but it was also this way before this PR so I'll open it as a separate issue. |
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.
Code looks pretty good. Two requests form me here:
- On mobile, when you tap the Zotistics button again, can it close the popup? I think it would be a better experience, feel free to push back here.
- Can you update the patch notes in this PR?
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.
🚀
Summary
Popover
component has some weird interactions withOnMouseLeave
andOnMouseEnter
events. When the mouse leaves the area, sometimes the popup doesn't close. To avoid this, we can use thePopper
component, which is basically a lower-levelPopover
.Demo:
graphDemo.mp4
Test Plan
Issues
Closes #672