Skip to content

propsal for exact active behavior customization in router-link #35

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

Closed
wants to merge 1 commit into from

Conversation

posva
Copy link
Member

@posva posva commented May 10, 2019

@Akryum
Copy link
Member

Akryum commented May 10, 2019

What if instead the exact prop could get a function so that the user can customize the behavior however he wants?

@posva
Copy link
Member Author

posva commented May 10, 2019

it could be an extra option, indeed, but I wonder if it's necessary since it could also be done through the scoped slot (although it depends on what arguments you want to pass to the function):

const isExact = (current, location) => location.path === current.path
<router-link v-slot="{ location }">
  <a :class="matchLocation($route, location) && 'is-active'">Link</a>
</router-link>

@Akryum
Copy link
Member

Akryum commented May 10, 2019

Yeah I think it should all be achievable with the slot API, and we should make sure it's easy to match several parts of the location. Then we could remove the exact prop altogether? 🐈

<router-link v-slot="{ location, navigate, exact }">
  <a
    :href="location"
    :class="{ 'router-link-active-exact': exact.path && exact.query }"
    @click="navigate"
  >
    Link
  </a>
</router-link>

@Akryum
Copy link
Member

Akryum commented May 10, 2019

I feel this new extact="path+query" syntax is not necessary with the slot API. To much new API surface and learning cost for little benefit imo.

@posva
Copy link
Member Author

posva commented May 10, 2019

I agree it can all be achieved through the scoped slot haha but it does make it much easier to use, so I don't think we should remove it altogether hehe. The most common use would be exact="all" and exact="path", so removing the + syntax and only allowing those values seems like a fair trade off to me while we cover most use cases and people can still match only the query or the hash by using the scoped slot

@Akryum
Copy link
Member

Akryum commented May 10, 2019

So instead of having a new way of using exact, we could have a new boolean prop named exact-path:

<router-link to="/" exact-path>Home</router-link>

@Akryum
Copy link
Member

Akryum commented May 10, 2019

In the end, I think it would be less confusing and easier to learn that having different ways of using exact.

@posva
Copy link
Member Author

posva commented May 10, 2019

yes, we could but because it's either using exact or exact-path so it would be confusing (it's my point in the alternatives, but I think you saw it), it means they should be refactored into one prop for it to be cleaner. It's also not possible to extend to new usages while the string version is.
To me having a new possible value is less to learn than having two props

@Akryum
Copy link
Member

Akryum commented May 10, 2019

Anyway, I think having this:
image
means the API is too complicated.

@Akryum
Copy link
Member

Akryum commented May 10, 2019

Maybe we could just remove the exact prop and make the user rely on the classes added by router-link, like .router-link-active-exact and maybe a new one, .router-link-active-exact-path?

@Akryum
Copy link
Member

Akryum commented May 10, 2019

So instead of having an exact prop, you can do:

<router-link class="exact" to="/">Home</router-link>
<router-link to="/foobar">Foobar</router-link>
:not(.exact).router-link-active,
.exact.router-link-active-exact {
  background: green;
}

@posva
Copy link
Member Author

posva commented May 10, 2019

About the table

It's funny, I felt the same way when I wrote it, but it should be treated as examples not as a table that would be included in the documentation. I realized it doesn't feel that complicated if I ignore the table and read it as a sentence that says we can match some specific parts of a location only and match multiple ones with a + operator

router-link, like .router-link-active-exact and maybe a new one, .router-link-active-exact-path

that's another alternative I forgot to add but it's more complicating and also adds even more props to the router-link (active-link-class companions). Also, if we have one for path, why not query and hash? That's what people could ask for and that would create even more props

@Akryum
Copy link
Member

Akryum commented May 10, 2019

That's what people could ask for and that would create even more props

Then they can just use the slot API.

I really feel this is all overkill, and that the exact prop is kinda only handy for the / path. The more complex use cases should be covered by the slot API. The exact prop is currently necessary because we don't have this slot API.

@Akryum
Copy link
Member

Akryum commented May 10, 2019

In the end I think we should just leave the exact prop as it is, being mostly useful for to="/" and use the slot API which will cover all possible use cases the exact prop may not (even with this RFC).

@Akryum
Copy link
Member

Akryum commented May 10, 2019

Another possible change: remove exact prop and change the default behavior of the router-link-active class for the / path, so it's like exact by default. I mean in how many apps you don't put exact on the home link in the nav? I think it would remove a gotcha and something use have to think and learn how to work around.

@posva
Copy link
Member Author

posva commented May 10, 2019

In the end I think we should just leave the exact prop as it is, being mostly useful for to="/" and use the slot API which will cover all possible use case the exact prop may not.

It's a possibility but there is a real need for matching the path only without the query and the hash. Since using a scoped slot will always be a lot more work than using the regular slot api, I think we should allow at least exact="path" for convenience. TBH the rest is really there as a proposal to gather feedback if this is necessary and I don't like the + because it's magic.

:not(.exact).router-link-active,
.exact.router-link-active-exact {
  background: green;
}

I didn't think about this one! It's a good one. I still want framework/library users to be able to provide a single class for .router-link-active (in tailwindcss for example) and provide the exact prop to have their styling without writing CSS

@Akryum
Copy link
Member

Akryum commented May 10, 2019

I just though about an issue with the proposal: what if you want to have styling for let's say router-link-active-exact and router-link-active-exact-path (both of them)?

@posva
Copy link
Member Author

posva commented May 10, 2019

I just though about an issue with the proposal: what if you want to have styling for let's say router-link-active-exact and router-link-active-exact-path (both of them)?

Do you mean all of the them at the same time, different styling?

the way I was thinking about the exact prop is:

  • no prop -> apply active and active-exact separately
  • true -> apply active and active-exact at the same time while the fullPath matches
  • 'path' -> apply active and active-exact at the same time while the path matches

@Akryum
Copy link
Member

Akryum commented May 10, 2019

Do you mean all of the them at the same time, different styling?

Yes

@posva
Copy link
Member Author

posva commented May 14, 2019

Closing in favor of a much more simple RFC https://github.com/vuejs/rfcs/pull/37/files

@posva posva closed this May 14, 2019
@posva posva deleted the router/link-exact-active-behavior branch May 14, 2019 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants