Skip to content
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

feat(fluent-editor): add before-editor-init props #2670

Merged
merged 4 commits into from
Dec 18, 2024

Conversation

kagol
Copy link
Member

@kagol kagol commented Dec 18, 2024

PR

FluentEditor 基于 Quill,沿用 Quill 的格式和模块扩展逻辑,通过 FluentEditor.register() 注册自定义格式和模块,该方法需要在编辑器初始化 new FluentEditor() 之前执行。

目前 @opentiny/vue-fluent-editor 并未对外暴露 FluentEditor 类,因此设计 before-editor-init 这个 props,通过该钩子函数暴露 FluentEditor 类,该钩子会在 FluentEditor 初始化之前执行,实现新模块的注册。

效果如下:

image

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced a new optional callback beforeEditorInit to enhance editor initialization.
    • Added beforeEditorInit property to the FluentEditor component for improved configuration options.
    • New Vue components created to demonstrate the beforeEditorInit functionality with custom formatting options.
  • Bug Fixes

    • Improved handling of text input, particularly for Chinese characters.
    • Streamlined multi-upload options and clarified handling of rejected files during uploads.
  • Enhancements

    • Refined logic for inserting tables and fullscreen functionality.
    • Updated image upload processing for better asynchronous handling.

Copy link

coderabbitai bot commented Dec 18, 2024

Walkthrough

The pull request introduces an optional beforeEditorInit callback to the Fluent Editor component across multiple files. This feature allows developers to perform additional processing before the editor initializes. Changes are made to the renderless implementation, Vue component props for mobile and PC versions, and the core initialization logic. The modifications enhance the editor's configuration flexibility by providing a hook for pre-initialization customization.

Changes

File Change Summary
packages/renderless/src/fluent-editor/index.ts - Added optional beforeEditorInit callback to init function
- Refined handling of multi-upload options
- Updated composition event and file upload handling
packages/vue/src/fluent-editor/src/index.ts - Added beforeEditorInit prop with default empty function
packages/vue/src/fluent-editor/src/mobile-first.vue - Added beforeEditorInit prop to component
packages/vue/src/fluent-editor/src/pc.vue - Added beforeEditorInit prop to component
examples/sites/demos/apis/fluent-editor.js - Added before-editor-init prop to fluent-editor component
- Updated options and zIndex default values from double to single quotes
examples/sites/demos/pc/app/fluent-editor/before-editor-init-composition-api.vue - Added new component with beforeEditorInit method and custom formatting styles
examples/sites/demos/pc/app/fluent-editor/before-editor-init.vue - Added new component with content data property
examples/sites/demos/pc/app/fluent-editor/webdoc/fluent-editor.js - Added new demo entry for before-editor-init functionality

Possibly related PRs

  • feat(fluent-editor): export default toolbar #2350: The addition of the beforeEditorInit callback in both the main PR and this PR indicates a focus on enhancing the initialization process of the Fluent Editor, suggesting a direct relationship in functionality.
  • feat(fluent-editor): add format painter #2149: This PR adds a new feature to the toolbar, which may relate to the overall functionality improvements in the editor's initialization and handling of uploads in the main PR.
  • docs(fluent-editor): add formula demo #2516: The introduction of a formula demo in this PR complements the enhancements made in the main PR regarding the editor's capabilities, particularly in handling complex content like mathematical expressions.
  • docs(form): [form] optimize demo and docs #2401: While primarily focused on form documentation, the changes in this PR may relate to the overall improvements in user interaction and clarity in the editor's functionality as described in the main PR.

Suggested reviewers

  • zzcr

Poem

🐰 A rabbit's ode to editor's might,
Hooks before init, a coder's delight!
Flexibility springs, like carrots so sweet,
Customization now feels wonderfully neat.
Code hops with joy, new features take flight! 🚀


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the enhancement New feature or request (功能增强) label Dec 18, 2024
Copy link

[e2e-test-warn]
The component to be tested is missing.

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

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (3)
packages/renderless/src/fluent-editor/index.ts (3)

209-210: Internationalization logic for Chinese character detection.

The REGEX approach for checking Chinese characters is useful, but keep in mind broader multilingual scenarios. If more languages must be handled, consider an extensible solution or a more general approach for a wide range of character sets.


348-353: Assess explicit handling of the multi-upload flag vs. equality check.

