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

Enhancement: Use bottom placement for flow run scatter plot popover #1559

Closed
wants to merge 3 commits into from

Conversation

pleek91
Copy link
Collaborator

@pleek91 pleek91 commented Jul 7, 2023

Description

Fixes an issue where horizontally scrubbing through flow runs was difficult due to the popover appearing directly to the right of the flow run point on the chart.

Closes #1460

Also removed the custom logic for closing the popover in favor of the built in functionality which didn't exist when this component was first created.

Also added a big a padding between the popover and the flow run point so even if the popover opens in the direction of the next point if they are right next to each other the next point should be visible.

@pleek91 pleek91 requested a review from a team as a code owner July 7, 2023 20:43
@netlify
Copy link

netlify bot commented Jul 7, 2023

Deploy Preview for prefect-ui-library ready!

Name Link
🔨 Latest commit ab665ff
🔍 Latest deploy log https://app.netlify.com/sites/prefect-ui-library/deploys/64a889691389d200088031c5
😎 Deploy Preview https://deploy-preview-1559--prefect-ui-library.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.


.flow-run-pop-over__content { @apply
p-2
pointer-events-none
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this prevent interacting with the contents of the popover? So I wouldn't be able to hover a point and then go to the run associated with it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm I feel like you are correct, I wanted to add some spacing between the trigger and the content but not have it as part of the interaction. But I think you're right this will cascade to the content itself. I'll double check

@zhen0
Copy link
Member

zhen0 commented Oct 27, 2023

@pleek91 - shall we close this one?

@pleek91 pleek91 closed this Oct 27, 2023
@pleek91 pleek91 deleted the flow-run-statter-plot-popovers branch January 22, 2024 23:05
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.

Obtrusive Flow Runs chart mouseover.
3 participants