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

Tag prop on <router-link> doesn't work since 2.0.0-rc.6 #707

Closed
VincentGarreau opened this issue Oct 1, 2016 · 13 comments
Closed

Tag prop on <router-link> doesn't work since 2.0.0-rc.6 #707

VincentGarreau opened this issue Oct 1, 2016 · 13 comments

Comments

@VincentGarreau
Copy link

VincentGarreau commented Oct 1, 2016

Versions

vue.js : 2.x
vue-router: > 2.0.0-rc.5

Reproduction Link

https://jsfiddle.net/9b80nxx1/6/
Update (link above is broken) : http://jsfiddle.net/VincentGarreau/4L240m47/1/

Steps to reproduce

Click on "Other"

What is actually happening?

The route is not updated with vue-router >= 2.0.0-rc.6, but it's working with <= 2.0.0-rc.5.

@fnlctrl
Copy link
Member

fnlctrl commented Oct 1, 2016

Thanks! It's indeed a bug.
https://github.com/vuejs/vue-router/blob/dev/src/components/link.js#L68-L70
Click event is not bound for <router-link> when this.tag is not 'a' and it doesn't have a child <a>..

fnlctrl added a commit that referenced this issue Oct 1, 2016
Fix click event not bound for `<router-link>` when tag is not `<a>` and doesn't have child `<a>`
@LinusBorg
Copy link
Member

LinusBorg commented Oct 2, 2016

I'm not sure it qualifies as a bug, but it's definitely worth an enhancement.

The point is that one should use the tag attribute to define the element that will receive the "active" class, and define the actual <a> element in the slot (in cases where the two should not be the same).

If my understanding is correct, there's three things to do:

  1. Explain this in the docs.
  2. make this work with button
  3. Fallback: add event to the root tag element if neither a not button was found in the slot.

@fnlctrl
Copy link
Member

fnlctrl commented Oct 2, 2016

@LinusBorg It's not about active classes, it's the element itself didn't get click events bound on it. It used to work in all previous versions.

@LinusBorg
Copy link
Member

LinusBorg commented Oct 2, 2016

I think Evan added it specifically for the purpose I explained:

e5f0964

Also see the explanation in the migration guide about v-link-active.

It is just missing fallback handling in case no link is provided in the slot.

@fnlctrl
Copy link
Member

fnlctrl commented Oct 2, 2016

Well then I guess we need to ask @yyx990803 to explain all this..

@LinusBorg
Copy link
Member

http://vuejs.org/guide/migration-vue-router.html#v-link-active-deprecated

This described behaviour should simply be extended to include button elements also, and fall back to the root element of neither are found.

@lukepolo
Copy link

Will this be fixed then ? Wondering if I should go ahead and re-do all my navigation

@LinusBorg
Copy link
Member

There's already a PR in review.

Just out of curiosity: So all your navigation does work without any <a> elements?

@lukepolo
Copy link

<router-link :to="{ path : '/site/' + site.id}" tag="li" class="btn btn-primary">Repository</router-link>

Vue Router would attach the event to the li tag , since im in a SPA it doesn't require the use of the a tag .

@LinusBorg
Copy link
Member

LinusBorg commented Oct 10, 2016

I would rather consider it good form to use the appropriate tags for navigation elements. For Screenreaders, or so the URL shows up at the bottom of the window when you hover the link etc.

Nontheless, the described fallback should be implemented, the review is still inprocess though.

@lukepolo
Copy link

I agree , have already switched it up

On Monday, October 10, 2016, Thorsten Lünborg notifications@github.com
wrote:

I would rather consider it good form to use the appropriate tags for
navigation elements. For Screenreaders, or so the URL shows up at the
bottom of the window when you hover the link etc.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#707 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB-I7IkegOmsWSYqb6HNRePzy_wKZiDpks5qyrxegaJpZM4KLtXW
.

@mattattui
Copy link

@LinusBorg My understanding is that ARIA provides role="link" to allow (for instance) screenreaders to discern non-<a> links.

That said, I didn't realise until researching this reply that HTML5 changed the definition of the <a> element to allow it to wrap blocks (in HTML 4, it was an inline tag). So if Vue-router is only for HTML5 doctypes, I think it's fine to no longer support this behaviour… though it was a bit of an unwelcome surprise, since this breaking change wasn't listed in the release notes for RC7 or 2.0.

@LinusBorg
Copy link
Member

Tjhe breaking change was unintended and will be fixed with the mentioned PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants