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

Improve click handling when pushstate is activated. #28

Closed
wants to merge 3 commits into from
Closed

Improve click handling when pushstate is activated. #28

wants to merge 3 commits into from

Conversation

Macrofig
Copy link
Contributor

@Macrofig Macrofig commented Jan 9, 2017

  • Handle javascript void
  • Handle target blank
  • Handle metkey click

Resolves #23

- Handle javascript void
- Handle target blank
- Handle metkey click
// href has some JS in it, let it run
if(node.href.trim().indexOf('javascript') === 0) {
try {
eval(node.href);
Copy link
Contributor

Choose a reason for hiding this comment

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

if you don't call ev.preventDefault() ... which should happen just by returning, won't it do the browser's default behavior?

Also, you might want to check for javascript: ... less likely someone has a link like javascript.com or something.

}

// Do not push state if meta key was pressed, mimicing standard browser behavior
if (e.metaKey) {
Copy link
Member

Choose a reason for hiding this comment

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

@justinbmeyer
Copy link
Contributor

can-route-pushstate listens on the body ... one idea is to have a plugin "beat" can-route-pushstate to the punch. Listen "lower" than can-route-pushstate, check if .stopPropagation() should be called to prevent can-route-pushstate's .preventDefault()

@justinbmeyer
Copy link
Contributor

  • check other libraries that write out <a> tags. Do they do this?

@chasenlehara
Copy link
Member

Closing in favor of #77

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.

None yet

3 participants