-
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(fluent-editor): add formula demo #2516
Conversation
WalkthroughThis pull request introduces two new Vue components, 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: 2
🧹 Outside diff range and nitpick comments (8)
packages/vue/src/fluent-editor/package.json (1)
MathLive dependency should be added to @opentiny/vue-fluent-editor
The verification shows that:
- MathLive is actively used in the fluent-editor component for formula editing functionality
- It's currently only listed as a dependency in the examples/sites/package.json
- The @opentiny/fluent-editor package (3.22.0) doesn't include mathlive in its dependencies
The formula editing feature requires mathlive to work correctly, as evidenced by:
- Direct imports of 'mathlive' and its CSS files in the formula editor component
- Usage of mathlive module configuration in the editor options
- Integration with mathlive for formula rendering and editing
🔗 Analysis chain
Line range hint
13-20
: Query: MathLive dependency appears to be missingThe AI summary mentions MathLive library integration, but it's not listed as a dependency. Should
mathlive
be added as a dependency for the formula editing feature to work correctly?
Let me gather more information about the formula editing feature and its dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if mathlive is a dependency of @opentiny/fluent-editor # or if it needs to be explicitly added # Check if mathlive is already included in fluent-editor's dependencies curl -s https://registry.npmjs.org/@opentiny/fluent-editor/3.22.0 | jq '.dependencies' # Search for mathlive usage in the codebase rg -l 'mathlive'Length of output: 549
Script:
#!/bin/bash # Check the content of formula-related files to understand the implementation rg -A 5 "mathlive" examples/sites/demos/pc/app/fluent-editor/options-formula.vue rg -A 5 "mathlive" examples/sites/demos/pc/app/fluent-editor/webdoc/fluent-editor.js # Check if mathlive is listed in any package.json files rg -l "mathlive" "**/package.json" # Check the content of the example package.json that contains mathlive cat examples/sites/package.json | jq '.dependencies'Length of output: 2014
examples/sites/demos/pc/app/fluent-editor/options-formula-composition-api.vue (2)
1-7
: Consider internationalizing the text and improving content display
- The hardcoded Chinese text "内容:" should be internationalized for better maintainability and accessibility.
- Consider formatting the displayed content for better readability in the demo.
<template> <div> <tiny-fluent-editor v-model="content" :options="options" ref="editorRef"></tiny-fluent-editor> - 内容:<br /> - {{ content }} + {{ t('content') }}:<br /> + <pre>{{ JSON.stringify(JSON.parse(content), null, 2) }}</pre> </div> </template>
18-29
: Improve code organization and translate comments
- Move the initial content to a named constant for better maintainability.
- Translate Chinese comments to English.
+const INITIAL_FORMULA = { + ops: [ + { + insert: { + mathlive: { + value: '\\sum_{i=1}^{n} i^2', + mode: 'dialog' + } + } + }, + { insert: '\n' } + ] +} const editorRef = ref() -const content = ref('{"ops":[{"insert":{"mathlive":{"value":"\\sum_{i=1}^{n} i^2","mode":"dialog"}}},{"insert":"\n"}]}') +const content = ref(JSON.stringify(INITIAL_FORMULA)) const options = ref({ modules: { - // 工具栏 + // Toolbar configuration toolbar: ['formula'], - // 可编辑公式 + // Enable formula editing mathlive: true } })examples/sites/demos/pc/app/fluent-editor/options-formula.vue (3)
1-7
: Consider implementing internationalization for the text labels.The hardcoded Chinese text "内容:" should be internationalized to support multiple languages and maintain consistency with the project's documentation standards.
- 内容:<br /> + {{ t('content') }}<br />
12-13
: Translate Chinese comments to English for consistency.Keep documentation and comments in English to maintain consistency across the codebase.
-// 引入 mathlive 模块和样式 +// Import mathlive module and styles
34-38
: Consider using created() hook and component props for initialization.Direct manipulation of the quill instance in mounted() could be fragile. Consider:
- Moving the initialization to created() hook
- Using component props for initial content
- Adding error handling for the quill instance access
export default { + props: { + initialFormula: { + type: Object, + default: () => INITIAL_FORMULA + } + }, mounted() { - this.$refs.editorRef.state.quill.setContents({ - 'ops': [{ 'insert': { 'mathlive': { 'value': '\\sum_{i=1}^{n} i^2', 'mode': 'dialog' } } }, { 'insert': '\n' }] - }) + try { + this.$refs.editorRef?.state.quill?.setContents(this.initialFormula) + } catch (error) { + console.error('Failed to initialize editor content:', error) + } } }examples/sites/demos/pc/app/fluent-editor/webdoc/fluent-editor.js (1)
54-65
: Consider adding English description for consistency.The demo entry is well-structured and provides clear documentation in Chinese. However, for consistency with other demos like 'basic-usage' that have English descriptions, consider adding an English description.
desc: { 'zh-CN': '通过设置 <code>options</code> 中的 <code>mathlive</code> 为 true 开启可编辑公式功能。', - 'en-US': '' + 'en-US': 'Enable editable formula functionality by setting <code>mathlive</code> to true in the <code>options</code> configuration.' },examples/sites/package.json (1)
44-44
: Document the dependency purpose.Consider adding a comment in package.json to document that mathlive is required for the fluent editor's formula editing feature. This helps future maintainers understand why this dependency exists.
"@vueuse/head": "0.7.13", "github-markdown-css": "^5.1.0", "highlight.js": "^11.5.1", "marked": "^4.3.0", - "mathlive": "^0.101.1", + // Required for fluent editor's formula editing feature + "mathlive": "^0.101.1", "prismjs": "^1.28.0",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
examples/sites/demos/pc/app/fluent-editor/options-formula-composition-api.vue
(1 hunks)examples/sites/demos/pc/app/fluent-editor/options-formula.vue
(1 hunks)examples/sites/demos/pc/app/fluent-editor/webdoc/fluent-editor.js
(1 hunks)examples/sites/package.json
(1 hunks)packages/vue/src/fluent-editor/package.json
(1 hunks)
🔇 Additional comments (6)
examples/sites/demos/pc/app/fluent-editor/options-formula-composition-api.vue (2)
1-36
: Verify formula feature demonstration completeness
As this is a demo component, we should ensure it adequately demonstrates the key features of the formula editing capability:
- Formula insertion
- Formula editing
- Different formula modes (if available)
- Error handling
#!/bin/bash
# Description: Check for comprehensive formula feature documentation
# Check if there are other formula-related demos or documentation
rg -g '*.{vue,md}' -l 'mathlive|formula' examples/sites/demos/pc/app/fluent-editor/
# Check for formula-related tests
rg -g '*test*.{js,ts,vue}' -l 'mathlive|formula'
9-17
: Translate comments and verify mathlive compatibility
- The Chinese comment should be in English for consistency.
- We should verify the mathlive version compatibility with the project.
examples/sites/demos/pc/app/fluent-editor/options-formula.vue (2)
13-15
: Verify mathlive dependency declaration.
The component imports mathlive and its CSS files, but we should ensure these dependencies are properly declared in package.json.
✅ Verification successful
Dependency mathlive
is properly declared in package.json
The verification confirms that mathlive
is correctly declared in examples/sites/package.json
with version constraint ^0.101.1
, which aligns with the imports used in the component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if mathlive is declared in package.json
rg -l "mathlive" | grep -i "package.json"
# Check the version constraints
rg '"mathlive":\s*"[^"]*"' -g 'package.json'
Length of output: 168
21-33
: 🛠️ Refactor suggestion
Extract duplicated formula content into a constant.
The formula content is duplicated between data() and mounted(). Consider extracting it into a constant to maintain DRY principles and improve maintainability.
+const INITIAL_FORMULA = {
+ ops: [
+ {
+ insert: {
+ mathlive: {
+ value: '\\sum_{i=1}^{n} i^2',
+ mode: 'dialog'
+ }
+ }
+ },
+ { insert: '\n' }
+ ]
+}
export default {
data() {
return {
- content: '{"ops":[{"insert":{"mathlive":{"value":"\\sum_{i=1}^{n} i^2","mode":"dialog"}}},{"insert":"\n"}]}',
+ content: JSON.stringify(INITIAL_FORMULA),
options: {
// ...
}
}
}
}
Likely invalid or redundant comment.
examples/sites/demos/pc/app/fluent-editor/webdoc/fluent-editor.js (1)
64-64
: Verify the referenced Vue file exists.
The demo references 'options-formula.vue'. Let's verify this file exists in the correct location.
✅ Verification successful
File exists and contains the required component ✓
The verification confirms that:
- The file
options-formula.vue
exists at the correct location:examples/sites/demos/pc/app/fluent-editor/options-formula.vue
- The file contains the
TinyFluentEditor
component as expected
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the referenced Vue file exists in the demos directory
# Expected: File should exist in the same directory as other demo files
# Find the demo file
fd --type f "options-formula.vue" examples/sites/demos/pc/app/fluent-editor
# If found, verify it contains TinyFluentEditor component
if fd --quiet "options-formula.vue" examples/sites/demos/pc/app/fluent-editor; then
rg -l "TinyFluentEditor" $(fd --type f "options-formula.vue" examples/sites/demos/pc/app/fluent-editor)
fi
Length of output: 438
examples/sites/package.json (1)
44-44
: Verify mathlive version and security.
The addition of mathlive is appropriate for the formula editing feature. However, let's verify the version choice.
✅ Verification successful
✓ The mathlive version is up-to-date and secure
The version 0.101.1 specified in package.json is currently the latest version available on npm, and no security vulnerabilities were found for this package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest version and security advisories for mathlive
# Get latest version from npm
echo "Latest version from npm:"
curl -s https://registry.npmjs.org/mathlive/latest | jq '.version'
# Check for known vulnerabilities
echo "Checking for vulnerabilities:"
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "mathlive") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 546
@@ -11,7 +11,7 @@ | |||
"//postversion": "pnpm build" | |||
}, | |||
"dependencies": { | |||
"@opentiny/fluent-editor": "~3.20.0", | |||
"@opentiny/fluent-editor": "~3.22.0", |
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.
💡 Codebase verification
Version bump is compatible but MathLive dependency is missing
The version bump to ~3.22.0 is compatible, but there's an implementation concern:
- The formula feature requires MathLive (v0.101.1) as shown in the demo files, but it's not declared as a dependency in the component's package.json
- The feature is actively used in the demo files with imports from 'mathlive'
- This dependency should be explicitly declared in packages/vue/src/fluent-editor/package.json to ensure proper installation
🔗 Analysis chain
Verify compatibility with the new fluent-editor version
The version bump from ~3.20.0 to ~3.22.0 appears to be related to adding formula support. Let's verify this version contains the required features.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the changelog or release notes for version 3.22.0 to confirm formula support
# and verify no breaking changes were introduced
# Search for changelog files
fd -t f -e md -e txt -i "changelog|release|history"
# Check package.json in fluent-editor dependency for peer dependencies
curl -s https://registry.npmjs.org/@opentiny/fluent-editor/3.22.0 | jq '.peerDependencies'
Length of output: 549
Script:
#!/bin/bash
# Let's check the actual usage of mathlive in the formula-related files
rg -A 5 "mathlive" examples/sites/demos/pc/app/fluent-editor/options-formula.vue
rg -A 5 "mathlive" examples/sites/demos/pc/app/fluent-editor/options-formula-composition-api.vue
# Check if there are any formula-related features in the fluent-editor component
rg -l "formula" packages/vue/src/fluent-editor/
# Check the package.json in examples to see how mathlive is declared
cat examples/sites/package.json | jq '.dependencies["mathlive"]'
Length of output: 1436
onMounted(() => { | ||
editorRef.value.state.quill.setContents({ | ||
'ops': [{ 'insert': { 'mathlive': { 'value': '\\sum_{i=1}^{n} i^2', 'mode': 'dialog' } } }, { 'insert': '\n' }] | ||
}) | ||
}) |
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
Remove redundant content initialization
The initial content is already set via v-model binding. The manual initialization in onMounted
is redundant and could lead to race conditions.
-onMounted(() => {
- editorRef.value.state.quill.setContents({
- 'ops': [{ 'insert': { 'mathlive': { 'value': '\\sum_{i=1}^{n} i^2', 'mode': 'dialog' } } }, { 'insert': '\n' }]
- })
-})
Additionally, if you need to programmatically update the content, consider exposing a proper method through the TinyFluentEditor component instead of accessing the Quill instance directly.
📝 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.
onMounted(() => { | |
editorRef.value.state.quill.setContents({ | |
'ops': [{ 'insert': { 'mathlive': { 'value': '\\sum_{i=1}^{n} i^2', 'mode': 'dialog' } } }, { 'insert': '\n' }] | |
}) | |
}) |
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
New Features
Bug Fixes
Documentation