Fix 353323: Keep indentation when start a list, and refactor

packages/roosterjs-content-model-api/lib/modelApi/list/setListType.ts

@@ -1,15 +1,20 @@
 import { splitSelectedParagraphByBr } from '../block/splitSelectedParagraphByBr';
 import {
+    copyFormat,
     createListItem,
     createListLevel,
     getOperationalBlocks,
     isBlockGroupOfType,
+    ListFormats,
+    ListFormatsToKeep,
+    ListFormatsToMove,
     mutateBlock,
     normalizeContentModel,
     setParagraphNotImplicit,
     updateListMetadata,
 } from 'roosterjs-content-model-dom';
 import type {
+    ContentModelBlockFormat,
     ContentModelListItem,
     ReadonlyContentModelBlock,
     ReadonlyContentModelDocument,
@@ -54,19 +59,9 @@ export function setListType(
             }
 
             if (alreadyInExpectedType) {
-                //if the list item has margins or textAlign, we need to apply them to the block to preserve the indention and alignment
+                // if the list item has margins or textAlign, we need to apply them to the block to preserve the indention and alignment
                 block.blocks.forEach(x => {
-                    if (block.format.marginLeft) {
-                        x.format.marginLeft = block.format.marginLeft;
-                    }
-
-                    if (block.format.marginRight) {
-                        x.format.marginRight = block.format.marginRight;
-                    }
-
-                    if (block.format.textAlign) {
-                        x.format.textAlign = block.format.textAlign;
-                    }
+                    copyFormat<ContentModelBlockFormat>(x.format, block.format, ListFormats);
                 });
             }
         } else {
@@ -109,19 +104,17 @@ export function setListType(
 
                     newListItem.blocks.push(mutableBlock);
 
-                    if (mutableBlock.format.marginRight) {
-                        newListItem.format.marginRight = mutableBlock.format.marginRight;
-                        mutableBlock.format.marginRight = undefined;
-                    }
-
-                    if (mutableBlock.format.marginLeft) {
-                        newListItem.format.marginLeft = mutableBlock.format.marginLeft;
-                        mutableBlock.format.marginLeft = undefined;
-                    }
-
-                    if (mutableBlock.format.textAlign) {
-                        newListItem.format.textAlign = mutableBlock.format.textAlign;
-                    }
+                    copyFormat<ContentModelBlockFormat>(
+                        newListItem.format,
+                        mutableBlock.format,
+                        ListFormatsToMove,
+                        true /*deleteOriginalFormat*/
+                    );
+                    copyFormat<ContentModelBlockFormat>(
+                        newListItem.format,
+                        mutableBlock.format,
+                        ListFormatsToKeep
+                    );
 
                     mutateBlock(parent).blocks.splice(index, 1, newListItem);
                     existingListItems.push(newListItem);

packages/roosterjs-content-model-api/test/modelApi/list/setListTypeTest.ts

@@ -470,6 +470,7 @@ describe('indent', () => {
                     },
                     format: {
                         textAlign: 'start',
+                        direction: 'rtl',
                     },
                 },
             ],

packages/roosterjs-content-model-dom/lib/index.ts

@@ -77,6 +77,13 @@ export { mergeTextSegments } from './modelApi/common/mergeTextSegments';
 export { normalizeSegmentFormat } from './modelApi/common/normalizeSegmentFormat';
 
 export { setParagraphNotImplicit } from './modelApi/block/setParagraphNotImplicit';
+export {
+    copyFormat,
+    ListFormats,
+    ListFormatsToKeep,
+    ListFormatsToMove,
+    ParagraphFormats,
+} from './modelApi/block/copyFormat';
 export { getOrderedListNumberStr } from './modelApi/list/getOrderedListNumberStr';
 export { getAutoListStyleType } from './modelApi/list/getAutoListStyleType';
 

packages/roosterjs-content-model-dom/lib/modelApi/block/copyFormat.ts

@@ -0,0 +1,76 @@
+import type {
+    ContentModelBlockFormat,
+    ContentModelFormatBase,
+} from 'roosterjs-content-model-types';
+
+/**
+ * When copy format between list and paragraph, these are the formats that we can copy and remove from the source
+ */
+export const ListFormatsToMove: (keyof ContentModelBlockFormat)[] = [
+    'marginRight',
+    'marginLeft',
+    'paddingRight',
+    'paddingLeft',
+];
+
+/**
+ * When copy format between list and paragraph, these are the formats that we can copy and keep in the source
+ */
+export const ListFormatsToKeep: (keyof ContentModelBlockFormat)[] = [
+    'direction',
+    'textAlign',
+    'htmlAlign',
+];
+
+/**
+ * When copy format from one block to another, these are all the formats that we can copy
+ */
+export const ListFormats: (keyof ContentModelBlockFormat)[] = ListFormatsToMove.concat(
+    ListFormatsToKeep
+);
+
+/**
+ * When copy format between paragraphs, these are the formats that we can copy
+ */
+export const ParagraphFormats: (keyof ContentModelBlockFormat)[] = [
+    'backgroundColor',
+    'direction',
+    'textAlign',
+    'htmlAlign',
+    'lineHeight',
+    'textIndent',
+    'marginTop',
+    'marginRight',
+    'marginBottom',
+    'marginLeft',
+    'paddingTop',
+    'paddingRight',
+    'paddingBottom',
+    'paddingLeft',
+];
+
+/**
+ * Copy formats from source to target with only specified keys
+ * @param targetFormat The format object to copy format to
+ * @param sourceFormat The format object to copy format from
+ * @param formatKeys The format keys to copy
+ * @param deleteOriginalFormat True to delete the original format from sourceFormat, false to keep it. @default false
+ */
+export function copyFormat<T extends ContentModelFormatBase>(
+    targetFormat: T,
+    sourceFormat: T,
+    formatKeys: (keyof T)[],
+    deleteOriginalFormat?: boolean
+) {
+    for (const key of formatKeys) {
+        if (sourceFormat[key] !== undefined) {
+            Object.assign(targetFormat, {
+                [key]: sourceFormat[key],
+            });
+
+            if (deleteOriginalFormat) {
+                delete sourceFormat[key];
+            }
+        }
+    }
+}

packages/roosterjs-content-model-dom/lib/modelApi/common/normalizeContentModel.ts

@@ -1,4 +1,5 @@
 import { isBlockEmpty } from './isEmpty';
+import { ListFormats } from '../block/copyFormat';
 import { mutateBlock } from './mutate';
 import { normalizeParagraph } from './normalizeParagraph';
 import { unwrapBlock } from './unwrapBlock';
@@ -21,7 +22,7 @@ export function normalizeContentModel(group: ReadonlyContentModelBlockGroup) {
             case 'BlockGroup':
                 if (block.blockGroupType == 'ListItem' && block.levels.length == 0) {
                     i += block.blocks.length;
-                    unwrapBlock(group, block);
+                    unwrapBlock(group, block, ListFormats);
                 } else {
                     normalizeContentModel(block);
                 }

packages/roosterjs-content-model-dom/lib/modelApi/common/unwrapBlock.ts

@@ -1,6 +1,8 @@
+import { copyFormat } from '../../modelApi/block/copyFormat';
 import { mutateBlock } from './mutate';
 import { setParagraphNotImplicit } from '../block/setParagraphNotImplicit';
 import type {
+    ContentModelBlockFormat,
     ReadonlyContentModelBlock,
     ReadonlyContentModelBlockGroup,
 } from 'roosterjs-content-model-types';
@@ -12,15 +14,32 @@ import type {
  */
 export function unwrapBlock(
     parent: ReadonlyContentModelBlockGroup | null,
-    groupToUnwrap: ReadonlyContentModelBlockGroup & ReadonlyContentModelBlock
+    groupToUnwrap: ReadonlyContentModelBlockGroup & ReadonlyContentModelBlock,
+    formatsToKeep?: (keyof ContentModelBlockFormat)[]
 ) {
     const index = parent?.blocks.indexOf(groupToUnwrap) ?? -1;
 
     if (index >= 0) {
         groupToUnwrap.blocks.forEach(setParagraphNotImplicit);
 
         if (parent) {
-            mutateBlock(parent)?.blocks.splice(index, 1, ...groupToUnwrap.blocks.map(mutateBlock));
+            mutateBlock(parent)?.blocks.splice(
+                index,
+                1,
+                ...groupToUnwrap.blocks.map(x => {
+                    const mutableBlock = mutateBlock(x);
+
+                    if (formatsToKeep) {
+                        copyFormat<ContentModelBlockFormat>(
+                            mutableBlock.format,
+                            groupToUnwrap.format,
+                            formatsToKeep
+                        );
+                    }
+
+                    return mutableBlock;
+                })
+            );
         }
     }
 }

packages/roosterjs-content-model-dom/test/modelApi/block/copyFormatTest.ts

@@ -0,0 +1,167 @@
+import { ContentModelBlockFormat } from 'roosterjs-content-model-types';
+import {
+    copyFormat,
+    ListFormatsToKeep,
+    ListFormatsToMove,
+    ParagraphFormats,
+} from '../../../lib/modelApi/block/copyFormat';
+
+describe('copyFormat', () => {
+    it('empty format', () => {
+        const targetFormat = {};
+        const sourceFormat = {};
+
+        copyFormat<ContentModelBlockFormat>(targetFormat, sourceFormat, []);
+
+        expect(targetFormat).toEqual({});
+    });
+
+    it('has format, no keys', () => {
+        const targetFormat: ContentModelBlockFormat = {};
+        const sourceFormat: ContentModelBlockFormat = {
+            backgroundColor: 'blue',
+            direction: 'rtl',
+            textAlign: 'end',
+            htmlAlign: 'end',
+            lineHeight: '2',
+            textIndent: '10',
+            marginTop: '10',
+            marginRight: '10',
+            marginBottom: '10',
+            marginLeft: '10',
+            paddingTop: '10',
+            paddingRight: '10',
+            paddingBottom: '10',
+            paddingLeft: '10',
+        };
+
+        copyFormat<ContentModelBlockFormat>(targetFormat, sourceFormat, []);
+
+        expect(targetFormat).toEqual(targetFormat);
+    });
+
+    it('copy list format, keep source', () => {
+        const targetFormat: ContentModelBlockFormat = {};
+        const sourceFormat: ContentModelBlockFormat = {
+            backgroundColor: 'blue',
+            direction: 'rtl',
+            textAlign: 'end',
+            htmlAlign: 'end',
+            lineHeight: '2',
+            textIndent: '10',
+            marginTop: '10',
+            marginRight: '10',
+            marginBottom: '10',
+            marginLeft: '10',
+            paddingTop: '10',
+            paddingRight: '10',
+            paddingBottom: '10',
+            paddingLeft: '10',
+        };
+
+        copyFormat<ContentModelBlockFormat>(targetFormat, sourceFormat, ListFormatsToKeep, false);
+
+        expect(targetFormat).toEqual({
+            direction: 'rtl',
+            textAlign: 'end',
+            htmlAlign: 'end',
+        });
+
+        expect(sourceFormat).toEqual({
+            backgroundColor: 'blue',
+            direction: 'rtl',
+            textAlign: 'end',
+            htmlAlign: 'end',
+            lineHeight: '2',
+            textIndent: '10',
+            marginTop: '10',
+            marginRight: '10',
+            marginBottom: '10',
+            marginLeft: '10',
+            paddingTop: '10',
+            paddingRight: '10',
+            paddingBottom: '10',
+            paddingLeft: '10',
+        });
+    });
+
+    it('copy list format, remove source', () => {
+        const targetFormat: ContentModelBlockFormat = {};
+        const sourceFormat: ContentModelBlockFormat = {
+            backgroundColor: 'blue',
+            direction: 'rtl',
+            textAlign: 'end',
+            htmlAlign: 'end',
+            lineHeight: '2',
+            textIndent: '10',
+            marginTop: '10',
+            marginRight: '10',
+            marginBottom: '10',
+            marginLeft: '10',
+            paddingTop: '10',
+            paddingRight: '10',
+            paddingBottom: '10',
+            paddingLeft: '10',
+        };
+        copyFormat<ContentModelBlockFormat>(targetFormat, sourceFormat, ListFormatsToMove, true);
+        expect(targetFormat).toEqual({
+            marginRight: '10',
+            marginLeft: '10',
+            paddingRight: '10',
+            paddingLeft: '10',
+        });
+        expect(sourceFormat).toEqual({
+            backgroundColor: 'blue',
+            direction: 'rtl',
+            textAlign: 'end',
+            htmlAlign: 'end',
+            lineHeight: '2',
+            textIndent: '10',
+            marginTop: '10',
+            marginBottom: '10',
+            paddingTop: '10',
+            paddingBottom: '10',
+        });
+    });
+
+    it('copy paragraph format', () => {
+        const targetFormat: ContentModelBlockFormat = {};
+        const sourceFormat: ContentModelBlockFormat = {
+            backgroundColor: 'blue',
+            direction: 'rtl',
+            textAlign: 'end',
+            htmlAlign: 'end',
+            lineHeight: '2',
+            textIndent: '10',
+            marginTop: '10',
+            marginRight: '10',
+            marginBottom: '10',
+            marginLeft: '10',
+            paddingTop: '10',
+            paddingRight: '10',
+            paddingBottom: '10',
+            paddingLeft: '10',
+        };
+
+        copyFormat<ContentModelBlockFormat>(targetFormat, sourceFormat, ParagraphFormats);
+
+        expect(targetFormat).toEqual({
+            backgroundColor: 'blue',
+            direction: 'rtl',
+            textAlign: 'end',
+            htmlAlign: 'end',
+            lineHeight: '2',
+            textIndent: '10',
+            marginTop: '10',
+            marginRight: '10',
+            marginBottom: '10',
+            marginLeft: '10',
+            paddingTop: '10',
+            paddingRight: '10',
+            paddingBottom: '10',
+            paddingLeft: '10',
+        });
+
+        expect(sourceFormat).toEqual(targetFormat);
+    });
+});