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

docs: add 'Active links' #2183

Merged
merged 2 commits into from
Mar 23, 2024
Merged

Conversation

skirtles-code
Copy link
Contributor

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.

Copy link

netlify bot commented Mar 21, 2024

Deploy Preview for vue-router ready!

Name Link
🔨 Latest commit 87c2d14
🔍 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
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.

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks again! 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?

@posva
Copy link
Member

posva commented Mar 22, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@skirtles-code skirtles-code changed the title docs: add RouterLink classes docs: add 'Active links' Mar 23, 2024
@posva
Copy link
Member

posva commented Mar 23, 2024

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

@posva posva merged commit 4e82939 into vuejs:main Mar 23, 2024
4 checks passed
niceplugin added a commit to niceplugin/Vuejs-Router-KO that referenced this pull request Mar 31, 2024
* docs: add RouterLink classes

* Rename page to 'Active links'
niceplugin added a commit to niceplugin/Vuejs-Router-KO that referenced this pull request Mar 31, 2024
* docs: add 'Active links' (vuejs#2183)

* docs: add RouterLink classes

* Rename page to 'Active links'

* docs: update certificate links
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants