Skip to content

Support target="_blank" for <router-link> #830

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Nov 13, 2016
Merged

Support target="_blank" for <router-link> #830

Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a678b2e
Support `target="_blank"` for `<router-link>`
fnlctrl Oct 25, 2016
85f720a
Fix trailing spaces
fnlctrl Oct 25, 2016
75e6c24
small fix
fnlctrl Oct 25, 2016
21a0305
yet another small fix
fnlctrl Oct 25, 2016
4947134
hopefully final fix...
fnlctrl Oct 25, 2016
a87897d
fix implementation
fnlctrl Nov 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/components/link.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ export default {
// don't redirect on right click
/* istanbul ignore if */
if (e.button !== 0) return
// don't redirect if `target="_blank"`
/* istanbul ignore if */
const target = this.$el && this.$el.getAttribute('target')
Copy link
Member

Choose a reason for hiding this comment

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

We should use e.target here. this.$el may be a wrapping <li> if tag prop is used.

if (target && target.toLowerCase().split(' ').indexOf('_blank') > -1) return
Copy link
Contributor

Choose a reason for hiding this comment

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

Since code is written in ES Net, shouldn't it be target.toLowerCase().split(' ').includes('_blank')?

Copy link
Member Author

Choose a reason for hiding this comment

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

That cannot be transpiled, and would require a polyfill.

Copy link
Member

Choose a reason for hiding this comment

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

how about just /\b_blank\b/i.test(target)


e.preventDefault()
if (this.replace) {
router.replace(to)
Expand Down