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

Zotistics graph pops up on hover #727

Merged
merged 13 commits into from
Oct 26, 2023
Merged

Zotistics graph pops up on hover #727

merged 13 commits into from
Oct 26, 2023

Conversation

Douglas-Hong
Copy link
Contributor

@Douglas-Hong Douglas-Hong commented Oct 11, 2023

Summary

  • The Zotistics graph now pops up when the user hovers over the Zotistics course info button. The popup closes when the mouse moves away from the popup or button.
  • The Popover component has some weird interactions with OnMouseLeave and OnMouseEnter events. When the mouse leaves the area, sometimes the popup doesn't close. To avoid this, we can use the Popper component, which is basically a lower-level Popover.
  • Some minor styling adjustments were made. The popup is a bit smaller so that it doesn't take much space, and so that the "View on Zotistics" link doesn't get cut off.

Demo:

graphDemo.mp4

hover

Test Plan

  • Verify that the graph pops up in the Search and Added tabs.
  • Verify that the graph looks okay on mobile devices.
  • Verify that the "View on Zotistics" link still works.

Issues

Closes #672

@Douglas-Hong
Copy link
Contributor Author

Douglas-Hong commented Oct 11, 2023

@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.

@EricPedley
Copy link
Member

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.

Copy link
Member

@EricPedley EricPedley left a 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:

  1. 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.
  2. Can you update the patch notes in this PR?

Copy link
Member

@EricPedley EricPedley left a comment

Choose a reason for hiding this comment

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

🚀

@EricPedley EricPedley merged commit b655b97 into main Oct 26, 2023
6 checks passed
@ap0nia ap0nia deleted the hover-zotistics-graph branch November 8, 2023 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC: Display Zotistics graph on hover
2 participants