Content Model: Improve the behavior of deleting general element (micr…

…osoft#1680)

* Content Model: prototype of delete general element

* Handle general better

* improve

packages/roosterjs-content-model/lib/domToModel/processors/generalProcessor.ts

@@ -9,6 +9,7 @@ import { stackFormat } from '../utils/stackFormat';
 
 const generalBlockProcessor: ElementProcessor<HTMLElement> = (group, element, context) => {
     const block = createGeneralBlock(element);
+    const isSelectedBefore = context.isInSelection;
 
     stackFormat(
         context,
@@ -19,17 +20,19 @@ const generalBlockProcessor: ElementProcessor<HTMLElement> = (group, element, co
         },
         () => {
             addBlock(group, block);
+
             context.elementProcessors.child(block, element, context);
         }
     );
+
+    if (isSelectedBefore && context.isInSelection) {
+        block.isSelected = true;
+    }
 };
 
 const generalSegmentProcessor: ElementProcessor<HTMLElement> = (group, element, context) => {
     const segment = createGeneralSegment(element, context.segmentFormat);
-
-    if (context.isInSelection && !element.firstChild) {
-        segment.isSelected = true;
-    }
+    const isSelectedBefore = context.isInSelection;
 
     addDecorators(segment, context);
     addSegment(group, segment);
@@ -44,6 +47,10 @@ const generalSegmentProcessor: ElementProcessor<HTMLElement> = (group, element,
             context.elementProcessors.child(segment, element, context);
         }
     );
+
+    if (isSelectedBefore && context.isInSelection) {
+        segment.isSelected = true;
+    }
 };
 
 /**

packages/roosterjs-content-model/lib/modelApi/block/getLeafSiblingBlock.ts

@@ -1,12 +1,28 @@
 import { ContentModelBlock } from '../../publicTypes/block/ContentModelBlock';
 import { ContentModelBlockGroup } from '../../publicTypes/group/ContentModelBlockGroup';
+import { ContentModelParagraph } from '../../publicTypes/block/ContentModelParagraph';
+import { ContentModelSegment } from '../../publicTypes/segment/ContentModelSegment';
+import { isGeneralSegment } from '../common/isGeneralSegment';
 
 /**
  * @internal
  */
 export type BlockAndPath = {
+    /**
+     * The sibling block
+     */
     block: ContentModelBlock;
+
+    /**
+     * Path of this sibling block
+     */
     path: ContentModelBlockGroup[];
+
+    /**
+     * If the input block is under a general segment, it is possible there are sibling segments under the same paragraph.
+     * Use this property to return the sibling sibling under the same paragraph
+     */
+    siblingSegment?: ContentModelSegment;
 };
 
 /**
@@ -45,6 +61,32 @@ export function getLeafSiblingBlock(
             }
 
             return { block: nextBlock, path: newPath };
+        } else if (isGeneralSegment(group)) {
+            // For general segment, we need to check if there is sibling segment under the same paragraph
+            // First let's find the parent paragraph of this segment
+            newPath.shift();
+
+            let segmentIndex = -1;
+            const segment = group;
+            const para = newPath[0]?.blocks.find(
+                x => x.blockType == 'Paragraph' && (segmentIndex = x.segments.indexOf(segment)) >= 0
+            ) as ContentModelParagraph;
+
+            if (para) {
+                // Now we have found the parent paragraph, so let's check if it has a sibling segment
+                const siblingSegment = para.segments[segmentIndex + (isNext ? 1 : -1)];
+
+                if (siblingSegment) {
+                    // Return this block, path and segment since we have found it
+                    return { block: para, path: newPath, siblingSegment };
+                } else {
+                    // No sibling segment, let's keep go upper level
+                    block = para;
+                }
+            } else {
+                // Parent sibling is not found (in theory this should never happen), just return null
+                break;
+            }
         } else if (group.blockGroupType != 'Document' && group.blockGroupType != 'TableCell') {
             newPath.shift();
             block = group;

packages/roosterjs-content-model/lib/modelApi/common/isGeneralSegment.ts

@@ -0,0 +1,14 @@
+import { ContentModelBlockGroup } from '../../publicTypes/group/ContentModelBlockGroup';
+import { ContentModelGeneralSegment } from '../../publicTypes/segment/ContentModelGeneralSegment';
+
+/**
+ * @internal
+ */
+export function isGeneralSegment(
+    group: ContentModelBlockGroup | ContentModelGeneralSegment
+): group is ContentModelGeneralSegment {
+    return (
+        group.blockGroupType == 'General' &&
+        (<ContentModelGeneralSegment>group).segmentType == 'General'
+    );
+}

packages/roosterjs-content-model/lib/modelApi/selection/deleteSelections.ts

