fixed tooltip shows up on Mobile Web while navigating the reports #2044
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
parasharrajat
changed the title
roryabraham
requested changes
Mar 24, 2021
There was a problem hiding this comment.
Overall looks good @parasharrajat, just some small requests on the comments and variable naming.
| this.setState({isHovered}, isHovered ? this.props.onHoverIn : this.props.onHoverOut); | ||
| } | ||
|
|
||
| // we reset the Hover block in case touchmove was not first after touctstart |
There was a problem hiding this comment.
Can you help me understand the race condition that can occur here?
There was a problem hiding this comment.
I think it is possible that the touchmove never fires.
roryabraham
approved these changes
Mar 24, 2021
85 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please review @roryabraham
Details
Blocked the hover events on the
Hoverablecomponent on touch events.Fixed Issues
Fixes #1490
#1632 (comment)
Tests
Open the App on any Mobile web platform and click on the names on the report title on the Home page. It should open the report and no tooltip should be shown.
Tested On
Screenshots
N/A
1616610966592.mp4