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

Pass correct popper element in the registerAPI #105 #106

Merged
merged 2 commits into from
Jun 24, 2019

Conversation

urbany
Copy link
Contributor

@urbany urbany commented Jun 12, 2019

Fixes #105

@simonihmig
Copy link
Collaborator

Thanks @urbany for working on this! Indeed my latest changes seem to have introduced this. 😔

Apparently there is some missing test coverage. Would you be able to add a test case to https://github.com/kybishop/ember-popper/blob/master/tests/integration/components/ember-popper/register-api-test.js?

Also another test is failing here, but this might be the flakey one of #95. I already restarted Travis a number of times though...

@simonihmig
Copy link
Collaborator

I already restarted Travis a number of times though...

Ah yes, it has finally passed after the third retry 🤔. So seems not related...

@urbany urbany force-pushed the fix-105-undefined-popper-element branch from 3d94af7 to 35296e9 Compare June 12, 2019 20:08
@urbany
Copy link
Contributor Author

urbany commented Jun 12, 2019

@simonihmig test added (I confirmed the test fails without the fix) and passes with the fix. I noticed that some tests fail randomly, but all pass most times.

PS: Thank you for your work on this addon!

@simonihmig simonihmig added the bug label Jun 13, 2019
@simonihmig simonihmig changed the title fix: Pass correct popper element in the API #105 Pass correct popper element in the registerAPI #105 Jun 13, 2019
Copy link
Collaborator

@simonihmig simonihmig left a comment

Choose a reason for hiding this comment

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

Awesome, thanks @urbany!

@simonihmig
Copy link
Collaborator

The failing test in Ember 2.18 is from the newly added test. It seems https://github.com/kybishop/ember-popper/pull/106/files#diff-255dc3991591e84507576a55a13a6173R52 returns null here!? 🤔

@urbany
Copy link
Contributor Author

urbany commented Jun 14, 2019

@simonihmig I tried to fix this but couldn't, don't know what's wrong sorry :s

@simonihmig
Copy link
Collaborator

simonihmig commented Jun 23, 2019

@urbany I added a commit to your PR that should hopefully fix the failing test! It is related to a bug in ember-angle-bracket-invocation-polyfill (used in the Ember 2.18 test scenario), which makes the attributes (here class) render after didRender.

Let's see what Travis thinks of this...

@simonihmig simonihmig merged commit f72449f into kybishop:master Jun 24, 2019
@simonihmig
Copy link
Collaborator

Published as 0.10.1! 🎉

@urbany
Copy link
Contributor Author

urbany commented Jun 24, 2019

@simonihmig great! Thank you for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

api._popperElement is undefined
2 participants