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

fix target _blank when anchor has children #1290

Merged
merged 3 commits into from
Mar 30, 2017
Merged

Conversation

RezaHaidari
Copy link
Contributor

Couple of days ago, we have faced an issue with out non-SPA project:
Once you add a child element to a <router-link> with target="_blank" attribute, it wouldn't fire on a new tab. check this fiddle:
https://jsfiddle.net/asynchronous/dnLs1dm4/10/

I've changed gaurdEvent method in link component so it could fix this issue.

Couple of days ago, we have faced an issue with out non-SPA project:
Once you add a child element to a ``<router-link>`` with ``target="_blank"`` attribute, it wouldn't fire on a new tab. check this fiddle:
https://jsfiddle.net/asynchronous/dnLs1dm4/10/

I've changed ``gaurdEvent`` method in ``link`` component so it could fix this issue.
@posva
Copy link
Member

posva commented Mar 29, 2017

What browser are you facing the issue? It's working properly on Chrome

@RezaHaidari
Copy link
Contributor Author

@posva we've checked on chrome and firefox (both ubuntu and windows).

are you sure you're clicking on span link ?

@posva
Copy link
Member

posva commented Mar 29, 2017

yes, both links behave the same (chrome 57)

@Javid-Izadfar
Copy link

@posva anchor with child is not opening in new tab for me too.

ezgif-1-9275d1a181

@posva
Copy link
Member

posva commented Mar 29, 2017

I see, it doesn't work on firefox.
Maybe we should use currentTarget instead of target
BTW, target should be the a link in that case (topmost element). It may be a Firefox bug

@Javid-Izadfar
Copy link

Javid-Izadfar commented Mar 29, 2017

Nah man, same thing on Chrome ... even multiple PCs :/
you are the first person who says it works fine for him

@RezaHaidari
Copy link
Contributor Author

@posva yes currentTarget fixed that issue.

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

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

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.

4 participants