-
Notifications
You must be signed in to change notification settings - Fork 6.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: external link jump #4319
fix: external link jump #4319
Conversation
|
Important Review skippedReview was skipped due to path filters Files ignored due to path filters (1)
You can disable this status message by setting the WalkthroughThe change introduces a new dependency in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchPanel
participant Router
User->>SearchPanel: Enter search
SearchPanel->>SearchPanel: Check if path is valid HTTP URL
alt External Link
SearchPanel->>User: Open external URL in new tab (window.open)
else Internal Link
SearchPanel->>Router: router.push(to.path, { replace: true })
end
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/effects/layouts/src/widgets/global-search/search-panel.vue (1 hunks)
Additional comments not posted (1)
packages/effects/layouts/src/widgets/global-search/search-panel.vue (1)
102-106
: LGTM!The changes to the
handleEnter
function enhance the navigation logic by handling external links differently from internal routing. External URLs are opened directly in the browser usingwindow.location.href
, while internal paths are managed through the router with thereplace: true
option. This ensures a smooth navigation experience for the user.The code changes are approved.
packages/effects/layouts/src/widgets/global-search/search-panel.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/effects/layouts/package.json (1 hunks)
- packages/effects/layouts/src/widgets/global-search/search-panel.vue (2 hunks)
Additional comments not posted (2)
packages/effects/layouts/package.json (1)
27-27
: Verify the usage and impact of the added dependency@vben-core/shared
.The new dependency
@vben-core/shared
has been added to thepackage.json
file. While this change can enhance code reuse and modularity, it's crucial to ensure that the added dependency is actually used and does not introduce any unnecessary bloat or complexity.Please provide more context on how this dependency is being utilized within the
@vben/layouts
package and confirm that it does not have any unintended side effects.Additionally, consider running the following script to verify the usage of the
@vben-core/shared
package:Verification successful
The dependency
@vben-core/shared
is actively used across the codebase.The
@vben-core/shared
package is imported in multiple files, indicating its significant role in providing shared utilities or components. This justifies its addition as a dependency in thepackage.json
file.
- Files utilizing
@vben-core/shared
include:
packages/utils/src/helpers/generate-routes-frontend.ts
packages/stores/src/modules/tabbar.ts
packages/utils/src/helpers/generate-menus.ts
packages/utils/src/helpers/generate-routes-backend.ts
packages/@core/composables/src/use-content-style.ts
packages/@core/preferences/src/use-preferences.ts
packages/@core/ui-kit/popup-ui/src/modal/use-modal.ts
- And several others.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `@vben-core/shared` package. # Test: Search for import statements of `@vben-core/shared`. Expect: At least one occurrence. rg --type typescript -g '!node_modules' -g '!dist' $'from \'@vben-core/shared\''Length of output: 120
Script:
#!/bin/bash # Description: Verify the usage of the `@vben-core/shared` package. # Test: Search for import statements of `@vben-core/shared` in .ts files. Expect: At least one occurrence. rg --glob '*.ts' -g '!node_modules' -g '!dist' $'from \'@vben-core/shared\''Length of output: 1484
packages/effects/layouts/src/widgets/global-search/search-panel.vue (1)
103-107
: LGTM!The changes to the
handleEnter
function look good. The function now handles external links differently from internal routing by opening them in a new browser tab, which is an improvement to the navigation logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/effects/layouts/package.json (1 hunks)
Additional comments not posted (1)
packages/effects/layouts/package.json (1)
27-27
: LGTM!The added dependency
@vben-core/shared
looks good.
* fix: external link jump * fix: external link jump * chore: update deps --------- Co-authored-by: Li Kui <[email protected]>
Description
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit