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

Dont trigger element handler when it doesn't exist #130

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yringler
Copy link

In our project, sometimes navigation would end up where we didn't expect.
It turns out that sometimes the href-to document handler picks up and reacts to a click event when it shouldn't.
Testing that the element being responded to still exists in the DOM fixed it for us.

@GavinJoyce
Copy link
Contributor

@yringler, thanks. Can you say more about what this is protecting against? When does the href-to document handler picks up and reacts to a click event when it shouldn't? Perhaps you could add a test?

@yringler
Copy link
Author

It's picking up a click on a component which extends the ember linkto component.
After the ember linkto finishes navigation, the handler from href-to is hit.

I don't have time to look into it further, unfortunately. For now we're just using my fork (I love yarn (which supports specifying a github url/branch!).

@yringler
Copy link
Author

yringler commented Jul 28, 2020

On further review, it looks like this is related to

isNotLinkComponent() {
. We check in that method if click came from a link component, but not if it came from a component which inherits from link.
I'll either switch our code to using link component directly, or update the PR

Edit: when I click on a link-to, by the time the handler is running the link to doesn't exist anymore (navigation already happened), and the check this.applicationInstance.lookup('-view-registry:main')[id] returns undefined.

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