Merge branch 'master' into u/jisong/checkdepimprove

microsoft · Sep 20, 2023 · fa85530 · fa85530
2 parents d8fbc78 + 187e032
commit fa85530
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 51 deletions.
diff --git 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
@@ -162,15 +162,17 @@ export default class DOMEventPlugin implements PluginWithState<DOMEventPluginSta
     };
 
     private onFocus = () => {
-        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
@@ -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(<Event>{});
+
+        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
@@ -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;
 }