Skip to content
This repository was archived by the owner on Nov 9, 2024. It is now read-only.

Conversation

@atomiks
Copy link
Owner

@atomiks atomiks commented Nov 14, 2018

Wraps Tippy's .enable(), .disable(), .show(), and .hide() methods using props.

isVisible must be accompanied by trigger="manual"

For some reason the isVisible tests are failing. Anyone want to take a look? Seems like trigger="manual" causes a problem but I have no idea why.

Manual dev environment tests show it working as expected

}

if (this.isManualTrigger && isVisible === true) {
this.tip.show()
Copy link

@jwinn jwinn Nov 14, 2018

Choose a reason for hiding this comment

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

Per atomiks/tippyjs#230, shouldn't this be?

Suggested change
this.tip.show()
setTimeout(this.tip.show)

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm going to release a new tippy.js patch that changes the global click listener to use capture phase (instead of bubbling) by default. That will prevent the need to do this.

Copy link

@jwinn jwinn Nov 14, 2018

Choose a reason for hiding this comment

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

May want to update the docs on how isVisible and showOnInit relate (or don't) to help avoid confusion down the road. Nice job on the project and thank you for all the work.

atomiks added a commit that referenced this pull request Nov 16, 2018
atomiks added a commit that referenced this pull request Nov 16, 2018
* Switch to react-testing-library

* Remove PR #20 tests for now
@atomiks
Copy link
Owner Author

atomiks commented Nov 16, 2018

Switching to react-testing-library instead of enzyme seems to be working better 🤷‍♂️

Not sure what was wrong, but the tests are passing now and coverage 100%

@atomiks atomiks merged commit 8ed0123 into master Nov 16, 2018
@atomiks atomiks deleted the method-props branch November 16, 2018 05:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants