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 click, drag, and keydown behavior #3208

Merged
merged 4 commits into from
Jun 1, 2023
Merged

Conversation

monicawoj
Copy link
Contributor

Resolves MetricsGraph issues regarding point selection, range selection, freezing the tooltip, and taking screenshots.

Problem:
We've had several reports of bugs when interacting with the MetricsGraph. Sometimes, the MetricsGraph goes into a mode where instead of clicking on a point it will zoom in, the MetricsGraph Tooltip doesn't work anymore either.

Solution:
Currently, we have special logic in place that freezes the Metrics Graph tooltip when the user holds down the 'Shift' key. However, 'Shift' is a special key that is used for certain shortcuts (such as taking a screenshots on Mac). The issue here is that when the shift key was pressed, followed by 'command' in order to take a screenshot, the application was paused as it entered screenshot mode and isShiftDown in our useKeyDown hook did not update on 'keyup'. The last captured event was 'keydown' of the shift key, therefore isShiftDown remained stuck in a 'down' state, even though it was no longer down. Manually resetting isShiftDown state to false when the user hits the "Meta" key (in Mac = command, in Windows = Control) was able to solve this issue.

Additionally, clicks outside of the tooltip have been disabled when shift is held down in order to ensure that only the tooltip can be clicked while the tooltip is frozen. This was able to resolve issues around unexpected zooming.

@monicawoj monicawoj added bug Something isn't working area/ui Something about the UI labels Jun 1, 2023
@monicawoj monicawoj requested a review from a team as a code owner June 1, 2023 12:02
Copy link
Member

@metalmatze metalmatze left a comment

Choose a reason for hiding this comment

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

Very nice! 👏
I couldn't trigger it anymore now! 🎉

Nice idea combining it with the Meta key 👍

@metalmatze metalmatze merged commit fe4ea42 into main Jun 1, 2023
@metalmatze metalmatze deleted the metrics-graph-selection-fix branch June 1, 2023 14:23
@manojVivek
Copy link
Contributor

Great find, Monica! 👏🏼 👏🏼 👏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui Something about the UI bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants