search

moribvndvs / ng2-idle

Responding to idle users in Angular (not AngularJS) applications.
https://moribvndvs.github.io/ng2-idle
Apache License 2.0
322 stars 129 forks source link

Bug/e2e support protractor waitforangular #74

Closed wesley-trantham closed 6 years ago

wesley-trantham commented 6 years ago

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x")

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior? (You can also link to an open issue here) https://github.com/HackedByChinese/ng2-idle/issues/71

What is the new behavior? E2E tests run in protractor. Keypress events are noticeably slower, but at least work.

Does this PR introduce a breaking change? (check one with "x")

[x] Yes
[ ] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Idle service's constructor now has an additional parameter. While unlikely to break most implementations, it is still a breaking change.

Other information:

I noticed after I made these changes that there is a setInterval call in keepAlive.ts. If you'd like I can fix that to also work with E2E.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.01%) to 99.368% when pulling 978e053df2ce9ae7162ad09c1899b0709e27488e on wesley-trantham:bug/e2e-support-protractor-waitforangular into 2e1aaaf5dfa380844a2b5a2aeec47981fd7f9826 on HackedByChinese:master.

jtcrowson commented 6 years ago

@wesley-trantham you have merge conflicts.

@HackedByChinese any timetable on reviewing this fix?

wesley-trantham commented 6 years ago

I actually saw a comment today on issue #40 that I should have found before I opened my issue. If I were to do it over again I would probably implement that solution suggested there, as it is easy, and probably works. I would think this is the correct fix, and not a work-around. It would be nice to see it go in.