-
Notifications
You must be signed in to change notification settings - Fork 269
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
docs: add algolia search #2597
docs: add algolia search #2597
Conversation
WalkthroughThe changes in this pull request introduce a search functionality to the webpage by adding external CSS and JavaScript resources. This includes a new Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 |
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: 1
🧹 Outside diff range and nitpick comments (2)
examples/sites/src/style.css (2)
31-36
: Consider using CSS Grid or Flexbox for search box positioningUsing absolute positioning with fixed left values might cause layout issues across different screen sizes. Consider using modern layout techniques like CSS Grid or Flexbox for more maintainable positioning.
-.search-box { - position: absolute; - z-index: 1000; - top: 12px; - left: 360px; -} +.header-container { + display: flex; + align-items: center; + gap: 1rem; +} +.search-box { + flex: 0 1 auto; +}
61-81
: Standardize breakpoints for better maintainabilityThe media query breakpoints (1023px, 768px, 430px) seem arbitrary. Consider using standard breakpoints and storing them in CSS variables for better maintainability.
+:root { + --breakpoint-mobile: 480px; + --breakpoint-tablet: 768px; + --breakpoint-desktop: 1024px; +} -@media screen and (max-width: 1023px) { +@media screen and (max-width: var(--breakpoint-desktop)) { /* ... */ } -@media screen and (max-width: 768px) { +@media screen and (max-width: var(--breakpoint-tablet)) { /* ... */ } -@media screen and (max-width: 430px) { +@media screen and (max-width: var(--breakpoint-mobile)) { /* ... */ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
examples/sites/index.html
(1 hunks)examples/sites/src/style.css
(1 hunks)examples/sites/src/views/layout/layout.vue
(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.21.2)
examples/sites/index.html
36-36: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (3)
examples/sites/src/style.css (1)
56-59
: LGTM: CSS variables for theming
Good practice using CSS variables for theming. This makes it easier to maintain consistent styling across the application.
examples/sites/index.html (1)
34-41
:
Security: Protect Algolia API key
The API key is exposed in client-side code. Consider moving it to environment variables and serving it through a backend endpoint.
🧰 Tools
🪛 Gitleaks (8.21.2)
36-36: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
examples/sites/src/views/layout/layout.vue (1)
14-14
: LGTM: Disabled filter as it's replaced by Algolia search
The change aligns with the PR objective of replacing the old search functionality with Algolia search.
@@ -21,12 +21,24 @@ | |||
window.DEPLOY_URL = '' | |||
window.process = { env: {} } | |||
</script> | |||
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@docsearch/css@3"/> |
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.
🛠️ Refactor suggestion
Add Subresource Integrity (SRI) hash for external CSS
For security, add an integrity hash when loading external resources from CDN.
-<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@docsearch/css@3"/>
+<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@docsearch/css@3"
+ integrity="sha384-[HASH]"
+ crossorigin="anonymous"/>
Committable suggestion skipped: line range outside the PR's diff.
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: 2
🧹 Outside diff range and nitpick comments (1)
examples/sites/src/style.css (1)
31-40
: Consider using z-index CSS variables for consistencyThe z-index values should align with the existing CSS variables defined in
:root
. Consider using--docs-float-settings-zindex
(999) or creating a new variable for search-related z-indices..search-box { position: absolute; - z-index: 1000; + z-index: var(--docs-float-settings-zindex); top: 12px; left: 360px; } .DocSearch-Container { - z-index: 1000; + z-index: var(--docs-float-settings-zindex); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
examples/sites/index.html
(1 hunks)examples/sites/src/App.vue
(1 hunks)examples/sites/src/style.css
(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.21.2)
examples/sites/index.html
36-36: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (3)
examples/sites/src/style.css (1)
65-85
: LGTM: Responsive design breakpoints
The media queries appropriately handle search box positioning across different screen sizes (1023px, 768px, 430px).
examples/sites/index.html (2)
24-24
: Add Subresource Integrity (SRI) hash for external CSS
For security, add an integrity hash when loading external resources from CDN.
34-41
:
Security: Protect Algolia API credentials
The API key is exposed in client-side code. While this might be a public search-only API key, it's recommended to:
- Move these credentials to environment variables
- Verify that this is a search-only API key with restricted permissions
🧰 Tools
🪛 Gitleaks (8.21.2)
36-36: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
</head> | ||
|
||
<body class="of-hidden"> | ||
<div id="header"></div> | ||
<app-root></app-root> | ||
<div id="app" class="wp100 hp100 pt60 of-hidden"></div> | ||
<div class="search-box" style="display: none;"></div> | ||
<script src="https://cdn.jsdelivr.net/npm/@docsearch/js@3"></script> |
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.
🛠️ Refactor suggestion
Add SRI hash for external JavaScript
Similarly, add integrity hash for the DocSearch JavaScript file.
-<script src="https://cdn.jsdelivr.net/npm/@docsearch/js@3"></script>
+<script src="https://cdn.jsdelivr.net/npm/@docsearch/js@3"
+ integrity="sha384-[HASH]"
+ crossorigin="anonymous"></script>
Committable suggestion skipped: line range outside the PR's diff.
const searchBox = document.querySelector('.search-box') | ||
if (searchBox) { | ||
searchBox.style.display = 'block' | ||
} |
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.
🛠️ Refactor suggestion
Consider using Vue's reactivity system instead of direct DOM manipulation
Rather than directly manipulating the DOM, consider using Vue's reactivity system:
- Move the search box into the Vue template
- Use v-show for visibility control
Example approach:
// In template
-<div class="search-box" style="display: none;"></div>
+<div class="search-box" v-show="showSearch"></div>
// In setup()
+const showSearch = ref(true)
-const searchBox = document.querySelector('.search-box')
-if (searchBox) {
- searchBox.style.display = 'block'
-}
Committable suggestion skipped: line range outside the PR's diff.
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: 1
🧹 Outside diff range and nitpick comments (12)
examples/sites/demos/pc/webdoc/develop-demo-en.md (2)
Line range hint
7-24
: Consider adding version pinning to package commandsFor better reproducibility and to prevent potential breaking changes, consider specifying versions in the installation commands.
-yarn create vite +yarn create vite@latest
Line range hint
39-48
: Security: Limit exposed environment variablesThe current configuration exposes all environment variables to the client side. Consider explicitly listing only the required environment variables instead of spreading the entire
process.env
.export default defineConfig({ plugins: [vue()], define: { - 'process.env': { ...process.env } + 'process.env': { + TINY_MODE: process.env.TINY_MODE, + // Add other required variables explicitly + } } })examples/sites/demos/pc/webdoc/installation-en.md (1)
Line range hint
83-112
: Sync missing runtime options documentation with English versionThe Chinese documentation includes detailed information about different runtime options (tiny-vue-pc.mjs, tiny-vue-mobile.mjs, etc.) that should also be added to the English version for consistency.
Add the following table to the English version:
| Runtime Name | Description | |--------------------------|-------------------------------------------| | tiny-vue-pc.mjs | Contains all PC template components | | tiny-vue-mobile.mjs | Contains all mobile template components | | tiny-vue-mobile-first.mjs| Contains all multi-end template components | | tiny-vue-simple.mjs | Contains commonly used components |🧰 Tools
🪛 LanguageTool
[style] ~108-~108: Specify a number, remove phrase, or simply use “many” or “numerous”
Context: ...ical Project Upgrade due toTinyVue
of a large number of projects and@opentiny/vue2
or`@opentin...(LARGE_NUMBER_OF)
examples/sites/demos/pc/webdoc/theme.md (2)
Line range hint
80-103
: Enhance the micro frontends documentation with more context.The micro frontends section would benefit from additional context:
- Explain what style isolation means in this context
- Provide examples of common micro frontend frameworks
- Add troubleshooting tips for common issues
Line range hint
32-71
: Add explanatory comments to the theme customization example.The code example would be more helpful with comments explaining each customization option:
themeTool.changeTheme({ name: 'my-app-custom-styles', data: { + // Primary brand color 'tv-base-color-brand': '#1476ff', + // Medium font size for general text 'tv-font-size-md': '12px', + // Large font size for headings 'tv-font-size-lg': '16px', + // Extra large font size for main headings 'tv-font-size-xxl': '20px' }, css: ` .tiny-button { + // Custom button border radius --tv-Button-border-radius: 6px; + // Button base styles min-width: 80px; border:none; padding : 2px 20px; } .tiny-button.tiny-button--primary{ + // Primary button background color background-color: #508de3; } ` })examples/sites/demos/pc/webdoc/theme-en.md (1)
Line range hint
1-15
: Improve English phrasing in the introduction section.The English translation could be improved for better clarity:
-537/5000 -A set of global CSS variables is defined in the TinyVue component library to unify theme styles, such as fonts, colors, spacing, and rounding values, and component-level CSS variables are also defined within each component. +The TinyVue component library defines a set of global CSS variables to unify theme styles (fonts, colors, spacing, border radius) and component-specific CSS variables within each component.examples/sites/demos/pc/webdoc/form-valid-en.md (1)
Line range hint
118-139
: Improve clarity in the trigger section.The trigger section could be clearer:
-Configure the way to trigger the verification rules through `trigger`. When it is `change`, the verification is triggered when the input box value changes. When it is `blur`, -the verification is triggered after the input box value is out of focus. Can be set to an array `['change', 'blur']` to trigger both scenarios. The default is to trigger both scenarios. -If it is only triggered when the verification method is actively called, it can be set to an empty array `[]`. +The `trigger` property configures when validation rules are executed: +- `change`: Validates when the input value changes +- `blur`: Validates when the input loses focus +- `['change', 'blur']`: Validates in both scenarios (default behavior) +- `[]`: Only validates when explicitly called through the validation method🧰 Tools
🪛 LanguageTool
[style] ~121-~121: To form a complete sentence, be sure to include a subject.
Context: ...er the input box value is out of focus. Can be set to an array['change', 'blur']
...(MISSING_IT_THERE)
examples/sites/demos/pc/webdoc/installation.md (5)
Line range hint
3-15
: Consider adding LTS information and version compatibility matrixThe version compatibility information is clear, but consider enhancing it with:
- Long-term support (LTS) status for each major version
- Detailed compatibility matrix including Node.js versions
- End-of-life dates for older versions
Line range hint
33-43
: Enhance Vite configuration exampleThe Vite configuration example could be improved by:
- Adding TypeScript type annotations
- Including comments explaining the purpose of the
define
block- Showing how to properly type the
process.env
objectConsider updating the example:
// vite.config.js -import { defineConfig } from 'vite' +import { defineConfig, UserConfig } from 'vite' import vue from '@vitejs/plugin-vue' -export default defineConfig({ +export default defineConfig((): UserConfig => ({ plugins: [vue()], define: { + // Required for @opentiny/vue environment variables 'process.env': { ...process.env } } -}) +}))
Line range hint
60-82
: Consider adding alternative CDN providersFor better reliability and availability, consider:
- Adding alternative CDN providers (e.g., jsDelivr, CDNJS)
- Including integrity hashes for security
- Adding a note about production usage recommendations
Line range hint
137-190
: Enhance troubleshooting sectionConsider adding:
- A structured troubleshooting guide with common error patterns
- Links to relevant documentation sections
- Environment-specific issues (Node.js, package managers)
- Development tools integration issues (IDE, linters)
CDN URLs need attention and performance optimization guidance is required
Several CDN URLs are returning HTTP 302 redirects which may impact performance:
- All @opentiny/* package URLs return 302 redirects
- Only Vue and ECharts CDN URLs return 200 OK
Additionally, the documentation should include performance best practices:
- Add
async
attribute to non-critical scripts- Use
<link rel="preload">
for critical resources- Add
crossorigin
attribute for CORS requests- Consider using SRI (Subresource Integrity) hashes
- Document browser caching behavior
🔗 Analysis chain
Line range hint
83-136
: Add CDN performance optimization guidanceConsider adding recommendations for:
- Using async/defer attributes for script loading
- Implementing preload/prefetch strategies
- Browser caching considerations
- Production deployment best practices
Also, verify that all CDN URLs are accessible and up-to-date.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify CDN URLs accessibility # Test all CDN URLs mentioned in the documentation urls=( "https://registry.npmmirror.com/vue/3.4.27/files/dist/vue.runtime.esm-browser.js" "https://registry.npmmirror.com/echarts/5.4.1/files/dist/echarts.esm.js" "https://registry.npmmirror.com/@opentiny/vue-runtime/3.17/files/dist3/tiny-vue-pc.mjs" "https://registry.npmmirror.com/@opentiny/vue-runtime/3.17/files/dist3/tiny-vue-icon.mjs" "https://registry.npmmirror.com/@opentiny/vue-runtime/3.17/files/dist3/tiny-vue-locale.mjs" "https://registry.npmmirror.com/@opentiny/vue-runtime/3.17/files/dist3/tiny-vue-common.mjs" "https://registry.npmmirror.com/@opentiny/vue-theme/3.17/files/index.css" ) for url in "${urls[@]}"; do echo "Checking $url" curl -s -I "$url" | head -n 1 doneLength of output: 2966
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (18)
examples/sites/demos/pc/webdoc/develop-demo-en.md
(2 hunks)examples/sites/demos/pc/webdoc/develop-demo.md
(2 hunks)examples/sites/demos/pc/webdoc/envpreparation-open-en.md
(1 hunks)examples/sites/demos/pc/webdoc/envpreparation-open.md
(1 hunks)examples/sites/demos/pc/webdoc/faq-en.md
(1 hunks)examples/sites/demos/pc/webdoc/faq.md
(1 hunks)examples/sites/demos/pc/webdoc/form-valid-en.md
(15 hunks)examples/sites/demos/pc/webdoc/form-valid.md
(15 hunks)examples/sites/demos/pc/webdoc/i18n-en.md
(2 hunks)examples/sites/demos/pc/webdoc/i18n.md
(4 hunks)examples/sites/demos/pc/webdoc/import-components-en.md
(4 hunks)examples/sites/demos/pc/webdoc/import-components.md
(6 hunks)examples/sites/demos/pc/webdoc/installation-en.md
(4 hunks)examples/sites/demos/pc/webdoc/installation.md
(5 hunks)examples/sites/demos/pc/webdoc/theme-en.md
(3 hunks)examples/sites/demos/pc/webdoc/theme.md
(10 hunks)examples/sites/src/views/components/components.vue
(1 hunks)examples/sites/src/views/components/demo.vue
(1 hunks)
✅ Files skipped from review due to trivial changes (10)
- examples/sites/demos/pc/webdoc/develop-demo.md
- examples/sites/demos/pc/webdoc/faq-en.md
- examples/sites/demos/pc/webdoc/i18n.md
- examples/sites/demos/pc/webdoc/form-valid.md
- examples/sites/demos/pc/webdoc/envpreparation-open.md
- examples/sites/demos/pc/webdoc/i18n-en.md
- examples/sites/demos/pc/webdoc/envpreparation-open-en.md
- examples/sites/demos/pc/webdoc/import-components-en.md
- examples/sites/demos/pc/webdoc/faq.md
- examples/sites/demos/pc/webdoc/import-components.md
🧰 Additional context used
🪛 LanguageTool
examples/sites/demos/pc/webdoc/form-valid-en.md
[grammar] ~6-~6: A word may be missing after ‘the’.
Context: ...nction andrules
Configuration rules of the. ## API ### validate When submitting ...
(THE_SENT_END)
[uncategorized] ~74-~74: Loose punctuation mark.
Context: ...at is, non-empty verification. - true
: Set totrue
, indicates that the field ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~74-~74: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...`, indicates that the field is mandatory and the label is marked with a red asterisk...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~76-~76: Loose punctuation mark.
Context: ...arked with a red asterisk *. - false
: default value, indicating that the fiel...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (4)
examples/sites/src/views/components/demo.vue (1)
6-6
: Semantic HTML improvement: Using h2 for demo title
Good change - using semantic h2 element instead of div improves accessibility and document structure while maintaining existing styling.
examples/sites/src/views/components/components.vue (2)
149-151
: Semantic HTML improvement: Using h2 for API group name
Good change - consistently using semantic h2 element improves document structure and accessibility. Maintains existing functionality and styling.
158-160
: Semantic HTML improvement: Using h3 for API key
Good change - using h3 element creates proper heading hierarchy under the h2 API group name. Maintains styling while improving document structure.
examples/sites/demos/pc/webdoc/installation-en.md (1)
Line range hint 60-85
: Security: Add Subresource Integrity (SRI) hashes for CDN resources
When loading resources from CDN, it's recommended to include integrity hashes to prevent malicious code injection through compromised CDN servers.
Example with SRI:
-<link rel="stylesheet" href="https://unpkg.com/@opentiny/vue-theme/index.css" />
+<link rel="stylesheet"
+ href="https://unpkg.com/@opentiny/vue-theme/index.css"
+ integrity="sha384-[actual-hash]"
+ crossorigin="anonymous" />
🧰 Tools
🪛 LanguageTool
[style] ~61-~61: Consider a shorter alternative to avoid wordiness.
Context: ...e>.
(IN_ORDER_TO_PREMIUM)
# Form Verification Configuration | ||
|
||
<br> | ||
<br> | ||
|
||
Form validation is based on`async-validator`Encapsulated, mainly used`validate`Function and`rules`Configuration rules of the. | ||
|
||
### API | ||
## API |
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.
🛠️ Refactor suggestion
Fix grammar issues in the introduction.
The introduction needs grammatical improvements:
-Form validation is based on`async-validator`Encapsulated, mainly used`validate`Function and`rules`Configuration rules of the.
+Form validation is encapsulated based on `async-validator`, primarily using the `validate` function and `rules` configuration.
📝 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.
# Form Verification Configuration | |
<br> | |
<br> | |
Form validation is based on`async-validator`Encapsulated, mainly used`validate`Function and`rules`Configuration rules of the. | |
### API | |
## API | |
# Form Verification Configuration | |
<br> | |
<br> | |
Form validation is encapsulated based on `async-validator`, primarily using the `validate` function and `rules` configuration. | |
## API |
🧰 Tools
🪛 LanguageTool
[grammar] ~6-~6: A word may be missing after ‘the’.
Context: ...nction andrules
Configuration rules of the. ## API ### validate When submitting ...
(THE_SENT_END)
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
Summary by CodeRabbit
New Features
Bug Fixes
Documentation