vuejs / router

🚦 The official router for Vue.js
https://router.vuejs.org/
MIT License
3.75k stars 1.15k forks source link

docs: add 'Active links' #2183

Closed skirtles-code closed 3 months ago

skirtles-code commented 3 months ago

This adds a page explaining the RouterLink classes.

This currently isn't covered in the main guide. It is mentioned in the migration guide for the exact prop. There's also an explanation in the related RFC.

I've put it near the end of the Essentials section. It mentions alias and redirect, so it needed to come after those are introduced. Putting it after the guide to passing props was an arbitrary choice, it could just as easily have gone before.

There's already an advanced guide to Extending RouterLink. That page seems to assume that people are already familiar with the existing RouterLink classes and the related concepts of active and exact. This new page covers those topics.

I haven't covered the edge case around the handling of a path: '' on children. I could try to add something about that, though I'm not sure whether it would be more confusing than helpful.

netlify[bot] commented 3 months ago

Deploy Preview for vue-router ready!

Name Link
Latest commit 87c2d14580631579ad1d4daf605e98a830e12068
Latest deploy log https://app.netlify.com/sites/vue-router/deploys/65fe24e1b06c8a00080134e0
Deploy Preview https://deploy-preview-2183--vue-router.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

Lighthouse
1 paths audited
Performance: 97 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

posva commented 3 months ago

I haven't covered the edge case around the handling of a path: '' on children. I could try to add something about that, though I'm not sure whether it would be more confusing than helpful.

I think it's fine leaving it out as most people won't have a route that can be visited directly while having a child with path ''.

skirtles-code commented 3 months ago

I haven't covered the edge case around the handling of a path: '' on children. I could try to add something about that, though I'm not sure whether it would be more confusing than helpful.

I think it's fine leaving it out as most people won't have a route that can be visited directly while having a child with path ''.

We might be thinking of slightly different edge cases.

To clarify, the specific edge case I'm most concerned about is the one shown here:

The routes here are:

const routes = [
  {
    path: '/user',
    children: [
      {
        path: '',
        name: 'A',
        component: UserView
      }, {
        path: 'roles',
        name: 'B',
        component: UserRolesView
      }
    ]
  }
]

Route A is a sibling of route B, not an ancestor, yet it is shown as active when the current location is /user/roles. The way I've described it in the guide would seem to imply that only ancestors can be active.

When I was writing my description of how 'active' is defined, I stayed clear of mentioning route.matched, because I didn't think most readers would know what that was, but in my mind they are closely related concepts. The edge case above is one instance where a link is shown as 'active' despite the route not being in route.matched.

skirtles-code commented 3 months ago

I'm wondering if we should name the page something else that includes active/matching. We could even omit the RouterLink keyword. So maybe, just "Active Links" works better. What do you think?

Yeah, I think that makes sense. I've also renamed the first section heading so it doesn't clash with the main heading.

posva commented 3 months ago

Ah i see now what you meant. It would be inconvenient if it was the other way around but it doesn’t for against the idea of the matched array