@@ -54,6 +54,7 @@ export interface DeleteSelectionResult {
 
 const DeleteSelectionIteratingOptions: IterateSelectionsOption = {
     contentUnderSelectedTableCell: 'ignoreForTableOrCell', // When a table cell is selected, we replace all content for this cell, so no need to go into its content
+    contentUnderSelectedGeneralElement: 'generalElementOnly', // When a general element is selected, we replace the whole element so no need to go into its content
     includeListFormatHolder: 'never',
 };
 
@@ -190,24 +191,35 @@ const deleteSelectionStep2: DeleteSelectionStep = (context, options) => {
         if (segmentToDelete) {
             context.isChanged = deleteSegment(segments, segmentToDelete, isForward, onDeleteEntity);
         } else if ((blockToDelete = getLeafSiblingBlock(path, paragraph, isForward))) {
-            const { block, path } = blockToDelete;
+            const { block, path, siblingSegment } = blockToDelete;
 
             if (block.blockType == 'Paragraph') {
-                if (isForward) {
-                    context.lastParagraph = block;
+                if (siblingSegment) {
+                    // When selection is under general segment, need to check if it has a sibling sibling, and delete from it
+                    context.isChanged = deleteSegment(
+                        block.segments,
+                        siblingSegment,
+                        isForward,
+                        onDeleteEntity
+                    );
                 } else {
-                    if (block.segments[block.segments.length - 1]?.segmentType == 'Br') {
-                        block.segments.pop();
+                    if (isForward) {
+                        context.lastParagraph = block;
+                    } else {
+                        if (block.segments[block.segments.length - 1]?.segmentType == 'Br') {
+                            block.segments.pop();
+                        }
+
+                        context.insertPoint = createInsertPoint(marker, block, path, tableContext);
+                        context.lastParagraph = paragraph;
+                        delete block.cachedElement;
                     }
 
-                    context.insertPoint = createInsertPoint(marker, block, path, tableContext);
-                    context.lastParagraph = paragraph;
-                    delete block.cachedElement;
+                    context.isChanged = true;
                 }
 
                 // When go across table, getLeafSiblingBlock will return null, when we are here, we must be in the same table context
                 context.lastTableContext = tableContext;
-                context.isChanged = true;
             } else {
                 context.isChanged = deleteBlock(path[0].blocks, block, isForward, onDeleteEntity);
             }

packages/roosterjs-content-model/lib/modelApi/selection/iterateSelections.ts

@@ -29,6 +29,15 @@ export interface IterateSelectionsOption {
      */
     contentUnderSelectedTableCell?: 'include' | 'ignoreForTable' | 'ignoreForTableOrCell';
 
+    /**
+     * For a selected general element, this property determines how do we handle its content.
+     * contentOnly: (Default) When the whole general element is selected, we only invoke callback for its selected content
+     * generalElementOnly: When the whole general element is selected, we only invoke callback for the general element (using block or
+     * segment parameter depends on if it is a block or segment), but skip all its content.
+     * both: When general element is selected, we invoke callback first for its content, then for general element itself
+     */
+    contentUnderSelectedGeneralElement?: 'contentOnly' | 'generalElementOnly' | 'both';
+
     /**
      * Whether call the callback for the list item format holder segment
      * anySegment: call the callback if any segment is selected under a list item
@@ -84,6 +93,8 @@ function internalIterateSelections(
     const parent = path[0];
     const includeListFormatHolder = option?.includeListFormatHolder || 'allSegments';
     const contentUnderSelectedTableCell = option?.contentUnderSelectedTableCell || 'include';
+    const contentUnderSelectedGeneralElement =
+        option?.contentUnderSelectedGeneralElement || 'contentOnly';
 
     let hasSelectedSegment = false;
     let hasUnselectedSegment = false;
@@ -94,7 +105,35 @@ function internalIterateSelections(
         switch (block.blockType) {
             case 'BlockGroup':
                 const newPath = [block, ...path];
-                if (internalIterateSelections(newPath, callback, option, table, treatAllAsSelect)) {
+
+                if (block.blockGroupType == 'General') {
+                    const isSelected = treatAllAsSelect || block.isSelected;
+                    const handleGeneralContent =
+                        !isSelected ||
+                        contentUnderSelectedGeneralElement == 'both' ||
+                        contentUnderSelectedGeneralElement == 'contentOnly';
+                    const handleGeneralElement =
+                        isSelected &&
+                        (contentUnderSelectedGeneralElement == 'both' ||
+                            contentUnderSelectedGeneralElement == 'generalElementOnly' ||
+                            block.blocks.length == 0);
+
+                    if (
+                        (handleGeneralContent &&
+                            internalIterateSelections(
+                                newPath,
+                                callback,
+                                option,
+                                table,
+                                isSelected
+                            )) ||
+                        (handleGeneralElement && callback(path, table, block))
+                    ) {
+                        return true;
+                    }
+                } else if (
+                    internalIterateSelections(newPath, callback, option, table, treatAllAsSelect)
+                ) {
                     return true;
                 }
                 break;
@@ -158,24 +197,41 @@ function internalIterateSelections(
 
                 for (let i = 0; i < block.segments.length; i++) {
                     const segment = block.segments[i];
+                    const isSelected = treatAllAsSelect || segment.isSelected;
 
-                    if (treatAllAsSelect || segment.isSelected) {
-                        segments.push(segment);
-                        hasSelectedSegment = true;
-                    } else if (segment.segmentType == 'General') {
-                        const newPath = [segment, ...path];
+                    if (segment.segmentType == 'General') {
+                        const handleGeneralContent =
+                            !isSelected ||
+                            contentUnderSelectedGeneralElement == 'both' ||
+                            contentUnderSelectedGeneralElement == 'contentOnly';
+                        const handleGeneralElement =
+                            isSelected &&
+                            (contentUnderSelectedGeneralElement == 'both' ||
+                                contentUnderSelectedGeneralElement == 'generalElementOnly' ||
+                                segment.blocks.length == 0);
 
                         if (
+                            handleGeneralContent &&
                             internalIterateSelections(
-                                newPath,
+                                [segment, ...path],
                                 callback,
                                 option,
                                 table,
-                                treatAllAsSelect
+                                isSelected
                             )
                         ) {
                             return true;
                         }
+
+                        if (handleGeneralElement) {
+                            segments.push(segment);
+                        }
+                    } else if (isSelected) {
+                        segments.push(segment);
+                    }
+
+                    if (isSelected) {
+                        hasSelectedSegment = true;
                     } else {
                         hasUnselectedSegment = true;
                     }
@@ -188,7 +244,7 @@ function internalIterateSelections(
 
             case 'Divider':
             case 'Entity':
-                if (block.isSelected && callback(path, table, block)) {
+                if ((treatAllAsSelect || block.isSelected) && callback(path, table, block)) {
                     return true;
                 }
 

packages/roosterjs-content-model/lib/modelApi/selection/setSelection.ts

@@ -1,9 +1,9 @@
 import { ContentModelBlock } from '../../publicTypes/block/ContentModelBlock';
 import { ContentModelBlockGroup } from '../../publicTypes/group/ContentModelBlockGroup';
-import { ContentModelGeneralSegment } from '../../publicTypes/segment/ContentModelGeneralSegment';
 import { ContentModelSegment } from '../../publicTypes/segment/ContentModelSegment';
 import { ContentModelTable } from '../../publicTypes/block/ContentModelTable';
 import { Coordinates } from 'roosterjs-editor-types';
+import { isGeneralSegment } from '../common/isGeneralSegment';
 import { Selectable } from '../../publicTypes/selection/Selectable';
 
 /**
@@ -176,13 +176,6 @@ function setSelectionToSegment(
     }
 }
 
-function isGeneralSegment(group: ContentModelBlockGroup): group is ContentModelGeneralSegment {
-    return (
-        group.blockGroupType == 'General' &&
-        (<ContentModelGeneralSegment>group).segmentType == 'General'
-    );
-}
-
 function setIsSelected(selectable: Selectable, value: boolean) {
     if (value) {
         selectable.isSelected = true;

packages/roosterjs-content-model/lib/modelToDom/handlers/handleGeneralModel.ts

@@ -1,7 +1,7 @@
 import { applyFormat } from '../utils/applyFormat';
 import { ContentModelBlockHandler } from '../../publicTypes/context/ContentModelHandler';
 import { ContentModelGeneralBlock } from '../../publicTypes/group/ContentModelGeneralBlock';
-import { ContentModelGeneralSegment } from '../../publicTypes/segment/ContentModelGeneralSegment';
+import { isGeneralSegment } from '../../modelApi/common/isGeneralSegment';
 import { isNodeOfType } from '../../domUtils/isNodeOfType';
 import { ModelToDomContext } from '../../publicTypes/context/ModelToDomContext';
 import { NodeType } from 'roosterjs-editor-types';
@@ -42,7 +42,3 @@ export const handleGeneralModel: ContentModelBlockHandler<ContentModelGeneralBlo
 
     return refNode;
 };
-
-function isGeneralSegment(block: ContentModelGeneralBlock): block is ContentModelGeneralSegment {
-    return (block as ContentModelGeneralSegment).segmentType == 'General';
-}

packages/roosterjs-content-model/lib/publicTypes/group/ContentModelGeneralBlock.ts

@@ -2,13 +2,15 @@ import { ContentModelBlockBase } from '../block/ContentModelBlockBase';
 import { ContentModelBlockFormat } from '../format/ContentModelBlockFormat';
 import { ContentModelBlockGroupBase } from './ContentModelBlockGroupBase';
 import { ContentModelSegmentFormat } from '../format/ContentModelSegmentFormat';
+import { Selectable } from '../selection/Selectable';
 
 /**
  * Content Model for general Block element
  */
 export interface ContentModelGeneralBlock
     extends ContentModelBlockGroupBase<'General'>,
-        ContentModelBlockBase<'BlockGroup', ContentModelBlockFormat & ContentModelSegmentFormat> {
+        ContentModelBlockBase<'BlockGroup', ContentModelBlockFormat & ContentModelSegmentFormat>,
+        Selectable {
     /**
      * A reference to original HTML node that this model was created from
      */