Using “UploaderDfls.enableMultiUpload === true || UploaderDfls.enableMultiUpload.file” is concise, but consider explicitly handling or documenting each condition to avoid confusion over “true” vs. object properties. Also, verifying “hasRejectedFile” is a good practice for user-facing feedback.


531-531: Overwriting “result.data”?

This line replaces the previously defined “result.data” with a new object containing the files array. If there are other properties in “result.data,” those get overridden. Confirm that’s the intended behavior, or consider merging properties if preserving existing fields is necessary.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 926fb6e and 68d2adc.

📒 Files selected for processing (4)
  • packages/renderless/src/fluent-editor/index.ts (8 hunks)
  • packages/vue/src/fluent-editor/src/index.ts (1 hunks)
  • packages/vue/src/fluent-editor/src/mobile-first.vue (1 hunks)
  • packages/vue/src/fluent-editor/src/pc.vue (1 hunks)
🔇 Additional comments (8)
packages/renderless/src/fluent-editor/index.ts (5)

52-53: Hook invocation for advanced configuration.

Calling the optional "beforeEditorInit" callback allows users to register additional modules or perform setup tasks before the editor is instantiated. This approach is flexible and well-structured. However, consider documenting how the callback is expected to behave and what parameters developers can safely modify.


199-200: Potential performance caution with container.innerHTML retrieval.

Accessing large HTML strings can be expensive. If there's a scenario where users paste extensive content, consider verifying that repeatedly reading container.innerHTML does not cause performance bottlenecks.


303-304: Multi-upload feature condition checks.

The condition ensures that multiple upload is enabled for both files and images. This logic is clear, but confirm that fallback scenarios (e.g., “enableMultiUpload” is false) are adequately handled, especially if partial multi-upload is allowed (e.g., images only, but not files).


495-499: Consistent multi-image upload logic.

The “imageEnableMultiUpload” flag enables multiple image uploads, which is consistent with the file upload approach. Ensure a uniform error handling flow for rejected images to maintain clarity in how the system responds if any file fails validation.


868-868: Double-click detection on blot-formatter overlay.

Filtering by classList and data-image ensures the correct target is processed for image preview. Just confirm that there are no referencing issues if the event target changes (e.g., if nested elements hold the data attribute).

packages/vue/src/fluent-editor/src/index.ts (1)

68-71: Added beforeEditorInit property.

Defining this prop as a function allows for a clean extension point before editor creation. The default no-op function is a sensible fallback, but ensure the documentation clarifies that the function has full access to the FluentEditor class and how it might affect initialization.

packages/vue/src/fluent-editor/src/mobile-first.vue (1)

105-106: Prop passing for beforeEditorInit.

Exposing “beforeEditorInit” aligns with the PC version, ensuring consistent usage across platforms. Users relying on the mobile-first editor can now also implement pre-initialization logic.

packages/vue/src/fluent-editor/src/pc.vue (1)

106-107: Consistent hooking for PC.

The addition of “beforeEditorInit” brings feature parity with the mobile version. This helps unify the editor configuration approach. No issues discovered.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (8)
examples/sites/demos/apis/fluent-editor.js (1)

8-17: Enhance documentation for before-editor-init prop

While the documentation is generally good, consider adding:

  1. A note about when exactly this hook is called in the initialization lifecycle
  2. An example signature showing the FluentEditor type/interface
  {
    name: 'before-editor-init',
    type: '(FluentEditor) => void',
    defaultValue: '',
    desc: {
-     'zh-CN': 'FluentEditor 初始化之前执行的钩子,用于注册自定义 FluentEditor 模块和格式。',
+     'zh-CN': 'FluentEditor 初始化之前执行的钩子,用于注册自定义 FluentEditor 模块和格式。在调用 new FluentEditor() 之前执行。示例:(FluentEditor: typeof Quill) => void',
      'en-US': ''
    },
    pcDemo: 'before-editor-init'
  }
examples/sites/demos/pc/app/fluent-editor/webdoc/fluent-editor.js (1)

68-78: Add missing English translations

