fix: the anchor and tabs conflict #5168
Conversation
Pro-linx
requested review from
anncwb,
vince292007 and
mynetfan
as code owners
|
WalkthroughThe pull request introduces a systematic change across multiple files in the layout and store modules, focusing on replacing Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/effects/layouts/src/basic/content/content.vue (1)
103-103: LGTM! Consider documenting the route key strategyThe change from
route.fullPathtoroute.pathis consistent across both cached and non-cached component scenarios. This effectively prevents unnecessary re-renders when only the URL anchor changes.Consider adding a comment or documentation explaining the route key strategy, as this is a crucial architectural decision that affects component lifecycle and state management. This will help future maintainers understand why
route.pathis used instead ofroute.fullPath.Also applies to: 109-109
packages/stores/src/modules/tabbar.ts (2)
549-549: LGTM! Consider documenting the path handling behaviorThe change to use
pathinstead offullPathis correct and forms the foundation for fixing the anchor navigation issue. The URL decoding ensures proper handling of special characters in paths.Consider adding a comment explaining why we use
pathinstead offullPath:/** * @zh_CN 获取标签页路径 + * @description Uses base path without query parameters to ensure consistent tab handling + * and prevent duplicate tabs when using anchor navigation * @param tab */ function getTabPath(tab: RouteRecordNormalized | TabDefinition) { return decodeURIComponent((tab as TabDefinition).path); }
Line range hint
1-565: Consider documenting the tab path handling strategyThe systematic change from
fullPathtopatheffectively addresses the anchor navigation issue. However, this represents a significant architectural decision in how tab uniqueness is determined.Consider:
- Adding a comment in the store's documentation explaining this tab path handling strategy
- Updating relevant component documentation to clarify that tabs with different query parameters will be treated as the same tab
- Adding test cases to verify the behavior with various URL patterns (base paths, query parameters, and anchors)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/effects/layouts/src/authentication/form.vue(1 hunks)packages/effects/layouts/src/basic/content/content.vue(1 hunks)packages/effects/layouts/src/basic/tabbar/use-tabbar.ts(2 hunks)packages/effects/layouts/src/iframe/iframe-router-view.vue(1 hunks)packages/stores/src/modules/tabbar.ts(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/effects/layouts/src/iframe/iframe-router-view.vue
🔇 Additional comments (6)
packages/effects/layouts/src/authentication/form.vue (1)
18-18: LGTM! Verify component state preservation
The change from route.fullPath to route.path is correct and aligns with fixing the anchor navigation issue. This ensures the component doesn't re-render when only the anchor portion of the URL changes.
Let's verify that this component maintains its state correctly with the new key:
#!/bin/bash
# Check if there are any other instances where fullPath might still be used
# that could affect this component's state preservation
rg -g '!*.{json,md}' 'fullPath.*authentication' packages/effects/layouts/src/basic/tabbar/use-tabbar.ts (2)
46-46: LGTM! Key change for tab management
The change to use route.path for the current active tab is correct and consistent with the PR's objective to fix anchor-related issues.
Let's verify the tab management behavior:
#!/bin/bash
# Check for any remaining fullPath usage in tab-related code
# that might conflict with this change
rg -g '!*.{json,md}' 'fullPath.*tab'147-147: LGTM! Verify maximize behavior
The change to use tab.path in the maximize handler ensures consistent navigation behavior and prevents anchor-related issues when maximizing tabs.
Let's verify the maximize functionality:
#!/bin/bash
# Check for any maximize-related handlers that might still use fullPath
rg -g '!*.{json,md}' 'fullPath.*(maximize|restore)'packages/stores/src/modules/tabbar.ts (3)
191-191: LGTM! Consistent with the tab path handling changes
The change maintains consistency with the new tab path handling approach, ensuring that tabs are compared using their base paths rather than full paths with query parameters.
61-65: LGTM! Verify tab closing behavior with query parameters
The change from fullPath to path looks good and aligns with fixing the anchor navigation issue. This ensures that tabs with the same base path but different query parameters or anchors are treated as the same tab.
Let's verify that tabs with query parameters are handled correctly:
#!/bin/bash
# Search for any special handling of query parameters in tab management
rg -g '!*.{json,lock}' -A 3 'query.*params' .293-293: Verify if query parameters should be preserved when opening in new window
While the change is consistent with other path handling updates, we should verify if query parameters need to be preserved when opening tabs in new windows. The current implementation might drop important URL parameters.
Let's check how query parameters are handled in new window openings:
#!/bin/bash
# Search for the implementation of openRouteInNewWindow
rg -A 5 'function openRouteInNewWindow' .
# Search for any tests related to opening routes in new windows
rg -g '*.{spec,test}.{ts,js,tsx,jsx}' 'openRouteInNewWindow|openTabInNewWindow' .
Description
修复使用锚点跳转时，会刷新content区域。若带有锚点地址刷新网页时，标签页重复显示的问题。
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
New Features
Bug Fixes
Documentation