feat: sync 3.20.0 to dev #2639
feat: sync 3.20.0 to dev #2639
Conversation
* feat: change default router link * feat: change default router link
#2619) * fix(anchor): [anchor] fixed the bug that querySelect cannot select special id * fix(anchor): [anchor] fixed the bug that querySelect cannot select special id
…2623) * fix: fixed the problem of global search js resource access failure * feat: update docs version
…nu (#2627) * feat(tree-menu): [tree-menu] Resolve the warning issue on the tree-menu page * feat(tree-menu): [tree-menu] Resolve the warning issue on the tree-menu page
… errors (#2629) * feat: support global search in multiple environments and fix pipeline errors * feat: support global search in multiple environments and fix pipeline errors
WalkthroughThe changes in this pull request primarily involve the removal of search functionality across various components of the application. This includes the deletion of DocSearch resources from the HTML and adjustments to visibility settings for the search box. Additionally, several files have been updated to reflect new dependencies, version changes, and modifications to routing and component logic. The overall structure of the application remains intact, but the search-related features have been significantly altered or removed. Changes
Possibly related PRs
Suggested labels
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 (
|
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
packages/vue/src/tree-menu/src/props.ts (1)
Line range hint
93-97: Document the metaErp-specific widthAdapt property.While the property implementation is correct, the comment "metaErp 新增属性" suggests this is a specific feature. Consider adding JSDoc documentation to clarify:
- The purpose of this property
- When it should be used
- How it affects the component's behavior
- // metaErp 新增属性 + /** + * Enables width adaptation for metaErp integration. + * @description Controls whether the tree menu width should adapt dynamically + */ widthAdapt: { type: Boolean, default: false }examples/sites/vite-dosearch-plugin.js (1)
7-10: Consider using more robust code transformationThe current string replacement approach is fragile and could break if the docsearch library updates its implementation.
Consider using AST transformation instead of string replacement for more reliable code modification. This would make the plugin more maintainable and less likely to break with dependency updates.
examples/sites/vite.config.ts (1)
46-46: Document the purpose of the VITE_BUILD_TARGET environment variableThe
isInnerflag's purpose and its impact on the build process should be documented.Consider adding a comment explaining when and why the 'inner' build target should be used:
+ // VITE_BUILD_TARGET='inner' enables additional search functionality through the docsearch plugin const isInner = env.VITE_BUILD_TARGET === 'inner'packages/renderless/src/anchor/index.ts (1)
101-102: Consider caching DOM query resultsMultiple DOM queries for the same elements could impact performance. Consider caching the results where appropriate.
For example, in the linkClick function:
export const linkClick = (...) => (e: Event, item: IAnchorLinkItem) => { + let cachedLinkEl // ... - const linkEl = getEleMentBySelect(scrollContainer, item.link) as HTMLElement + if (!cachedLinkEl) { + cachedLinkEl = getElementBySelector(scrollContainer, item.link) as HTMLElement + } const top = cachedLinkEl?.getBoundingClientRect().top - ...Also applies to: 240-248
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (20)
examples/sites/index.html(1 hunks)examples/sites/package.json(2 hunks)examples/sites/playground/App.vue(2 hunks)examples/sites/src/App.vue(0 hunks)examples/sites/src/main.js(1 hunks)examples/sites/src/menus.jsx(2 hunks)examples/sites/src/router.js(2 hunks)examples/sites/src/style.css(4 hunks)examples/sites/src/tools/docsearch.js(1 hunks)examples/sites/src/views/components/demo.vue(1 hunks)examples/sites/vite-dosearch-plugin.js(1 hunks)examples/sites/vite.config.ts(3 hunks)packages/renderless/package.json(1 hunks)packages/renderless/src/anchor/index.ts(5 hunks)packages/renderless/src/color-select-panel/vue.ts(1 hunks)packages/vue-common/package.json(1 hunks)packages/vue-common/src/index.ts(1 hunks)packages/vue/src/fluent-editor/package.json(2 hunks)packages/vue/src/tree-menu/package.json(1 hunks)packages/vue/src/tree-menu/src/props.ts(1 hunks)
💤 Files with no reviewable changes (1)
- examples/sites/src/App.vue
✅ Files skipped from review due to trivial changes (6)
- packages/renderless/package.json
- examples/sites/index.html
- packages/vue-common/package.json
- packages/vue/src/tree-menu/package.json
- packages/vue/src/fluent-editor/package.json
- examples/sites/package.json
🧰 Additional context used
🪛 Gitleaks (8.21.2)
examples/sites/src/main.js
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (19)
packages/vue/src/tree-menu/src/props.ts (1)
66-69: LGTM! The showExpand prop follows component conventions.
The new prop follows the established pattern for boolean flags in the component and is appropriately positioned near other visibility-related properties.
examples/sites/src/router.js (2)
3-3: Consider UX implications of removing theme persistence
The removal of storage-related imports ($local, CURRENT_THEME_KEY) indicates that user theme preferences will no longer persist between sessions. This might impact user experience, especially for returning visitors who previously had their preferences saved.
Let's verify if there are any other components still relying on the removed storage functionality:
Consider implementing a different persistence mechanism or documenting the rationale behind removing this feature in the PR description.
37-38:
Reconsider hardcoding Chinese language as default
The redirection logic now defaults to Chinese (ZH_CN_LANG) for all unmatched routes, which might not be ideal for international users. Consider:
- Using browser's language preference (
navigator.language) - Implementing a language detection middleware
- Adding a language selection landing page
Here's a suggested improvement:
- const langPath = LANG_PATH_MAP[ZH_CN_LANG]
- const theme = THEME_ROUTE_MAP[DEFAULT_THEME]
+ const browserLang = navigator.language.toLowerCase()
+ const supportedLang = LANG_PATH_MAP[browserLang] ? browserLang : ZH_CN_LANG
+ const langPath = LANG_PATH_MAP[supportedLang]
+ const theme = THEME_ROUTE_MAP[DEFAULT_THEME]Let's check if there are any language-related configurations or middleware in the codebase:
examples/sites/vite-dosearch-plugin.js (1)
1-18:
Consider adding safety checks and error handling
The plugin modifies third-party code by injecting a global handler, which could lead to runtime errors. Several improvements are recommended:
- Add safety checks for the global handler:
export default function viteDosearchPlugin() {
return {
name: 'vite-dosearch-plugin',
enforce: 'pre',
transform(code, id) {
if (id.includes('@docsearch')) {
const newCode = code.replace(
'Promise((function(t){var n=new XMLHttpRequest;',
- 'Promise((function(t){t=window.handleGlobalSearchData(t);var n=new XMLHttpRequest;'
+ 'Promise((function(t){t=typeof window.handleGlobalSearchData === "function" ? window.handleGlobalSearchData(t) : t;var n=new XMLHttpRequest;'
)
return {
code: newCode,
map: null
}
}
}
}
}- Consider adding a warning if the handler is missing:
export default function viteDosearchPlugin() {
+ let warned = false;
return {
name: 'vite-dosearch-plugin',
enforce: 'pre',
transform(code, id) {
if (id.includes('@docsearch')) {
+ if (!warned) {
+ console.warn('[vite-dosearch-plugin] Ensure window.handleGlobalSearchData is defined before docsearch initialization');
+ warned = true;
+ }
const newCode = code.replace(Let's verify the docsearch dependency and usage:
examples/sites/vite.config.ts (2)
3-3: LGTM: Import statement is correctly placed
The import statement for the local plugin is properly positioned among other plugin imports.
120-121: Verify the plugin order and dependencies
The placement of viteDosearchPlugin at the end of the plugins array needs verification to ensure it doesn't conflict with other plugins.
Let's check for potential plugin conflicts:
✅ Verification successful
Plugin order is safe and well-configured
The viteDosearchPlugin placement is correct and safe because:
- The plugin has
enforce: 'pre'configuration, ensuring it runs before other plugins - It only transforms code that includes '@docsearch' and makes a specific, isolated modification
- It's conditionally loaded only when
isInneris true, which is appropriate for its purpose - The plugin's transformation doesn't interfere with other plugins in the chain as it only modifies docsearch-specific code
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other plugins that might interact with docsearch
rg -l "docsearch" --type ts --type js
# Check for environment variable usage
rg "VITE_BUILD_TARGET" --type ts --type js
Length of output: 620
Script:
#!/bin/bash
# Let's check the content of the docsearch plugin to understand its dependencies
cat examples/sites/vite-dosearch-plugin.js
# Also check the vite config to see the full plugin configuration
cat examples/sites/vite.config.ts
Length of output: 5811
packages/vue-common/src/index.ts (1)
179-179: Ensure consistent handling of reactive globalDesignConfig
The reassignment correctly handles the possibility that globalDesignConfig is a reactive object with a .value property, ensuring it is always an object for subsequent use.
packages/renderless/src/color-select-panel/vue.ts (1)
Line range hint 61-65: Switch to using hooks.onMounted for lifecycle management
Using hooks.onMounted ensures that the lifecycle hook is properly managed within the renderless function, enhancing consistency and flexibility.
examples/sites/src/style.css (5)
7-8: Restore bold text styling after reset.css
Adding font-weight: 700; to b and strong tags ensures that bold text is rendered correctly after including reset.css.
26-27: Set vertical alignment for table cells
Applying vertical-align: middle; to .tiny-grid td and .tiny-grid th aligns cell content centrally, improving table presentation.
40-40: Increase specificity of CSS selector for search container
Adjusting the selector to .DocSearch.DocSearch-Container increases specificity, ensuring the styles are applied correctly without conflicts.
79-81: Adjust padding for search button on mobile devices
Adding padding to .search-box .DocSearch-Button within the media query enhances its appearance on screens up to 768px wide.
47-47:
Fix missing semicolon in CSS declaration
Adding the missing semicolon to the transition property corrects a syntax error, ensuring the CSS is parsed correctly.
Apply this diff to fix the syntax error:
- transition: 0.4s
+ transition: 0.4s;Likely invalid or redundant comment.
examples/sites/src/menus.jsx (1)
29-41: Verify if restricting documentation menus to DEV mode is intentional
The change makes documentation menus only available in development mode. This could impact users trying to access documentation in production environments.
Consider:
- Documenting this behavior change in the README or relevant documentation
- Adding a configuration flag to optionally enable documentation in production if needed
examples/sites/src/main.js (1)
52-55: Verify search functionality in non-open environments
The conditional implementation of doSearchEverySite for non-open environments should be tested to ensure consistent search behavior.
✅ Verification successful
Let me gather more context about the search functionality implementation.
Search functionality implementation is simple and properly scoped
The implementation of doSearchEverySite is straightforward - it sets up a global handler that replaces the OpenTiny URL with the current origin in search results. This is a reasonable approach for non-open environments and doesn't require additional verification as it:
- Only affects URL replacement in search results
- Is properly scoped to non-open environments
- Has a simple, focused implementation without complex dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if doSearchEverySite is properly implemented
rg -A 5 "doSearchEverySite" ./examples/sites/src/tools/
Length of output: 548
Script:
#!/bin/bash
# Check the complete implementation of doSearchEverySite and related constants
rg -A 15 "OPEN_TINY_URL|doSearchEverySite" ./examples/sites/src/
# Check where and how the search functionality is used
ast-grep --pattern 'doSearchEverySite()'
Length of output: 2859
examples/sites/playground/App.vue (3)
26-26: LGTM: Version array updated to include 3.20.
The version array maintains the three latest versions, which is consistent with the PR objective of syncing version 3.20.
87-88: LGTM: Improved readability of design theme imports.
The multi-line formatting enhances code readability while maintaining the same functionality.
Line range hint 313-317: LGTM: Added mobile responsiveness for demo container.
The media query ensures proper content handling on small screens (≤640px) by enabling horizontal scrolling.
examples/sites/src/views/components/demo.vue (1)
322-324: LGTM: Enhanced visual styling of demo container.
The updated styles improve the visual presentation with a white background, border, and rounded corners.
| if (typeof data.content === 'string') { | ||
| data.content = data.content.replaceAll(OPEN_TINY_URL, window.location.origin) | ||
| } |
There was a problem hiding this comment.
Ensure compatibility of String.prototype.replaceAll
The replaceAll method may not be supported in all browsers, especially older ones. To improve compatibility, consider using replace with a global regular expression.
Apply this diff to enhance compatibility:
- data.content = data.content.replaceAll(OPEN_TINY_URL, window.location.origin)
+ data.content = data.content.replace(new RegExp(OPEN_TINY_URL, 'g'), window.location.origin)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (typeof data.content === 'string') { | |
| data.content = data.content.replaceAll(OPEN_TINY_URL, window.location.origin) | |
| } | |
| if (typeof data.content === 'string') { | |
| data.content = data.content.replace(new RegExp(OPEN_TINY_URL, 'g'), window.location.origin) | |
| } |
| docsearch({ | ||
| appId: 'AGPA5UXHMH', | ||
| apiKey: '5fa09fc20270efa61d68e2c2eb0f56df', | ||
| indexName: 'opentiny', | ||
| container: '.search-box', | ||
| debug: false | ||
| }) |
There was a problem hiding this comment.
Critical: Move DocSearch API credentials to environment variables
The DocSearch API credentials are hardcoded in the source code, which poses a security risk. These should be moved to environment variables.
Apply this change:
docsearch({
- appId: 'AGPA5UXHMH',
- apiKey: '5fa09fc20270efa61d68e2c2eb0f56df',
+ appId: import.meta.env.VITE_DOCSEARCH_APP_ID,
+ apiKey: import.meta.env.VITE_DOCSEARCH_API_KEY,
indexName: 'opentiny',
container: '.search-box',
debug: false
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| docsearch({ | |
| appId: 'AGPA5UXHMH', | |
| apiKey: '5fa09fc20270efa61d68e2c2eb0f56df', | |
| indexName: 'opentiny', | |
| container: '.search-box', | |
| debug: false | |
| }) | |
| docsearch({ | |
| appId: import.meta.env.VITE_DOCSEARCH_APP_ID, | |
| apiKey: import.meta.env.VITE_DOCSEARCH_API_KEY, | |
| indexName: 'opentiny', | |
| container: '.search-box', | |
| debug: false | |
| }) |
🧰 Tools
🪛 Gitleaks (8.21.2)
46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| const getEleMentBySelect = (parent, selector) => | ||
| selector?.startsWith('#') ? document.getElementById(selector.slice(1)) : parent.querySelector(selector) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve naming convention and add input validation
The function name getEleMentBySelect has inconsistent capitalization. Also, consider adding input validation for security.
-const getEleMentBySelect = (parent, selector) =>
+const getElementById = (id) => document.getElementById(id)
+const querySelector = (parent, selector) => parent?.querySelector(selector)
+
+const getElementBySelector = (parent, selector) => {
+ if (!selector || typeof selector !== 'string') {
+ return null
+ }
+ return selector.startsWith('#')
+ ? getElementById(selector.slice(1))
+ : querySelector(parent, selector)
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const getEleMentBySelect = (parent, selector) => | |
| selector?.startsWith('#') ? document.getElementById(selector.slice(1)) : parent.querySelector(selector) | |
| const getElementById = (id) => document.getElementById(id) | |
| const querySelector = (parent, selector) => parent?.querySelector(selector) | |
| const getElementBySelector = (parent, selector) => { | |
| if (!selector || typeof selector !== 'string') { | |
| return null | |
| } | |
| return selector.startsWith('#') | |
| ? getElementById(selector.slice(1)) | |
| : querySelector(parent, selector) | |
| } |
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Release Notes
New Features
showExpandandwidthAdaptto tree menu configuration.Bug Fixes
Updates
Documentation
filterAttrsfunction for clarity.Chores