From 187e032cd6cb30f28d9e8d555911f9ce5aee8274 Mon Sep 17 00:00:00 2001 From: Jiuqing Song Date: Wed, 20 Sep 2023 13:25:52 -0700 Subject: [PATCH] Do not restore cached selection when call select (#2075) --- .../lib/coreApi/select.ts | 113 +++++++++++------- .../lib/corePlugins/DOMEventPlugin.ts | 18 +-- .../test/corePlugins/domEventPluginTest.ts | 10 ++ .../corePluginState/DOMEventPluginState.ts | 5 + 4 files changed, 95 insertions(+), 51 deletions(-) diff --git a/packages/roosterjs-editor-core/lib/coreApi/select.ts b/packages/roosterjs-editor-core/lib/coreApi/select.ts index 50f0a340234..24ca95f8687 100644 --- a/packages/roosterjs-editor-core/lib/coreApi/select.ts +++ b/packages/roosterjs-editor-core/lib/coreApi/select.ts @@ -1,5 +1,6 @@ import { contains, createRange, safeInstanceOf } from 'roosterjs-editor-dom'; import { + EditorCore, NodePosition, PluginEventType, PositionType, @@ -21,6 +22,35 @@ import { * @param arg4 (optional) An offset number, or a PositionType */ export const select: Select = (core, arg1, arg2, arg3, arg4) => { + let rangeEx = buildRangeEx(core, arg1, arg2, arg3, arg4); + + if (rangeEx) { + const skipReselectOnFocus = core.domEvent.skipReselectOnFocus; + + // We are applying a new selection, so we don't need to apply cached selection in DOMEventPlugin. + // Set skipReselectOnFocus to skip this behavior + core.domEvent.skipReselectOnFocus = true; + + try { + applyRangeEx(core, rangeEx); + } finally { + core.domEvent.skipReselectOnFocus = skipReselectOnFocus; + } + } else { + core.domEvent.tableSelectionRange = core.api.selectTable(core, null); + core.domEvent.imageSelectionRange = core.api.selectImage(core, null); + } + + return !!rangeEx; +}; + +function buildRangeEx( + core: EditorCore, + arg1: Range | SelectionRangeEx | NodePosition | Node | SelectionPath | null, + arg2?: NodePosition | number | PositionType | TableSelection | null, + arg3?: Node, + arg4?: number | PositionType +) { let rangeEx: SelectionRangeEx | null = null; if (isSelectionRangeEx(arg1)) { @@ -65,53 +95,50 @@ export const select: Select = (core, arg1, arg2, arg3, arg4) => { : null; } - if (rangeEx) { - switch (rangeEx.type) { - case SelectionRangeTypes.TableSelection: - if (contains(core.contentDiv, rangeEx.table)) { - core.domEvent.imageSelectionRange = core.api.selectImage(core, null); - core.domEvent.tableSelectionRange = core.api.selectTable( - core, - rangeEx.table, - rangeEx.coordinates - ); - rangeEx = core.domEvent.tableSelectionRange; - } - break; - case SelectionRangeTypes.ImageSelection: - if (contains(core.contentDiv, rangeEx.image)) { - core.domEvent.tableSelectionRange = core.api.selectTable(core, null); - core.domEvent.imageSelectionRange = core.api.selectImage(core, rangeEx.image); - rangeEx = core.domEvent.imageSelectionRange; - } - break; - case SelectionRangeTypes.Normal: - core.domEvent.tableSelectionRange = core.api.selectTable(core, null); - core.domEvent.imageSelectionRange = core.api.selectImage(core, null); + return rangeEx; +} - if (contains(core.contentDiv, rangeEx.ranges[0])) { - core.api.selectRange(core, rangeEx.ranges[0]); - } else { - rangeEx = null; - } - break; - } +function applyRangeEx(core: EditorCore, rangeEx: SelectionRangeEx | null) { + switch (rangeEx?.type) { + case SelectionRangeTypes.TableSelection: + if (contains(core.contentDiv, rangeEx.table)) { + core.domEvent.imageSelectionRange = core.api.selectImage(core, null); + core.domEvent.tableSelectionRange = core.api.selectTable( + core, + rangeEx.table, + rangeEx.coordinates + ); + rangeEx = core.domEvent.tableSelectionRange; + } + break; + case SelectionRangeTypes.ImageSelection: + if (contains(core.contentDiv, rangeEx.image)) { + core.domEvent.tableSelectionRange = core.api.selectTable(core, null); + core.domEvent.imageSelectionRange = core.api.selectImage(core, rangeEx.image); + rangeEx = core.domEvent.imageSelectionRange; + } + break; + case SelectionRangeTypes.Normal: + core.domEvent.tableSelectionRange = core.api.selectTable(core, null); + core.domEvent.imageSelectionRange = core.api.selectImage(core, null); - core.api.triggerEvent( - core, - { - eventType: PluginEventType.SelectionChanged, - selectionRangeEx: rangeEx, - }, - true /** broadcast **/ - ); - } else { - core.domEvent.tableSelectionRange = core.api.selectTable(core, null); - core.domEvent.imageSelectionRange = core.api.selectImage(core, null); + if (contains(core.contentDiv, rangeEx.ranges[0])) { + core.api.selectRange(core, rangeEx.ranges[0]); + } else { + rangeEx = null; + } + break; } - return !!rangeEx; -}; + core.api.triggerEvent( + core, + { + eventType: PluginEventType.SelectionChanged, + selectionRangeEx: rangeEx, + }, + true /** broadcast **/ + ); +} function isSelectionRangeEx(obj: any): obj is SelectionRangeEx { const rangeEx = obj as SelectionRangeEx; diff --git a/packages/roosterjs-editor-core/lib/corePlugins/DOMEventPlugin.ts b/packages/roosterjs-editor-core/lib/corePlugins/DOMEventPlugin.ts index 2c1fa9da916..9f0e9306113 100644 --- a/packages/roosterjs-editor-core/lib/corePlugins/DOMEventPlugin.ts +++ b/packages/roosterjs-editor-core/lib/corePlugins/DOMEventPlugin.ts @@ -162,15 +162,17 @@ export default class DOMEventPlugin implements PluginWithState { - const { table, coordinates } = this.state.tableSelectionRange || {}; - const { image } = this.state.imageSelectionRange || {}; + if (!this.state.skipReselectOnFocus) { + const { table, coordinates } = this.state.tableSelectionRange || {}; + const { image } = this.state.imageSelectionRange || {}; - if (table && coordinates) { - this.editor?.select(table, coordinates); - } else if (image) { - this.editor?.select(image); - } else if (this.state.selectionRange) { - this.editor?.select(this.state.selectionRange); + if (table && coordinates) { + this.editor?.select(table, coordinates); + } else if (image) { + this.editor?.select(image); + } else if (this.state.selectionRange) { + this.editor?.select(this.state.selectionRange); + } } this.state.selectionRange = null; diff --git a/packages/roosterjs-editor-core/test/corePlugins/domEventPluginTest.ts b/packages/roosterjs-editor-core/test/corePlugins/domEventPluginTest.ts index cb17db99cff..72bdd7c47c9 100644 --- a/packages/roosterjs-editor-core/test/corePlugins/domEventPluginTest.ts +++ b/packages/roosterjs-editor-core/test/corePlugins/domEventPluginTest.ts @@ -188,6 +188,16 @@ describe('DOMEventPlugin verify event handlers while allow keyboard event propag expect(select.calls.argsFor(0)[0]).toBe(range); expect(state.selectionRange).toBeNull(); }); + + it('Skip applying selection when skipReselectOnFocus is true', () => { + const range = document.createRange(); + state.selectionRange = range; + state.skipReselectOnFocus = true; + eventMap.focus({}); + + expect(select).not.toHaveBeenCalled(); + expect(state.selectionRange).toBeNull(); + }); }); describe('DOMEventPlugin verify event handlers while disallow keyboard event propagation', () => { diff --git a/packages/roosterjs-editor-types/lib/corePluginState/DOMEventPluginState.ts b/packages/roosterjs-editor-types/lib/corePluginState/DOMEventPluginState.ts index dbc727eeabb..596b4b0ff4a 100644 --- a/packages/roosterjs-editor-types/lib/corePluginState/DOMEventPluginState.ts +++ b/packages/roosterjs-editor-types/lib/corePluginState/DOMEventPluginState.ts @@ -39,4 +39,9 @@ export default interface DOMEventPluginState { * Image selection range */ imageSelectionRange: ImageSelectionRange | null; + + /** + * When set to true, onFocus event will not trigger reselect cached range + */ + skipReselectOnFocus?: boolean; }