The demo documentation is well-structured but missing English translations for name and description.

  name: {
    'zh-CN': '初始化前的钩子',
-   'en-US': ''
+   'en-US': 'Pre-initialization Hook'
  },
  desc: {
    'zh-CN': '<p>通过 <code>before-editor-init</code> 设置 FluentEditor 初始化前的钩子函数,主要用于注册 FluentEditor 自定义格式和模块。<br>这个示例增加了两个新的格式:good / bad,并在工具栏增加了对应的图标用于设置这两种格式。<br>选中一段文本,点击点赞图标,会将文本色设置成绿色;点击点踩图标,会将文本色设置成红色。</p>',
-   'en-US': ''
+   'en-US': '<p>Use <code>before-editor-init</code> to set a hook function that executes before FluentEditor initialization, primarily for registering custom formats and modules.<br>This example adds two new formats: good / bad, with corresponding toolbar icons.<br>When text is selected, clicking the thumbs-up icon sets the text color to green; clicking thumbs-down sets it to red.</p>'
  }
examples/sites/demos/pc/app/fluent-editor/before-editor-init-composition-api.vue (6)

9-10: Consider formatting the displayed content for better readability

The raw content display could be improved by formatting the JSON string or adding proper styling.

-    内容:<br />
-    {{ content }}
+    内容:<br />
+    <pre>{{ JSON.stringify(JSON.parse(content), null, 2) }}</pre>

23-24: Consider moving SVG icons to separate files

Large SVG strings make the code harder to read and maintain. Consider moving them to separate .svg files and importing them.

+// icons/good-icon.svg
+// icons/bad-icon.svg
+import goodIcon from './icons/good-icon.svg'
+import badIcon from './icons/bad-icon.svg'

-const goodIcon = `<svg t="1734490908682"...`
-const badIcon = `<svg t="1734491308472"...`

31-41: Consider defining color values consistently

The color values are defined in the toolbar handlers but not in the style definitions. Consider using a consistent approach by defining colors as constants or moving them to a theme configuration.

+const COLORS = {
+  good: '#5cb300',
+  bad: '#f23030'
+}

 const GoodStyle = new Parchment.StyleAttributor('good', 'color', {
-  scope: Parchment.Scope.INLINE
+  scope: Parchment.Scope.INLINE,
+  defaultValue: COLORS.good
 })

45-46: Consider using English for comments

For better international collaboration, consider using English for comments.

-    // 工具栏
+    // Toolbar configuration

58-69: Add type safety and use consistent color values

The toolbar handlers could benefit from TypeScript type safety and should use the same color values defined earlier.

+interface FluentEditor {
+  format: (format: string, value: string) => void;
+}

 onMounted(() => {
   const fluentEditor = fluentEditorRef.value.state.quill
   const toolbar = fluentEditor.getModule('toolbar')

   toolbar.addHandler('good', function (value) {
-    this.quill.format('good', value ? '#5cb300' : '')
+    this.quill.format('good', value ? COLORS.good : '')
   })

   toolbar.addHandler('bad', function (value) {
-    this.quill.format('bad', value ? '#f23030' : '')
+    this.quill.format('bad', value ? COLORS.bad : '')
   })
 })

1-70: Implementation successfully demonstrates the before-editor-init feature

The example effectively showcases how to use the new before-editor-init prop to extend the editor's functionality by registering custom formats and modules. This aligns perfectly with the PR objectives.

Consider adding error handling for the editor initialization process and documenting the expected timing of the beforeEditorInit callback in relation to the editor's lifecycle.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 68d2adc and 64e150c.

📒 Files selected for processing (4)
  • examples/sites/demos/apis/fluent-editor.js (4 hunks)
  • examples/sites/demos/pc/app/fluent-editor/before-editor-init-composition-api.vue (1 hunks)
  • examples/sites/demos/pc/app/fluent-editor/before-editor-init.vue (1 hunks)
  • examples/sites/demos/pc/app/fluent-editor/webdoc/fluent-editor.js (1 hunks)
🔇 Additional comments (2)
examples/sites/demos/apis/fluent-editor.js (1)

77-77: LGTM! Consistent string literal style

The changes to defaultValue formatting maintain consistency with single quotes.

Also applies to: 88-88

examples/sites/demos/pc/app/fluent-editor/before-editor-init-composition-api.vue (1)

14-20: LGTM! Clean setup with proper imports and initialization

The component setup follows Vue Composition API best practices with appropriate imports and reactive references.

Comment on lines 9 to 22
<script>
import { TinyFluentEditor } from '@opentiny/vue'

