Fix marginTop/marginBottom returning undefined when selection spans unmeaningful paragraphs

demo/scripts/controlsV2/demoButtons/spaceBeforeAfterButtons.ts

@@ -12,7 +12,7 @@ export const spaceAfterButton: RibbonButton<typeof spaceAfterButtonKey> = {
     key: spaceAfterButtonKey,
     unlocalizedText: 'Remove space after',
     iconName: 'CaretDown8',
-    isChecked: formatState => !formatState.marginBottom || parseInt(formatState.marginBottom) <= 0,
+    isChecked: formatState => !!formatState.marginBottom && parseInt(formatState.marginBottom) > 0,
     onClick: editor => {
         const marginBottom = getFormatState(editor).marginBottom;
         setParagraphMargin(

packages/roosterjs-content-model-api/lib/publicApi/utils/formatParagraphWithContentModel.ts

@@ -17,7 +17,11 @@ export function formatParagraphWithContentModel(
         (model, context) => {
             splitSelectedParagraphByBr(model);
 
-            const paragraphs = getSelectedParagraphs(model, true /*mutate*/);
+            const paragraphs = getSelectedParagraphs(
+                model,
+                true /*mutate*/,
+                false /*removeUnmeaningful*/
+            );
 
             paragraphs.forEach(setStyleCallback);
             context.newPendingFormat = 'preserve';

packages/roosterjs-content-model-api/test/publicApi/utils/formatParagraphWithContentModelTest.ts

@@ -11,7 +11,11 @@ import {
 } from 'roosterjs-content-model-types';
 import {
     createContentModelDocument,
+    createListItem,
+    createListLevel,
     createParagraph,
+    createTable,
+    createTableCell,
     createText,
 } from 'roosterjs-content-model-dom';
 
@@ -132,4 +136,107 @@ describe('formatParagraphWithContentModel', () => {
         expect(splitSelectedParagraphByBrSpy).toHaveBeenCalledTimes(1);
         expect(splitSelectedParagraphByBrSpy).toHaveBeenCalledWith(model);
     });
+
+    it('multiple paragraphs selected', () => {
+        model = createContentModelDocument();
+        const para1 = createParagraph();
+        const para2 = createParagraph();
+        const text1 = createText('test1');
+        const text2 = createText('test2');
+
+        text1.isSelected = true;
+        text2.isSelected = true;
+
+        para1.segments.push(text1);
+        para2.segments.push(text2);
+        model.blocks.push(para1, para2);
+
+        formatParagraphWithContentModel(
+            editor,
+            apiName,
+            paragraph => (paragraph.format.backgroundColor = 'red')
+        );
+
+        expect(para1.format.backgroundColor).toBe('red');
+        expect(para2.format.backgroundColor).toBe('red');
+    });
+
+    it('paragraph in list item', () => {
+        model = createContentModelDocument();
+        const listItem = createListItem([createListLevel('UL')]);
+        const para = createParagraph();
+        const text = createText('item');
+
+        text.isSelected = true;
+        para.segments.push(text);
+        listItem.blocks.push(para);
+        model.blocks.push(listItem);
+
+        formatParagraphWithContentModel(
+            editor,
+            apiName,
+            paragraph => (paragraph.format.backgroundColor = 'blue')
+        );
+
+        expect(para.format.backgroundColor).toBe('blue');
+        expect(splitSelectedParagraphByBrSpy).toHaveBeenCalledTimes(1);
+    });
+
+    it('paragraph in table cell', () => {
+        model = createContentModelDocument();
+        const table = createTable(1);
+        const cell = createTableCell();
+        const para = createParagraph();
+        const text = createText('cell content');
+
+        text.isSelected = true;
+        para.segments.push(text);
+        cell.blocks.push(para);
+        table.rows[0].cells.push(cell);
+        model.blocks.push(table);
+
+        formatParagraphWithContentModel(
+            editor,
+            apiName,
+            paragraph => (paragraph.format.backgroundColor = 'green')
+        );
+
+        expect(para.format.backgroundColor).toBe('green');
+        expect(splitSelectedParagraphByBrSpy).toHaveBeenCalledTimes(1);
+    });
+
+    it('returns false when no paragraphs are selected', () => {
+        model = createContentModelDocument();
+        const para = createParagraph();
+        para.segments.push(createText('unselected'));
+        model.blocks.push(para);
+
+        let callbackReturn: boolean | undefined;
+        (editor.formatContentModel as jasmine.Spy).and.callFake(
+            (callback: ContentModelFormatter, options: FormatContentModelOptions) => {
+                context = {
+                    newEntities: [],
+                    newImages: [],
+                    deletedEntities: [],
+                    rawEvent: options.rawEvent,
+                };
+                callbackReturn = callback(model, context);
+            }
+        );
+
+        formatParagraphWithContentModel(editor, apiName, () => {});
+
+        expect(callbackReturn).toBe(false);
+    });
+
+    it('passes apiName to formatContentModel options', () => {
+        model = createContentModelDocument();
+
+        formatParagraphWithContentModel(editor, apiName, () => {});
+
+        expect(editor.formatContentModel).toHaveBeenCalledWith(
+            jasmine.any(Function),
+            jasmine.objectContaining({ apiName })
+        );
+    });
 });

packages/roosterjs-content-model-dom/lib/modelApi/selection/collectSelections.ts

@@ -203,6 +203,18 @@ export function getSelectedParagraphs(
     mutate: true
 ): ShallowMutableContentModelParagraph[];
 
+/**
+ * Get any array of selected paragraphs from a content model, return mutable paragraphs
+ * @param model The Content Model to get selection from
+ * @param mutate Set to true to indicate we will mutate the selected paragraphs
+ * @param removeUnmeaningful True to remove unmeaningful selection like only selection marker is selected, or head/tail paragraph is selected with selection marker at the wrong place
+ */
+export function getSelectedParagraphs(
+    model: ReadonlyContentModelDocument,
+    mutate: true,
+    removeUnmeaningful: boolean
+): ShallowMutableContentModelParagraph[];
+
 /**
  * Get any array of selected paragraphs from a content model (Readonly)
  * @param model The Content Model to get selection from
@@ -213,12 +225,15 @@ export function getSelectedParagraphs(
 
 export function getSelectedParagraphs(
     model: ReadonlyContentModelDocument,
-    mutate?: boolean
+    mutate?: boolean,
+    removeUnmeaningful: boolean = true
 ): ReadonlyContentModelParagraph[] {
     const selections = collectSelections(model, { includeListFormatHolder: 'never' });
     const result: ReadonlyContentModelParagraph[] = [];
 
-    removeUnmeaningfulSelections(selections);
+    if (removeUnmeaningful) {
+        removeUnmeaningfulSelections(selections);
+    }
 
     selections.forEach(({ block }) => {
         if (block?.blockType == 'Paragraph') {

packages/roosterjs-content-model-dom/test/modelApi/selection/collectSelectionsTest.ts

@@ -424,6 +424,62 @@ describe('getSelectedParagraphs', () => {
             [p1]
         );
     });
+
+    it('removeUnmeaningful=false keeps trailing selection marker paragraph', () => {
+        const s1 = createText('test1');
+        const m2 = createSelectionMarker({ fontSize: '20px' });
+        const p1 = createParagraph(false, { lineHeight: '10px' });
+        const p2 = createParagraph(false, { lineHeight: '20px' });
+
+        p1.segments.push(s1);
+        p2.segments.push(m2);
+
+        spyOn(iterateSelections, 'iterateSelections').and.callFake((_, callback) => {
+            callback([], undefined, p1, [s1]);
+            callback([], undefined, p2, [m2]);
+            return false;
+        });
+
+        const result = getSelectedParagraphs(null!, true /*mutate*/, false /*removeUnmeaningful*/);
+
+        expect(result).toEqual([p1, p2]);
+    });
+
+    it('removeUnmeaningful=false keeps leading selection marker paragraph', () => {
+        const s2 = createText('test2');
+        const m1 = createSelectionMarker({ fontSize: '10px' });
+        const p1 = createParagraph(false, { lineHeight: '10px' });
+        const p2 = createParagraph(false, { lineHeight: '20px' });
+
+        p1.segments.push(m1);
+        p2.segments.push(s2);
+
+        spyOn(iterateSelections, 'iterateSelections').and.callFake((_, callback) => {
+            callback([], undefined, p1, [m1]);
+            callback([], undefined, p2, [s2]);
+            return false;
+        });
+
+        const result = getSelectedParagraphs(null!, true /*mutate*/, false /*removeUnmeaningful*/);
+
+        expect(result).toEqual([p1, p2]);
+    });
+
+    it('mutate=true returns paragraphs as mutable', () => {
+        const p1 = createParagraph(false, { lineHeight: '10px' });
+        const text = createText('hello');
+        p1.segments.push(text);
+
+        spyOn(iterateSelections, 'iterateSelections').and.callFake((_, callback) => {
+            callback([], undefined, p1, [text]);
+            return false;
+        });
+
+        const result = getSelectedParagraphs(null!, true /*mutate*/);
+
+        expect(result.length).toBe(1);
+        expect(result[0]).toBe(p1);
+    });
 });
 
 describe('getFirstSelectedTable', () => {