feat(NcAppNavigationItem): use exact route matching#7939
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7939 +/- ##
=======================================
Coverage 54.60% 54.60%
=======================================
Files 106 106
Lines 3443 3443
Branches 1005 1005
=======================================
Hits 1880 1880
Misses 1322 1322
Partials 241 241 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ShGKme
left a comment
There was a problem hiding this comment.
In v8 there was exact prop because NcAppNavigationItem is a wrapper over RouterLink which had the exact prop in the past in Vue 2. In other words, removed NcAppNavigationItem's exact prop followed removed <RouterLink>'s exact prop.
This prop was removed in Vue Router v4 (Vue 3) ~5 years ago: https://github.com/vuejs/rfcs/blob/master/active-rfcs/0028-router-active-link.md
In my opinion, we should not bring back a prop removed in the base component.
Also, isExactActive is different from old exact. It acts as exact-path, ignoring query and hash.
Adding a prop with the same name but different behaviour seems even more confusing to me and harder to catch. This means that a prop is not removed but changed between v8 and v9, which is still a breaking change from v8, but different from mentioned in the breaking change notes.
In the current case, I'd use isExactActive instead of isExact without additional configuration to always the the full path.
In addition, I'd make active optional but allow override router's active by active prop. So a developer can manually specify when a link is active in a more complex case (for example, with query/hash).
da6e056 to
11ecdb8
Compare
exact prop for exact route matching11ecdb8 to
51c2d1f
Compare
51c2d1f to
f731b21
Compare
|
@ShGKme can you review the requested changes |
… prop to be able to overwrite the vue-router state Signed-off-by: Wolfgang <github@linux-dude.de>
f731b21 to
6e010cf
Compare
|
Rebased PR solving conflict with 377667a |
Resolves: #7965
updated
This PR changes the state to use from
isActivetoisExactActiveand madeactivean optional prop to overwrite the default behaviour.old
exact -
Pass in true if you want the matching behaviour to be non-inclusive: https://router.vuejs.org/api/#exactThe exact prop for NcAppNavigationItem was removed in v9 without replacement.
The news app could now use this for a new feature (nextcloud/news#3148), to use /starred for all items and /starred/:feedId for sub items.
This PR adds the exact prop to control which active state -
isExactActiveorisActive- to use.isExactActive: true if the [exact active class](https://v3.router.vuejs.org/api/#exact-active-class) should be applied. Allows to apply an arbitrary class🏁 Checklist