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

Allow click, doubleClick, and triggerEvent to trigger events on window. #943

Merged
merged 5 commits into from
Nov 12, 2020

Conversation

scalvert
Copy link
Contributor

@scalvert scalvert commented Nov 10, 2020

During a refactor that improved typings in this project, the getElement function was narrowed in what it would return - specifically it was correctly fixed to return elements, of which window is not. This had the side effect of removing support for triggering window events via triggerEvent, in addition to removing support for other helpers.

This PR restores that capability by adding a new getTarget util, which wraps the getElement util but additionally checks for the presence of window, and subsequently returns that in favor of an element.

TODO:

Update getElement calls in other helpers that are valid for window:

  • triggerEvent
  • click
  • doubleClick

@scalvert scalvert marked this pull request as draft November 10, 2020 18:35
@rwjblue
Copy link
Member

rwjblue commented Nov 10, 2020

We should evaluate the other DOM helpers, see which should allow window (I think most should not so the default getElement behavior is fine for them).

@scalvert
Copy link
Contributor Author

@rwjblue I did. It looks as though click and doubleClick are the only other ones that make sense with window. Updating the PR to include those.

@scalvert scalvert marked this pull request as ready for review November 10, 2020 20:16
@scalvert scalvert changed the title WIP: Ensure relevant test helpers support window Ensure relevant test helpers support window Nov 10, 2020
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

You've labeled this as breaking, but how is it breaking? It seems that we are widening the types / allowed values (you can now do a thing you couldn't before), no?

addon-test-support/@ember/test-helpers/dom/-target.ts Outdated Show resolved Hide resolved
addon-test-support/@ember/test-helpers/dom/click.ts Outdated Show resolved Hide resolved
@rwjblue rwjblue changed the title Ensure relevant test helpers support window Allow click, doubleClick, and triggerEvent to trigger events on window. Nov 12, 2020
@rwjblue rwjblue merged commit 2c825ed into master Nov 12, 2020
@rwjblue rwjblue deleted the fix-trigger-event branch November 12, 2020 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants