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

Unable to preventDefault on route-href when pushState is true #848

Open
powerbuoy opened this issue Jan 7, 2018 · 12 comments
Open

Unable to preventDefault on route-href when pushState is true #848

powerbuoy opened this issue Jan 7, 2018 · 12 comments

Comments

@powerbuoy
Copy link

powerbuoy commented Jan 7, 2018

I'm trying to prevent a link with a route-href attribute from navigating to the new URL.

When I configure the router to use hashChange instead of pushState it works fine. But if I switch to pushState preventDefault() does nothing:

import {inject} from 'aurelia-framework';

@inject(Element)
export class PreventHrefCustomAttribute {
	constructor (el) {
		this.el = el;
		this.onClick = e => {
			e.preventDefault(); // This only works when pushState: false, hashChange: true

			alert('The href should not be followed');
		};
	}

	attached () {
		this.el.addEventListener('click', this.onClick);
	}

	detached () {
		this.el.removeEventListener('click', this.onClick);
	}
}

<a route-href="route: item; params.bind: {id: item.id}" prevent-href>${item.name}</a>

@powerbuoy
Copy link
Author

So am I doing something wrong or is this expected behaviour? If so it's a little weird it behaves differently with hashChange.

@bigopon
Copy link
Member

bigopon commented Jan 21, 2018

Can you try

e.stopImmediatePropagation();
e.stopPropagation();

together with

this.el.addEventListener('click', this.onClick, /* add this */ true)

@powerbuoy
Copy link
Author

powerbuoy commented Jan 21, 2018

Unfortunately it doesn't change things... here's what my code looks like now:

constructor (el, ea) {
	this.el = el;
	this.ea = ea;

	this.onClick = e => {
		e.preventDefault();
		e.stopImmediatePropagation();
		e.stopPropagation();

		this.ea.publish('route-popup:open', {route: this.route, params: this.params});
	};
}

attached () {
	this.el.addEventListener('click', this.onClick, true);
}

detached () {
	this.el.removeEventListener('click', this.onClick);
}

FYI - my use case for this is to have a list of items with normal links (route-href) so that opening new tabs etc works as normal, but then also add my own attribute which opens a popup if the user does not Cmd+click.

@Alexander-Taran
Copy link
Contributor

@bigopon any other ideas?

@Eagle94T
Copy link

@powerbuoy did you try it with a mousedown event instead of the click?

The Link will be opened when the mousedown and mouseup occure on the same element.
At this moment, the mousedown event should already be triggered, and should be able to prevent the default behaviour.

MDN-Reference for mousedown and for click

@powerbuoy
Copy link
Author

@Eagle94T I did not. But even if that works it's inconsistent it doesn't work the same with and without pushState

@bigopon
Copy link
Member

bigopon commented Nov 12, 2018

@powerbuoy Maybe just ignore the router altogether ? adding an attribute router-ignore will make sure router does not participate in processing the event?

@powerbuoy
Copy link
Author

powerbuoy commented Nov 13, 2018

@bigopon I don't want the router to ignore it though. Most of the time I want the route-href to work as it should, and then in some cases I want my custom route-popup attribute to take over and open the route in a popup instead (think Pinterest where each post has its own page/URL but they open in popups on the home-page in high res screens).

The fact that it works when pushState: false and not when pushState: true sounds like a bug to me.

@bigopon
Copy link
Member

bigopon commented Nov 13, 2018

@powerbuoy Can you try the work around described here aurelia/history-browser#32 (comment)

@powerbuoy
Copy link
Author

@bigopon Honestly that's a little more hassle than I'm willing to get this working :P I'll try mousedown first at least.

@bigopon
Copy link
Member

bigopon commented Nov 13, 2018

@powerbuoy Sure x) it may look like a lot, but it boils down to ensure you register your click handler before all else, even the one from router history browser

@powerbuoy
Copy link
Author

@bigopon Ah I see. I'll take a look at it when I get a chance. This particular feature (open some URLs in popups in some resolutions) has been pushed down on the priority list a bit for the moment.

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

No branches or pull requests

4 participants