feat: menu routing support opens in a new window #4715
Conversation
|
WalkthroughThe pull request introduces significant enhancements to routing and menu configuration documentation, clarifying the distinctions between static and dynamic routes. It adds a new property, 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: 4
🧹 Outside diff range and nitpick comments (11)
packages/@core/base/shared/src/utils/window.ts (1)
26-35: LGTM with minor suggestions for improvementThe
openRouteInNewWindowfunction is well-implemented and aligns with the PR objectives. It correctly constructs the URL and uses the existingopenWindowfunction. However, consider the following improvements:
- Add JSDoc for the
pathparameter to improve developer understanding.- Consider adding error handling for invalid paths.
Here's a suggested improvement with JSDoc and basic error handling:
/** * Opens a route in a new window. * @param path - The route path to open. Can be absolute or relative. * @throws {Error} If the path is empty or not a string. */ function openRouteInNewWindow(path: string) { if (typeof path !== 'string' || path.trim() === '') { throw new Error('Invalid path provided'); } const { hash, origin } = location; const fullPath = path.startsWith('/') ? path : `/${path}`; const url = `${origin}${hash ? '/#' : ''}${fullPath}`; openWindow(url, { target: '_blank' }); }playground/src/locales/langs/zh-CN/demos.json (1)
52-52: LGTM: New translation added for 'Open in New Window' featureThe addition of "openInNewWindow": "新窗口打开" correctly implements the Chinese translation for the new feature to open menu items in a new window. This aligns with the PR objectives and the changes in routing configuration.
Consider adding a comment above this line to explain the purpose of this feature, which might help other developers understand its significance:
"menuWithQuery": "带参菜单", +// 允许在新窗口中打开菜单项 "openInNewWindow": "新窗口打开"playground/src/locales/langs/en-US/demos.json (1)
51-52: LGTM! Consider adding translations for other supported languages.The addition of
"openInNewWindow": "Open in New Window"is appropriate and aligns with the new feature. The English text is clear and concise.To maintain consistency across the application, consider adding corresponding translations for this new feature in other supported language files (if any) within the
locales/langs/directory.packages/@core/base/typings/src/vue-router.d.ts (1)
101-104: LGTM! Consider enhancing the JSDoc comment.The addition of the
openInNewWindowproperty is well-implemented and aligns with the PR objective. The property is correctly typed as an optional boolean.Consider expanding the JSDoc comment to provide more context:
/** * Indicates whether the route should open in a new window/tab when accessed. * @default false */ openInNewWindow?: boolean;This enhancement provides more clarity on the property's behavior and default value.
docs/src/guide/essentials/route.md (3)
Line range hint
1-24: LGTM! Clear explanation of routing types and menu generation.The updated introduction and routing types section provide a clearer explanation of how the framework handles routing and menu generation. The distinction between static and dynamic routes is well-defined.
Consider adding a brief example or diagram to illustrate the difference between static and dynamic routes for visual learners.
Line range hint
26-385: Excellent additions for route definition and page creation.The new sections on route definition and adding pages provide clear, practical guidance with well-structured examples. This will greatly assist users in implementing both simple and complex routing structures.
Consider adding a brief note about best practices for organizing route files in larger projects, such as grouping routes by feature or module.
Line range hint
385-552: Comprehensive update to RouteMeta interface and configuration options.The addition of the
openInNewWindowproperty to the RouteMeta interface is well-documented and aligns with the PR objectives. The expanded explanations for each configuration option provide valuable context for users.Consider adding a brief example demonstrating how to use the
openInNewWindowproperty in a route configuration to further clarify its usage.packages/stores/src/modules/tabbar.ts (1)
293-293: SimplifyopenTabInNewWindowmethodThe method has been refactored to use the new
openRouteInNewWindowfunction, which simplifies the logic and improves consistency. This change aligns well with the new feature for opening routes in new windows.Consider using optional chaining for a more concise expression:
-openRouteInNewWindow(tab.fullPath || tab.path); +openRouteInNewWindow(tab.fullPath ?? tab.path);This change would use
tab.pathonly iftab.fullPathisnullorundefined, which might be more precise depending on your use case.docs/src/en/guide/essentials/route.md (2)
Line range hint
1-38: LGTM! Clear explanations and updated examples.The introduction and route types sections have been significantly improved. The distinction between static and dynamic routes is now clearer, and the code examples for static routes configuration have been updated.
Consider adding a brief explanation of when to use static vs. dynamic routes to help developers choose the appropriate approach for their use case.
Line range hint
40-205: Great addition of detailed routing examples!The examples for secondary and multilevel routing are comprehensive and provide valuable guidance for implementing complex routing structures.
For the multilevel routing example, consider breaking it down into smaller sections with explanations for each level. This could help readers better understand the structure and purpose of each nested route.
packages/effects/layouts/src/basic/menu/use-navigation.ts (1)
9-9: Rename 'routeMetaMap' to 'routeMap' for claritySince the map now stores entire
routeobjects rather than just theirmetaproperties, consider renamingrouteMetaMaptorouteMapto better reflect its purpose.Apply this diff to rename the variable:
-const routeMetaMap = new Map<string, RouteRecordNormalized>(); +const routeMap = new Map<string, RouteRecordNormalized>(); -routes.forEach((route) => { - routeMetaMap.set(route.path, route); -}); +routes.forEach((route) => { + routeMap.set(route.path, route); +}); -const route = routeMetaMap.get(path); +const route = routeMap.get(path);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
- docs/src/en/guide/essentials/route.md (1 hunks)
- docs/src/guide/essentials/route.md (2 hunks)
- packages/@core/base/shared/src/utils/window.ts (1 hunks)
- packages/@core/base/typings/src/vue-router.d.ts (1 hunks)
- packages/@core/ui-kit/shadcn-ui/src/components/breadcrumb/breadcrumb-background.vue (2 hunks)
- packages/effects/layouts/src/basic/menu/use-navigation.ts (1 hunks)
- packages/stores/src/modules/tabbar.ts (2 hunks)
- playground/src/locales/langs/en-US/demos.json (1 hunks)
- playground/src/locales/langs/zh-CN/demos.json (1 hunks)
- playground/src/router/routes/modules/demos.ts (2 hunks)
- playground/src/views/demos/features/new-window/index.vue (1 hunks)
- pnpm-workspace.yaml (3 hunks)
🧰 Additional context used
🔇 Additional comments (18)
playground/src/views/demos/features/new-window/index.vue (2)
1-3: LGTM: Script section is well-structured.The script section is concise, uses TypeScript, and correctly imports the Fallback component. It follows the Vue 3 composition API style with
<script setup>.
5-11: LGTM: Template structure and Fallback component usage are correct.The Fallback component is properly implemented with appropriate props for description, status, and title. This effectively communicates that the page has been opened in a new window.
packages/@core/base/shared/src/utils/window.ts (1)
37-37: Export statement looks goodThe export statement correctly includes both the new
openRouteInNewWindowfunction and the existingopenWindowfunction, making them available for use in other parts of the application.playground/src/locales/langs/zh-CN/demos.json (1)
51-51: LGTM: Trailing comma addedAdding a trailing comma after the last item in an object or array is a good practice. It makes future additions easier and reduces diff noise in version control.
packages/@core/ui-kit/shadcn-ui/src/components/breadcrumb/breadcrumb-background.vue (3)
9-9: Improved props handling and click logicThe changes in the script section enhance the component's functionality and readability:
Props handling (line 9): The switch to direct destructuring from
defineProps<Props>()simplifies the code and aligns with modern Vue 3 practices.
handleClickfunction (lines 13-17): The addition of theindexparameter and the check for the last breadcrumb item prevent unnecessary 'select' events for the current page, improving the component's behavior.These changes improve both the code quality and the breadcrumb's functionality.
Also applies to: 13-17
28-31: Updated click event handler in templateThe modification to the click event handler in the template (lines 28-31) correctly implements the changes made to the
handleClickfunction:
- It now passes both the
indexanditem.pathtohandleClick, aligning with the updated function signature.- This change enables the new logic in
handleClickto identify the last breadcrumb item.- The use of
@click.stopis maintained, preventing unintended event propagation.This update ensures consistency between the template and the script, maintaining the improved breadcrumb functionality.
Line range hint
1-31: Overall improvement to breadcrumb componentThe changes made to this component collectively enhance its functionality and align with modern Vue 3 practices:
- The props handling has been simplified using direct destructuring.
- The
handleClickfunction now considers the breadcrumb item's position, preventing unnecessary actions on the last item.- The template has been updated to correctly utilize the new
handleClickfunction signature.These improvements contribute to a more robust and user-friendly breadcrumb navigation, aligning well with the PR's objective of enhancing routing capabilities. The code is now more maintainable and provides a better user experience by disabling clicks on the current page's breadcrumb.
pnpm-workspace.yaml (1)
46-46: LGTM: Minor version updates for development dependencies.The changes include minor version updates for the following packages:
@types/node: ^22.7.7 to ^22.7.8@typescript-eslint/eslint-pluginand@typescript-eslint/parser: ^8.10.0 to ^8.11.0eslint-config-turbo: ^2.2.1 to ^2.2.3turbo: ^2.2.1 to ^2.2.3These updates are likely to include bug fixes and small improvements. The synchronized updates for related packages (
@typescript-eslint/*andturbo-related) are good practice.Please ensure these updates align with the project's dependency update policy and that they don't introduce any unexpected changes in the development environment.
Also applies to: 51-52, 87-87, 156-156
docs/src/guide/essentials/route.md (2)
Line range hint
553-568: Useful addition of route refresh functionality.The new section on route refreshing provides a clear and concise example of how to programmatically refresh routes using the
useRefreshhook. This is a valuable addition to the documentation.
Line range hint
1-568: Comprehensive and well-structured update to routing documentation.This update significantly enhances the routing and menu configuration documentation. Key improvements include:
- Clearer explanations of routing types and automatic menu generation.
- Detailed examples for second-level and multi-level routes.
- Step-by-step guide for adding new pages.
- Addition of the
openInNewWindowproperty to the RouteMeta interface.- Expanded documentation for each configuration option.
- New section on route refreshing.
These changes provide users with a more comprehensive understanding of the routing system and practical guidance for implementation.
packages/stores/src/modules/tabbar.ts (2)
7-7: Update import statement to useopenRouteInNewWindowThe import statement has been updated to use
openRouteInNewWindowinstead ofopenWindow. This change aligns with the refactoring of the window opening functionality.
Line range hint
1-593: Summary of changes and their impactThe changes in this file are minimal and focused on implementing the new feature for opening routes in new windows. The modifications are well-contained and don't introduce any breaking changes to the existing tab management functionality. These updates align perfectly with the PR objective of adding menu routing support for opening in a new window.
Key points:
- The import statement has been updated to use the new
openRouteInNewWindowfunction.- The
openTabInNewWindowmethod has been simplified to use this new function.These changes enhance the tab management system by providing a more streamlined way to open tabs in new windows, improving the overall user experience.
docs/src/en/guide/essentials/route.md (3)
Line range hint
207-258: LGTM! Clear instructions for adding new pages.The step-by-step guide for adding a new page, including both route configuration and component creation, is well-structured and easy to follow.
Line range hint
392-406: LGTM! Useful example for route refreshing.The addition of a code example for refreshing the current route is a helpful feature for developers. It's concise and easy to understand.
Line range hint
260-390: Excellent documentation of route configuration options!The comprehensive list of route configuration options, including detailed explanations, provides valuable guidance for developers. The addition of the
openInNewWindowproperty to the RouteMeta interface aligns well with the PR objective of supporting menu routing that opens in a new window.To ensure consistency across the codebase, let's verify the implementation of the
openInNewWindowproperty:✅ Verification successful
Verification Successful
The
openInNewWindowproperty is properly defined and utilized across the codebase as intended.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation and usage of the openInNewWindow property # Test 1: Check for the property definition in relevant files echo "Checking for openInNewWindow property definition:" rg "openInNewWindow\??:\s*boolean" --type ts # Test 2: Search for usage of the openInNewWindow property in the codebase echo "Checking for openInNewWindow property usage:" rg "openInNewWindow" --type vue --type tsLength of output: 401
Script:
#!/bin/bash # Description: Verify the usage of the openInNewWindow property in .vue and .ts files # Test 1: Check for the property definition in relevant files echo "Checking for openInNewWindow property definition:" rg "openInNewWindow\??:\s*boolean" --type ts # Test 2: Search for usage of the openInNewWindow property in the codebase echo "Checking for openInNewWindow property usage:" rg "openInNewWindow" -g "*.vue" -g "*.ts"Length of output: 983
playground/src/router/routes/modules/demos.ts (2)
177-177: LGTM: FullScreenDemo title updateThe title update for the FullScreenDemo route improves clarity and consistency.
203-213: LGTM: NewWindowDemo route additionThe new route for NewWindowDemo is well-structured and aligns perfectly with the PR objective of adding support for opening menu routes in a new window. The
openInNewWindow: trueproperty in the meta object is crucial for implementing this feature.packages/effects/layouts/src/basic/menu/use-navigation.ts (1)
20-21: Confirm the functionality of 'openRouteInNewWindow'Ensure that the
openRouteInNewWindowfunction correctly handles route paths and includes necessary query parameters or state. Missing these could lead to incomplete or incorrect navigation when opening in a new window.Run the following script to check the implementation of
openRouteInNewWindow:#!/bin/bash # Description: Find the implementation of 'openRouteInNewWindow' and display its code. # Expected: The function definition of 'openRouteInNewWindow'. rg 'function openRouteInNewWindow' --type ts -A 10
Description
close #4699
fixed #4707
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
Release Notes
New Features
openInNewWindowproperty.Documentation
Bug Fixes
UI Enhancements