Conversation
WalkthroughThe pull request migrates fluent-editor's i18n system from a custom implementation to the external Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant FE as FluentEditor
participant I18N as quill-i18n Module
participant Modules as Editor Modules
participant UI as UI Components
App->>FE: Initialize with i18n config (locale, messages)
FE->>FE: Merge built-in locales with user messages
FE->>I18N: Configure with merged locale config
I18N-->>FE: Ready (locale set)
FE->>Modules: Pass i18nModule instance
Modules->>Modules: Register locale data via i18nModule.addMessages()
Modules->>I18N: Subscribe to I18N_LOCALE_CHANGE
I18N-->>Modules: Event listener registered
App->>FE: User changes language
FE->>I18N: setLocale(newLang)
I18N->>I18N: Update active locale
I18N-->>Modules: Emit I18N_LOCALE_CHANGE event
Modules->>Modules: Handle locale change event
Modules->>FE: Call getLangText(key) for text resolution
FE->>I18N: Request translated text
I18N-->>FE: Return localized text
FE-->>UI: Update displayed text
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can generate walkthrough in a markdown collapsible section to save space.Enable the |
| // Temporary backward-compat syntax support: `_i18n"key"` | ||
| // TODO: remove this legacy syntax support in a future major version. | ||
| const LEGACY_I18N_KEY_REGEX = /^_i18n(?:"([^"]+)"|'([^']+)'|`([^`]+)`)$/ | ||
|
|
||
| function resolveLegacyI18nKey(name: string): string { | ||
| const key = name.trim() | ||
| const matched = key.match(LEGACY_I18N_KEY_REGEX) | ||
| if (!matched) return name | ||
| return matched[1] || matched[2] || matched[3] || name | ||
| } |
There was a problem hiding this comment.
此处对原先的解析方法做了兼容,后续版本变动再放弃兼容旧的使用方式
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/fluent-editor/src/modules/mind-map/modules/context-menu.ts (2)
59-62:⚠️ Potential issue | 🔴 CriticalBug:
handler.getText('delete')returns undefined.Same issue as
updateContextMenuItems()- should use'deleteContent'to match the key defined inresolveTexts().🐛 Proposed fix
addContextMenuItem(blot, handler.getText('copy'), () => handleCopy(blot)) addContextMenuItem(blot, handler.getText('cut'), () => handleCut(blot)) addContextMenuItem(blot, handler.getText('paste'), () => handlePaste(blot)) - addContextMenuItem(blot, handler.getText('delete'), () => handleDeleteContent(blot)) + addContextMenuItem(blot, handler.getText('deleteContent'), () => handleDeleteContent(blot))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/fluent-editor/src/modules/mind-map/modules/context-menu.ts` around lines 59 - 62, The context menu is calling handler.getText('delete') which returns undefined; change the key to match resolveTexts() by using 'deleteContent' wherever getText is called for the delete action (specifically update the calls in addContextMenuItem for the delete menu item that invokes handleDeleteContent(blot)); ensure consistency with updateContextMenuItems() and resolveTexts() so handler.getText('deleteContent') is used instead of 'delete'.
36-46:⚠️ Potential issue | 🔴 CriticalBug:
this.texts.deleteis undefined; should bethis.texts.deleteContent.The
resolveTexts()method (lines 25-34) definesdeleteContent,deleteNode, anddeleteIcon, butupdateContextMenuItems()referencesthis.texts.deletewhich doesn't exist. This will set the 4th menu item's text toundefined.🐛 Proposed fix
updateContextMenuItems() { if (!this.blot.contextMenu) return const menuItems = this.blot.contextMenu.querySelectorAll('.ql-mind-map-context-menu-item') if (menuItems.length >= 4) { menuItems[0].textContent = this.texts.copy menuItems[1].textContent = this.texts.cut menuItems[2].textContent = this.texts.paste - menuItems[3].textContent = this.texts.delete + menuItems[3].textContent = this.texts.deleteContent } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/fluent-editor/src/modules/mind-map/modules/context-menu.ts` around lines 36 - 46, The fourth context-menu item is being set from a non-existent property this.texts.delete causing undefined text; in updateContextMenuItems() replace the reference to this.texts.delete with the correct property this.texts.deleteContent (the resolved names are defined in resolveTexts()), i.e., set menuItems[3].textContent = this.texts.deleteContent so the "delete" menu entry displays the intended string.packages/fluent-editor/src/modules/table-up/index.ts (1)
28-48:⚠️ Potential issue | 🟠 MajorMissing cleanup for I18N_LOCALE_CHANGE event listener.
The event listener attached in the constructor is never removed. This can cause memory leaks if the module is destroyed while the listener remains attached to the quill instance.
Store the handler reference and remove it in a
destroy()override:🛠️ Proposed fix
constructor(public quill: FluentEditor, options: Partial<any>) { super(quill, options) if (!this.quill.options['format-painter']) this.quill.options['format-painter'] = {} const currentIgnoreFormat = this.quill.options['format-painter'].ignoreFormat || [] this.quill.options['format-painter'].ignoreFormat = Array.from( new Set([ ...currentIgnoreFormat, 'table-up-cell-inner', ]), ) - this.quill.on(I18N_LOCALE_CHANGE, () => { + this.localeChangeHandler = () => { this.refreshUI() const toolbar = this.quill.getModule('toolbar') as Toolbar if (toolbar && (this.quill.theme as QuillTheme).pickers) { const [, select] = (toolbar.controls as [string, HTMLElement][] || []).find(([name]) => name === this.statics.toolName) || [] if (select && select.tagName.toLocaleLowerCase() === 'select') { const picker = (this.quill.theme as QuillTheme).pickers.find(picker => picker.select === select) if (picker) { this.buildCustomSelect(this.options.customSelect, picker) } } } Object.keys(this.modules).forEach((key) => { if (isFunction(this.modules[key].destroy)) { this.modules[key].destroy() } }) this.modules = {} this.initModules() - }) + } + this.quill.on(I18N_LOCALE_CHANGE, this.localeChangeHandler) } + + destroy() { + this.quill.off(I18N_LOCALE_CHANGE, this.localeChangeHandler) + if (super.destroy) { + super.destroy() + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/fluent-editor/src/modules/table-up/index.ts` around lines 28 - 48, The I18N_LOCALE_CHANGE listener registered with this.quill.on in the constructor is never removed, causing leaks; capture the handler (e.g., assign the arrow function to this._i18nHandler and call this.quill.on(I18N_LOCALE_CHANGE, this._i18nHandler)) and then override/detect destroy() (or module teardown) to call this.quill.off(I18N_LOCALE_CHANGE, this._i18nHandler) (or removeListener) before cleaning this.modules; update references to the handler name and ensure it is cleared (this._i18nHandler = null) when removed.
🧹 Nitpick comments (7)
packages/fluent-editor/src/modules/custom-image/preview/preview-modal.ts (1)
113-120: Consider removing document event listeners in the destroy method.The
keydownandwheelevent listeners attached todocumentare not removed in thedestroy()method (lines 248-263). While the global singleton pattern means the instance typically lives for the application lifetime, the presence of adestroy()method suggests cleanup is intended. Store references to the listener functions and callremoveEventListenerindestroy()to prevent potential memory leaks if the modal is ever destroyed and recreated.♻️ Proposed fix to store and remove event listeners
Store listener references as class properties and remove them in destroy:
export class ImagePreviewModal { private modal: HTMLElement | null = null private overlay: HTMLElement | null = null private previewImage: HTMLImageElement | null = null private scaleTooltip: HTMLElement | null = null + private keydownListener: ((e: KeyboardEvent) => void) | null = null + private wheelListener: ((e: WheelEvent) => void) | null = null private currentScale: number = 1// 绑定事件 this.overlay.addEventListener('click', () => this.hide()) - document.addEventListener('keydown', (e) => { + this.keydownListener = (e: KeyboardEvent) => { if (e.key === 'Escape') { this.hide() } - }) + } + document.addEventListener('keydown', this.keydownListener) // 绑定滚轮缩放事件 - document.addEventListener('wheel', e => this.onMouseWheel(e), { passive: false }) + this.wheelListener = (e: WheelEvent) => this.onMouseWheel(e) + document.addEventListener('wheel', this.wheelListener, { passive: false })destroy() { this.hideScaleTooltip() + if (this.keydownListener) { + document.removeEventListener('keydown', this.keydownListener) + this.keydownListener = null + } + if (this.wheelListener) { + document.removeEventListener('wheel', this.wheelListener) + this.wheelListener = null + } if (this.overlay && this.overlay.parentNode) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/fluent-editor/src/modules/custom-image/preview/preview-modal.ts` around lines 113 - 120, Store the document event handlers as class properties (e.g., this.handleKeydown and this.handleWheel) instead of inline arrow functions so they can be removed later; wire this.handleKeydown to call this.hide() when e.key === 'Escape' and wire this.handleWheel to call this.onMouseWheel(e), attach them with document.addEventListener where currently added, and then remove them in destroy() via document.removeEventListener('keydown', this.handleKeydown) and document.removeEventListener('wheel', this.handleWheel, { passive: false }) (or matching options) to ensure proper cleanup.packages/fluent-editor/src/modules/custom-uploader.ts (1)
99-99: Prefer typed narrowing overas anyingetMaxSize/getMultiple.At Line 99 and Line 111,
as anyremoves useful compile-time checks and makes future regressions easier to miss. Keep these maps strongly typed (same pattern asgetAccept) so config handling stays explicit.Typed alternative
- const map = (maxSize || {}) as any + const map: Partial<Record<UploadKind, number>> = + typeof maxSize === 'number' ? {} : (maxSize ?? {}) - const map = (multiple || {}) as any + const map: Partial<Record<UploadKind, boolean>> = + typeof multiple === 'boolean' ? {} : (multiple ?? {})Also applies to: 111-111
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/fluent-editor/src/modules/custom-uploader.ts` at line 99, The use of "as any" on the local map in getMaxSize/getMultiple (e.g., the variable named map) removes type safety; replace the cast with a properly typed map (mirror the pattern used in getAccept) such as a typed Record<string, number|null> for getMaxSize and Record<string, boolean> for getMultiple, or explicitly type the map variable via an interface/generic so assignments and lookups are checked at compile time; update the map declaration in getMaxSize and getMultiple accordingly and adjust any downstream uses to match the new types to preserve strong typing and avoid the as any cast.packages/fluent-editor/src/modules/flow-chart/i18n/index.ts (1)
4-6: LGTM!The migration to
addMessagesAPI aligns withquill-i18n. The separate locale registration is clean.Consider adding type safety by importing and using the
I18ntype fromquill-i18ninstead ofany, if available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/fluent-editor/src/modules/flow-chart/i18n/index.ts` around lines 4 - 6, Replace the loose any type on the registerFlowChartI18N parameter with the quill-i18n I18n type to add compile-time safety: import the I18n (or equivalent) type from 'quill-i18n' and change the function signature registerFlowChartI18N(i18nModule: any) to use that imported I18n type so calls to i18nModule.addMessages('en-US', FLOW_CHART_EN_US) and i18nModule.addMessages('zh-CN', FLOW_CHART_ZH_CN) are type-checked.packages/docs/package.json (1)
30-31: Version pinning inconsistency after major version jump.
quill-toolbar-tipjumped from^0.0.13to1.1.0— a significant version change. Additionally, this dependency is now pinned without a caret prefix, unlikequill-table-up(^3.4.0), which may prevent patch updates from being applied.Consider using
^1.1.0for consistency unless the pinned version is intentional to avoid potential breaking changes in future releases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/docs/package.json` around lines 30 - 31, The package.json dependency for quill-toolbar-tip is pinned as "1.1.0" while quill-table-up uses a caret ("^3.4.0"), creating an inconsistency after the major jump; update the dependency entry for quill-toolbar-tip to use a caret (e.g., "^1.1.0") in packages/docs/package.json (adjust the "quill-toolbar-tip" value) unless the pin was intentional, to allow patch/minor updates consistently with "quill-table-up".packages/fluent-editor/src/modules/mind-map/i18n/index.ts (1)
4-6: Consider adding type annotation for better maintainability.The function works correctly with the new
addMessages()API. However, usinganytype fori18nModulereduces type safety.💡 Suggested type improvement
+import type { I18n } from 'quill-i18n' + import { MIND_MAP_EN_US } from './en-us' import { MIND_MAP_ZH_CN } from './zh-cn' -export function registerMindMapI18N(i18nModule: any) { +export function registerMindMapI18N(i18nModule: I18n) { i18nModule.addMessages('en-US', MIND_MAP_EN_US) i18nModule.addMessages('zh-CN', MIND_MAP_ZH_CN) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/fluent-editor/src/modules/mind-map/i18n/index.ts` around lines 4 - 6, The parameter i18nModule in registerMindMapI18N is typed as any which reduces type safety; replace any with a proper interface/type that includes the addMessages(locale: string, messages: object) method (or import the existing i18n module type if available) and update the function signature to use that type so calls to i18nModule.addMessages('en-US', MIND_MAP_EN_US) and addMessages('zh-CN', MIND_MAP_ZH_CN) are type-checked.packages/docs/fluent-editor/demos/collaborative-editing.spec.ts (1)
98-105: Redundant if/else branches - both paths are identical.The
if (lv <= 2)andelsebranches execute the same code. This appears to be leftover from a refactor.♻️ Proposed simplification
for (const lv of levels) { await p1.locator('.ql-editor').first().click() await selectAll(p1) - if (lv <= 2) { - await p1.locator('.ql-toolbar .ql-picker.ql-header').click() - await p1.locator('.ql-toolbar').getByRole('button', { name: `Heading ${lv}` }).click() - } - else { - await p1.locator('.ql-toolbar .ql-picker.ql-header').click() - await p1.locator('.ql-toolbar').getByRole('button', { name: `Heading ${lv}` }).click() - } + await p1.locator('.ql-toolbar .ql-picker.ql-header').click() + await p1.locator('.ql-toolbar').getByRole('button', { name: `Heading ${lv}` }).click() await expect.poll(() => headingMatched(p2, lv, 'Title')).toBeTruthy() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/docs/fluent-editor/demos/collaborative-editing.spec.ts` around lines 98 - 105, The if/else in the collaborative-editing spec is redundant: both branches perform identical actions for variable lv. Remove the conditional and replace with a single sequence that clicks the header picker and the corresponding heading button using the existing p1 locators (e.g., p1.locator('.ql-toolbar .ql-picker.ql-header').click() and p1.locator('.ql-toolbar').getByRole('button', { name: `Heading ${lv}` }).click()); keep the lv reference and existing locator calls (p1.locator(...), getByRole(...)) unchanged.packages/fluent-editor/src/modules/flow-chart/modules/context-menu.ts (1)
1-1: ImportI18N_LOCALE_CHANGEconstant for consistency.This file uses the string literal
'i18n-locale-change'on line 18, while other modules (e.g.,mind-map/control-panel.ts,mind-map/context-menu.ts) import and use theI18N_LOCALE_CHANGEconstant fromquill-i18n. Using the constant ensures consistency and prevents breakage if the event name changes.♻️ Proposed fix
-import type { I18n } from 'quill-i18n' +import { type I18n, I18N_LOCALE_CHANGE } from 'quill-i18n'Then update line 18:
- this.quill.on('i18n-locale-change', (event: { locale: string, oldLocale: string }) => { + this.quill.on(I18N_LOCALE_CHANGE, (event: { locale: string, oldLocale: string }) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/fluent-editor/src/modules/flow-chart/modules/context-menu.ts` at line 1, Import the I18N_LOCALE_CHANGE constant from 'quill-i18n' and replace the literal string 'i18n-locale-change' used in this file (in the context-menu event registration/handler around the current line 18) with the I18N_LOCALE_CHANGE symbol so the module uses the shared event constant like other modules (e.g., mind-map/control-panel.ts, mind-map/context-menu.ts).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/docs/fluent-editor/demos/mind-map.spec.ts`:
- Around line 34-38: Rename the variable miniMapCount to mindMapCount for
consistency and then replace the manual immediate count check with Playwright's
auto-retrying assertion: after clicking mindMapButton and waiting if needed, use
the locator('.ql-mind-map-item') with toHaveCount(mindMapCount + 1) instead of
awaiting page.locator(...).count() to avoid the race condition and leverage
Playwright's built-in waiting.
In `@packages/docs/fluent-editor/demos/table-up-shortcut.vue`:
- Around line 86-89: The watch on lang can call editor methods before the editor
is mounted; guard the watcher to only access editor when defined by checking
editor is truthy (or using optional chaining) inside the watch callback and/or
defer registering the watcher until onMounted completes; update the watcher
around the ref lang to verify editor (the editor variable or instance used in
the demo) before calling getModule('i18n') and setLocale so it no longer throws
if editor is undefined.
In `@packages/docs/fluent-editor/docs/demo/i18n.md`:
- Line 9: Update the docs to use the new API parameter name `locale` rather than
`lang`: change the example reference `setLocale(lang)` to `setLocale(locale)`
and search for other occurrences of `lang` in the i18n.md example to rename them
to `locale` so the docs match the breaking API and the migration contract for
`setLocale`.
In `@packages/fluent-editor/src/config/i18n/en-us.ts`:
- Around line 77-80: The i18n bundle is missing mode-specific keys used by the
dynamic lookup (e.g., link.enter-${mode}); update the 'link' object to include
explicit keys for each mode variant referenced at runtime (for example add
'enter-link-formula' and 'enter-link-video' alongside the existing
'enter-link'), mapping them to the appropriate tooltip text (or same text as
'enter-link') so lookups like link.enter-formula and link.enter-video resolve
correctly when using the dynamic lookup.
In `@packages/fluent-editor/src/modules/custom-image/specs/custom-image-spec.ts`:
- Around line 138-140: The code unsafely casts event.target to HTMLElement when
building imageSrc; add a runtime guard like checking "event.target instanceof
HTMLElement" before calling getAttribute. Locate the target/imageSrc logic
(variables named target and imageSrc) and change it to first verify event.target
is an HTMLElement, then read (target.getAttribute('src') ||
target.getAttribute('data-image')) into imageSrc; if the guard fails set
imageSrc to undefined/null or handle accordingly to avoid calling getAttribute
on non-HTMLElements.
In `@packages/fluent-editor/src/modules/custom-uploader.ts`:
- Around line 87-90: getAccept currently checks .length and can return a string
cast to string[] causing validateFile (which calls accept.some) to crash; update
the getAccept logic (function getAccept, variables mimetypes, map, fromKind) to
explicitly narrow to arrays using Array.isArray before returning — if fromKind
is an array return it, otherwise if map.file is an array return map.file, else
return an empty array (string[]). Ensure the function always returns a proper
string[] so callers like validateFile can safely call Array.prototype methods.
In `@packages/fluent-editor/src/modules/link/modules/tooltip.ts`:
- Around line 38-40: The current setTemplate() call inside the
I18N_LOCALE_CHANGE handler causes listen() to run each locale change,
re-attaching editor/root listeners and producing duplicates; to fix, stop
re-binding listeners on locale updates by either removing the listen()
invocation from setTemplate() so setTemplate() only updates UI strings, or make
listen() idempotent by adding a guard (e.g., a boolean like
this._listenersAttached) inside listen() to early-return if already attached;
update references to I18N_LOCALE_CHANGE, setTemplate(), and listen()
accordingly.
- Line 50: Guard against an empty data-mode before composing the i18n key: when
setting data-before-title on this.root, check this.root.dataset.mode (or fall
back to a safe default like 'default' or an empty string) and only call
this.quill.getLangText with a valid key (e.g., build the key using the guarded
mode variable) so you don't produce "link.enter-undefined"; update the
assignment that sets data-before-title to use the guarded mode value and/or a
conditional lookup.
---
Outside diff comments:
In `@packages/fluent-editor/src/modules/mind-map/modules/context-menu.ts`:
- Around line 59-62: The context menu is calling handler.getText('delete') which
returns undefined; change the key to match resolveTexts() by using
'deleteContent' wherever getText is called for the delete action (specifically
update the calls in addContextMenuItem for the delete menu item that invokes
handleDeleteContent(blot)); ensure consistency with updateContextMenuItems() and
resolveTexts() so handler.getText('deleteContent') is used instead of 'delete'.
- Around line 36-46: The fourth context-menu item is being set from a
non-existent property this.texts.delete causing undefined text; in
updateContextMenuItems() replace the reference to this.texts.delete with the
correct property this.texts.deleteContent (the resolved names are defined in
resolveTexts()), i.e., set menuItems[3].textContent = this.texts.deleteContent
so the "delete" menu entry displays the intended string.
In `@packages/fluent-editor/src/modules/table-up/index.ts`:
- Around line 28-48: The I18N_LOCALE_CHANGE listener registered with
this.quill.on in the constructor is never removed, causing leaks; capture the
handler (e.g., assign the arrow function to this._i18nHandler and call
this.quill.on(I18N_LOCALE_CHANGE, this._i18nHandler)) and then override/detect
destroy() (or module teardown) to call this.quill.off(I18N_LOCALE_CHANGE,
this._i18nHandler) (or removeListener) before cleaning this.modules; update
references to the handler name and ensure it is cleared (this._i18nHandler =
null) when removed.
---
Nitpick comments:
In `@packages/docs/fluent-editor/demos/collaborative-editing.spec.ts`:
- Around line 98-105: The if/else in the collaborative-editing spec is
redundant: both branches perform identical actions for variable lv. Remove the
conditional and replace with a single sequence that clicks the header picker and
the corresponding heading button using the existing p1 locators (e.g.,
p1.locator('.ql-toolbar .ql-picker.ql-header').click() and
p1.locator('.ql-toolbar').getByRole('button', { name: `Heading ${lv}`
}).click()); keep the lv reference and existing locator calls (p1.locator(...),
getByRole(...)) unchanged.
In `@packages/docs/package.json`:
- Around line 30-31: The package.json dependency for quill-toolbar-tip is pinned
as "1.1.0" while quill-table-up uses a caret ("^3.4.0"), creating an
inconsistency after the major jump; update the dependency entry for
quill-toolbar-tip to use a caret (e.g., "^1.1.0") in packages/docs/package.json
(adjust the "quill-toolbar-tip" value) unless the pin was intentional, to allow
patch/minor updates consistently with "quill-table-up".
In `@packages/fluent-editor/src/modules/custom-image/preview/preview-modal.ts`:
- Around line 113-120: Store the document event handlers as class properties
(e.g., this.handleKeydown and this.handleWheel) instead of inline arrow
functions so they can be removed later; wire this.handleKeydown to call
this.hide() when e.key === 'Escape' and wire this.handleWheel to call
this.onMouseWheel(e), attach them with document.addEventListener where currently
added, and then remove them in destroy() via
document.removeEventListener('keydown', this.handleKeydown) and
document.removeEventListener('wheel', this.handleWheel, { passive: false }) (or
matching options) to ensure proper cleanup.
In `@packages/fluent-editor/src/modules/custom-uploader.ts`:
- Line 99: The use of "as any" on the local map in getMaxSize/getMultiple (e.g.,
the variable named map) removes type safety; replace the cast with a properly
typed map (mirror the pattern used in getAccept) such as a typed Record<string,
number|null> for getMaxSize and Record<string, boolean> for getMultiple, or
explicitly type the map variable via an interface/generic so assignments and
lookups are checked at compile time; update the map declaration in getMaxSize
and getMultiple accordingly and adjust any downstream uses to match the new
types to preserve strong typing and avoid the as any cast.
In `@packages/fluent-editor/src/modules/flow-chart/i18n/index.ts`:
- Around line 4-6: Replace the loose any type on the registerFlowChartI18N
parameter with the quill-i18n I18n type to add compile-time safety: import the
I18n (or equivalent) type from 'quill-i18n' and change the function signature
registerFlowChartI18N(i18nModule: any) to use that imported I18n type so calls
to i18nModule.addMessages('en-US', FLOW_CHART_EN_US) and
i18nModule.addMessages('zh-CN', FLOW_CHART_ZH_CN) are type-checked.
In `@packages/fluent-editor/src/modules/flow-chart/modules/context-menu.ts`:
- Line 1: Import the I18N_LOCALE_CHANGE constant from 'quill-i18n' and replace
the literal string 'i18n-locale-change' used in this file (in the context-menu
event registration/handler around the current line 18) with the
I18N_LOCALE_CHANGE symbol so the module uses the shared event constant like
other modules (e.g., mind-map/control-panel.ts, mind-map/context-menu.ts).
In `@packages/fluent-editor/src/modules/mind-map/i18n/index.ts`:
- Around line 4-6: The parameter i18nModule in registerMindMapI18N is typed as
any which reduces type safety; replace any with a proper interface/type that
includes the addMessages(locale: string, messages: object) method (or import the
existing i18n module type if available) and update the function signature to use
that type so calls to i18nModule.addMessages('en-US', MIND_MAP_EN_US) and
addMessages('zh-CN', MIND_MAP_ZH_CN) are type-checked.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc80e2ff-6eda-4ec6-9b37-05f921754b91
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (40)
packages/docs/fluent-editor/demos/collaborative-editing.spec.tspackages/docs/fluent-editor/demos/collaborative-editing.vuepackages/docs/fluent-editor/demos/file-upload.spec.tspackages/docs/fluent-editor/demos/file-upload.vuepackages/docs/fluent-editor/demos/flow-chart.spec.tspackages/docs/fluent-editor/demos/i18n-custom.vuepackages/docs/fluent-editor/demos/i18n.vuepackages/docs/fluent-editor/demos/mind-map.spec.tspackages/docs/fluent-editor/demos/table-up-shortcut.vuepackages/docs/fluent-editor/docs/demo/i18n.mdpackages/docs/package.jsonpackages/fluent-editor/package.jsonpackages/fluent-editor/src/assets/link.scsspackages/fluent-editor/src/assets/style.scsspackages/fluent-editor/src/config/editor.config.tspackages/fluent-editor/src/config/i18n/en-us.tspackages/fluent-editor/src/config/i18n/zh-cn.tspackages/fluent-editor/src/config/types/editor-modules.interface.tspackages/fluent-editor/src/core/fluent-editor.tspackages/fluent-editor/src/fluent-editor.tspackages/fluent-editor/src/modules/counter.tspackages/fluent-editor/src/modules/custom-image/index.tspackages/fluent-editor/src/modules/custom-image/preview/preview-modal.tspackages/fluent-editor/src/modules/custom-image/preview/preview.csspackages/fluent-editor/src/modules/custom-image/specs/custom-image-spec.tspackages/fluent-editor/src/modules/custom-uploader.tspackages/fluent-editor/src/modules/file/modules/file-module.tspackages/fluent-editor/src/modules/flow-chart/i18n/index.tspackages/fluent-editor/src/modules/flow-chart/modules/context-menu.tspackages/fluent-editor/src/modules/flow-chart/modules/control-panel.tspackages/fluent-editor/src/modules/i18n.tspackages/fluent-editor/src/modules/index.tspackages/fluent-editor/src/modules/link/modules/tooltip.tspackages/fluent-editor/src/modules/mind-map/i18n/index.tspackages/fluent-editor/src/modules/mind-map/modules/context-menu.tspackages/fluent-editor/src/modules/mind-map/modules/control-panel.tspackages/fluent-editor/src/modules/shortcut-key/index.tspackages/fluent-editor/src/modules/table-up/index.tspackages/fluent-editor/src/modules/toolbar/toolbar-tip.tspackages/fluent-editor/src/themes/snow.ts
💤 Files with no reviewable changes (2)
- packages/fluent-editor/src/modules/custom-image/preview/preview.css
- packages/fluent-editor/src/modules/i18n.ts
| const miniMapCount = await page.locator('.ql-mind-map-item').count() | ||
| await expect(mindMapButton).toBeVisible() | ||
| await mindMapButton.click() | ||
| await page.waitForTimeout(500) | ||
| const editor = page.locator('.ql-editor') | ||
| await expect(editor).toBeVisible() | ||
| await expect(await page.locator('.ql-mind-map-item').count()).toBe(miniMapCount + 1) |
There was a problem hiding this comment.
Fix typo and use proper Playwright assertion for auto-waiting.
- Variable
miniMapCountappears to be a typo — should bemindMapCountfor consistency. - Line 38 evaluates the count immediately with
await page.locator(...).count(), creating a potential race condition. Playwright'stoHaveCount()assertion auto-retries and should be used instead.
🐛 Proposed fix
- const miniMapCount = await page.locator('.ql-mind-map-item').count()
+ const mindMapCount = await page.locator('.ql-mind-map-item').count()
await expect(mindMapButton).toBeVisible()
await mindMapButton.click()
await page.waitForTimeout(500)
- await expect(await page.locator('.ql-mind-map-item').count()).toBe(miniMapCount + 1)
+ await expect(page.locator('.ql-mind-map-item')).toHaveCount(mindMapCount + 1)📝 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 miniMapCount = await page.locator('.ql-mind-map-item').count() | |
| await expect(mindMapButton).toBeVisible() | |
| await mindMapButton.click() | |
| await page.waitForTimeout(500) | |
| const editor = page.locator('.ql-editor') | |
| await expect(editor).toBeVisible() | |
| await expect(await page.locator('.ql-mind-map-item').count()).toBe(miniMapCount + 1) | |
| const mindMapCount = await page.locator('.ql-mind-map-item').count() | |
| await expect(mindMapButton).toBeVisible() | |
| await mindMapButton.click() | |
| await page.waitForTimeout(500) | |
| await expect(page.locator('.ql-mind-map-item')).toHaveCount(mindMapCount + 1) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/docs/fluent-editor/demos/mind-map.spec.ts` around lines 34 - 38,
Rename the variable miniMapCount to mindMapCount for consistency and then
replace the manual immediate count check with Playwright's auto-retrying
assertion: after clicking mindMapButton and waiting if needed, use the
locator('.ql-mind-map-item') with toHaveCount(mindMapCount + 1) instead of
awaiting page.locator(...).count() to avoid the race condition and leverage
Playwright's built-in waiting.
| const lang = ref('en-US') | ||
| watch(lang, (value) => { | ||
| (editor.getModule('i18n') as I18n).setLocale(value) | ||
| }) |
There was a problem hiding this comment.
Guard against editor being undefined before mount completes.
If lang is modified before onMounted finishes (e.g., programmatically or via rapid user interaction), editor will be undefined, causing a runtime error.
🛡️ Proposed fix
const lang = ref('en-US')
watch(lang, (value) => {
+ if (!editor) return
(editor.getModule('i18n') as I18n).setLocale(value)
})📝 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 lang = ref('en-US') | |
| watch(lang, (value) => { | |
| (editor.getModule('i18n') as I18n).setLocale(value) | |
| }) | |
| const lang = ref('en-US') | |
| watch(lang, (value) => { | |
| if (!editor) return | |
| (editor.getModule('i18n') as I18n).setLocale(value) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/docs/fluent-editor/demos/table-up-shortcut.vue` around lines 86 -
89, The watch on lang can call editor methods before the editor is mounted;
guard the watcher to only access editor when defined by checking editor is
truthy (or using optional chaining) inside the watch callback and/or defer
registering the watcher until onMounted completes; update the watcher around the
ref lang to verify editor (the editor variable or instance used in the demo)
before calling getModule('i18n') and setLocale so it no longer throws if editor
is undefined.
| Welcome to commit PR for more language support. | ||
|
|
||
| 可通过函数 `changeLanguage({ lang, langText })` 修改当前语言 | ||
| 可通过 `i18n` 模块的 `setLocale(lang)` 修改当前语言 |
There was a problem hiding this comment.
Use locale naming in docs to match the breaking API.
Line 9 uses setLocale(lang), while the migration contract is setLocale(locale). Keeping docs terminology aligned reduces confusion during upgrades.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/docs/fluent-editor/docs/demo/i18n.md` at line 9, Update the docs to
use the new API parameter name `locale` rather than `lang`: change the example
reference `setLocale(lang)` to `setLocale(locale)` and search for other
occurrences of `lang` in the i18n.md example to rename them to `locale` so the
docs match the breaking API and the migration contract for `setLocale`.
| 'link': { | ||
| '': 'Hyperlink', | ||
| 'enter-link': 'Enter link: ', | ||
| }, |
There was a problem hiding this comment.
Add missing mode-specific link tooltip keys.
Only enter-link is defined. With dynamic lookup (link.enter-${mode}), formula/video modes can resolve to missing keys.
Suggested fix
'link': {
'': 'Hyperlink',
'enter-link': 'Enter link: ',
+ 'enter-formula': 'Enter formula: ',
+ 'enter-video': 'Enter video URL: ',
},📝 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.
| 'link': { | |
| '': 'Hyperlink', | |
| 'enter-link': 'Enter link: ', | |
| }, | |
| 'link': { | |
| '': 'Hyperlink', | |
| 'enter-link': 'Enter link: ', | |
| 'enter-formula': 'Enter formula: ', | |
| 'enter-video': 'Enter video URL: ', | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/fluent-editor/src/config/i18n/en-us.ts` around lines 77 - 80, The
i18n bundle is missing mode-specific keys used by the dynamic lookup (e.g.,
link.enter-${mode}); update the 'link' object to include explicit keys for each
mode variant referenced at runtime (for example add 'enter-link-formula' and
'enter-link-video' alongside the existing 'enter-link'), mapping them to the
appropriate tooltip text (or same text as 'enter-link') so lookups like
link.enter-formula and link.enter-video resolve correctly when using the dynamic
lookup.
| const target = event.target | ||
| const imageSrc = target.getAttribute('src') || target.getAttribute('data-image') | ||
| const imageSrc = (target as HTMLElement).getAttribute('src') || (target as HTMLElement).getAttribute('data-image') | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify strictness settings and locate similar unsafe casts.
fd tsconfig.json -x sh -c 'echo "== {} =="; rg -n "strictNullChecks|strict" "{}"'
rg -n --type=ts "event\\.target as HTMLElement"Repository: opentiny/tiny-editor
Length of output: 897
🏁 Script executed:
# Check the file and surrounding context
head -n 160 packages/fluent-editor/src/modules/custom-image/specs/custom-image-spec.ts | tail -n 30Repository: opentiny/tiny-editor
Length of output: 986
🏁 Script executed:
# Check the full method implementation
sed -n '135,150p' packages/fluent-editor/src/modules/custom-image/specs/custom-image-spec.tsRepository: opentiny/tiny-editor
Length of output: 392
🏁 Script executed:
# Look at imports and class definition to understand the event type
head -n 50 packages/fluent-editor/src/modules/custom-image/specs/custom-image-spec.tsRepository: opentiny/tiny-editor
Length of output: 1699
🏁 Script executed:
# Check if there are other guards/checks in the CustomImageSpec class
sed -n '1,160p' packages/fluent-editor/src/modules/custom-image/specs/custom-image-spec.ts | grep -n "instanceof\|?.target\|null check\|guard"Repository: opentiny/tiny-editor
Length of output: 145
🏁 Script executed:
# Look at other event handlers in the same file to see if they use similar patterns
rg -n "event\.target" packages/fluent-editor/src/modules/custom-image/specs/custom-image-spec.ts -A 2Repository: opentiny/tiny-editor
Length of output: 911
🏁 Script executed:
# Check if getAttribute is being called on event.target elsewhere without guards in this file
rg -n "getAttribute" packages/fluent-editor/src/modules/custom-image/specs/custom-image-spec.ts -B 2 -A 1Repository: opentiny/tiny-editor
Length of output: 466
Add runtime type guard before calling getAttribute.
Line 139 uses an unsafe type cast without runtime validation. Even though event.target is typed as HTMLElement, the cast provides no runtime protection—if event.target is null or a different node type, getAttribute() will fail. The class already uses proper guards elsewhere (e.g., line 113: instanceof HTMLElement), so apply the same pattern here.
Proposed fix
onImageDoubleClick = (event: MouseEvent) => {
const target = event.target
+ if (!(target instanceof HTMLElement)) {
+ return
+ }
- const imageSrc = (target as HTMLElement).getAttribute('src') || (target as HTMLElement).getAttribute('data-image')
+ const imageSrc = target.getAttribute('src') || target.getAttribute('data-image')
if (imageSrc) {
const modal = getImagePreviewModal()
modal.show(imageSrc)
}
}📝 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 target = event.target | |
| const imageSrc = target.getAttribute('src') || target.getAttribute('data-image') | |
| const imageSrc = (target as HTMLElement).getAttribute('src') || (target as HTMLElement).getAttribute('data-image') | |
| const target = event.target | |
| if (!(target instanceof HTMLElement)) { | |
| return | |
| } | |
| const imageSrc = target.getAttribute('src') || target.getAttribute('data-image') | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/fluent-editor/src/modules/custom-image/specs/custom-image-spec.ts`
around lines 138 - 140, The code unsafely casts event.target to HTMLElement when
building imageSrc; add a runtime guard like checking "event.target instanceof
HTMLElement" before calling getAttribute. Locate the target/imageSrc logic
(variables named target and imageSrc) and change it to first verify event.target
is an HTMLElement, then read (target.getAttribute('src') ||
target.getAttribute('data-image')) into imageSrc; if the guard fails set
imageSrc to undefined/null or handle accordingly to avoid calling getAttribute
on non-HTMLElements.
| const map = (mimetypes || {}) as any | ||
| const fromKind = map[kind] | ||
| if (fromKind?.length) return fromKind | ||
| if (map.file?.length && kind !== 'file') return map.file | ||
| if (fromKind?.length) return fromKind as string[] | ||
| if (map.file?.length && kind !== 'file') return map.file as string[] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/fluent-editor/src/modules/custom-uploader.ts | head -150Repository: opentiny/tiny-editor
Length of output: 6321
getAccept can return a non-array, causing accept.some() to crash at runtime.
At lines 89–90, the .length property check cannot distinguish arrays from strings (both have .length). If a consumer passes malformed config like { image: "jpeg" } instead of { image: ["jpeg"] }, the code returns a string cast as string[], and validateFile crashes at line 122 with TypeError: accept.some is not a function.
The getMaxSize (lines 100–102) and getMultiple (lines 112–114) methods safely use typeof guards, but getAccept must narrow with Array.isArray() before returning.
Proposed fix
- const map = (mimetypes || {}) as any
+ const map: Partial<Record<UploadKind, string[]>> = (mimetypes || {})
const fromKind = map[kind]
- if (fromKind?.length) return fromKind as string[]
- if (map.file?.length && kind !== 'file') return map.file as string[]
+ if (Array.isArray(fromKind) && fromKind.length > 0) return fromKind
+ if (kind !== 'file' && Array.isArray(map.file) && map.file.length > 0) return map.file
return []📝 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 map = (mimetypes || {}) as any | |
| const fromKind = map[kind] | |
| if (fromKind?.length) return fromKind | |
| if (map.file?.length && kind !== 'file') return map.file | |
| if (fromKind?.length) return fromKind as string[] | |
| if (map.file?.length && kind !== 'file') return map.file as string[] | |
| const map: Partial<Record<UploadKind, string[]>> = (mimetypes || {}) | |
| const fromKind = map[kind] | |
| if (Array.isArray(fromKind) && fromKind.length > 0) return fromKind | |
| if (kind !== 'file' && Array.isArray(map.file) && map.file.length > 0) return map.file | |
| return [] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/fluent-editor/src/modules/custom-uploader.ts` around lines 87 - 90,
getAccept currently checks .length and can return a string cast to string[]
causing validateFile (which calls accept.some) to crash; update the getAccept
logic (function getAccept, variables mimetypes, map, fromKind) to explicitly
narrow to arrays using Array.isArray before returning — if fromKind is an array
return it, otherwise if map.file is an array return map.file, else return an
empty array (string[]). Ensure the function always returns a proper string[] so
callers like validateFile can safely call Array.prototype methods.
| this.quill.on(I18N_LOCALE_CHANGE, () => { | ||
| this.setTemplate() | ||
| }) |
There was a problem hiding this comment.
Avoid rebinding listeners on every locale switch.
At Line 39, calling setTemplate() on locale change re-runs listen() (Line 52), which re-attaches editor/root listeners each time. This can cause duplicate handlers and repeated side effects after multiple locale toggles.
Suggested fix
constructor(public quill: FluentEditor, bounds) {
super(quill, bounds)
this.setTemplate()
@@
- this.quill.on(I18N_LOCALE_CHANGE, () => {
- this.setTemplate()
- })
+ this.quill.on(I18N_LOCALE_CHANGE, () => {
+ this.refreshI18nText()
+ })
}
setTemplate() {
@@
- this.root.setAttribute('data-before-title', this.quill.getLangText(`link.enter-${this.root.dataset.mode}`))
+ const mode = this.root.dataset.mode || 'link'
+ this.root.setAttribute('data-before-title', this.quill.getLangText(`link.enter-${mode}`))
this.textbox = this.root.querySelector('input[type="text"]')
this.listen()
}
+
+refreshI18nText() {
+ const mode = this.root.dataset.mode || 'link'
+ this.textbox?.setAttribute('data-link', this.quill.getLangText('linkplaceholder'))
+ this.root.setAttribute('data-before-title', this.quill.getLangText(`link.enter-${mode}`))
+}Also applies to: 43-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/fluent-editor/src/modules/link/modules/tooltip.ts` around lines 38 -
40, The current setTemplate() call inside the I18N_LOCALE_CHANGE handler causes
listen() to run each locale change, re-attaching editor/root listeners and
producing duplicates; to fix, stop re-binding listeners on locale updates by
either removing the listen() invocation from setTemplate() so setTemplate() only
updates UI strings, or make listen() idempotent by adding a guard (e.g., a
boolean like this._listenersAttached) inside listen() to early-return if already
attached; update references to I18N_LOCALE_CHANGE, setTemplate(), and listen()
accordingly.
| '<a class="ql-preview"><i class="icon-share"></i></a>', | ||
| '<a class="ql-remove"><i class="icon-delete"></i></a>', | ||
| ].join('') | ||
| this.root.setAttribute('data-before-title', this.quill.getLangText(`link.enter-${this.root.dataset.mode}`)) |
There was a problem hiding this comment.
Guard data-mode before building the i18n key.
At Line 50, this.root.dataset.mode can be empty, producing link.enter-undefined.
Suggested fix
- this.root.setAttribute('data-before-title', this.quill.getLangText(`link.enter-${this.root.dataset.mode}`))
+ const mode = this.root.dataset.mode || 'link'
+ this.root.setAttribute('data-before-title', this.quill.getLangText(`link.enter-${mode}`))📝 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.
| this.root.setAttribute('data-before-title', this.quill.getLangText(`link.enter-${this.root.dataset.mode}`)) | |
| const mode = this.root.dataset.mode || 'link' | |
| this.root.setAttribute('data-before-title', this.quill.getLangText(`link.enter-${mode}`)) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/fluent-editor/src/modules/link/modules/tooltip.ts` at line 50, Guard
against an empty data-mode before composing the i18n key: when setting
data-before-title on this.root, check this.root.dataset.mode (or fall back to a
safe default like 'default' or an empty string) and only call
this.quill.getLangText with a valid key (e.g., build the key using the guarded
mode variable) so you don't produce "link.enter-undefined"; update the
assignment that sets data-before-title to use the guarded mode value and/or a
conditional lookup.
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
{ i18n?: { lang?: string; langText?: Record<string, string>; } }->{ i18n?: { locale?: string; messages?: Record<string, any>; } }changeLanguage({ lang, langText })改为setLocale(locale)I18N.register(...)-> 使用i18n.addMessages(locale, messages)I18N.parserText(key, lang)-> 使用i18n.t(key)。但仍然兼容quill.getLangText(key)CHANGE_LANGUAGE_EVENT->I18N_LOCALE_CHANGEI18N类型 -> 使用I18n类型Summary by CodeRabbit
New Features
Bug Fixes
Chores