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 issue with multiple tooltips showing #256

Merged
merged 1 commit into from
Jun 1, 2017

Conversation

gragland
Copy link
Contributor

This fixes an issue with createSliderWithTooltip() where multiple tooltips will be shown at once if the mouse is dragged over two or more handles quickly.

Reproduced here:
http://codepen.io/anon/pen/jmEPxV?editors=0011

This was due to the fact that multiple setStates would fire back to back and the second would apply old state values. Passing a function to setState (atomic update) was the fix.

@paranoidjk
Copy link
Member

sorry, i may not understand correctly about your problem.

I took a look at your codepen demo, and multiple tooltips will be shown is because that the two handle is very closely, so they both trigger onMouseEnter event, and should this be reasonable ?

@gragland
Copy link
Contributor Author

The bug is that the tooltips don't disappear. See this GIF: https://media.giphy.com/media/3o7bucOydJQsjY6UwM/giphy.gif

Pretty sure this is an issue with onMouseLeave setState not being atomic (has old state values because it's fired before first setState has been applied).

@paranoidjk
Copy link
Member

@gragland CI is broken, try upgrade enzyme to latest.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 49.587% when pulling 84a5860 on gragland:patch-1 into fb009b3 on react-component:master.

@gragland
Copy link
Contributor Author

@paranoidjk Looks like it failed because coverage decreased. Is that the case? I can write a few tests for createSliderWithTooltip if so.

@afc163
Copy link
Member

afc163 commented Apr 14, 2017

Add test case about your fix.

@paranoidjk
Copy link
Member

Pretty sure this is an issue with onMouseLeave setState not being atomic (has old state values because it's fired before first setState has been applied).

I'd like to say, it's because multiple setState had been batch process.

@benjycui
Copy link
Member

benjycui commented Jun 1, 2017

ping~ @gragland

@paranoidjk
Copy link
Member

@gragland I will handle the test case.

@paranoidjk paranoidjk merged commit 7ef40ba into react-component:master Jun 1, 2017
paranoidjk added a commit that referenced this pull request Jun 1, 2017
@paranoidjk
Copy link
Member

add test at 756dcc8

jspyhotdev pushed a commit to jspyhotdev/React-slider that referenced this pull request Jul 13, 2018
Senior-Hayato-Suzuki pushed a commit to Senior-Hayato-Suzuki/react-slider that referenced this pull request Feb 16, 2020
Senior-Hayato-Suzuki added a commit to Senior-Hayato-Suzuki/react-slider that referenced this pull request Feb 26, 2020
blue-git-pro added a commit to blue-git-pro/dashboard-next.js that referenced this pull request May 14, 2021
web3gru pushed a commit to web3gru/slider that referenced this pull request May 13, 2023
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.

5 participants