Skip to content

Commit

Permalink
fix(link): Fix active links when parent link redirects to child (#2772)
Browse files Browse the repository at this point in the history
* fix(ActiveLink): Fix active links when parent link redirects to child

Fix active parent link when parent link redirects to a child and when the user navigates between child links

fix #2724

* test(active-links): add more tests for redirects

* chore(lint): add prettier conf to specs for longer lines

* test(active-links): adapt redirect tests to  added content

* chore(link): remove commented code
  • Loading branch information
Domino9697 authored and posva committed Jul 5, 2019
1 parent da1a114 commit 64785a9
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 21 deletions.
46 changes: 44 additions & 2 deletions examples/active-links/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,48 @@ const Users = {

const User = { template: '<div>{{ $route.params.username }}</div>' }

const Gallery = {
template: `
<div>
<h2>Gallery</h2>
<router-view></router-view>
</div>
`
}

const Image = { template: '<div>{{ $route.params.imageId }}</div>' }

const router = new VueRouter({
mode: 'history',
base: __dirname,
routes: [
{ path: '/', component: Home },
{ path: '/about', component: About },
{ path: '/users', component: Users,
{
path: '/redirect-gallery',
name: 'redirect-gallery',
redirect: { name: 'gallery' }
},
{
path: '/redirect-image',
name: 'redirect-image',
redirect: { name: 'image', params: { imageId: 'image1' }}
},
{
path: '/users',
component: Users,
children: [{ path: ':username', name: 'user', component: User }]
},
{
path: '/gallery',
component: Gallery,
children: [
{ path: ':username', name: 'user', component: User }
{
path: '',
name: 'gallery',
redirect: { name: 'image', params: { imageId: 'image1' }}
},
{ path: ':imageId', component: Image, name: 'image' }
]
}
]
Expand Down Expand Up @@ -66,6 +99,15 @@ new Vue({
<router-link tag="li" to="/about">
<a>/about (active class on outer element)</a>
</router-link>
<li><router-link to="/gallery">/gallery (redirect to /gallery/image1)</router-link></li>
<li><router-link :to="{ name: 'gallery' }">/gallery named link (redirect to /gallery/image1)</router-link></li>
<li><router-link :to="{ name: 'image', params: {imageId: 'image2'} }">/gallery/image2</router-link></li>
<li><router-link :to="{ name: 'image', params: {imageId: 'image1'} }">/gallery/image1</router-link></li>
<li><router-link to="/redirect-gallery">/redirect-gallery (redirect to /gallery)</router-link></li>
<li><router-link :to="{ name: 'redirect-gallery' }">/redirect-gallery named (redirect to /gallery)</router-link></li>
<li><router-link to="/redirect-image">/redirect-image (redirect to /gallery/image1)</router-link></li>
<li><router-link :to="{ name: 'redirect-image' }" >/redirect-image named (redirect to /gallery/image1)</router-link></li>
</ul>
<router-view class="view"></router-view>
</div>
Expand Down
44 changes: 26 additions & 18 deletions src/components/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import { createRoute, isSameRoute, isIncludedRoute } from '../util/route'
import { extend } from '../util/misc'
import { normalizeLocation } from '../util/location'

// work around weird flow bug
const toTypes: Array<Function> = [String, Object]
Expand Down Expand Up @@ -31,26 +32,31 @@ export default {
render (h: Function) {
const router = this.$router
const current = this.$route
const { location, route, href } = router.resolve(this.to, current, this.append)
const { location, route, href } = router.resolve(
this.to,
current,
this.append
)

const classes = {}
const globalActiveClass = router.options.linkActiveClass
const globalExactActiveClass = router.options.linkExactActiveClass
// Support global empty active class
const activeClassFallback = globalActiveClass == null
? 'router-link-active'
: globalActiveClass
const exactActiveClassFallback = globalExactActiveClass == null
? 'router-link-exact-active'
: globalExactActiveClass
const activeClass = this.activeClass == null
? activeClassFallback
: this.activeClass
const exactActiveClass = this.exactActiveClass == null
? exactActiveClassFallback
: this.exactActiveClass
const compareTarget = location.path
? createRoute(null, location, null, router)
const activeClassFallback =
globalActiveClass == null ? 'router-link-active' : globalActiveClass
const exactActiveClassFallback =
globalExactActiveClass == null
? 'router-link-exact-active'
: globalExactActiveClass
const activeClass =
this.activeClass == null ? activeClassFallback : this.activeClass
const exactActiveClass =
this.exactActiveClass == null
? exactActiveClassFallback
: this.exactActiveClass

const compareTarget = route.redirectedFrom
? createRoute(null, normalizeLocation(route.redirectedFrom), null, router)
: route

classes[exactActiveClass] = isSameRoute(current, compareTarget)
Expand All @@ -70,7 +76,9 @@ export default {

const on = { click: guardEvent }
if (Array.isArray(this.event)) {
this.event.forEach(e => { on[e] = handler })
this.event.forEach(e => {
on[e] = handler
})
} else {
on[this.event] = handler
}
Expand All @@ -88,9 +96,9 @@ export default {
if (a) {
// in case the <a> is a static node
a.isStatic = false
const aData = a.data = extend({}, a.data)
const aData = (a.data = extend({}, a.data))
aData.on = on
const aAttrs = a.data.attrs = extend({}, a.data.attrs)
const aAttrs = (a.data.attrs = extend({}, a.data.attrs))
aAttrs.href = href
} else {
// doesn't have <a> child, apply listener to self
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/specs/.prettierrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"printWidth": 120
}
21 changes: 20 additions & 1 deletion test/e2e/specs/active-links.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module.exports = {
browser
.url('http://localhost:8080/active-links/')
.waitForElementVisible('#app', 1000)
.assert.count('li a', 11)
.assert.count('li a', 19)
// assert correct href with base
.assert.attributeContains('li:nth-child(1) a', 'href', '/active-links/')
.assert.attributeContains('li:nth-child(2) a', 'href', '/active-links/')
Expand All @@ -22,6 +22,14 @@ module.exports = {
.assert.attributeContains('li:nth-child(9) a', 'href', '/active-links/users/evan?foo=bar&baz=qux')
.assert.attributeContains('li:nth-child(10) a', 'href', '/active-links/about')
.assert.attributeContains('li:nth-child(11) a', 'href', '/active-links/about')
.assert.attributeContains('li:nth-child(12) a', 'href', '/active-links/gallery')
.assert.attributeContains('li:nth-child(13) a', 'href', '/active-links/gallery/')
.assert.attributeContains('li:nth-child(14) a', 'href', '/active-links/gallery/image2')
.assert.attributeContains('li:nth-child(15) a', 'href', '/active-links/gallery/image1')
.assert.attributeContains('li:nth-child(16) a', 'href', '/active-links/redirect-gallery')
.assert.attributeContains('li:nth-child(17) a', 'href', '/active-links/redirect-gallery')
.assert.attributeContains('li:nth-child(18) a', 'href', '/active-links/redirect-image')
.assert.attributeContains('li:nth-child(19) a', 'href', '/active-links/redirect-image')
.assert.containsText('.view', 'Home')

assertActiveLinks(1, [1, 2], null, [1, 2])
Expand All @@ -36,6 +44,17 @@ module.exports = {
assertActiveLinks(10, [1, 10], [11], [10], [11])
assertActiveLinks(11, [1, 10], [11], [10], [11])

// redirects
assertActiveLinks(12, [1, 12, 13, 15], null, [15])
assertActiveLinks(13, [1, 12, 13, 15], null, [15])
assertActiveLinks(14, [1, 12, 13, 14], null, [14])
assertActiveLinks(15, [1, 12, 13, 15], null, [15])
// different level redirect
assertActiveLinks(16, [1, 12, 13, 15], null, [15])
assertActiveLinks(17, [1, 12, 13, 15], null, [15])
assertActiveLinks(18, [1, 12, 13, 15], null, [15])
assertActiveLinks(19, [1, 12, 13, 15], null, [15])

browser.end()

function assertActiveLinks (n, activeA, activeLI, exactActiveA, exactActiveLI) {
Expand Down

0 comments on commit 64785a9

Please sign in to comment.