Merge pull request #2768 from microsoft/u/juliaroldi/image-fixes

Fix Image range selection
microsoft · Aug 19, 2024 · 13ba45a · 13ba45a
2 parents 024877f + f6bb152
commit 13ba45a
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 83 deletions.
diff --git a/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts b/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts
@@ -21,7 +21,6 @@ import type {
     ParsedTable,
     TableSelectionInfo,
     TableCellCoordinate,
-    RangeSelection,
     MouseUpEvent,
 } from 'roosterjs-content-model-types';
 
@@ -362,6 +361,23 @@ class SelectionPlugin implements PluginWithState<SelectionPluginState> {
                         this.selectBeforeOrAfterElement(editor, selection.image);
                     }
                 }
+
+                if (
+                    (isModifierKey(rawEvent) || rawEvent.shiftKey) &&
+                    selection.image &&
+                    !this.isSafari
+                ) {
+                    const range = selection.image.ownerDocument.createRange();
+                    range.selectNode(selection.image);
+                    this.setDOMSelection(
+                        {
+                            type: 'range',
+                            range,
+                            isReverted: false,
+                        },
+                        null /* tableSelection */
+                    );
+                }
                 break;
 
             case 'range':
@@ -696,7 +712,6 @@ class SelectionPlugin implements PluginWithState<SelectionPluginState> {
                 if (this.isSafari) {
                     this.state.selection = newSelection;
                 }
-                this.trySelectSingleImage(newSelection);
             }
         }
     };
@@ -768,21 +783,6 @@ class SelectionPlugin implements PluginWithState<SelectionPluginState> {
             this.state.mouseDisposer = undefined;
         }
     }
-
-    private trySelectSingleImage(selection: RangeSelection) {
-        if (!selection.range.collapsed) {
-            const image = isSingleImageInSelection(selection.range);
-            if (image) {
-                this.setDOMSelection(
-                    {
-                        type: 'image',
-                        image: image,
-                    },
-                    null /*tableSelection*/
-                );
-            }
-        }
-    }
 }
 
 /**

diff --git a/packages/roosterjs-content-model-core/test/corePlugin/selection/SelectionPluginTest.ts b/packages/roosterjs-content-model-core/test/corePlugin/selection/SelectionPluginTest.ts
@@ -649,10 +649,18 @@ describe('SelectionPlugin handle image selection', () => {
             key: 'A',
             stopPropagation: stopPropagationSpy,
             ctrlKey: true,
+            shiftKey: false,
         } as any;
 
         const mockedImage = {
             parentNode: { childNodes: [] },
+            ownerDocument: {
+                createRange: () => {
+                    return {
+                        selectNode: (node: any) => {},
+                    };
+                },
+            },
         } as any;
 
         mockedImage.parentNode.childNodes.push(mockedImage);
@@ -674,7 +682,7 @@ describe('SelectionPlugin handle image selection', () => {
         });
 
         expect(stopPropagationSpy).not.toHaveBeenCalled();
-        expect(setDOMSelectionSpy).not.toHaveBeenCalled();
+        expect(setDOMSelectionSpy).toHaveBeenCalled();
     });
 
     it('key down - other key, image has no parent', () => {
@@ -2823,40 +2831,4 @@ describe('SelectionPlugin selectionChange on image selected', () => {
         expect(setDOMSelectionSpy).not.toHaveBeenCalled();
         expect(getDOMSelectionSpy).toHaveBeenCalledTimes(1);
     });
-
-    it('onSelectionChange on image | 4', () => {
-        const image = document.createElement('img');
-        spyOn(isSingleImageInSelection, 'isSingleImageInSelection').and.returnValue(image);
-
-        const plugin = createSelectionPlugin({});
-        const state = plugin.getState();
-        const mockedOldSelection = {
-            type: 'image',
-            image: {} as any,
-        } as DOMSelection;
-
-        state.selection = mockedOldSelection;
-
-        plugin.initialize(editor);
-
-        const onSelectionChange = addEventListenerSpy.calls.argsFor(0)[1] as Function;
-        const mockedNewSelection = {
-            type: 'range',
-            range: {} as any,
-        } as any;
-
-        hasFocusSpy.and.returnValue(true);
-        isInShadowEditSpy.and.returnValue(false);
-        getDOMSelectionSpy.and.returnValue(mockedNewSelection);
-        getRangeAtSpy.and.returnValue({ startContainer: {} });
-
-        onSelectionChange();
-
-        expect(setDOMSelectionSpy).toHaveBeenCalled();
-        expect(setDOMSelectionSpy).toHaveBeenCalledWith({
-            type: 'image',
-            image,
-        });
-        expect(getDOMSelectionSpy).toHaveBeenCalledTimes(1);
-    });
 });
diff --git a/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts b/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts
@@ -17,10 +17,8 @@ import { updateWrapper } from './utils/updateWrapper';
 import {
     getSafeIdSelector,
     isElementOfType,
-    isModifierKey,
     isNodeOfType,
     mutateSegment,
-    toArray,
     unwrap,
 } from 'roosterjs-content-model-dom';
 import type { DragAndDropHelper } from '../pluginUtils/DragAndDrop/DragAndDropHelper';
@@ -168,41 +166,16 @@ export class ImageEditPlugin implements ImageEditor, EditorPlugin {
         }
     }
 
-    //Sometimes the cursor can be inside the editing image and inside shadow dom, then the cursor need to moved out of shadow dom
-    private selectBeforeEditingImage(editor: IEditor, element: HTMLElement) {
-        let parent = element.parentNode;
-        if (parent && isNodeOfType(parent, 'ELEMENT_NODE') && parent.shadowRoot) {
-            element = parent;
-            parent = parent.parentNode;
-        }
-        const index = parent && toArray(parent.childNodes).indexOf(element);
-        if (index !== null && index >= 0 && parent) {
-            const doc = editor.getDocument();
-            const range = doc.createRange();
-            range.setStart(parent, index);
-            range.collapse();
-            editor.setDOMSelection({
-                type: 'range',
-                range,
-                isReverted: false,
-            });
-        }
-    }
-
     private keyDownHandler(editor: IEditor, event: KeyDownEvent) {
         if (this.isEditing) {
             if (event.rawEvent.key === 'Escape') {
                 this.removeImageWrapper();
             } else {
-                const selection = editor.getDOMSelection();
-                const isImageSelection = selection?.type == 'image';
-                if (isImageSelection) {
-                    this.selectBeforeEditingImage(editor, selection.image);
-                }
                 this.applyFormatWithContentModel(
                     editor,
                     this.isCropMode,
-                    (isModifierKey(event.rawEvent) || event.rawEvent.shiftKey) && isImageSelection //if it's a modifier key over a image, the image should select the image
+                    true /** should selectImage */,
+                    false /* isApiOperation */
                 );
             }
         }
@@ -260,6 +233,7 @@ export class ImageEditPlugin implements ImageEditor, EditorPlugin {
                         if (shouldSelectImage) {
                             normalizeImageSelection(previousSelectedImage);
                         }
+
                         this.cleanInfo();
                         result = true;
                     }