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 delayHide issue #246 #247

Merged
merged 2 commits into from
Jan 30, 2017
Merged

Fix delayHide issue #246 #247

merged 2 commits into from
Jan 30, 2017

Conversation

huumanoid
Copy link
Contributor

Description

The following text assumes that reader has read description of issue #246.

At first, I've just added this.cleanTimer() in showTooltip method.
Problem described in #246 has completely gone.

But it's reasonable to ask: why behavior is different after ReactTooltip.rebuild()?
It was caused by invalid effect detection in bindListeners method.

I think, this should be considered as a bug:
effect

In the example above, there are one <ReactTooltip /> component with default setup, both buttons SampleText(1|2) attached to this ReactTooltip, data-effect of the second button has been set to solid.

I haven't get an idea why isCapture is implemented as decorator, but I've followed the same way and have added getEffect decorator. I wonder what you think about it.

Tests

Test stand still available at http://hmnid.ru:8090/ . Test stand for effect issue available at http://hmnid.ru:8090/effect.html

Demo with fixed react-tooltip available at http://hmnid:8091/ and http://hmnid.ru:8091/effect.html respectively.

Sources of demos available at https://github.com/huumanoid/react-tooltip-delayhide-issue-test

@wwayne wwayne merged commit bb57a5b into ReactTooltip:master Jan 30, 2017
@wwayne
Copy link
Collaborator

wwayne commented Jan 30, 2017

Really thanks for your work and detailed explanation :), I'm gonna build and release new version, it should be 3.2.3

@huumanoid
Copy link
Contributor Author

Very glad to see that you have already reviewed it and pushed it to npm. Thank you! It's important for my project.

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.

2 participants