From 7e7a3989afeb3177d8785e0a7e362b2e08ca5642 Mon Sep 17 00:00:00 2001 From: Francois Hendriks Date: Mon, 20 May 2019 17:57:20 +0200 Subject: [PATCH 1/5] 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 --- examples/active-links/app.js | 21 ++++++++++++++++++++- src/components/link.js | 5 +++-- test/e2e/specs/active-links.js | 8 +++++++- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/examples/active-links/app.js b/examples/active-links/app.js index a1a3238d4..11c331ea8 100644 --- a/examples/active-links/app.js +++ b/examples/active-links/app.js @@ -17,6 +17,17 @@ const Users = { const User = { template: '
{{ $route.params.username }}
' } +const Gallery = { + template: ` +
+

Gallery

+ +
+ ` +} + +const Image = { template: '
{{ $route.params.imageId }}
' } + const router = new VueRouter({ mode: 'history', base: __dirname, @@ -27,7 +38,11 @@ const router = new VueRouter({ children: [ { path: ':username', name: 'user', component: User } ] - } + }, + { path: '/gallery', component: Gallery, name: 'gallery', + children: [ + { path: ':imageId', component: Image, name: 'image' } + ], redirect: { name: 'image', params: { imageId: 'image1' }}} ] }) @@ -66,6 +81,10 @@ new Vue({ /about (active class on outer element) + +
  • /gallery (redirect to /gallery/image1)
    • +
  • /gallery/image2
    • +
  • /gallery/image1
    • diff --git a/src/components/link.js b/src/components/link.js index 2662d8263..f7ae238fc 100644 --- a/src/components/link.js +++ b/src/components/link.js @@ -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 = [String, Object] @@ -49,8 +50,8 @@ export default { const exactActiveClass = this.exactActiveClass == null ? exactActiveClassFallback : this.exactActiveClass - const compareTarget = location.path - ? createRoute(null, location, null, router) + const compareTarget = route.redirectedFrom + ? createRoute(null, normalizeLocation(route.redirectedFrom), null, router) : route classes[exactActiveClass] = isSameRoute(current, compareTarget) diff --git a/test/e2e/specs/active-links.js b/test/e2e/specs/active-links.js index fd142e4e5..8d5b549ec 100644 --- a/test/e2e/specs/active-links.js +++ b/test/e2e/specs/active-links.js @@ -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', 14) // 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/') @@ -22,6 +22,9 @@ 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/image2') + .assert.attributeContains('li:nth-child(14) a', 'href', '/active-links/gallery/image1') .assert.containsText('.view', 'Home') assertActiveLinks(1, [1, 2], null, [1, 2]) @@ -35,6 +38,9 @@ module.exports = { assertActiveLinks(9, [1, 3, 5, 7, 9], null, [9]) assertActiveLinks(10, [1, 10], [11], [10], [11]) assertActiveLinks(11, [1, 10], [11], [10], [11]) + assertActiveLinks(12, [1, 12, 14], null, [14]) + assertActiveLinks(13, [1, 12, 13], null, [13]) + assertActiveLinks(14, [1, 12, 14], null, [14]) browser.end() From 4e6083fae073ec0214e8ca58687c3ecb8d6cef77 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Fri, 5 Jul 2019 11:02:12 +0200 Subject: [PATCH 2/5] test(active-links): add more tests for redirects --- examples/active-links/app.js | 37 ++++++++++++++++++++++++++------ src/components/link.js | 41 ++++++++++++++++++++++-------------- 2 files changed, 55 insertions(+), 23 deletions(-) diff --git a/examples/active-links/app.js b/examples/active-links/app.js index 11c331ea8..1715d58b6 100644 --- a/examples/active-links/app.js +++ b/examples/active-links/app.js @@ -34,15 +34,33 @@ const router = new VueRouter({ routes: [ { path: '/', component: Home }, { path: '/about', component: About }, - { path: '/users', component: Users, - children: [ - { path: ':username', name: 'user', component: User } - ] + { + path: '/redirect-gallery', + name: 'redirect-gallery', + redirect: { name: 'gallery' } + }, + { + path: '/redirect-image', + name: 'redirect-image', + redirect: { name: 'image', params: { imageId: 'image1' }} }, - { path: '/gallery', component: Gallery, name: 'gallery', + { + path: '/users', + component: Users, + children: [{ path: ':username', name: 'user', component: User }] + }, + { + path: '/gallery', + component: Gallery, children: [ + { + path: '', + name: 'gallery', + redirect: { name: 'image', params: { imageId: 'image1' }} + }, { path: ':imageId', component: Image, name: 'image' } - ], redirect: { name: 'image', params: { imageId: 'image1' }}} + ] + } ] }) @@ -82,9 +100,14 @@ new Vue({ /about (active class on outer element) -
  • /gallery (redirect to /gallery/image1)
    • +
  • /gallery (redirect to /gallery/image1)
    • +
  • /gallery named link (redirect to /gallery/image1)
  • /gallery/image2
  • /gallery/image1
    • +
  • /redirect-gallery (redirect to /gallery)
    • +
  • /redirect-gallery named (redirect to /gallery)
    • +
  • /redirect-image (redirect to /gallery/image1)
    • +
  • /redirect-image named (redirect to /gallery/image1)
    • diff --git a/src/components/link.js b/src/components/link.js index f7ae238fc..1c7abd0ce 100644 --- a/src/components/link.js +++ b/src/components/link.js @@ -32,24 +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 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) + // : route const compareTarget = route.redirectedFrom ? createRoute(null, normalizeLocation(route.redirectedFrom), null, router) : route @@ -71,7 +78,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 } @@ -89,9 +98,9 @@ export default { if (a) { // in case the 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 child, apply listener to self From c4f05d1d38739e72a2d6c8b640690ea1e3dff326 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Fri, 5 Jul 2019 11:15:06 +0200 Subject: [PATCH 3/5] chore(lint): add prettier conf to specs for longer lines --- test/e2e/specs/.prettierrc | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 test/e2e/specs/.prettierrc diff --git a/test/e2e/specs/.prettierrc b/test/e2e/specs/.prettierrc new file mode 100644 index 000000000..963354f23 --- /dev/null +++ b/test/e2e/specs/.prettierrc @@ -0,0 +1,3 @@ +{ + "printWidth": 120 +} From 0ba6dc5f6c35c3e7738b94197e715fd06e70eac3 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Fri, 5 Jul 2019 11:15:38 +0200 Subject: [PATCH 4/5] test(active-links): adapt redirect tests to added content --- test/e2e/specs/active-links.js | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/test/e2e/specs/active-links.js b/test/e2e/specs/active-links.js index 8d5b549ec..686e94af2 100644 --- a/test/e2e/specs/active-links.js +++ b/test/e2e/specs/active-links.js @@ -9,7 +9,7 @@ module.exports = { browser .url('http://localhost:8080/active-links/') .waitForElementVisible('#app', 1000) - .assert.count('li a', 14) + .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/') @@ -23,8 +23,13 @@ module.exports = { .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/image2') - .assert.attributeContains('li:nth-child(14) a', 'href', '/active-links/gallery/image1') + .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]) @@ -38,9 +43,17 @@ module.exports = { assertActiveLinks(9, [1, 3, 5, 7, 9], null, [9]) assertActiveLinks(10, [1, 10], [11], [10], [11]) assertActiveLinks(11, [1, 10], [11], [10], [11]) - assertActiveLinks(12, [1, 12, 14], null, [14]) - assertActiveLinks(13, [1, 12, 13], null, [13]) - assertActiveLinks(14, [1, 12, 14], null, [14]) + + // 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() From ac7d83c3fa2a3df920b90264220e9e5e244c75d5 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Fri, 5 Jul 2019 11:16:54 +0200 Subject: [PATCH 5/5] chore(link): remove commented code --- src/components/link.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/link.js b/src/components/link.js index 1c7abd0ce..d944e8b82 100644 --- a/src/components/link.js +++ b/src/components/link.js @@ -54,9 +54,7 @@ export default { this.exactActiveClass == null ? exactActiveClassFallback : this.exactActiveClass - // const compareTarget = location.path - // ? createRoute(null, location, null, router) - // : route + const compareTarget = route.redirectedFrom ? createRoute(null, normalizeLocation(route.redirectedFrom), null, router) : route