export default {
components: {
TinyFluentEditor
},
data() {
return {
content: '{"ops":[{"insert":"Hello "},{"attributes":{"bold":true},"insert":"FluentEditor"},{"insert":"!"}]}'
}
}
}
</script>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add beforeEditorInit method implementation

Add the method to demonstrate custom format registration:

  export default {
    components: {
      TinyFluentEditor
    },
    data() {
      return {
        content: '{"ops":[{"insert":"Hello "},{"attributes":{"bold":true},"insert":"FluentEditor"},{"insert":"!"}]}'
      }
    },
+   methods: {
+     beforeEditorInit(FluentEditor) {
+       // Register custom formats
+       FluentEditor.register('formats/good', {
+         className: 'ql-good',
+         style: { color: 'green' }
+       })
+       FluentEditor.register('formats/bad', {
+         className: 'ql-bad',
+         style: { color: 'red' }
+       })
+     }
+   }
  }
📝 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.

Suggested change
<script>
import { TinyFluentEditor } from '@opentiny/vue'
export default {
components: {
TinyFluentEditor
},
data() {
return {
content: '{"ops":[{"insert":"Hello "},{"attributes":{"bold":true},"insert":"FluentEditor"},{"insert":"!"}]}'
}
}
}
</script>
<script>
import { TinyFluentEditor } from '@opentiny/vue'
export default {
components: {
TinyFluentEditor
},
data() {
return {
content: '{"ops":[{"insert":"Hello "},{"attributes":{"bold":true},"insert":"FluentEditor"},{"insert":"!"}]}'
}
},
methods: {
beforeEditorInit(FluentEditor) {
// Register custom formats
FluentEditor.register('formats/good', {
className: 'ql-good',
style: { color: 'green' }
})
FluentEditor.register('formats/bad', {
className: 'ql-bad',
style: { color: 'red' }
})
}
}
}
</script>

Comment on lines 1 to 7
<template>
<div>
<tiny-fluent-editor v-model="content"></tiny-fluent-editor>
内容:<br />
{{ content }}
</div>
</template>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add missing before-editor-init implementation

The example is missing the key feature it's meant to demonstrate. Consider adding the before-editor-init prop with custom format registration:

  <template>
    <div>
-     <tiny-fluent-editor v-model="content"></tiny-fluent-editor>
+     <tiny-fluent-editor
+       v-model="content"
+       :before-editor-init="beforeEditorInit"
+     ></tiny-fluent-editor>
      内容:<br />
      {{ content }}
    </div>
  </template>

Committable suggestion skipped: line range outside the PR's diff.

