fix: search menu did not correctly carry routing parameters #5441
Conversation
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/effects/layouts/src/widgets/global-search/search-panel.vueOops! Something went wrong! :( ESLint: 9.17.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
Poem
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 (2)
packages/effects/layouts/src/widgets/global-search/search-panel.vue (2)
14-15: Consider using path aliases for imports.Using relative paths for imports can make the code harder to maintain if the file structure changes. Consider using path aliases (e.g.,
@/basic/menu/use-navigation) for better maintainability.
Line range hint
89-105: Add error handling for navigation.While the navigation implementation looks good, consider adding error handling for the navigation call to gracefully handle any routing failures:
async function handleEnter() { if (searchResults.value.length === 0) { return; } const result = searchResults.value; const index = activeIndex.value; if (result.length === 0 || index < 0) { return; } const to = result[index]; if (to) { searchHistory.value.push(to); handleClose(); await nextTick(); - await navigation(to.path); + try { + await navigation(to.path); + } catch (error) { + console.error('Navigation failed:', error); + // Consider showing a user-friendly error message + } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/effects/layouts/src/widgets/global-search/search-panel.vue(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Test (windows-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Test (ubuntu-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (2)
packages/effects/layouts/src/widgets/global-search/search-panel.vue (2)
88-88: LGTM!The navigation setup follows Vue Composition API best practices with clear and descriptive naming.
14-14: Verify routing parameter handling in useNavigation.To ensure this fix properly addresses the routing parameter issue, let's verify the implementation of
useNavigation:✅ Verification successful
Implementation correctly handles routing parameters
The
useNavigationhook properly handles routing parameters through vue-router's official API, preserving query parameters from route metadata and correctly structuring navigation calls.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the implementation of useNavigation to verify routing parameter handling # Search for the useNavigation implementation echo "Searching for useNavigation implementation..." ast-grep --pattern 'export function useNavigation() { $$$ }' # Search for tests related to routing parameter handling echo "Searching for related tests..." rg -l "useNavigation.*routing|route.*params.*navigation" -g "*.{spec,test}.{ts,js,vue}"Length of output: 326
Script:
#!/bin/bash # Try multiple approaches to find the implementation # 1. Look for the exact file from the import echo "Checking the direct file path..." cat packages/effects/layouts/src/basic/menu/use-navigation.ts 2>/dev/null || echo "File not found at expected path" # 2. Search for any file containing useNavigation echo -e "\nSearching for files containing useNavigation..." rg -l "useNavigation" # 3. Search for navigation-related function exports with more flexible pattern echo -e "\nSearching for navigation-related exports..." ast-grep --pattern 'export $_ useNavigation'Length of output: 1577
Description
修复在携带路由参数情况下 搜索菜单未正确携带路由参数跳转
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
useRouterwith a new navigation method