Merge pull request #1345 from microsoft/u/juliaroldi/image-editing-re…

…factor

U/juliaroldi/image editing refactor

demo/scripts/controls/getToggleablePlugins.ts

@@ -40,6 +40,7 @@ export default function getToggleablePlugins(initState: BuildInPluginState) {
         paste: pluginList.paste ? new Paste() : null,
         watermark: pluginList.watermark ? new Watermark(initState.watermarkText) : null,
         imageEdit,
+        imageSelection: pluginList.imageSelection ? new ImageSelection() : null,
         cutPasteListChain: pluginList.cutPasteListChain ? new CutPasteListChain() : null,
         tableCellSelection: pluginList.tableCellSelection ? new TableCellSelection() : null,
         tableResize: pluginList.tableResize ? new TableResize() : null,
@@ -56,7 +57,6 @@ export default function getToggleablePlugins(initState: BuildInPluginState) {
                 ? createTableEditMenuProvider()
                 : null,
         contextMenu: pluginList.contextMenu ? createContextMenuPlugin() : null,
-        imageSelection: pluginList.imageSelection ? new ImageSelection() : null,
     };
 
     return Object.values(plugins);

demo/scripts/controls/sidePane/editorOptions/ExperimentalFeatures.tsx

@@ -24,8 +24,6 @@ const FeatureNames: Partial<Record<ExperimentalFeatures, string>> = {
         'Use pending style format to do formatting when selection is collapsed',
     [ExperimentalFeatures.NormalizeList]:
         'Normalize list to make sure it can be displayed correctly in other client',
-    [ExperimentalFeatures.ImageSelection]:
-        'Image Selection: the selected image data will be stored by editor core',
     [ExperimentalFeatures.ReuseAllAncestorListElements]:
         "Reuse ancestor list elements even if they don't match the types from the list item.",
 };

demo/scripts/controls/sidePane/editorOptions/Plugins.tsx

@@ -61,10 +61,7 @@ export default class Plugins extends React.Component<PluginsProps, {}> {
                         'Show customized context menu for special cases'
                     )}
                     {this.renderPluginItem('tableCellSelection', 'Table Cell Selection')}
-                    {this.renderPluginItem(
-                        'imageSelection',
-                        'Image Selection (requires Image Selection experimental feature)'
-                    )}
+                    {this.renderPluginItem('imageSelection', 'Image Selection')}
                 </tbody>
             </table>
         );

packages/roosterjs-editor-core/lib/corePlugins/CopyPastePlugin.ts

@@ -117,6 +117,9 @@ export default class CopyPastePlugin implements PluginWithState<CopyPastePluginS
 
                     if (image) {
                         newRange = createRange(image);
+                        if (isCut) {
+                            this.deleteImage(this.editor, selection.image.id);
+                        }
                     }
                 } else {
                     newRange =
@@ -280,4 +283,10 @@ export default class CopyPastePlugin implements PluginWithState<CopyPastePluginS
             table.style.removeProperty('height');
         }
     }
+
+    private deleteImage(editor: IEditor, imageId: string) {
+        editor.queryElements('#' + imageId, node => {
+            editor.deleteNode(node);
+        });
+    }
 }

packages/roosterjs-editor-core/lib/editor/Editor.ts

@@ -476,11 +476,7 @@ export default class Editor implements IEditor {
                 table: arg1,
                 coordinates: arg2,
             };
-        } else if (
-            this.isFeatureEnabled(ExperimentalFeatures.ImageSelection) &&
-            safeInstanceOf(arg1, 'HTMLImageElement') &&
-            typeof arg2 == 'undefined'
-        ) {
+        } else if (safeInstanceOf(arg1, 'HTMLImageElement') && typeof arg2 == 'undefined') {
             rangeEx = {
                 type: SelectionRangeTypes.ImageSelection,
                 ranges: [],

packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/ImageEdit.ts

@@ -19,7 +19,6 @@ import {
     getEntityFromElement,
     getEntitySelector,
     getObjectKeys,
-    matchesSelector,
     safeInstanceOf,
     toArray,
     wrap,
@@ -29,8 +28,8 @@ import {
     doubleCheckResize,
     getSideResizeHTML,
     getCornerResizeHTML,
-    getResizeBordersHTML,
     OnShowResizeHandle,
+    getResizeBordersHTML,
 } from './imageEditors/Resizer';
 import {
     ExperimentalFeatures,
@@ -42,19 +41,14 @@ import {
     PluginEvent,
     PluginEventType,
     EntityOperation,
-    Entity,
-    Keys,
-    PositionType,
     CreateElementData,
     KnownCreateElementDataIndex,
     ModeIndependentColor,
+    SelectionRangeTypes,
+    Entity,
 } from 'roosterjs-editor-types';
 import type { CompatibleImageEditOperation } from 'roosterjs-editor-types/lib/compatibleTypes';
 
-const SHIFT_KEYCODE = 16;
-const CTRL_KEYCODE = 17;
-const ALT_KEYCODE = 18;
-
 const DIRECTIONS = 8;
 const DirectionRad = (Math.PI * 2) / DIRECTIONS;
 const DirectionOrder = ['nw', 'n', 'ne', 'e', 'se', 's', 'sw', 'w'];
@@ -184,45 +178,28 @@ export default class ImageEdit implements EditorPlugin {
      */
     onPluginEvent(e: PluginEvent) {
         switch (e.eventType) {
-            case PluginEventType.MouseDown:
-                this.setEditingImage(null);
-                break;
-
-            case PluginEventType.MouseUp:
-                const target = e.rawEvent.target;
+            case PluginEventType.SelectionChanged:
                 if (
-                    e.isClicking &&
-                    e.rawEvent.button == 0 &&
-                    safeInstanceOf(target, 'HTMLImageElement') &&
-                    target.isContentEditable &&
-                    matchesSelector(target, this.options.imageSelector)
+                    e.selectionRangeEx &&
+                    e.selectionRangeEx.type === SelectionRangeTypes.ImageSelection
                 ) {
-                    this.setEditingImage(target, ImageEditOperation.ResizeAndRotate);
+                    this.setEditingImage(
+                        e.selectionRangeEx.image,
+                        ImageEditOperation.ResizeAndRotate
+                    );
                 }
-
                 break;
-
+            case PluginEventType.MouseDown:
+                this.setEditingImage(null);
+                break;
             case PluginEventType.KeyDown:
-                const key = e.rawEvent.which;
-                if (key == Keys.DELETE || key == Keys.BACKSPACE) {
-                    // Set current editing image to null and select the image if any, and do not prevent default of the event
-                    // so that browser will delete the selected image for us
-                    this.setEditingImage(null, true /*selectImage*/);
-                } else if (key == Keys.ESCAPE && this.image) {
-                    // Press ESC should cancel current editing operation, resume back to original edit info
-                    this.editInfo = getEditInfoFromImage(this.image);
-                    this.setEditingImage(null);
-                    e.rawEvent.preventDefault();
-                } else if (key != SHIFT_KEYCODE && key != CTRL_KEYCODE && key != ALT_KEYCODE) {
-                    // For other key, just unselect current image and select it. If this is an input key, the image will be replaced
-                    this.setEditingImage(null, true /*selectImage*/);
-                }
+                this.setEditingImage(null);
                 break;
-
             case PluginEventType.ContentChanged:
                 if (
-                    e.source != ChangeSource.InsertEntity ||
-                    (<Entity>e.data).type != IMAGE_EDIT_WRAPPER_ENTITY_TYPE
+                    (e.source !== ChangeSource.Format && e.source !== ChangeSource.InsertEntity) ||
+                    (e.source === ChangeSource.InsertEntity &&
+                        (<Entity>e.data)?.type != IMAGE_EDIT_WRAPPER_ENTITY_TYPE)
                 ) {
                     // After contentChanged event, the current image wrapper may not be valid any more, remove all of them if any
                     this.editor.queryElements(
@@ -232,7 +209,6 @@ export default class ImageEdit implements EditorPlugin {
                 }
 
                 break;
-
             case PluginEventType.EntityOperation:
                 if (e.entity.type == IMAGE_EDIT_WRAPPER_ENTITY_TYPE) {
                     if (e.operation == EntityOperation.ReplaceTemporaryContent) {
@@ -288,7 +264,7 @@ export default class ImageEdit implements EditorPlugin {
             typeof operationOrSelect === 'number' ? operationOrSelect : ImageEditOperation.None;
         const selectImage = typeof operationOrSelect === 'number' ? false : !!operationOrSelect;
 
-        if (this.image) {
+        if (!image && this.image) {
             // When there is image in editing, clean up any cached objects and elements
             this.clearDndHelpers();
 
@@ -306,7 +282,6 @@ export default class ImageEdit implements EditorPlugin {
             if (selectImage) {
                 this.editor.select(this.image);
             }
-
             this.image = null;
             this.editInfo = null;
             this.lastSrc = null;
@@ -328,7 +303,7 @@ export default class ImageEdit implements EditorPlugin {
                 this.allowedOperations;
 
             // Create and update editing wrapper and elements
-            const wrapper = this.createWrapper(operation);
+            this.createWrapper(operation);
             this.updateWrapper();
 
             // Init drag and drop
@@ -339,16 +314,15 @@ export default class ImageEdit implements EditorPlugin {
                 ...this.createDndHelpers(ImageEditElementClass.CropContainer, Cropper),
             ];
 
-            // Put cursor next to the image
-            this.editor.select(wrapper, PositionType.After);
+            this.editor.select(this.image);
         }
     }
 
     /**
      * quit editing mode when editor lose focus
      */
     private onBlur = () => {
-        this.setEditingImage(null);
+        this.setEditingImage(null, true);
     };
 
     /**
@@ -367,12 +341,11 @@ export default class ImageEdit implements EditorPlugin {
         wrapper.style.position = 'relative';
         wrapper.style.maxWidth = '100%';
         // keep the same vertical align
-        const originalVerticalAlign = this.image.ownerDocument.defaultView
-            .getComputedStyle(this.image)
-            .getPropertyValue('vertical-align');
+        const originalVerticalAlign = this.getStylePropertyValue(this.image, 'vertical-align');
         if (originalVerticalAlign) {
             wrapper.style.verticalAlign = originalVerticalAlign;
         }
+
         wrapper.style.display = Browser.isSafari ? 'inline-block' : 'inline-flex';
 
         // Cache current src so that we can compare it after edit see if src is changed
@@ -418,6 +391,12 @@ export default class ImageEdit implements EditorPlugin {
         return wrapper;
     }
 
+    private getStylePropertyValue(element: HTMLElement, property: string): string {
+        return element.ownerDocument.defaultView
+            .getComputedStyle(this.image)
+            .getPropertyValue(property);
+    }
+
     /**
      * Get image wrapper from image
      * @param image The image to get wrapper from

packages/roosterjs-editor-plugins/lib/plugins/ImageEdit/imageEditors/Resizer.ts

@@ -11,11 +11,7 @@ import { ImageEditElementClass } from '../types/ImageEditElementClass';
  * will be picked up by ImageEdit code
  */
 export interface OnShowResizeHandle {
-    (
-        elementData: CreateElementData,
-        x: DNDDirectionX,
-        y: DnDDirectionY
-    ): void
+    (elementData: CreateElementData, x: DNDDirectionX, y: DnDDirectionY): void;
 }
 
 const enum HandleTypes {
@@ -55,17 +51,17 @@ export const Resizer: DragAndDropHandler<DragAndDropContext, ResizeInfo> = {
                 // first sure newHeight is right，calculate newWidth
                 newWidth = newHeight * ratio;
                 if (newWidth < options.minWidth) {
-                  newWidth = options.minWidth;
-                  newHeight = newWidth / ratio;
+                    newWidth = options.minWidth;
+                    newHeight = newWidth / ratio;
                 }
-              } else {
+            } else {
                 // first sure newWidth is right，calculate newHeight
                 newHeight = newWidth / ratio;
                 if (newHeight < options.minHeight) {
-                  newHeight = options.minHeight;
-                  newWidth = newHeight * ratio;
+                    newHeight = options.minHeight;
+                    newWidth = newHeight * ratio;
                 }
-              }
+            }
         }
 
         editInfo.widthPx = newWidth;
@@ -142,18 +138,19 @@ export function getCornerResizeHTML(
 
     Xs.forEach(x =>
         Ys.forEach(y => {
-            let elementData = (x == '') == (y == '')
-            ? getResizeHandleHTML(
-                  x,
-                  y,
-                  resizeBorderColor,
-                  handlesExperimentalFeatures
-                      ? HandleTypes.CircularHandlesCorner
-                      : HandleTypes.SquareHandles
-              )
-            : null;
+            let elementData =
+                (x == '') == (y == '')
+                    ? getResizeHandleHTML(
+                          x,
+                          y,
+                          resizeBorderColor,
+                          handlesExperimentalFeatures
+                              ? HandleTypes.CircularHandlesCorner
+                              : HandleTypes.SquareHandles
+                      )
+                    : null;
             if (onShowResizeHandle) {
-                onShowResizeHandle(elementData, x, y)
+                onShowResizeHandle(elementData, x, y);
             }
             result.push(elementData);
         })
@@ -179,16 +176,17 @@ export function getSideResizeHTML(
     const result: CreateElementData[] = [];
     Xs.forEach(x =>
         Ys.forEach(y => {
-            let elementData = (x == '') != (y == '')
-            ? getResizeHandleHTML(
-                  x,
-                  y,
-                  resizeBorderColor,
-                  handlesExperimentalFeatures
-                      ? HandleTypes.CircularHandlesCorner
-                      : HandleTypes.SquareHandles
-              )
-            : null;
+            let elementData =
+                (x == '') != (y == '')
+                    ? getResizeHandleHTML(
+                          x,
+                          y,
+                          resizeBorderColor,
+                          handlesExperimentalFeatures
+                              ? HandleTypes.CircularHandlesCorner
+                              : HandleTypes.SquareHandles
+                      )
+                    : null;
             if (onShowResizeHandle) {
                 onShowResizeHandle(elementData, x, y);
             }

packages/roosterjs-editor-plugins/lib/plugins/ImageSelection/ImageSelection.ts

@@ -2,7 +2,6 @@ import { createRange, safeInstanceOf } from 'roosterjs-editor-dom';
 
 import {
     EditorPlugin,
-    ExperimentalFeatures,
     IEditor,
     PluginEvent,
     PluginEventType,
@@ -47,7 +46,7 @@ export default class ImageSelection implements EditorPlugin {
     }
 
     onPluginEvent(event: PluginEvent) {
-        if (this.editor && this.editor.isFeatureEnabled(ExperimentalFeatures.ImageSelection)) {
+        if (this.editor) {
             switch (event.eventType) {
                 case PluginEventType.EnteredShadowEdit:
                 case PluginEventType.LeavingShadowEdit:
@@ -57,7 +56,7 @@ export default class ImageSelection implements EditorPlugin {
                     }
                     break;
 
-                case PluginEventType.MouseDown:
+                case PluginEventType.MouseUp:
                     const target = event.rawEvent.target;
                     if (safeInstanceOf(target, 'HTMLImageElement')) {
                         if (event.rawEvent.button === mouseRightButton) {
@@ -68,7 +67,7 @@ export default class ImageSelection implements EditorPlugin {
                         }
                     }
                     break;
-                case PluginEventType.KeyDown:
+                case PluginEventType.KeyUp:
                     const key = event.rawEvent.key;
                     const keyDownSelection = this.editor.getSelectionRangeEx();
                     if (keyDownSelection.type === SelectionRangeTypes.ImageSelection) {