Vue-router: Active class not always applied correctly when having subroutes

[denjaland]

When having subroutes, and applying an active class, it goes wrong when there are replacement variables.

Especially in a case where you navigate to .../:id/... it goes wrong; the route will be shown active using a startsWith() because we can not apply 'exact' because of the subrouting.

I think the parent should only be shown active is the part BEFORE the last SLASH in the route matches exactly in stead of working with .startsWith().

Vue.js / vue-router versions

2.1.6 / 2.1.1

Reproduction Link

https://jsfiddle.net/dwa0kde5/2/

Steps to reproduce

Click the links on the fiddle; the active class shows a red border to the links; you'll soon see where it goes wrong :-)

What is Expected?

When vieweing id 10 / 100 / 1XXX, the /1 route should NOT be active. I believe the slashes should be determining where the parent route ends.

What is actually happening?

[denjaland]

I added the following issue a note to better show the issue: https://jsfiddle.net/dwa0kde5/2/

[fnlctrl]

Actually it's intended behavior since you're not using exact matching:
http://router.vuejs.org/en/api/router-link.html

The default active class matching behavior is inclusive match. For example, will get this class applied as long as the current path starts with /a.

So you'll have to add the exact prop so that links with different params are not marked as active: https://jsfiddle.net/m1n7o6kj/

Though currently vue-router cannot achieve your desired result: mark current route and it's children's link active, and exclude other routes.

[denjaland]

You're not getting the issue; when setting the exact to true; the parent is no longer being active; in a multilevel navigation, you typically want to show the path in your activation that is active; click sub1; you want main1 and sub1 to be active. (Active on the main would "open" the submenu in the navigation)

[LinusBorg]

@fnlctrl I agree with @denjaland: The 'not-exact' match is too lose when using dynamic segments:

A link with a path of /users/1 should not match for /users/123, and should not get the active class - but it does.

[denjaland]

@LinusBorg - I guess you meant it should NOT match for /users/123, right?

[fnlctrl]

@LinusBorg That's inclusive match by design and by definition (and as explained in the docs, /a will match anything that starts with /a ).. So I didn't see it as a bug, but rather a not supported feature.

@denjaland Route matching works independently of current view state. A route is active only when it matches the url, with the current exact option (inclusive match or not).

[denjaland]

Right. But you do see the use case though?
In my opinion, it may have been designed like that, but the use case is pretty obvious, and I would definitely add support for it.
If you prefer to call it a feature request rather than a bug, be my guest :-)

[fnlctrl]

Right. But you do see the use case though?

Yeah... let's mark this as a feature request.

[LinusBorg]

@denjaland

@LinusBorg - I guess you meant it should NOT match for /users/123, right?

Yep - corrected.

@fnlctrl Well, I would argue that it's something in between a bug and an enhancement - the current functionality is definitely lacking when we have routes with dynamic segments, and weither it was designed to not support them is something Evan would have to explain ^^

But there's no need to argue semantics, we should simply see that we get this supported somehow.

[fnlctrl]

Well, it's indeed lacking, but it's been here all the way since the start of 2.0 and the behavior has been precisely documented (and people didn't seem to have trouble with it until now).. So I guess it's rather not designed to support(being unaware of this use case when designing) than designed not to support 😂

[RyuuGan]

I just want to add that it would be awesome if the link will be active if the subroute is matched, especially when the urls are not matching, like /countries and /country/:id.

https://jsfiddle.net/dwa0kde5/3/

[rpkilby]

@RyuuGan - that sounds like a separate feature request. For now, I'd recommend just using /countries/:id.

@denjaland - this seems like this could be handled by appending a / to the end of URL pattern. eg,

• /users/1 incorrectly matches against /users/123
• /users/1/ should not match against /users/123/

[fnlctrl]

Appending a trailing slash would mean different behavior for /123 and /123/ (and surprise people), so it's not an appropriate fix.
Maybe we can add a subroute prop to <router-link> that makes it active when the current route is its subroute?

[LinusBorg]

Like the "subroute" prop idea!

[rpkilby]

Appending a trailing slash would mean different behavior for /123 and /123/ (and surprise people), so it's not an appropriate fix.

@fnlctrl - I'm saying that the user could change their routes to include the ending /, which should fix the issue, right? I'm not arguing that the router should automatically append it on their behalf.

[fnlctrl]

@rpkilby Vue-router currently normalizes paths, and removes trailing slashes by default, so for now it won't work even if the user manually appended a /. https://jsfiddle.net/0yLc0vz1/

[denjaland]

@rpkilby
Appending the / in my routes is a nice workaround for the time being, but I personally don't like the idea. .../:id and .../:id/ should behave in the same way, and I don't like the idea of being forced in appending the slash.
(yes, I'm one of those users that sometimes just changes the link in my browser directly if I know how to easily go to object X ;-) You would require me to add a trailing slash there ;-) )

@fnlctrl // cc @LinusBorg
I don't really like the subroute idea - what if it has more subroutes... You want me to add all possible subroutes to my main route just to activate it? So if I add a subroute somewhere along, I need to remember to update all my links in parent components to make sure it's included?

[fnlctrl]

@denjaland Ah, I meant a subroute flag that's either true or false, like exact. Now it seems the name was confusing - so maybe a match prop that accepts one of 'exact', 'subroute', 'default'?
<router-link match="subroute"></router-link>
(And I guess we'll continue to support exact prop, and match would be of higher precedence)

[denjaland]

So more like a "hasSubroutes" flag you mean? Because you'll need an indication on the parent. Not sure what this brings though...

[denjaland]

Imo, a slash in the path indicates that it is a subroute of the url before the slash.
Maybe we can introduce another class and keep "active" when the url "beginsWith" but introduce "hasActiveSubRoute" when a subroute with real slashes is activated.
I believe this might lead us too far though unless someone can tell me a use case in which you really want/need active activated not considering the slashes.

If there is a real need, it maybe better to change "exact" from a boolean value to a "matchtype" which can have "exact", "subroute" or "startswith" option...

This will support all but avoid conflicting parameters where you need to give one parameter precendence over the other. I like tobe able to see how something behaves when I change a parameter :-)

[fnlctrl]

I didn't mean hasSubroutes... match="subroutes" means being active when current route is a subroute of <router-link>'s route. And to prevent more misunderstandings, by subroute I mean a route defined inside a children array, not a part of a url divided by /.

(Maybe you missed my previous note that adding a trailing slash won't work https://github.com/vuejs/vue-router/issues/1014#issuecomment-268732548)

If there is a real need, it maybe better to change "exact" from a boolean value to a "matchtype" which can have "exact", "subroute" or "startswith" option...

...which is exactly what I proposed before 😄 ,

maybe a match prop that accepts one of 'exact', 'subroute', 'default'?

[denjaland]

Okay then - just as long as the new prop replaces the existing "exact" one for matching behaviour, I'm totally on board with you now ;-)

[yyx990803]

Seems to be the same issue with #1091 and fixed via #1101.

[denjaland]

Yes - the fiddle at https://jsfiddle.net/dwa0kde5/2/ confirms it works with the latest. Thanks a bunch!

[OnyekaIjeh]

Please I have this issue.
Parent route only has the active class on 'account-profile' and not when on the other child routes

export default [
{
path: "/account",
component: Account,
children: [
{
path: "",
name: "account",
redirect: { name: "account-profile" }
},
{
path: "profile",
name: "account-profile",
component: AccountProfile
},
{
path: "password",
name: "account-password",
component: AccountPassword
},
{
path: "analytics",
name: "account-analytics",
component: AccountAnalytics
}
]
}
];