Update logic to decide if we need to merge a table on paste. (#2022)

* Fix TableSelectionCopy

* update unit tests

* Fix 2

* address comment

packages-content-model/roosterjs-content-model-editor/lib/publicApi/utils/paste.ts

@@ -1,9 +1,14 @@
-import { ContentModelBlockFormat, FormatParser } from 'roosterjs-content-model-types';
 import { domToContentModel } from 'roosterjs-content-model-dom';
 import { formatWithContentModel } from './formatWithContentModel';
+import { FormatWithContentModelContext } from '../../publicTypes/parameter/FormatWithContentModelContext';
 import { IContentModelEditor } from '../../publicTypes/IContentModelEditor';
 import { mergeModel } from '../../modelApi/common/mergeModel';
 import { NodePosition } from 'roosterjs-editor-types';
+import {
+    ContentModelBlockFormat,
+    ContentModelDocument,
+    FormatParser,
+} from 'roosterjs-content-model-types';
 import ContentModelBeforePasteEvent, {
     ContentModelBeforePasteEventData,
 } from '../../publicTypes/event/ContentModelBeforePasteEvent';
@@ -84,19 +89,8 @@ export default function paste(
         formatWithContentModel(
             editor,
             'Paste',
-            (model, context) => {
-                if (customizedMerge) {
-                    customizedMerge(model, pasteModel);
-                } else {
-                    mergeModel(model, pasteModel, context, {
-                        mergeFormat: applyCurrentFormat ? 'keepSourceEmphasisFormat' : 'none',
-                        mergeTable:
-                            pasteModel.blocks.length === 1 &&
-                            pasteModel.blocks[0].blockType === 'Table',
-                    });
-                }
-                return true;
-            },
+            (model, context) =>
+                mergePasteContent(model, context, pasteModel, applyCurrentFormat, customizedMerge),
             {
                 changeSource: ChangeSource.Paste,
                 getChangeData: () => clipboardData,
@@ -105,6 +99,45 @@ export default function paste(
     }
 }
 
+/**
+ * @internal
+ * Export only for unit test
+ */
+export function mergePasteContent(
+    model: ContentModelDocument,
+    context: FormatWithContentModelContext,
+    pasteModel: ContentModelDocument,
+    applyCurrentFormat: boolean,
+    customizedMerge:
+        | undefined
+        | ((source: ContentModelDocument, target: ContentModelDocument) => void)
+): boolean {
+    if (customizedMerge) {
+        customizedMerge(model, pasteModel);
+    } else {
+        mergeModel(model, pasteModel, context, {
+            mergeFormat: applyCurrentFormat ? 'keepSourceEmphasisFormat' : 'none',
+            mergeTable: shouldMergeTable(pasteModel),
+        });
+    }
+    return true;
+}
+
+function shouldMergeTable(pasteModel: ContentModelDocument): boolean | undefined {
+    // If model contains a table and a paragraph element after the table with a single BR segment, remove the Paragraph after the table
+    if (
+        pasteModel.blocks.length == 2 &&
+        pasteModel.blocks[0].blockType === 'Table' &&
+        pasteModel.blocks[1].blockType === 'Paragraph' &&
+        pasteModel.blocks[1].segments.length === 1 &&
+        pasteModel.blocks[1].segments[0].segmentType === 'Br'
+    ) {
+        pasteModel.blocks.splice(1);
+    }
+    // Only merge table when the document contain a single table.
+    return pasteModel.blocks.length === 1 && pasteModel.blocks[0].blockType === 'Table';
+}
+
 function createBeforePasteEventData(
     editor: IContentModelEditor,
     clipboardData: ClipboardData,

packages-content-model/roosterjs-content-model-editor/test/editor/plugins/ContentModelCopyPastePluginTest.ts

@@ -1,5 +1,6 @@
 import * as cloneModelFile from '../../../lib/modelApi/common/cloneModel';
 import * as contentModelToDomFile from 'roosterjs-content-model-dom/lib/modelToDom/contentModelToDom';
+import * as createRangeF from 'roosterjs-editor-dom/lib/selection/createRange';
 import * as deleteSelectionsFile from '../../../lib/modelApi/edit/deleteSelection';
 import * as extractClipboardItemsFile from 'roosterjs-editor-dom/lib/clipboard/extractClipboardItems';
 import * as iterateSelectionsFile from '../../../lib/modelApi/selection/iterateSelections';
@@ -199,7 +200,7 @@ describe('ContentModelCopyPastePlugin |', () => {
         it('Selection not Collapsed and table selection', () => {
             // Arrange
             const table = document.createElement('table');
-            table.id = 'image';
+            table.id = 'table';
             // Arrange
             selectionRangeExValue = <SelectionRangeEx>{
                 type: SelectionRangeTypes.TableSelection,
@@ -209,6 +210,7 @@ describe('ContentModelCopyPastePlugin |', () => {
                 table,
             };
 
+            spyOn(createRangeF, 'default').and.callThrough();
             spyOn(deleteSelectionsFile, 'deleteSelection');
             spyOn(contentModelToDomFile, 'contentModelToDom').and.callFake(() => {
                 const container = document.createElement('div');
@@ -228,6 +230,7 @@ describe('ContentModelCopyPastePlugin |', () => {
             domEvents.copy?.(<Event>{});
 
             // Assert
+            expect(createRangeF.default).toHaveBeenCalledWith(<any>table.parentElement);
             expect(getSelectionRangeEx).toHaveBeenCalled();
             expect(deleteSelectionsFile.deleteSelection).not.toHaveBeenCalled();
             expect(contentModelToDomFile.contentModelToDom).toHaveBeenCalledWith(
@@ -264,6 +267,7 @@ describe('ContentModelCopyPastePlugin |', () => {
                 image,
             };
 
+            spyOn(createRangeF, 'default').and.callThrough();
             spyOn(deleteSelectionsFile, 'deleteSelection');
             spyOn(contentModelToDomFile, 'contentModelToDom').and.callFake(() => {
                 div.appendChild(image);
@@ -280,6 +284,7 @@ describe('ContentModelCopyPastePlugin |', () => {
             domEvents.copy?.(<Event>{});
 
             // Assert
+            expect(createRangeF.default).toHaveBeenCalledWith(<any>image);
             expect(getSelectionRangeEx).toHaveBeenCalled();
             expect(deleteSelectionsFile.deleteSelection).not.toHaveBeenCalled();
             expect(contentModelToDomFile.contentModelToDom).toHaveBeenCalledWith(
@@ -394,7 +399,7 @@ describe('ContentModelCopyPastePlugin |', () => {
         it('Selection not Collapsed and table selection', () => {
             // Arrange
             const table = document.createElement('table');
-            table.id = 'image';
+            table.id = 'table';
             selectionRangeExValue = <SelectionRangeEx>{
                 type: SelectionRangeTypes.TableSelection,
                 ranges: [new Range()],
@@ -403,6 +408,7 @@ describe('ContentModelCopyPastePlugin |', () => {
                 table,
             };
 
+            spyOn(createRangeF, 'default').and.callThrough();
             spyOn(deleteSelectionsFile, 'deleteSelection');
             spyOn(contentModelToDomFile, 'contentModelToDom').and.callFake(() => {
                 const container = document.createElement('div');
@@ -422,6 +428,7 @@ describe('ContentModelCopyPastePlugin |', () => {
             domEvents.cut?.(<Event>{});
 
             // Assert
+            expect(createRangeF.default).toHaveBeenCalledWith(<any>table.parentElement);
             expect(getSelectionRangeEx).toHaveBeenCalled();
             expect(contentModelToDomFile.contentModelToDom).toHaveBeenCalledWith(
                 document,
@@ -460,6 +467,7 @@ describe('ContentModelCopyPastePlugin |', () => {
                 image,
             };
 
+            spyOn(createRangeF, 'default').and.callThrough();
             spyOn(deleteSelectionsFile, 'deleteSelection');
             spyOn(contentModelToDomFile, 'contentModelToDom').and.callFake(() => {
                 div.appendChild(image);
@@ -476,6 +484,7 @@ describe('ContentModelCopyPastePlugin |', () => {
             domEvents.cut?.(<Event>{});
 
             // Assert
+            expect(createRangeF.default).toHaveBeenCalledWith(<any>image);
             expect(getSelectionRangeEx).toHaveBeenCalled();
             expect(contentModelToDomFile.contentModelToDom).toHaveBeenCalledWith(
                 document,