@@ -5,6 +5,16 @@ export default {
name: 'fluent-editor',
type: 'component',
props: [
{
name: 'before-editor-init',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

新添加的属性,需要标注下在什么版本添加的哈

Copy link

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 64e150c and a4071b1.

📒 Files selected for processing (1)
  • examples/sites/demos/apis/fluent-editor.js (4 hunks)
🔇 Additional comments (1)
examples/sites/demos/apis/fluent-editor.js (1)

8-20: 🛠️ Refactor suggestion

Add English description and fix default value type.

  1. Please add an English description for international developers.
  2. The defaultValue should be a function since the type is (FluentEditor) => void.

Apply this diff:

  {
    name: 'before-editor-init',
    type: '(FluentEditor) => void',
-   defaultValue: '',
+   defaultValue: '() => void',
    meta: {
      stable: '3.21.0'
    },
    desc: {
      'zh-CN': 'FluentEditor 初始化之前执行的钩子,用于注册自定义 FluentEditor 模块和格式。',
-     'en-US': ''
+     'en-US': 'A hook executed before FluentEditor initialization, used to register custom FluentEditor modules and formats.'
    },
    pcDemo: 'before-editor-init'
  }

Likely invalid or redundant comment.

@@ -64,7 +77,7 @@ export default {
{
name: 'options',
type: 'object',
defaultValue: "",
defaultValue: '',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix inconsistent default values for options and zIndex.

The default values should match their respective types:

  • options should be an empty object ({}) as its type is 'object'
  • zIndex should be a number as its type is 'number'

Apply this diff:

  {
    name: 'options',
    type: 'object',
-   defaultValue: '',
+   defaultValue: '{}',
    // ...
  }
  // ...
  {
    name: 'zIndex',
    type: 'number',
-   defaultValue: '',
+   defaultValue: '1000',
    // ...
  }

Also applies to: 91-91

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (2)
examples/sites/demos/pc/app/fluent-editor/before-editor-init.vue (2)

9-10: Consider internationalizing the hardcoded text

The Chinese text "内容:" should be externalized to support internationalization.

-    内容:<br />
+    {{ t('content') }}<br />

23-23: Consider using a more maintainable content format

The initial content is defined as a JSON string, which is hard to read and maintain. Consider using a computed property or a separate constant.

-      content: '{"ops":[{"insert":"Hello "},{"attributes":{"bold":true},"insert":"FluentEditor"},{"insert":"!"}]}',
+      content: {
+        ops: [
+          { insert: 'Hello ' },
+          { attributes: { bold: true }, insert: 'FluentEditor' },
+          { insert: '!' }
+        ]
+      }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a4071b1 and 7eb790c.

📒 Files selected for processing (1)
  • examples/sites/demos/pc/app/fluent-editor/before-editor-init.vue (1 hunks)
🔇 Additional comments (1)
examples/sites/demos/pc/app/fluent-editor/before-editor-init.vue (1)

6-6: Implementation aligns with PR objectives

The before-editor-init prop is correctly implemented and demonstrates the intended functionality of allowing custom format registration before editor initialization.

Comment on lines +62 to +73
mounted() {
const fluentEditor = this.$refs.fluentEditorRef.state.quill
const toolbar = fluentEditor.getModule('toolbar')

toolbar.addHandler('good', function (value) {
this.quill.format('good', value ? '#5cb300' : '')
})

toolbar.addHandler('bad', function (value) {
this.quill.format('bad', value ? '#f23030' : '')
})
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add cleanup and error handling

  1. Add cleanup in the unmounted/beforeDestroy hook
  2. Add error handling for toolbar operations
  3. Consider using a more stable way to access the Quill instance
   mounted() {
+    try {
       const fluentEditor = this.$refs.fluentEditorRef.state.quill
       const toolbar = fluentEditor.getModule('toolbar')
 
       toolbar.addHandler('good', function (value) {
         this.quill.format('good', value ? '#5cb300' : '')
       })
 
       toolbar.addHandler('bad', function (value) {
         this.quill.format('bad', value ? '#f23030' : '')
       })
+    } catch (error) {
+      console.error('Failed to setup toolbar handlers:', error)
+    }
   },
+  beforeDestroy() {
+    try {
+      const toolbar = this.$refs.fluentEditorRef?.state.quill?.getModule('toolbar')
+      if (toolbar) {
+        toolbar.addHandler('good', null)
+        toolbar.addHandler('bad', null)
+      }
+    } catch (error) {
+      console.error('Failed to cleanup toolbar handlers:', error)
+    }
+  }
📝 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.

Suggested change
mounted() {
const fluentEditor = this.$refs.fluentEditorRef.state.quill
const toolbar = fluentEditor.getModule('toolbar')
toolbar.addHandler('good', function (value) {
this.quill.format('good', value ? '#5cb300' : '')
})
toolbar.addHandler('bad', function (value) {
this.quill.format('bad', value ? '#f23030' : '')
})
}
mounted() {
try {
const fluentEditor = this.$refs.fluentEditorRef.state.quill
const toolbar = fluentEditor.getModule('toolbar')
toolbar.addHandler('good', function (value) {
this.quill.format('good', value ? '#5cb300' : '')
})
toolbar.addHandler('bad', function (value) {
this.quill.format('bad', value ? '#f23030' : '')
})
} catch (error) {
console.error('Failed to setup toolbar handlers:', error)
}
},
beforeDestroy() {
try {
const toolbar = this.$refs.fluentEditorRef?.state.quill?.getModule('toolbar')
if (toolbar) {
toolbar.addHandler('good', null)
toolbar.addHandler('bad', null)
}
} catch (error) {
console.error('Failed to cleanup toolbar handlers:', error)
}
}

Comment on lines +41 to +60
beforeEditorInit(FluentEditor) {
const goodIcon = `<svg t="1734490908682" class="icon" viewBox="0 0 1024 1024" version="1.1" xmlns="http://www.w3.org/2000/svg" p-id="5918" width="200" height="200"><path d="M881.066667 394.666667c-21.333333-23.466667-51.2-36.266667-81.066667-36.266667H618.666667v-117.333333c0-44.8-29.866667-85.333333-87.466667-117.333334-17.066667-10.666667-38.4-12.8-57.6-8.533333-19.2 4.266667-36.266667 17.066667-46.933333 34.133333-2.133333 2.133333-2.133333 4.266667-4.266667 6.4l-125.866667 281.6H204.8c-59.733333 0-108.8 46.933333-108.8 106.666667v258.133333c0 57.6 49.066667 106.666667 108.8 106.666667h544c53.333333 0 98.133333-38.4 106.666667-89.6l51.2-337.066667c4.266667-34.133333-6.4-64-25.6-87.466666z m-593.066667 448H204.8c-25.6 0-44.8-19.2-44.8-42.666667v-256c0-23.466667 19.2-42.666667 44.8-42.666667h83.2v341.333334z m554.666667-373.333334L789.333333 806.4c-4.266667 21.333333-21.333333 36.266667-42.666666 36.266667H352V471.466667l130.133333-290.133334c2.133333-4.266667 4.266667-4.266667 6.4-4.266666 2.133333 0 4.266667 0 8.533334 2.133333 25.6 14.933333 55.466667 38.4 55.466666 64v149.333333c0 17.066667 14.933333 32 32 32h213.333334c12.8 0 25.6 4.266667 34.133333 14.933334 8.533333 6.4 12.8 19.2 10.666667 29.866666z" fill="#666666" p-id="5919"></path></svg>`
const badIcon = `<svg t="1734491308472" class="icon" viewBox="0 0 1024 1024" version="1.1" xmlns="http://www.w3.org/2000/svg" p-id="3380" width="200" height="200"><path d="M904.533333 522.666667L853.333333 185.6c-8.533333-51.2-55.466667-89.6-106.666666-89.6H204.8c-59.733333 0-108.8 46.933333-108.8 106.666667v258.133333c0 57.6 49.066667 106.666667 108.8 106.666667h91.733333l125.866667 281.6c2.133333 2.133333 2.133333 4.266667 4.266667 6.4 14.933333 23.466667 38.4 36.266667 64 36.266666 12.8 0 25.6-4.266667 38.4-10.666666 57.6-34.133333 87.466667-72.533333 87.466666-117.333334v-117.333333h183.466667c32 0 59.733333-12.8 81.066667-36.266667 19.2-25.6 29.866667-55.466667 23.466666-87.466666z m-616.533333-21.333334H204.8c-25.6 0-44.8-19.2-44.8-42.666666v-256c0-23.466667 19.2-42.666667 44.8-42.666667h83.2v341.333333zM832 567.466667c-8.533333 8.533333-21.333333 14.933333-34.133333 14.933333h-213.333334c-17.066667 0-32 14.933333-32 32v149.333333c0 25.6-29.866667 49.066667-55.466666 64-4.266667 2.133333-10.666667 2.133333-14.933334-4.266666L352 533.333333V160H746.666667c21.333333 0 40.533333 14.933333 42.666666 36.266667L842.666667 533.333333c2.133333 10.666667-2.133333 23.466667-10.666667 34.133334z" fill="#666666" p-id="3381"></path></svg>`
const icons = FluentEditor.import('ui/icons')
icons.good = goodIcon
icons.bad = badIcon

const Parchment = FluentEditor.import('parchment')

const GoodStyle = new Parchment.StyleAttributor('good', 'color', {
scope: Parchment.Scope.INLINE
})

const BadStyle = new Parchment.StyleAttributor('bad', 'color', {
scope: Parchment.Scope.INLINE
})

FluentEditor.register('formats/good', GoodStyle)
FluentEditor.register('formats/bad', BadStyle)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve maintainability and error handling

  1. The SVG icons should be moved to separate files or a dedicated icons module.
  2. Add error handling for FluentEditor import operations.
   beforeEditorInit(FluentEditor) {
+    try {
+      const { goodIcon, badIcon } = await import('./icons')
       const icons = FluentEditor.import('ui/icons')
       icons.good = goodIcon
       icons.bad = badIcon

       const Parchment = FluentEditor.import('parchment')
       // ... rest of the code
+    } catch (error) {
+      console.error('Failed to initialize editor:', error)
+    }
   }

Committable suggestion skipped: line range outside the PR's diff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request (功能增强)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants