refactor: drop exact from envelope and envelopeskeleton - #13625
Conversation
Signed-off-by: greta <gretadoci@gmail.com>
|
And that is compatible with the current router too? |
|
@coderabbitai review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughThe envelope components no longer use exact route matching. ChangesEnvelope route matching
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: 🔵 Low · up to Removing exact route matching may cause the wrong envelope to appear selected when a message route includes a draft identifier. This is a bounded UI correctness risk; the PR is otherwise mergeable with explicit owner awareness or follow-up. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Team
Run ID: faf7dcc4-503b-4348-8c23-1a02e832a215
📒 Files selected for processing (2)
src/components/Envelope.vuesrc/components/EnvelopeSkeleton.vue
💤 Files with no reviewable changes (1)
- src/components/Envelope.vue
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.
| :custom="to ? true : null" | ||
| :to="to" | ||
| :exact="to ? exact : null"> | ||
| :to="to"> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 '"vue-router"' --glob 'package*.json' --glob '*lock*' .
rg -n -C 4 'message|children:|EnvelopeSkeleton|:exact|isExactActive' srcRepository: nextcloud/mail
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- EnvelopeSkeleton.vue ---'
cat -n src/components/EnvelopeSkeleton.vue
printf '%s\n' '--- Vue Router dependency declarations ---'
rg -n -C 3 '"vue-router"' package.json package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null || true
printf '%s\n' '--- EnvelopeSkeleton usages and route definitions ---'
rg -n -C 5 'EnvelopeSkeleton|name:\s*['"'"'"'"'"']message|children\s*:|isExactActive|:exact' src --glob '*.vue' --glob '*.js' --glob '*.ts' --glob '*.json' | head -n 400Repository: nextcloud/mail
Length of output: 32299
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Change for EnvelopeSkeleton.vue ---'
git diff --unified=12 -- src/components/EnvelopeSkeleton.vue
printf '%s\n' '--- Envelope link target and active-state inputs ---'
sed -n '1,40p' src/components/Envelope.vue
sed -n '770,810p' src/components/Envelope.vue
rg -n -C 6 '<EnvelopeSkeleton|<Envelope\b|active=|:active=' src/components src/views --glob '*.vue' | head -n 300
printf '%s\n' '--- Router configuration ---'
cat -n src/router.js | sed -n '1,70p'Repository: nextcloud/mail
Length of output: 8332
🌐 Web query:
Vue Router 3.6.5 exact router-link isActive isIncludedRoute source query hash optional params
💡 Result:
In Vue Router 3.6.5, the router-link component determines active state based on route records and parameters, intentionally excluding query strings and hashes from the matching logic [1][2]. 1. isActive (router-link-active): This class is applied when the current route matches the route record and includes the same parameter values [1]. Because the matching is inclusive, it remains active for nested routes [3][1]. 2. isExactActive (router-link-exact-active): This class is applied only when the current route is an exact match for the link's target, meaning it must match the same route record and parameters, and it must not be an ancestor route [1]. 3. Optional Params: Optional parameters are handled within the path matching logic. For a link to be active, provided parameters must match the current route's parameters [4][5]. If a parameter is optional and not present, it does not prevent an active match provided the path segment otherwise aligns [6]. 4. Query/Hash: These are ignored during the active/exact active state calculation [1][2]. The router does not use them to determine if a link is "active," as the determination is strictly based on the route record and path parameters [1]. If you require active status to account for query parameters, you must implement custom logic using the v-slot API (which exposes isActive, route, etc.) to manually compare the current route's query object with your target query object [3][7].
Citations:
- 1: https://router.vuejs.org/guide/essentials/active-links
- 2: https://github.com/vuejs/rfcs/blob/master/active-rfcs/0028-router-active-link.md
- 3: https://v3.router.vuejs.org/api/
- 4: https://github.com/vuejs/router/blob/main/packages/router/src/RouterLink.ts
- 5: https://github.com/vuejs/router/blob/ea8e3e21/packages/router/src/RouterLink.ts
- 6: https://github.com/vuejs/router/blob/main/packages/router/src/matcher/index.ts
- 7: https://router.vuejs.org/guide/advanced/extending-router-link.html
Preserve exact active-state matching for draft routes.
Removing :exact allows the message link to remain active when the current message route includes its optional draftId. Because EnvelopeSkeleton styles the row from isActive, the wrong envelope can appear selected. Keep :exact and add a regression test.
2 participants
ref: #13622
Vue Router 4 matches routes exactly by default, making the
exactprop a no-op. Dropped the binding in Envelope.vue and the prop
declaration + template binding in EnvelopeSkeleton.vue.
🤖 AI (if applicable)
Summary by CodeRabbit