From 05f0ce0d0fc1a1386bf3a6d3fb55f902a94f2d09 Mon Sep 17 00:00:00 2001 From: vhuseinova-msft <98852890+vhuseinova-msft@users.noreply.github.com> Date: Wed, 3 Jul 2024 11:05:55 -0700 Subject: [PATCH 01/31] Export DefaultSanitizers to be available in roosterjs-content-model-plugins package #2739 --- packages/roosterjs-content-model-plugins/lib/index.ts | 1 + .../lib/paste/DefaultSanitizers.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/roosterjs-content-model-plugins/lib/index.ts b/packages/roosterjs-content-model-plugins/lib/index.ts index e00daab314d..4ff598eb9ce 100644 --- a/packages/roosterjs-content-model-plugins/lib/index.ts +++ b/packages/roosterjs-content-model-plugins/lib/index.ts @@ -2,6 +2,7 @@ export { TableEditPlugin } from './tableEdit/TableEditPlugin'; export { OnTableEditorCreatedCallback } from './tableEdit/OnTableEditorCreatedCallback'; export { TableEditFeatureName } from './tableEdit/editors/features/TableEditFeatureName'; export { PastePlugin } from './paste/PastePlugin'; +export { DefaultSanitizers } from './paste/DefaultSanitizers'; export { EditPlugin, EditOptions } from './edit/EditPlugin'; export { AutoFormatPlugin, AutoFormatOptions } from './autoFormat/AutoFormatPlugin'; diff --git a/packages/roosterjs-content-model-plugins/lib/paste/DefaultSanitizers.ts b/packages/roosterjs-content-model-plugins/lib/paste/DefaultSanitizers.ts index a1ca64d7e9c..c22f7987b20 100644 --- a/packages/roosterjs-content-model-plugins/lib/paste/DefaultSanitizers.ts +++ b/packages/roosterjs-content-model-plugins/lib/paste/DefaultSanitizers.ts @@ -1,7 +1,7 @@ import type { ValueSanitizer } from 'roosterjs-content-model-types'; /** - * @internal + * Default style sanitizers for PastePlugin. */ export const DefaultSanitizers: Record = { width: divParagraphSanitizer, From f57a68c5014b404a1e41a977ad685db01a2d47b0 Mon Sep 17 00:00:00 2001 From: Bryan Valverde U Date: Wed, 10 Jul 2024 13:33:24 -0600 Subject: [PATCH 02/31] Fix #2741 by changing Id selectors from `#{id}` to `[id="{id}"]` (#2742) * init * Fix build --- .../coreApi/setEditorStyle/ensureUniqueId.ts | 2 +- .../setDOMSelection/setDOMSelectionTest.ts | 2 +- .../setEditorStyle/ensureUniqueIdTest.ts | 44 ++++++++++++++++++- 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/ensureUniqueId.ts b/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/ensureUniqueId.ts index c3cb20c5363..c52e9f0bc47 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/ensureUniqueId.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/ensureUniqueId.ts @@ -7,7 +7,7 @@ export function ensureUniqueId(element: HTMLElement, idPrefix: string): string { const doc = element.ownerDocument; let i = 0; - while (!element.id || doc.querySelectorAll('#' + element.id).length > 1) { + while (!element.id || doc.querySelectorAll(`[id="${element.id}"]`).length > 1) { element.id = idPrefix + '_' + i++; } diff --git a/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts b/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts index 5ecbc2ffb25..26f8537f3c9 100644 --- a/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts +++ b/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts @@ -529,7 +529,7 @@ describe('setDOMSelection', () => { createRangeSpy.and.returnValue(mockedRange); querySelectorAllSpy.and.callFake(selector => { - return selector == '#image_0' ? ['', ''] : ['']; + return selector == '[id="image_0"]' ? ['', ''] : ['']; }); hasFocusSpy.and.returnValue(false); diff --git a/packages/roosterjs-content-model-core/test/coreApi/setEditorStyle/ensureUniqueIdTest.ts b/packages/roosterjs-content-model-core/test/coreApi/setEditorStyle/ensureUniqueIdTest.ts index d45aeeb931e..0237af7ef8e 100644 --- a/packages/roosterjs-content-model-core/test/coreApi/setEditorStyle/ensureUniqueIdTest.ts +++ b/packages/roosterjs-content-model-core/test/coreApi/setEditorStyle/ensureUniqueIdTest.ts @@ -38,10 +38,52 @@ describe('ensureUniqueId', () => { id: 'dup', } as any; querySelectorAllSpy.and.callFake((selector: string) => - selector == '#dup' ? [{}, {}] : [] + selector == '[id="dup"]' ? [{}, {}] : [] ); const result = ensureUniqueId(element, 'prefix'); expect(result).toBe('dup_0'); }); + + it('Should not throw when element id starts with number', () => { + const element = { + ownerDocument: doc, + id: '0', + } as any; + + let isFirst = true; + querySelectorAllSpy.and.callFake((_selector: string) => { + if (isFirst) { + isFirst = false; + return [{}, {}]; + } + return [{}]; + }); + + ensureUniqueId(element, 'prefix'); + + expect(querySelectorAllSpy).toHaveBeenCalledWith('[id="0"]'); + expect(element.id).toEqual('0_0'); + }); + + it('Should not throw when element id starts with hyphen', () => { + const element = { + ownerDocument: doc, + id: '-', + } as any; + + let isFirst = true; + querySelectorAllSpy.and.callFake((_selector: string) => { + if (isFirst) { + isFirst = false; + return [{}, {}]; + } + return [{}]; + }); + + ensureUniqueId(element, 'prefix'); + + expect(querySelectorAllSpy).toHaveBeenCalledWith('[id="-"]'); + expect(element.id).toEqual('-_0'); + }); }); From 5f184e83455891ac99bd4937b16cd1a45cf91bda Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Thu, 11 Jul 2024 11:58:25 -0300 Subject: [PATCH 03/31] fix selection --- .../lib/imageEdit/utils/getSelectedImage.ts | 19 ++- .../imageEdit/utils/getSelectedImageTest.ts | 147 ++++++++++++++++++ 2 files changed, 165 insertions(+), 1 deletion(-) diff --git a/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts b/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts index c517106f2b8..09da505b3db 100644 --- a/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts +++ b/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts @@ -3,16 +3,33 @@ import type { ReadonlyContentModelDocument } from 'roosterjs-content-model-types import type { ImageAndParagraph } from '../types/ImageAndParagraph'; /** + * Selecting directly on the image will only capture the image segment. + * However, if the selection is made while the image is within a wrapper, it will capture the span that encloses the image. + * In the last case, the selection will be marked as <---SelectionMarker---><---Image---><---SelectionMarker--->. + * This function accounts for both scenarios. * @internal */ export function getSelectedImage(model: ReadonlyContentModelDocument): ImageAndParagraph | null { const selections = getSelectedSegmentsAndParagraphs(model, false); - if (selections.length == 1 && selections[0][0].segmentType == 'Image' && selections[0][1]) { return { image: selections[0][0], paragraph: selections[0][1], }; + } else if ( + selections.length == 3 && + selections[1][1] && + selections[1][1].segments.every( + seg => + (seg.segmentType == 'Image' || seg.segmentType == 'SelectionMarker') && + seg.isSelected + ) && + selections[1][0].segmentType == 'Image' + ) { + return { + image: selections[1][0], + paragraph: selections[1][1], + }; } else { return null; } diff --git a/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts b/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts index 29261a8f640..9d4386c7f6c 100644 --- a/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts +++ b/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts @@ -118,4 +118,151 @@ describe('getSelectedImage', () => { const selections = getSelectedImage(model); expect(selections).toEqual(null); }); + + it('get selected Image in wrapper', () => { + const model: ContentModelDocument = { + blockGroupType: 'Document', + blocks: [ + { + blockType: 'Paragraph', + segments: [ + { + segmentType: 'SelectionMarker', + format: {}, + isSelected: true, + }, + { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: 'image_0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: true, + isSelected: true, + }, + { + segmentType: 'SelectionMarker', + format: {}, + isSelected: true, + }, + ], + format: {}, + segmentFormat: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + }, + }, + ], + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: '#000000', + }, + }; + + const selections = getSelectedImage(model); + expect(selections).toEqual({ + image: { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: 'image_0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: true, + isSelected: true, + }, + paragraph: { + blockType: 'Paragraph', + segments: [ + { + segmentType: 'SelectionMarker', + format: {}, + isSelected: true, + }, + { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: 'image_0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: true, + isSelected: true, + }, + { + segmentType: 'SelectionMarker', + format: {}, + isSelected: true, + }, + ], + format: {}, + segmentFormat: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + }, + }, + }); + }); + + it('do not selected Image in wrapper', () => { + const model: ContentModelDocument = { + blockGroupType: 'Document', + blocks: [ + { + blockType: 'Paragraph', + segments: [ + { + segmentType: 'SelectionMarker', + format: {}, + isSelected: true, + }, + { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: 'image_0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: true, + isSelected: true, + }, + ], + format: {}, + segmentFormat: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + }, + }, + ], + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: '#000000', + }, + }; + + const selections = getSelectedImage(model); + expect(selections).toEqual(null); + }); }); From 71e67d7f308d67101beb052066c8e6de8f3a45a2 Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Thu, 11 Jul 2024 12:05:36 -0300 Subject: [PATCH 04/31] fix --- .../lib/imageEdit/utils/getSelectedImage.ts | 3 +- .../imageEdit/utils/getSelectedImageTest.ts | 51 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts b/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts index 09da505b3db..4dedb38ee8e 100644 --- a/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts +++ b/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts @@ -24,7 +24,8 @@ export function getSelectedImage(model: ReadonlyContentModelDocument): ImageAndP (seg.segmentType == 'Image' || seg.segmentType == 'SelectionMarker') && seg.isSelected ) && - selections[1][0].segmentType == 'Image' + selections[1][0].segmentType == 'Image' && + selections[1][0].isSelectedAsImageSelection == true ) { return { image: selections[1][0], diff --git a/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts b/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts index 9d4386c7f6c..ec759cc4fa5 100644 --- a/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts +++ b/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts @@ -265,4 +265,55 @@ describe('getSelectedImage', () => { const selections = getSelectedImage(model); expect(selections).toEqual(null); }); + + it('do not selected Image in wrapper, if is not image selection', () => { + const model: ContentModelDocument = { + blockGroupType: 'Document', + blocks: [ + { + blockType: 'Paragraph', + segments: [ + { + segmentType: 'SelectionMarker', + format: {}, + isSelected: true, + }, + { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: 'image_0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: false, + isSelected: true, + }, + { + segmentType: 'SelectionMarker', + format: {}, + isSelected: true, + }, + ], + format: {}, + segmentFormat: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + }, + }, + ], + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: '#000000', + }, + }; + + const selections = getSelectedImage(model); + expect(selections).toEqual(null); + }); }); From f21e926d2bc20c4ae4cbee6a5d419c0b15e8ab14 Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Thu, 11 Jul 2024 12:18:51 -0300 Subject: [PATCH 05/31] fixes --- .../lib/imageEdit/utils/getSelectedImage.ts | 9 +- .../imageEdit/utils/getSelectedImageTest.ts | 111 ++++++++++++++++++ 2 files changed, 115 insertions(+), 5 deletions(-) diff --git a/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts b/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts index 4dedb38ee8e..212664355c3 100644 --- a/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts +++ b/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts @@ -11,6 +11,7 @@ import type { ImageAndParagraph } from '../types/ImageAndParagraph'; */ export function getSelectedImage(model: ReadonlyContentModelDocument): ImageAndParagraph | null { const selections = getSelectedSegmentsAndParagraphs(model, false); + console.log(selections); if (selections.length == 1 && selections[0][0].segmentType == 'Image' && selections[0][1]) { return { image: selections[0][0], @@ -19,11 +20,9 @@ export function getSelectedImage(model: ReadonlyContentModelDocument): ImageAndP } else if ( selections.length == 3 && selections[1][1] && - selections[1][1].segments.every( - seg => - (seg.segmentType == 'Image' || seg.segmentType == 'SelectionMarker') && - seg.isSelected - ) && + selections[1][1].segments + .filter(seg => seg.isSelected) + .every(seg => seg.segmentType == 'Image' || seg.segmentType == 'SelectionMarker') && selections[1][0].segmentType == 'Image' && selections[1][0].isSelectedAsImageSelection == true ) { diff --git a/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts b/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts index ec759cc4fa5..cc0fc316772 100644 --- a/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts +++ b/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts @@ -220,6 +220,117 @@ describe('getSelectedImage', () => { }); }); + it('get selected Image in wrapper near text', () => { + const model: ContentModelDocument = { + blockGroupType: 'Document', + blocks: [ + { + blockType: 'Paragraph', + segments: [ + { + segmentType: 'Text', + format: {}, + text: 'test', + }, + { + segmentType: 'SelectionMarker', + format: {}, + isSelected: true, + }, + { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: 'image_0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: true, + isSelected: true, + }, + { + segmentType: 'SelectionMarker', + format: {}, + isSelected: true, + }, + { + segmentType: 'Text', + format: {}, + text: 'test', + }, + ], + format: {}, + segmentFormat: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + }, + }, + ], + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: '#000000', + }, + }; + + const selections = getSelectedImage(model); + expect(selections).toEqual({ + image: { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: 'image_0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: true, + isSelected: true, + }, + paragraph: { + blockType: 'Paragraph', + segments: [ + { + segmentType: 'SelectionMarker', + format: {}, + isSelected: true, + }, + { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: 'image_0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: true, + isSelected: true, + }, + { + segmentType: 'SelectionMarker', + format: {}, + isSelected: true, + }, + ], + format: {}, + segmentFormat: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + }, + }, + }); + }); + it('do not selected Image in wrapper', () => { const model: ContentModelDocument = { blockGroupType: 'Document', From 11367de32397c0a2872e67870178d4ea5a03ff38 Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Thu, 11 Jul 2024 12:22:04 -0300 Subject: [PATCH 06/31] remove console.log --- .../lib/imageEdit/utils/getSelectedImage.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts b/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts index 212664355c3..428a91cb572 100644 --- a/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts +++ b/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts @@ -11,7 +11,6 @@ import type { ImageAndParagraph } from '../types/ImageAndParagraph'; */ export function getSelectedImage(model: ReadonlyContentModelDocument): ImageAndParagraph | null { const selections = getSelectedSegmentsAndParagraphs(model, false); - console.log(selections); if (selections.length == 1 && selections[0][0].segmentType == 'Image' && selections[0][1]) { return { image: selections[0][0], From 5365706f35a49c95b347d63ca63031962880c4fb Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Thu, 11 Jul 2024 12:34:03 -0300 Subject: [PATCH 07/31] fix test --- .../test/imageEdit/utils/getSelectedImageTest.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts b/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts index cc0fc316772..3d38e3f3fd0 100644 --- a/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts +++ b/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts @@ -296,6 +296,11 @@ describe('getSelectedImage', () => { paragraph: { blockType: 'Paragraph', segments: [ + { + segmentType: 'Text', + format: {}, + text: 'test', + }, { segmentType: 'SelectionMarker', format: {}, @@ -320,6 +325,11 @@ describe('getSelectedImage', () => { format: {}, isSelected: true, }, + { + segmentType: 'Text', + format: {}, + text: 'test', + }, ], format: {}, segmentFormat: { From 5e2b68093834c09e9bdd21d1b65632a2cf67660a Mon Sep 17 00:00:00 2001 From: Bryan Valverde U Date: Thu, 11 Jul 2024 12:22:36 -0600 Subject: [PATCH 08/31] Fix #2734 by Setting List Metadata `applyListStyleFromLevel: true` when toggling a list (#2743) * init * Tests * add missing file --- .../lib/modelApi/list/setListType.ts | 12 ++++ .../test/modelApi/list/setListTypeTest.ts | 26 ++++---- .../publicApi/list/toggleNumberingTest.ts | 1 + .../autoFormat/list/keyboardListTrigger.ts | 18 ++++++ .../list/keyboardListTriggerTest.ts | 62 ++++++++++++++++--- 5 files changed, 99 insertions(+), 20 deletions(-) diff --git a/packages/roosterjs-content-model-api/lib/modelApi/list/setListType.ts b/packages/roosterjs-content-model-api/lib/modelApi/list/setListType.ts index 977f1d17118..1144f8aa1ae 100644 --- a/packages/roosterjs-content-model-api/lib/modelApi/list/setListType.ts +++ b/packages/roosterjs-content-model-api/lib/modelApi/list/setListType.ts @@ -6,6 +6,7 @@ import { mutateBlock, normalizeContentModel, setParagraphNotImplicit, + updateListMetadata, } from 'roosterjs-content-model-dom'; import type { ContentModelListItem, @@ -121,6 +122,17 @@ export function setListType( mutateBlock(parent).blocks.splice(index, 1, newListItem); existingListItems.push(newListItem); + + const levelIndex = newListItem.levels.length - 1; + const level = mutateBlock(newListItem).levels[levelIndex]; + + if (level) { + updateListMetadata(level, metadata => + Object.assign({}, metadata, { + applyListStyleFromLevel: true, + }) + ); + } } else { existingListItems.forEach( x => (mutateBlock(x).levels[0].format.marginBottom = '0px') diff --git a/packages/roosterjs-content-model-api/test/modelApi/list/setListTypeTest.ts b/packages/roosterjs-content-model-api/test/modelApi/list/setListTypeTest.ts index 2d9c7120c63..dc55413d65b 100644 --- a/packages/roosterjs-content-model-api/test/modelApi/list/setListTypeTest.ts +++ b/packages/roosterjs-content-model-api/test/modelApi/list/setListTypeTest.ts @@ -78,7 +78,7 @@ describe('indent', () => { marginTop: undefined, textAlign: undefined, }, - dataset: {}, + dataset: { editingInfo: '{"applyListStyleFromLevel":true}' }, }, ], blocks: [para], @@ -138,7 +138,9 @@ describe('indent', () => { marginTop: '0px', textAlign: undefined, }, - dataset: {}, + dataset: { + editingInfo: '{"applyListStyleFromLevel":true}', + }, }, ], blocks: [para], @@ -364,7 +366,7 @@ describe('indent', () => { marginTop: undefined, textAlign: undefined, }, - dataset: {}, + dataset: { editingInfo: '{"applyListStyleFromLevel":true}' }, }, ], blocks: [para3], @@ -420,7 +422,7 @@ describe('indent', () => { levels: [ { listType: 'OL', - dataset: {}, + dataset: { editingInfo: '{"applyListStyleFromLevel":true}' }, format: { startNumberOverride: 1, direction: 'rtl', @@ -500,7 +502,7 @@ describe('indent', () => { levels: [ { listType: 'OL', - dataset: {}, + dataset: { editingInfo: '{"applyListStyleFromLevel":true}' }, format: { startNumberOverride: 1, direction: undefined, @@ -529,7 +531,7 @@ describe('indent', () => { levels: [ { listType: 'OL', - dataset: {}, + dataset: { editingInfo: '{"applyListStyleFromLevel":true}' }, format: { direction: undefined, marginBottom: undefined, @@ -584,7 +586,7 @@ describe('indent', () => { levels: [ { listType: 'OL', - dataset: {}, + dataset: { editingInfo: '{"applyListStyleFromLevel":true}' }, format: { startNumberOverride: 1, direction: undefined, @@ -641,7 +643,7 @@ describe('indent', () => { levels: [ { listType: 'OL', - dataset: {}, + dataset: { editingInfo: '{"applyListStyleFromLevel":true}' }, format: { startNumberOverride: 1, direction: undefined, @@ -669,7 +671,7 @@ describe('indent', () => { levels: [ { listType: 'OL', - dataset: {}, + dataset: { editingInfo: '{"applyListStyleFromLevel":true}' }, format: { startNumberOverride: undefined, direction: undefined, @@ -697,7 +699,7 @@ describe('indent', () => { levels: [ { listType: 'OL', - dataset: {}, + dataset: { editingInfo: '{"applyListStyleFromLevel":true}' }, format: { direction: undefined, marginBottom: undefined, @@ -779,7 +781,7 @@ describe('indent', () => { marginTop: undefined, textAlign: undefined, }, - dataset: {}, + dataset: { editingInfo: '{"applyListStyleFromLevel":true}' }, }, ], formatHolder: { @@ -809,7 +811,7 @@ describe('indent', () => { marginTop: undefined, textAlign: undefined, }, - dataset: {}, + dataset: { editingInfo: '{"applyListStyleFromLevel":true}' }, }, ], formatHolder: { diff --git a/packages/roosterjs-content-model-api/test/publicApi/list/toggleNumberingTest.ts b/packages/roosterjs-content-model-api/test/publicApi/list/toggleNumberingTest.ts index a2056062992..febaeab11cc 100644 --- a/packages/roosterjs-content-model-api/test/publicApi/list/toggleNumberingTest.ts +++ b/packages/roosterjs-content-model-api/test/publicApi/list/toggleNumberingTest.ts @@ -1,6 +1,7 @@ import * as setListType from '../../../lib/modelApi/list/setListType'; import { IEditor } from 'roosterjs-content-model-types'; import { toggleNumbering } from '../../../lib/publicApi/list/toggleNumbering'; + import { ContentModelDocument, ContentModelFormatter, diff --git a/packages/roosterjs-content-model-plugins/lib/autoFormat/list/keyboardListTrigger.ts b/packages/roosterjs-content-model-plugins/lib/autoFormat/list/keyboardListTrigger.ts index 14ee7c25f28..a3d707c1f7a 100644 --- a/packages/roosterjs-content-model-plugins/lib/autoFormat/list/keyboardListTrigger.ts +++ b/packages/roosterjs-content-model-plugins/lib/autoFormat/list/keyboardListTrigger.ts @@ -1,10 +1,13 @@ import { getListTypeStyle } from './getListTypeStyle'; +import { getOperationalBlocks, isBlockGroupOfType } from 'roosterjs-content-model-dom'; import { + getListAnnounceData, setListType, setModelListStartNumber, setModelListStyle, } from 'roosterjs-content-model-api'; import type { + ContentModelListItem, FormatContentModelContext, ReadonlyContentModelDocument, ShallowMutableContentModelParagraph, @@ -26,6 +29,7 @@ export function keyboardListTrigger( const { listType, styleType, index } = listStyleType; triggerList(model, listType, styleType, index); context.canUndoByBackspace = true; + setAnnounceData(model, context); return true; } @@ -48,9 +52,23 @@ const triggerList = ( isOrderedList ? { orderedStyleType: styleType, + applyListStyleFromLevel: false, } : { unorderedStyleType: styleType, + applyListStyleFromLevel: false, } ); }; +function setAnnounceData(model: ReadonlyContentModelDocument, context: FormatContentModelContext) { + const [paragraphOrListItems] = getOperationalBlocks( + model, + ['ListItem'], + [] // Set stop types to be empty so we can find list items even cross the boundary of table, then we can always operation on the list item if any + ); + + if (paragraphOrListItems && isBlockGroupOfType(paragraphOrListItems.block, 'ListItem')) { + const { path, block } = paragraphOrListItems; + context.announceData = getListAnnounceData([block, ...path]); + } +} diff --git a/packages/roosterjs-content-model-plugins/test/autoFormat/list/keyboardListTriggerTest.ts b/packages/roosterjs-content-model-plugins/test/autoFormat/list/keyboardListTriggerTest.ts index a2cb74f5401..efcc84ef7e9 100644 --- a/packages/roosterjs-content-model-plugins/test/autoFormat/list/keyboardListTriggerTest.ts +++ b/packages/roosterjs-content-model-plugins/test/autoFormat/list/keyboardListTriggerTest.ts @@ -12,7 +12,8 @@ describe('keyboardListTrigger', () => { context: FormatContentModelContext, expectedResult: boolean, shouldSearchForBullet: boolean = true, - shouldSearchForNumbering: boolean = true + shouldSearchForNumbering: boolean = true, + expectedContext?: any ) { const result = keyboardListTrigger( model, @@ -22,6 +23,9 @@ describe('keyboardListTrigger', () => { shouldSearchForNumbering ); expect(result).toBe(expectedResult); + if (expectedContext) { + expect(context).toEqual(expectedContext); + } } it('trigger numbering list', () => { @@ -49,7 +53,16 @@ describe('keyboardListTrigger', () => { }, paragraph, { canUndoByBackspace: true } as any, - true + true /* expectedResult */, + undefined /* shouldSearchForBullet */, + undefined /* shouldSearchForNumbering */, + { + canUndoByBackspace: true, + announceData: { + defaultStrings: 'announceListItemNumbering', + formatStrings: ['1'], + }, + } ); }); @@ -118,7 +131,13 @@ describe('keyboardListTrigger', () => { }, paragraph, { canUndoByBackspace: true } as any, - true + true, + undefined /* shouldSearchForBullet */, + undefined /* shouldSearchForNumbering */, + { + canUndoByBackspace: true, + announceData: { defaultStrings: 'announceListItemNumbering', formatStrings: ['2'] }, + } ); }); @@ -147,7 +166,10 @@ describe('keyboardListTrigger', () => { }, paragraph, { canUndoByBackspace: true } as any, - false + false, + undefined /* shouldSearchForBullet */, + undefined /* shouldSearchForNumbering */, + { canUndoByBackspace: true } ); }); @@ -176,7 +198,13 @@ describe('keyboardListTrigger', () => { }, paragraph, { canUndoByBackspace: true } as any, - true + true, + undefined /* shouldSearchForBullet */, + undefined /* shouldSearchForNumbering */, + { + canUndoByBackspace: true, + announceData: { defaultStrings: 'announceListItemBullet' }, + } ); }); @@ -205,7 +233,10 @@ describe('keyboardListTrigger', () => { }, paragraph, {} as any, - false + false, + undefined, + undefined, + {} ); }); @@ -384,7 +415,16 @@ describe('keyboardListTrigger', () => { }, paragraph, { canUndoByBackspace: true } as any, - true + true, + undefined /* shouldSearchForBullet */, + undefined /* shouldSearchForNumbering */, + { + canUndoByBackspace: true, + announceData: { + defaultStrings: 'announceListItemNumbering', + formatStrings: ['3'], + }, + } ); }); @@ -493,7 +533,13 @@ describe('keyboardListTrigger', () => { }, paragraph, { canUndoByBackspace: true } as any, - true + true, + undefined /* shouldSearchForBullet */, + undefined /* shouldSearchForNumbering */, + { + canUndoByBackspace: true, + announceData: { defaultStrings: 'announceListItemNumbering', formatStrings: ['A'] }, + } ); }); }); From aec6607cb235338db148a3fc938a72d884a101ef Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Thu, 11 Jul 2024 19:02:09 -0300 Subject: [PATCH 09/31] crop alternative --- .../lib/imageEdit/ImageEditPlugin.ts | 4 + .../lib/imageEdit/utils/getSelectedImage.ts | 18 +- .../utils/normalizeImageSelection.ts | 31 ++ .../imageEdit/utils/getSelectedImageTest.ts | 319 ------------------ .../utils/normalizeImageSelection.ts | 135 ++++++++ 5 files changed, 171 insertions(+), 336 deletions(-) create mode 100644 packages/roosterjs-content-model-plugins/lib/imageEdit/utils/normalizeImageSelection.ts create mode 100644 packages/roosterjs-content-model-plugins/test/imageEdit/utils/normalizeImageSelection.ts diff --git a/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts b/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts index ba856beb358..35cef77486a 100644 --- a/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts +++ b/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts @@ -9,6 +9,7 @@ import { getHTMLImageOptions } from './utils/getHTMLImageOptions'; import { getSelectedImage } from './utils/getSelectedImage'; import { getSelectedImageMetadata, updateImageEditInfo } from './utils/updateImageEditInfo'; import { ImageEditElementClass } from './types/ImageEditElementClass'; +import { normalizeImageSelection } from './utils/normalizeImageSelection'; import { Resizer } from './Resizer/resizerContext'; import { Rotator } from './Rotator/rotatorContext'; import { updateRotateHandle } from './Rotator/updateRotateHandle'; @@ -256,6 +257,9 @@ export class ImageEditPlugin implements ImageEditor, EditorPlugin { image.isSelectedAsImageSelection = shouldSelectImage; } ); + if (shouldSelectImage) { + normalizeImageSelection(previousSelectedImage); + } this.cleanInfo(); result = true; } diff --git a/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts b/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts index 428a91cb572..c517106f2b8 100644 --- a/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts +++ b/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/getSelectedImage.ts @@ -3,32 +3,16 @@ import type { ReadonlyContentModelDocument } from 'roosterjs-content-model-types import type { ImageAndParagraph } from '../types/ImageAndParagraph'; /** - * Selecting directly on the image will only capture the image segment. - * However, if the selection is made while the image is within a wrapper, it will capture the span that encloses the image. - * In the last case, the selection will be marked as <---SelectionMarker---><---Image---><---SelectionMarker--->. - * This function accounts for both scenarios. * @internal */ export function getSelectedImage(model: ReadonlyContentModelDocument): ImageAndParagraph | null { const selections = getSelectedSegmentsAndParagraphs(model, false); + if (selections.length == 1 && selections[0][0].segmentType == 'Image' && selections[0][1]) { return { image: selections[0][0], paragraph: selections[0][1], }; - } else if ( - selections.length == 3 && - selections[1][1] && - selections[1][1].segments - .filter(seg => seg.isSelected) - .every(seg => seg.segmentType == 'Image' || seg.segmentType == 'SelectionMarker') && - selections[1][0].segmentType == 'Image' && - selections[1][0].isSelectedAsImageSelection == true - ) { - return { - image: selections[1][0], - paragraph: selections[1][1], - }; } else { return null; } diff --git a/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/normalizeImageSelection.ts b/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/normalizeImageSelection.ts new file mode 100644 index 00000000000..79698c77895 --- /dev/null +++ b/packages/roosterjs-content-model-plugins/lib/imageEdit/utils/normalizeImageSelection.ts @@ -0,0 +1,31 @@ +import { mutateBlock } from 'roosterjs-content-model-dom'; +import type { ImageAndParagraph } from '../types/ImageAndParagraph'; + +/** + * Selecting directly on the image will only capture the image segment. + * However, if the selection is made while the image is within a wrapper, it will capture the span that encloses the image. + * In the last case, the selection will be marked as <---SelectionMarker---><---Image---><---SelectionMarker--->. + * To fix this behavior the extra selection markers are removed. + * @internal + */ +export function normalizeImageSelection(imageAndParagraph: ImageAndParagraph) { + const paragraph = imageAndParagraph.paragraph; + const index = paragraph.segments.indexOf(imageAndParagraph.image); + if (index > 0) { + const markerBefore = paragraph.segments[index - 1]; + const markerAfter = paragraph.segments[index + 1]; + if ( + markerBefore && + markerAfter && + markerAfter.segmentType == 'SelectionMarker' && + markerBefore.segmentType == 'SelectionMarker' && + markerAfter.isSelected && + markerBefore.isSelected + ) { + const mutatedParagraph = mutateBlock(paragraph); + mutatedParagraph.segments.splice(index - 1, 1); + mutatedParagraph.segments.splice(index, 1); + } + return imageAndParagraph; + } +} diff --git a/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts b/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts index 3d38e3f3fd0..29261a8f640 100644 --- a/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts +++ b/packages/roosterjs-content-model-plugins/test/imageEdit/utils/getSelectedImageTest.ts @@ -118,323 +118,4 @@ describe('getSelectedImage', () => { const selections = getSelectedImage(model); expect(selections).toEqual(null); }); - - it('get selected Image in wrapper', () => { - const model: ContentModelDocument = { - blockGroupType: 'Document', - blocks: [ - { - blockType: 'Paragraph', - segments: [ - { - segmentType: 'SelectionMarker', - format: {}, - isSelected: true, - }, - { - segmentType: 'Image', - src: 'test', - format: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - id: 'image_0', - maxWidth: '1800px', - }, - dataset: {}, - isSelectedAsImageSelection: true, - isSelected: true, - }, - { - segmentType: 'SelectionMarker', - format: {}, - isSelected: true, - }, - ], - format: {}, - segmentFormat: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - }, - }, - ], - format: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: '#000000', - }, - }; - - const selections = getSelectedImage(model); - expect(selections).toEqual({ - image: { - segmentType: 'Image', - src: 'test', - format: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - id: 'image_0', - maxWidth: '1800px', - }, - dataset: {}, - isSelectedAsImageSelection: true, - isSelected: true, - }, - paragraph: { - blockType: 'Paragraph', - segments: [ - { - segmentType: 'SelectionMarker', - format: {}, - isSelected: true, - }, - { - segmentType: 'Image', - src: 'test', - format: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - id: 'image_0', - maxWidth: '1800px', - }, - dataset: {}, - isSelectedAsImageSelection: true, - isSelected: true, - }, - { - segmentType: 'SelectionMarker', - format: {}, - isSelected: true, - }, - ], - format: {}, - segmentFormat: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - }, - }, - }); - }); - - it('get selected Image in wrapper near text', () => { - const model: ContentModelDocument = { - blockGroupType: 'Document', - blocks: [ - { - blockType: 'Paragraph', - segments: [ - { - segmentType: 'Text', - format: {}, - text: 'test', - }, - { - segmentType: 'SelectionMarker', - format: {}, - isSelected: true, - }, - { - segmentType: 'Image', - src: 'test', - format: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - id: 'image_0', - maxWidth: '1800px', - }, - dataset: {}, - isSelectedAsImageSelection: true, - isSelected: true, - }, - { - segmentType: 'SelectionMarker', - format: {}, - isSelected: true, - }, - { - segmentType: 'Text', - format: {}, - text: 'test', - }, - ], - format: {}, - segmentFormat: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - }, - }, - ], - format: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: '#000000', - }, - }; - - const selections = getSelectedImage(model); - expect(selections).toEqual({ - image: { - segmentType: 'Image', - src: 'test', - format: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - id: 'image_0', - maxWidth: '1800px', - }, - dataset: {}, - isSelectedAsImageSelection: true, - isSelected: true, - }, - paragraph: { - blockType: 'Paragraph', - segments: [ - { - segmentType: 'Text', - format: {}, - text: 'test', - }, - { - segmentType: 'SelectionMarker', - format: {}, - isSelected: true, - }, - { - segmentType: 'Image', - src: 'test', - format: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - id: 'image_0', - maxWidth: '1800px', - }, - dataset: {}, - isSelectedAsImageSelection: true, - isSelected: true, - }, - { - segmentType: 'SelectionMarker', - format: {}, - isSelected: true, - }, - { - segmentType: 'Text', - format: {}, - text: 'test', - }, - ], - format: {}, - segmentFormat: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - }, - }, - }); - }); - - it('do not selected Image in wrapper', () => { - const model: ContentModelDocument = { - blockGroupType: 'Document', - blocks: [ - { - blockType: 'Paragraph', - segments: [ - { - segmentType: 'SelectionMarker', - format: {}, - isSelected: true, - }, - { - segmentType: 'Image', - src: 'test', - format: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - id: 'image_0', - maxWidth: '1800px', - }, - dataset: {}, - isSelectedAsImageSelection: true, - isSelected: true, - }, - ], - format: {}, - segmentFormat: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - }, - }, - ], - format: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: '#000000', - }, - }; - - const selections = getSelectedImage(model); - expect(selections).toEqual(null); - }); - - it('do not selected Image in wrapper, if is not image selection', () => { - const model: ContentModelDocument = { - blockGroupType: 'Document', - blocks: [ - { - blockType: 'Paragraph', - segments: [ - { - segmentType: 'SelectionMarker', - format: {}, - isSelected: true, - }, - { - segmentType: 'Image', - src: 'test', - format: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - id: 'image_0', - maxWidth: '1800px', - }, - dataset: {}, - isSelectedAsImageSelection: false, - isSelected: true, - }, - { - segmentType: 'SelectionMarker', - format: {}, - isSelected: true, - }, - ], - format: {}, - segmentFormat: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: 'rgb(0, 0, 0)', - }, - }, - ], - format: { - fontFamily: 'Calibri', - fontSize: '11pt', - textColor: '#000000', - }, - }; - - const selections = getSelectedImage(model); - expect(selections).toEqual(null); - }); }); diff --git a/packages/roosterjs-content-model-plugins/test/imageEdit/utils/normalizeImageSelection.ts b/packages/roosterjs-content-model-plugins/test/imageEdit/utils/normalizeImageSelection.ts new file mode 100644 index 00000000000..22f8f732230 --- /dev/null +++ b/packages/roosterjs-content-model-plugins/test/imageEdit/utils/normalizeImageSelection.ts @@ -0,0 +1,135 @@ +import { ImageAndParagraph } from '../../../lib/imageEdit/types/ImageAndParagraph'; +import { normalizeImageSelection } from '../../../lib/imageEdit/utils/normalizeImageSelection'; +import type { ReadonlyContentModelImage } from 'roosterjs-content-model-types'; + +describe('normalizeImageSelection', () => { + it('normalize image selection', () => { + const image: ReadonlyContentModelImage = { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: 'image_0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: true, + isSelected: true, + }; + const imageAndParagraph: ImageAndParagraph = { + paragraph: { + blockType: 'Paragraph', + segments: [ + { + segmentType: 'SelectionMarker', + isSelected: true, + format: {}, + }, + image, + { + segmentType: 'SelectionMarker', + isSelected: true, + format: {}, + }, + ], + format: {}, + segmentFormat: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + }, + }, + image: image, + }; + + const result = normalizeImageSelection(imageAndParagraph); + expect(result?.paragraph).toEqual({ + blockType: 'Paragraph', + segments: [ + { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: 'image_0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: true, + isSelected: true, + }, + ], + format: {}, + segmentFormat: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + }, + }); + }); + + it('normalize image selection', () => { + const imageAndParagraph: ImageAndParagraph = { + paragraph: { + blockType: 'Paragraph', + segments: [ + { + segmentType: 'SelectionMarker', + format: {}, + }, + { + segmentType: 'Text', + text: 'test', + format: {}, + }, + { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: 'image_0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: true, + isSelected: true, + }, + { + segmentType: 'SelectionMarker', + isSelected: true, + format: {}, + }, + ], + format: {}, + segmentFormat: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + }, + }, + image: { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: 'image_0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: true, + isSelected: true, + }, + }; + + const result = normalizeImageSelection(imageAndParagraph); + expect(result).toBeUndefined(); + }); +}); From 1bf1e77bb1653f8999a91f6446ad886aa8cc2eb8 Mon Sep 17 00:00:00 2001 From: Bryan Valverde U Date: Mon, 15 Jul 2024 11:06:24 -0600 Subject: [PATCH 10/31] Fix #2733 by changing the approach to announce repeated announce messages (#2745) * init * move string to constant --- .../lib/coreApi/announce/announce.ts | 36 ++++++------------- .../corePlugin/lifecycle/LifecyclePlugin.ts | 4 +++ .../lib/utils/createAriaLiveElement.ts | 19 ++++++++++ .../test/coreApi/announce/announceTest.ts | 36 ++++++------------- .../lifecycle/LifecyclePluginTest.ts | 31 ++++++++++++++++ 5 files changed, 76 insertions(+), 50 deletions(-) create mode 100644 packages/roosterjs-content-model-core/lib/utils/createAriaLiveElement.ts diff --git a/packages/roosterjs-content-model-core/lib/coreApi/announce/announce.ts b/packages/roosterjs-content-model-core/lib/coreApi/announce/announce.ts index 2f04873238a..ad92200bd39 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/announce/announce.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/announce/announce.ts @@ -1,5 +1,8 @@ +import { createAriaLiveElement } from '../../utils/createAriaLiveElement'; import type { Announce } from 'roosterjs-content-model-types'; +const DOT_STRING = '.'; + /** * @internal * Announce the given data @@ -10,16 +13,16 @@ export const announce: Announce = (core, announceData) => { const { text, defaultStrings, formatStrings = [] } = announceData; const { announcerStringGetter } = core.lifecycle; const template = defaultStrings && announcerStringGetter?.(defaultStrings); - const textToAnnounce = formatString(template || text, formatStrings); - - if (textToAnnounce) { - let announceContainer = core.lifecycle.announceContainer; + let textToAnnounce = formatString(template || text, formatStrings); - if (!announceContainer || textToAnnounce == announceContainer.textContent) { - announceContainer?.parentElement?.removeChild(announceContainer); - announceContainer = createAriaLiveElement(core.physicalRoot.ownerDocument); + if (!core.lifecycle.announceContainer) { + core.lifecycle.announceContainer = createAriaLiveElement(core.physicalRoot.ownerDocument); + } - core.lifecycle.announceContainer = announceContainer; + if (textToAnnounce && core.lifecycle.announceContainer) { + const { announceContainer } = core.lifecycle; + if (textToAnnounce == announceContainer.textContent) { + textToAnnounce += DOT_STRING; } if (announceContainer) { @@ -41,20 +44,3 @@ function formatString(text: string | undefined, formatStrings: string[]) { return text; } - -function createAriaLiveElement(document: Document): HTMLDivElement { - const div = document.createElement('div'); - - div.style.clip = 'rect(0px, 0px, 0px, 0px)'; - div.style.clipPath = 'inset(100%)'; - div.style.height = '1px'; - div.style.overflow = 'hidden'; - div.style.position = 'absolute'; - div.style.whiteSpace = 'nowrap'; - div.style.width = '1px'; - div.ariaLive = 'assertive'; - - document.body.appendChild(div); - - return div; -} diff --git a/packages/roosterjs-content-model-core/lib/corePlugin/lifecycle/LifecyclePlugin.ts b/packages/roosterjs-content-model-core/lib/corePlugin/lifecycle/LifecyclePlugin.ts index 5e0cd991614..1e3e72b2aff 100644 --- a/packages/roosterjs-content-model-core/lib/corePlugin/lifecycle/LifecyclePlugin.ts +++ b/packages/roosterjs-content-model-core/lib/corePlugin/lifecycle/LifecyclePlugin.ts @@ -1,4 +1,5 @@ import { ChangeSource, getObjectKeys, setColor } from 'roosterjs-content-model-dom'; +import { createAriaLiveElement } from '../../utils/createAriaLiveElement'; import type { IEditor, LifecyclePluginState, @@ -74,6 +75,9 @@ class LifecyclePlugin implements PluginWithState { // Let other plugins know that we are ready this.editor.triggerEvent('editorReady', {}, true /*broadcast*/); + + // Initialize the Announce container. + this.state.announceContainer = createAriaLiveElement(editor.getDocument()); } /** diff --git a/packages/roosterjs-content-model-core/lib/utils/createAriaLiveElement.ts b/packages/roosterjs-content-model-core/lib/utils/createAriaLiveElement.ts new file mode 100644 index 00000000000..e255a8d7551 --- /dev/null +++ b/packages/roosterjs-content-model-core/lib/utils/createAriaLiveElement.ts @@ -0,0 +1,19 @@ +/** + * @internal + */ +export function createAriaLiveElement(document: Document): HTMLDivElement { + const div = document.createElement('div'); + + div.style.clip = 'rect(0px, 0px, 0px, 0px)'; + div.style.clipPath = 'inset(100%)'; + div.style.height = '1px'; + div.style.overflow = 'hidden'; + div.style.position = 'absolute'; + div.style.whiteSpace = 'nowrap'; + div.style.width = '1px'; + div.ariaLive = 'assertive'; + + document.body.appendChild(div); + + return div; +} diff --git a/packages/roosterjs-content-model-core/test/coreApi/announce/announceTest.ts b/packages/roosterjs-content-model-core/test/coreApi/announce/announceTest.ts index f614b638623..53727233613 100644 --- a/packages/roosterjs-content-model-core/test/coreApi/announce/announceTest.ts +++ b/packages/roosterjs-content-model-core/test/coreApi/announce/announceTest.ts @@ -28,9 +28,16 @@ describe('announce', () => { }); it('announce empty string', () => { + const mockedDiv = { + style: {}, + } as any; + createElementSpy.and.returnValue(mockedDiv); + announce(core, {}); - expect(createElementSpy).not.toHaveBeenCalled(); - expect(appendChildSpy).not.toHaveBeenCalled(); + + expect(createElementSpy).toHaveBeenCalled(); + expect(appendChildSpy).toHaveBeenCalled(); + expect(mockedDiv.textContent).toBeUndefined(); }); it('announce a given string', () => { @@ -180,39 +187,18 @@ describe('announce', () => { }); it('already has div with same text', () => { - const removeChildSpy = jasmine.createSpy('removeChild'); const mockedDiv = { textContent: 'test', - parentElement: { - removeChild: removeChildSpy, - }, - } as any; - const mockedDiv2 = { - style: {}, } as any; core.lifecycle.announceContainer = mockedDiv; - createElementSpy.and.returnValue(mockedDiv2); announce(core, { text: 'test', }); - expect(removeChildSpy).toHaveBeenCalledWith(mockedDiv); - expect(createElementSpy).toHaveBeenCalledWith('div'); - expect(appendChildSpy).toHaveBeenCalledWith(mockedDiv2); - expect(mockedDiv2).toEqual({ - style: { - clip: 'rect(0px, 0px, 0px, 0px)', - clipPath: 'inset(100%)', - height: '1px', - overflow: 'hidden', - position: 'absolute', - whiteSpace: 'nowrap', - width: '1px', - }, - ariaLive: 'assertive', - textContent: 'test', + expect(mockedDiv).toEqual({ + textContent: 'test.', }); }); }); diff --git a/packages/roosterjs-content-model-core/test/corePlugin/lifecycle/LifecyclePluginTest.ts b/packages/roosterjs-content-model-core/test/corePlugin/lifecycle/LifecyclePluginTest.ts index f687a441fa5..e4b48f607ce 100644 --- a/packages/roosterjs-content-model-core/test/corePlugin/lifecycle/LifecyclePluginTest.ts +++ b/packages/roosterjs-content-model-core/test/corePlugin/lifecycle/LifecyclePluginTest.ts @@ -1,13 +1,24 @@ import * as color from 'roosterjs-content-model-dom/lib/formatHandlers/utils/color'; +import * as createAriaLiveElementFile from '../../../lib/utils/createAriaLiveElement'; import { ChangeSource } from 'roosterjs-content-model-dom'; import { createLifecyclePlugin } from '../../../lib/corePlugin/lifecycle/LifecyclePlugin'; import { DarkColorHandler, IEditor } from 'roosterjs-content-model-types'; +const announceContainer = {} as Readonly; + describe('LifecyclePlugin', () => { + beforeEach(() => { + spyOn(createAriaLiveElementFile, 'createAriaLiveElement').and.returnValue( + announceContainer + ); + }); + it('init', () => { const div = document.createElement('div'); const plugin = createLifecyclePlugin({}, div); const triggerEvent = jasmine.createSpy('triggerEvent'); + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); + const state = plugin.getState(); plugin.initialize(({ @@ -15,6 +26,7 @@ describe('LifecyclePlugin', () => { getFocusedPosition: () => null, getColorManager: () => null, isDarkMode: () => false, + getDocument, })); expect(state).toEqual({ @@ -22,6 +34,7 @@ describe('LifecyclePlugin', () => { shadowEditFragment: null, styleElements: {}, announcerStringGetter: undefined, + announceContainer, }); expect(div.isContentEditable).toBeTrue(); @@ -48,6 +61,7 @@ describe('LifecyclePlugin', () => { }, div ); + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); const triggerEvent = jasmine.createSpy('triggerEvent'); const state = plugin.getState(); @@ -56,6 +70,7 @@ describe('LifecyclePlugin', () => { getFocusedPosition: () => null, getColorManager: () => null, isDarkMode: () => false, + getDocument, })); expect(state).toEqual({ @@ -63,6 +78,7 @@ describe('LifecyclePlugin', () => { shadowEditFragment: null, styleElements: {}, announcerStringGetter: mockedAnnouncerStringGetter, + announceContainer, }); expect(div.isContentEditable).toBeTrue(); @@ -79,12 +95,14 @@ describe('LifecyclePlugin', () => { div.contentEditable = 'true'; const plugin = createLifecyclePlugin({}, div); const triggerEvent = jasmine.createSpy('triggerEvent'); + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); plugin.initialize(({ triggerEvent, getFocusedPosition: () => null, getColorManager: () => null, isDarkMode: () => false, + getDocument, })); expect(div.isContentEditable).toBeTrue(); @@ -101,12 +119,14 @@ describe('LifecyclePlugin', () => { div.contentEditable = 'false'; const plugin = createLifecyclePlugin({}, div); const triggerEvent = jasmine.createSpy('triggerEvent'); + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); plugin.initialize(({ triggerEvent, getFocusedPosition: () => null, getColorManager: () => null, isDarkMode: () => false, + getDocument, })); expect(div.isContentEditable).toBeFalse(); @@ -124,12 +144,14 @@ describe('LifecyclePlugin', () => { const triggerEvent = jasmine.createSpy('triggerEvent'); const state = plugin.getState(); const mockedDarkColorHandler = 'HANDLER' as any; + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); const setColorSpy = spyOn(color, 'setColor'); plugin.initialize(({ triggerEvent, getColorManager: () => mockedDarkColorHandler, + getDocument, })); expect(setColorSpy).toHaveBeenCalledTimes(2); @@ -139,6 +161,7 @@ describe('LifecyclePlugin', () => { shadowEditFragment: null, styleElements: {}, announcerStringGetter: undefined, + announceContainer, }); plugin.onPluginEvent({ @@ -156,12 +179,14 @@ describe('LifecyclePlugin', () => { const triggerEvent = jasmine.createSpy('triggerEvent'); const state = plugin.getState(); const mockedDarkColorHandler = 'HANDLER' as any; + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); const setColorSpy = spyOn(color, 'setColor'); plugin.initialize(({ triggerEvent, getColorManager: () => mockedDarkColorHandler, + getDocument, })); expect(setColorSpy).toHaveBeenCalledTimes(2); @@ -171,6 +196,7 @@ describe('LifecyclePlugin', () => { shadowEditFragment: null, styleElements: {}, announcerStringGetter: undefined, + announceContainer, }); const mockedIsDarkColor = 'Dark' as any; @@ -208,6 +234,7 @@ describe('LifecyclePlugin', () => { div ); const triggerEvent = jasmine.createSpy('triggerEvent'); + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); const state = plugin.getState(); const mockedDarkColorHandler = 'HANDLER' as any; @@ -216,6 +243,7 @@ describe('LifecyclePlugin', () => { plugin.initialize(({ triggerEvent, getColorManager: () => mockedDarkColorHandler, + getDocument, })); expect(setColorSpy).toHaveBeenCalledTimes(0); @@ -225,6 +253,7 @@ describe('LifecyclePlugin', () => { shadowEditFragment: null, styleElements: {}, announcerStringGetter: undefined, + announceContainer, }); const mockedIsDarkColor = 'Dark' as any; @@ -242,10 +271,12 @@ describe('LifecyclePlugin', () => { it('Dispose plugin and clean up style nodes', () => { const div = document.createElement('div'); const plugin = createLifecyclePlugin({}, div); + const getDocument = jasmine.createSpy('getDocument').and.returnValue(document); plugin.initialize({ getColorManager: jasmine.createSpy(), triggerEvent: jasmine.createSpy(), + getDocument, }); const state = plugin.getState(); From a4fa3ff9c9f8ca1c32b27fa454336ecb43c1680f Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Tue, 16 Jul 2024 15:19:22 -0400 Subject: [PATCH 11/31] image selection --- .../lib/coreApi/setDOMSelection/setDOMSelection.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts b/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts index 39cdd9828be..c08de55b70b 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts @@ -27,7 +27,12 @@ const DEFAULT_SELECTION_BORDER_COLOR = '#DB626C'; export const setDOMSelection: SetDOMSelection = (core, selection, skipSelectionChangedEvent) => { const existingSelection = core.api.getDOMSelection(core); - if (existingSelection && selection && areSameSelections(existingSelection, selection)) { + if ( + existingSelection?.type !== 'image' && + existingSelection && + selection && + areSameSelections(existingSelection, selection) + ) { return; } From 5c236885a8e0ff3208d932ec2e8fbd70b16c0223 Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Tue, 16 Jul 2024 16:12:05 -0400 Subject: [PATCH 12/31] WIP --- .../lib/coreApi/setDOMSelection/setDOMSelection.ts | 7 +------ .../lib/corePlugin/selection/SelectionPlugin.ts | 1 + .../lib/editor/ExperimentalFeature.ts | 5 +++++ 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts b/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts index c08de55b70b..39cdd9828be 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts @@ -27,12 +27,7 @@ const DEFAULT_SELECTION_BORDER_COLOR = '#DB626C'; export const setDOMSelection: SetDOMSelection = (core, selection, skipSelectionChangedEvent) => { const existingSelection = core.api.getDOMSelection(core); - if ( - existingSelection?.type !== 'image' && - existingSelection && - selection && - areSameSelections(existingSelection, selection) - ) { + if (existingSelection && selection && areSameSelections(existingSelection, selection)) { return; } diff --git a/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts b/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts index b9245e0c0f9..7ced8fa2896 100644 --- a/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts +++ b/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts @@ -165,6 +165,7 @@ class SelectionPlugin implements PluginWithState { // Image selection if ( + editor.isExperimentalFeatureEnabled('PreserveImageSelection') && selection?.type == 'image' && (rawEvent.button == MouseLeftButton || (rawEvent.button == MouseRightButton && diff --git a/packages/roosterjs-content-model-types/lib/editor/ExperimentalFeature.ts b/packages/roosterjs-content-model-types/lib/editor/ExperimentalFeature.ts index bdd12a56d22..97efedcbfe3 100644 --- a/packages/roosterjs-content-model-types/lib/editor/ExperimentalFeature.ts +++ b/packages/roosterjs-content-model-types/lib/editor/ExperimentalFeature.ts @@ -9,3 +9,8 @@ export type ExperimentalFeature = * and use cached element when write back if it is not changed. */ 'PersistCache'; + +/** + * Workaround for the Legacy Image Edit + */ +'PreserveImageSelection'; From 89f1ca701b3a0a44acd7fb8d2ae8de445a33de9f Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Tue, 16 Jul 2024 17:06:00 -0400 Subject: [PATCH 13/31] WIP --- .../lib/editor/ExperimentalFeature.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/roosterjs-content-model-types/lib/editor/ExperimentalFeature.ts b/packages/roosterjs-content-model-types/lib/editor/ExperimentalFeature.ts index 97efedcbfe3..e9f0568b65b 100644 --- a/packages/roosterjs-content-model-types/lib/editor/ExperimentalFeature.ts +++ b/packages/roosterjs-content-model-types/lib/editor/ExperimentalFeature.ts @@ -8,9 +8,8 @@ export type ExperimentalFeature = * When this feature is enabled, we will persist a content model in memory as long as we can, * and use cached element when write back if it is not changed. */ - 'PersistCache'; - -/** - * Workaround for the Legacy Image Edit - */ -'PreserveImageSelection'; + | 'PersistCache' + /** + * Workaround for the Legacy Image Edit + */ + | 'PreserveImageSelection'; From a0db66352e17e542cf37ddcb923339866405117c Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Tue, 16 Jul 2024 17:52:32 -0400 Subject: [PATCH 14/31] WIP --- .../lib/coreApi/setDOMSelection/setDOMSelection.ts | 7 ++++++- .../lib/corePlugin/selection/SelectionPlugin.ts | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts b/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts index 39cdd9828be..aca3b350ac7 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts @@ -27,7 +27,12 @@ const DEFAULT_SELECTION_BORDER_COLOR = '#DB626C'; export const setDOMSelection: SetDOMSelection = (core, selection, skipSelectionChangedEvent) => { const existingSelection = core.api.getDOMSelection(core); - if (existingSelection && selection && areSameSelections(existingSelection, selection)) { + if ( + !core.experimentalFeatures.indexOf('PreserveImageSelections') && + existingSelection && + selection && + areSameSelections(existingSelection, selection) + ) { return; } diff --git a/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts b/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts index 7ced8fa2896..2c9a72e1de3 100644 --- a/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts +++ b/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts @@ -165,7 +165,7 @@ class SelectionPlugin implements PluginWithState { // Image selection if ( - editor.isExperimentalFeatureEnabled('PreserveImageSelection') && + !editor.isExperimentalFeatureEnabled('PreserveImageSelection') && selection?.type == 'image' && (rawEvent.button == MouseLeftButton || (rawEvent.button == MouseRightButton && From 9131b75a5b15eee391914f67a7906556e5f1946e Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Wed, 17 Jul 2024 09:15:07 -0400 Subject: [PATCH 15/31] WIP --- .../setDOMSelection/setDOMSelection.ts | 7 +- .../corePlugin/selection/SelectionPlugin.ts | 88 +++++++++++++++---- .../lib/editor/ExperimentalFeature.ts | 2 +- 3 files changed, 71 insertions(+), 26 deletions(-) diff --git a/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts b/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts index aca3b350ac7..39cdd9828be 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts @@ -27,12 +27,7 @@ const DEFAULT_SELECTION_BORDER_COLOR = '#DB626C'; export const setDOMSelection: SetDOMSelection = (core, selection, skipSelectionChangedEvent) => { const existingSelection = core.api.getDOMSelection(core); - if ( - !core.experimentalFeatures.indexOf('PreserveImageSelections') && - existingSelection && - selection && - areSameSelections(existingSelection, selection) - ) { + if (existingSelection && selection && areSameSelections(existingSelection, selection)) { return; } diff --git a/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts b/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts index 2c9a72e1de3..003052f7d69 100644 --- a/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts +++ b/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts @@ -22,9 +22,11 @@ import type { TableSelectionInfo, TableCellCoordinate, RangeSelection, + MouseUpEvent, } from 'roosterjs-content-model-types'; const MouseLeftButton = 0; +const MouseMiddleButton = 1; const MouseRightButton = 2; const Up = 'ArrowUp'; const Down = 'ArrowDown'; @@ -140,7 +142,7 @@ class SelectionPlugin implements PluginWithState { break; case 'mouseUp': - this.onMouseUp(); + this.onMouseUp(this.editor, event); break; case 'keyDown': @@ -164,8 +166,39 @@ class SelectionPlugin implements PluginWithState { let image: HTMLImageElement | null; // Image selection + if (editor.isExperimentalFeatureEnabled('LegacyImageSelection')) { + if ( + rawEvent.button === MouseRightButton && + (image = + this.getClickingImage(rawEvent) ?? + this.getContainedTargetImage(rawEvent, selection)) && + image.isContentEditable + ) { + this.selectImageWithRange(image, rawEvent); + return; + } else if (selection?.type == 'image' && selection.image !== rawEvent.target) { + this.selectBeforeOrAfterElement(editor, selection.image); + return; + } + } else { + if ( + (image = + this.getClickingImage(rawEvent) ?? + this.getContainedTargetImage(rawEvent, selection)) && + image.isContentEditable + ) { + this.setDOMSelection( + { + type: 'image', + image: image, + }, + null + ); + return; + } + } + if ( - !editor.isExperimentalFeatureEnabled('PreserveImageSelection') && selection?.type == 'image' && (rawEvent.button == MouseLeftButton || (rawEvent.button == MouseRightButton && @@ -175,22 +208,6 @@ class SelectionPlugin implements PluginWithState { this.setDOMSelection(null /*domSelection*/, null /*tableSelection*/); } - if ( - (image = - this.getClickingImage(rawEvent) ?? - this.getContainedTargetImage(rawEvent, selection)) && - image.isContentEditable - ) { - this.setDOMSelection( - { - type: 'image', - image: image, - }, - null - ); - return; - } - // Table selection if (selection?.type == 'table' && rawEvent.button == MouseLeftButton) { this.setDOMSelection(null /*domSelection*/, null /*tableSelection*/); @@ -229,6 +246,25 @@ class SelectionPlugin implements PluginWithState { } } + private selectImageWithRange(image: HTMLImageElement, event: Event) { + const range = image.ownerDocument.createRange(); + range.selectNode(image); + + const domSelection = this.editor?.getDOMSelection(); + if (domSelection?.type == 'image' && image == domSelection.image) { + event.preventDefault(); + } else { + this.setDOMSelection( + { + type: 'range', + isReverted: false, + range, + }, + null + ); + } + } + private onMouseMove = (event: Event) => { if (this.editor && this.state.tableSelection) { const hasTableSelection = !!this.state.tableSelection.lastCo; @@ -289,7 +325,21 @@ class SelectionPlugin implements PluginWithState { } }; - private onMouseUp() { + private onMouseUp(editor: IEditor, event: MouseUpEvent) { + let image: HTMLImageElement | null; + + if ( + editor.isExperimentalFeatureEnabled('LegacyImageSelection') && + (image = this.getClickingImage(event.rawEvent)) && + image.isContentEditable && + event.rawEvent.button != MouseMiddleButton && + (event.rawEvent.button == + MouseRightButton /* it's not possible to drag using right click */ || + event.isClicking) + ) { + this.selectImageWithRange(image, event.rawEvent); + } + this.detachMouseEvent(); } diff --git a/packages/roosterjs-content-model-types/lib/editor/ExperimentalFeature.ts b/packages/roosterjs-content-model-types/lib/editor/ExperimentalFeature.ts index e9f0568b65b..15159013fd9 100644 --- a/packages/roosterjs-content-model-types/lib/editor/ExperimentalFeature.ts +++ b/packages/roosterjs-content-model-types/lib/editor/ExperimentalFeature.ts @@ -12,4 +12,4 @@ export type ExperimentalFeature = /** * Workaround for the Legacy Image Edit */ - | 'PreserveImageSelection'; + | 'LegacyImageSelection'; From e1ff0f2b6704adf9a2e7ec3d88c6db5a772e79a9 Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Wed, 17 Jul 2024 10:24:04 -0400 Subject: [PATCH 16/31] legacy selection --- .../corePlugin/selection/SelectionPlugin.ts | 22 +++++++++++-------- .../selection/SelectionPluginTest.ts | 9 ++++++++ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts b/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts index 003052f7d69..ad491c51885 100644 --- a/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts +++ b/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts @@ -181,6 +181,10 @@ class SelectionPlugin implements PluginWithState { return; } } else { + if (selection?.type == 'image' && rawEvent.button == MouseLeftButton) { + this.setDOMSelection(null /*domSelection*/, null /*tableSelection*/); + } + if ( (image = this.getClickingImage(rawEvent) ?? @@ -196,16 +200,16 @@ class SelectionPlugin implements PluginWithState { ); return; } - } - if ( - selection?.type == 'image' && - (rawEvent.button == MouseLeftButton || - (rawEvent.button == MouseRightButton && - !this.getClickingImage(rawEvent) && - !this.getContainedTargetImage(rawEvent, selection))) - ) { - this.setDOMSelection(null /*domSelection*/, null /*tableSelection*/); + if ( + selection?.type == 'image' && + (rawEvent.button == MouseLeftButton || + (rawEvent.button == MouseRightButton && + !this.getClickingImage(rawEvent) && + !this.getContainedTargetImage(rawEvent, selection))) + ) { + this.setDOMSelection(null /*domSelection*/, null /*tableSelection*/); + } } // Table selection diff --git a/packages/roosterjs-content-model-core/test/corePlugin/selection/SelectionPluginTest.ts b/packages/roosterjs-content-model-core/test/corePlugin/selection/SelectionPluginTest.ts index e2243a23ec1..e2584338b5d 100644 --- a/packages/roosterjs-content-model-core/test/corePlugin/selection/SelectionPluginTest.ts +++ b/packages/roosterjs-content-model-core/test/corePlugin/selection/SelectionPluginTest.ts @@ -370,6 +370,9 @@ describe('SelectionPlugin handle image selection', () => { getColorManager: () => ({ getDarkColor: (color: string) => `${DEFAULT_DARK_COLOR_SUFFIX_COLOR}${color}`, }), + isExperimentalFeatureEnabled: () => { + return false; + }, } as any; plugin = createSelectionPlugin({}); plugin.initialize(editor); @@ -765,6 +768,9 @@ describe('SelectionPlugin handle table selection', () => { } }, announce: announceSpy, + isExperimentalFeatureEnabled: () => { + return false; + }, } as any; plugin = createSelectionPlugin({}); plugin.initialize(editor); @@ -2246,6 +2252,9 @@ describe('SelectionPlugin on Safari', () => { getColorManager: () => ({ getDarkColor: (color: string) => `${DEFAULT_DARK_COLOR_SUFFIX_COLOR}${color}`, }), + isExperimentalFeatureEnabled: () => { + return false; + }, } as any) as IEditor; }); From c67f409d8be0267fd2c7386196c953ef2c529b54 Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Wed, 17 Jul 2024 10:32:12 -0400 Subject: [PATCH 17/31] fix --- .../corePlugin/selection/SelectionPlugin.ts | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts b/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts index ad491c51885..d2aaf74b2be 100644 --- a/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts +++ b/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts @@ -181,7 +181,13 @@ class SelectionPlugin implements PluginWithState { return; } } else { - if (selection?.type == 'image' && rawEvent.button == MouseLeftButton) { + if ( + selection?.type == 'image' && + (rawEvent.button == MouseLeftButton || + (rawEvent.button == MouseRightButton && + !this.getClickingImage(rawEvent) && + !this.getContainedTargetImage(rawEvent, selection))) + ) { this.setDOMSelection(null /*domSelection*/, null /*tableSelection*/); } @@ -200,16 +206,6 @@ class SelectionPlugin implements PluginWithState { ); return; } - - if ( - selection?.type == 'image' && - (rawEvent.button == MouseLeftButton || - (rawEvent.button == MouseRightButton && - !this.getClickingImage(rawEvent) && - !this.getContainedTargetImage(rawEvent, selection))) - ) { - this.setDOMSelection(null /*domSelection*/, null /*tableSelection*/); - } } // Table selection From 9546b4e967c527105d15d161db1171ea8ad0ec21 Mon Sep 17 00:00:00 2001 From: Bryan Valverde U Date: Thu, 18 Jul 2024 08:22:19 -0600 Subject: [PATCH 18/31] Create getSafeIdSelector and replace all Id selectors with it. (#2747) * init * Add other occurence --- .../restoreSnapshotSelection.ts | 7 +- .../setDOMSelection/setDOMSelection.ts | 25 ++- .../coreApi/setEditorStyle/ensureUniqueId.ts | 4 +- .../coreApi/setEditorStyle/setEditorStyle.ts | 5 +- .../setDOMSelection/setDOMSelectionTest.ts | 165 +++++++++++++++++- .../setEditorStyle/ensureUniqueIdTest.ts | 15 +- .../lib/domUtils/getSafeIdSelector.ts | 20 +++ .../roosterjs-content-model-dom/lib/index.ts | 1 + .../test/domUtils/getSafeIdSelectorTest.ts | 29 +++ .../lib/imageEdit/ImageEditPlugin.ts | 4 +- .../test/imageEdit/ImageEditPluginTest.ts | 51 ++++++ 11 files changed, 309 insertions(+), 17 deletions(-) create mode 100644 packages/roosterjs-content-model-dom/lib/domUtils/getSafeIdSelector.ts create mode 100644 packages/roosterjs-content-model-dom/test/domUtils/getSafeIdSelectorTest.ts diff --git a/packages/roosterjs-content-model-core/lib/coreApi/restoreUndoSnapshot/restoreSnapshotSelection.ts b/packages/roosterjs-content-model-core/lib/coreApi/restoreUndoSnapshot/restoreSnapshotSelection.ts index 266400b67ff..b3c9c9bf732 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/restoreUndoSnapshot/restoreSnapshotSelection.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/restoreUndoSnapshot/restoreSnapshotSelection.ts @@ -1,5 +1,6 @@ -import type { DOMSelection, EditorCore, Snapshot } from 'roosterjs-content-model-types'; import { getPositionFromPath } from './getPositionFromPath'; +import { getSafeIdSelector } from 'roosterjs-content-model-dom'; +import type { DOMSelection, EditorCore, Snapshot } from 'roosterjs-content-model-types'; /** * @internal @@ -29,7 +30,7 @@ export function restoreSnapshotSelection(core: EditorCore, snapshot: Snapshot) { break; case 'table': const table = physicalRoot.querySelector( - '#' + snapshotSelection.tableId + getSafeIdSelector(snapshotSelection.tableId) ) as HTMLTableElement; if (table) { @@ -45,7 +46,7 @@ export function restoreSnapshotSelection(core: EditorCore, snapshot: Snapshot) { break; case 'image': const image = physicalRoot.querySelector( - '#' + snapshotSelection.imageId + getSafeIdSelector(snapshotSelection.imageId) ) as HTMLImageElement; if (image) { diff --git a/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts b/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts index 39cdd9828be..4b08be3e861 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts @@ -3,7 +3,12 @@ import { areSameSelections } from '../../corePlugin/cache/areSameSelections'; import { ensureUniqueId } from '../setEditorStyle/ensureUniqueId'; import { findLastedCoInMergedCell } from './findLastedCoInMergedCell'; import { findTableCellElement } from './findTableCellElement'; -import { isNodeOfType, parseTableCells, toArray } from 'roosterjs-content-model-dom'; +import { + getSafeIdSelector, + isNodeOfType, + parseTableCells, + toArray, +} from 'roosterjs-content-model-dom'; import type { ParsedTable, SelectionChangedEvent, @@ -59,7 +64,7 @@ export const setDOMSelection: SetDOMSelection = (core, selection, skipSelectionC `outline-style:solid!important; outline-color:${ imageSelectionColor || DEFAULT_SELECTION_BORDER_COLOR }!important;`, - [`#${ensureUniqueId(image, IMAGE_ID)}`] + [getSafeIdSelector(ensureUniqueId(image, IMAGE_ID))] ); core.api.setEditorStyle( core, @@ -105,13 +110,21 @@ export const setDOMSelection: SetDOMSelection = (core, selection, skipSelectionC }; const tableId = ensureUniqueId(table, TABLE_ID); + const tableSelector = getSafeIdSelector(tableId); + const tableSelectors = firstCell.row == 0 && firstCell.col == 0 && lastCell.row == parsedTable.length - 1 && lastCell.col == (parsedTable[lastCell.row]?.length ?? 0) - 1 - ? [`#${tableId}`, `#${tableId} *`] - : handleTableSelected(parsedTable, tableId, table, firstCell, lastCell); + ? [tableSelector, `${tableSelector} *`] + : handleTableSelected( + parsedTable, + tableSelector, + table, + firstCell, + lastCell + ); core.selection.selection = selection; @@ -163,7 +176,7 @@ export const setDOMSelection: SetDOMSelection = (core, selection, skipSelectionC function handleTableSelected( parsedTable: ParsedTable, - tableId: string, + tableSelector: string, table: HTMLTableElement, firstCell: TableCellCoordinate, lastCell: TableCellCoordinate @@ -214,7 +227,7 @@ function handleTableSelected( cellIndex >= firstCell.col && cellIndex <= lastCell.col ) { - const selector = `#${tableId}${middleElSelector} tr:nth-child(${currentRow})>${cell.tagName}:nth-child(${tdCount})`; + const selector = `${tableSelector}${middleElSelector} tr:nth-child(${currentRow})>${cell.tagName}:nth-child(${tdCount})`; selectors.push(selector, selector + ' *'); } diff --git a/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/ensureUniqueId.ts b/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/ensureUniqueId.ts index c52e9f0bc47..00bb8aaeabb 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/ensureUniqueId.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/ensureUniqueId.ts @@ -1,3 +1,5 @@ +import { getSafeIdSelector } from 'roosterjs-content-model-dom'; + /** * @internal */ @@ -7,7 +9,7 @@ export function ensureUniqueId(element: HTMLElement, idPrefix: string): string { const doc = element.ownerDocument; let i = 0; - while (!element.id || doc.querySelectorAll(`[id="${element.id}"]`).length > 1) { + while (!element.id || doc.querySelectorAll(getSafeIdSelector(element.id)).length > 1) { element.id = idPrefix + '_' + i++; } diff --git a/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/setEditorStyle.ts b/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/setEditorStyle.ts index a26a905e1f4..23255fa19b7 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/setEditorStyle.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/setEditorStyle/setEditorStyle.ts @@ -1,4 +1,5 @@ import { ensureUniqueId } from './ensureUniqueId'; +import { getSafeIdSelector } from 'roosterjs-content-model-dom'; import type { SetEditorStyle } from 'roosterjs-content-model-types'; const MAX_RULE_SELECTOR_LENGTH = 9000; @@ -34,7 +35,9 @@ export const setEditorStyle: SetEditorStyle = ( } if (cssRule) { - const rootSelector = '#' + ensureUniqueId(core.physicalRoot, CONTENT_DIV_ID); + const rootSelector = getSafeIdSelector( + ensureUniqueId(core.physicalRoot, CONTENT_DIV_ID) + ); const selectors = !subSelectors ? [rootSelector] : typeof subSelectors === 'string' diff --git a/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts b/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts index 26f8537f3c9..04da3ead0eb 100644 --- a/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts +++ b/packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts @@ -529,7 +529,7 @@ describe('setDOMSelection', () => { createRangeSpy.and.returnValue(mockedRange); querySelectorAllSpy.and.callFake(selector => { - return selector == '[id="image_0"]' ? ['', ''] : ['']; + return selector == '#image_0' ? ['', ''] : ['']; }); hasFocusSpy.and.returnValue(false); @@ -573,6 +573,127 @@ describe('setDOMSelection', () => { ['*::selection'] ); }); + + it('image selection with duplicated and unsupported id', () => { + const mockedSelection = { + type: 'image', + image: mockedImage, + } as any; + const selectNodeSpy = jasmine.createSpy('selectNode'); + const collapseSpy = jasmine.createSpy('collapse'); + const mockedRange = { + selectNode: selectNodeSpy, + collapse: collapseSpy, + }; + + mockedImage.id = '0_image_0'; + createRangeSpy.and.returnValue(mockedRange); + + querySelectorAllSpy.and.callFake(selector => { + return selector == '[id="0_image_0"]' ? ['', ''] : ['']; + }); + hasFocusSpy.and.returnValue(false); + + setDOMSelection(core, mockedSelection); + + expect(core.selection).toEqual({ + skipReselectOnFocus: undefined, + selection: mockedSelection, + imageSelectionBorderColor: DEFAULT_SELECTION_BORDER_COLOR, + tableCellSelectionBackgroundColor: DEFAULT_TABLE_CELL_SELECTION_BACKGROUND_COLOR, + } as any); + expect(triggerEventSpy).toHaveBeenCalledWith( + core, + { + eventType: 'selectionChanged', + newSelection: mockedSelection, + }, + true + ); + expect(selectNodeSpy).toHaveBeenCalledWith(mockedImage); + expect(collapseSpy).not.toHaveBeenCalledWith(); + expect(addRangeToSelectionSpy).toHaveBeenCalledWith(doc, mockedRange, undefined); + expect(setEditorStyleSpy).toHaveBeenCalledTimes(5); + expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelection', null); + expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelectionHideCursor', null); + expect(setEditorStyleSpy).toHaveBeenCalledWith( + core, + '_DOMSelectionHideSelection', + null + ); + expect(setEditorStyleSpy).toHaveBeenCalledWith( + core, + '_DOMSelection', + 'outline-style:solid!important; outline-color:#DB626C!important;', + ['[id="0_image_0_0"]'] + ); + expect(setEditorStyleSpy).toHaveBeenCalledWith( + core, + '_DOMSelectionHideSelection', + 'background-color: transparent !important;', + ['*::selection'] + ); + }); + + it('image selection with unsupported id', () => { + mockedImage.id = '0'; + const mockedSelection = { + type: 'image', + image: mockedImage, + } as any; + const selectNodeSpy = jasmine.createSpy('selectNode'); + const collapseSpy = jasmine.createSpy('collapse'); + const mockedRange = { + selectNode: selectNodeSpy, + collapse: collapseSpy, + }; + + createRangeSpy.and.returnValue(mockedRange); + + querySelectorAllSpy.and.returnValue([]); + hasFocusSpy.and.returnValue(false); + + setDOMSelection(core, mockedSelection); + + expect(core.selection).toEqual({ + skipReselectOnFocus: undefined, + selection: mockedSelection, + imageSelectionBorderColor: DEFAULT_SELECTION_BORDER_COLOR, + tableCellSelectionBackgroundColor: DEFAULT_TABLE_CELL_SELECTION_BACKGROUND_COLOR, + } as any); + expect(triggerEventSpy).toHaveBeenCalledWith( + core, + { + eventType: 'selectionChanged', + newSelection: mockedSelection, + }, + true + ); + expect(selectNodeSpy).toHaveBeenCalledWith(mockedImage); + expect(collapseSpy).not.toHaveBeenCalledWith(); + expect(addRangeToSelectionSpy).toHaveBeenCalledWith(doc, mockedRange, undefined); + expect(mockedImage.id).toBe('0'); + expect(setEditorStyleSpy).toHaveBeenCalledTimes(5); + expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelection', null); + expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelectionHideCursor', null); + expect(setEditorStyleSpy).toHaveBeenCalledWith( + core, + '_DOMSelectionHideSelection', + null + ); + expect(setEditorStyleSpy).toHaveBeenCalledWith( + core, + '_DOMSelection', + 'outline-style:solid!important; outline-color:#DB626C!important;', + ['[id="0"]'] + ); + expect(setEditorStyleSpy).toHaveBeenCalledWith( + core, + '_DOMSelectionHideSelection', + 'background-color: transparent !important;', + ['*::selection'] + ); + }); }); describe('Table selection', () => { @@ -634,7 +755,8 @@ describe('setDOMSelection', () => { lastRow: number, result: string[], selectionColor?: string, - expectedDarkSelectionColor?: string + expectedDarkSelectionColor?: string, + expectedId?: string ) { const mockedSelection = { type: 'table', @@ -676,7 +798,7 @@ describe('setDOMSelection', () => { }, true ); - expect(mockedTable.id).toBe('table_0'); + expect(mockedTable.id).toBe(expectedId || 'table_0'); expect(setEditorStyleSpy).toHaveBeenCalledTimes(5); expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelection', null); expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelectionHideCursor', null); @@ -711,6 +833,27 @@ describe('setDOMSelection', () => { ]); }); + it('Select Table Cells TR under Table Tag with unsupportedId', () => { + const table = buildTable(true); + table.id = '0'; + runTest( + table, + 1, + 0, + 1, + 1, + [ + '[id="0"]>TBODY> tr:nth-child(1)>TD:nth-child(2)', + '[id="0"]>TBODY> tr:nth-child(1)>TD:nth-child(2) *', + '[id="0"]>TBODY> tr:nth-child(2)>TD:nth-child(2)', + '[id="0"]>TBODY> tr:nth-child(2)>TD:nth-child(2) *', + ], + undefined /* selectionColor */, + undefined /* expectedDarkSelectionColor */, + '0' + ); + }); + it('Select Table Cells TBODY', () => { runTest(buildTable(false), 0, 0, 0, 1, [ '#table_0> tr:nth-child(1)>TD:nth-child(1)', @@ -891,6 +1034,22 @@ describe('setDOMSelection', () => { ]); }); + it('Select All with unsupported Id', () => { + const table = buildTable(true /* tbody */, false, false); + table.id = '0'; + runTest( + table, + 0, + 0, + 1, + 1, + ['[id="0"]', '[id="0"] *'], + undefined /* selectionColor */, + undefined /* expectedDarkSelectionColor */, + '0' + ); + }); + it('Select All with custom selection color', () => { const selectionColor = 'red'; core.selection.tableCellSelectionBackgroundColor = selectionColor; diff --git a/packages/roosterjs-content-model-core/test/coreApi/setEditorStyle/ensureUniqueIdTest.ts b/packages/roosterjs-content-model-core/test/coreApi/setEditorStyle/ensureUniqueIdTest.ts index 0237af7ef8e..499ceee73fb 100644 --- a/packages/roosterjs-content-model-core/test/coreApi/setEditorStyle/ensureUniqueIdTest.ts +++ b/packages/roosterjs-content-model-core/test/coreApi/setEditorStyle/ensureUniqueIdTest.ts @@ -38,13 +38,26 @@ describe('ensureUniqueId', () => { id: 'dup', } as any; querySelectorAllSpy.and.callFake((selector: string) => - selector == '[id="dup"]' ? [{}, {}] : [] + selector == '#dup' ? [{}, {}] : [] ); const result = ensureUniqueId(element, 'prefix'); expect(result).toBe('dup_0'); }); + it('Has duplicated and unsupported id', () => { + const element = { + ownerDocument: doc, + id: '0dup', + } as any; + querySelectorAllSpy.and.callFake((selector: string) => + selector == '[id="0dup"]' ? [{}, {}] : [] + ); + const result = ensureUniqueId(element, 'prefix'); + + expect(result).toBe('0dup_0'); + }); + it('Should not throw when element id starts with number', () => { const element = { ownerDocument: doc, diff --git a/packages/roosterjs-content-model-dom/lib/domUtils/getSafeIdSelector.ts b/packages/roosterjs-content-model-dom/lib/domUtils/getSafeIdSelector.ts new file mode 100644 index 00000000000..941c42f1c75 --- /dev/null +++ b/packages/roosterjs-content-model-dom/lib/domUtils/getSafeIdSelector.ts @@ -0,0 +1,20 @@ +const StartsWithUnsupportedCharacter = /^[.|\-|_|\d]/; + +/** + * Returns a safe Id to use in Native APIs. + * IDs that start with number or hyphen can throw errors if used. + * @see https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id + * @param id + * @returns + */ +export function getSafeIdSelector(id: string) { + if (!id) { + return id; + } + + if (id.match(StartsWithUnsupportedCharacter)) { + return `[id="${id}"]`; + } else { + return `#${id}`; + } +} diff --git a/packages/roosterjs-content-model-dom/lib/index.ts b/packages/roosterjs-content-model-dom/lib/index.ts index 10dc2a755ef..12077de7df0 100644 --- a/packages/roosterjs-content-model-dom/lib/index.ts +++ b/packages/roosterjs-content-model-dom/lib/index.ts @@ -19,6 +19,7 @@ export { updateMetadata, getMetadata, hasMetadata } from './modelApi/metadata/up export { isNodeOfType } from './domUtils/isNodeOfType'; export { isElementOfType } from './domUtils/isElementOfType'; export { getObjectKeys } from './domUtils/getObjectKeys'; +export { getSafeIdSelector } from './domUtils/getSafeIdSelector'; export { toArray } from './domUtils/toArray'; export { moveChildNodes, wrapAllChildNodes } from './domUtils/moveChildNodes'; export { wrap } from './domUtils/wrap'; diff --git a/packages/roosterjs-content-model-dom/test/domUtils/getSafeIdSelectorTest.ts b/packages/roosterjs-content-model-dom/test/domUtils/getSafeIdSelectorTest.ts new file mode 100644 index 00000000000..34afbbb072f --- /dev/null +++ b/packages/roosterjs-content-model-dom/test/domUtils/getSafeIdSelectorTest.ts @@ -0,0 +1,29 @@ +import { getSafeIdSelector } from '../../lib/domUtils/getSafeIdSelector'; + +describe('getSafeIdSelector', () => { + it('emptyString', () => { + const result = getSafeIdSelector(''); + + expect(result).toEqual(''); + }); + it('Valid', () => { + const result = getSafeIdSelector('Test'); + + expect(result).toEqual('#Test'); + }); + it('Invalid with number', () => { + const result = getSafeIdSelector('0_Test'); + + expect(result).toEqual('[id="0_Test"]'); + }); + it('Invalid with dash', () => { + const result = getSafeIdSelector('-Test'); + + expect(result).toEqual('[id="-Test"]'); + }); + it('Invalid with underscore', () => { + const result = getSafeIdSelector('_Test'); + + expect(result).toEqual('[id="_Test"]'); + }); +}); diff --git a/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts b/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts index 35cef77486a..22d57145b1d 100644 --- a/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts +++ b/packages/roosterjs-content-model-plugins/lib/imageEdit/ImageEditPlugin.ts @@ -14,8 +14,8 @@ import { Resizer } from './Resizer/resizerContext'; import { Rotator } from './Rotator/rotatorContext'; import { updateRotateHandle } from './Rotator/updateRotateHandle'; import { updateWrapper } from './utils/updateWrapper'; - import { + getSafeIdSelector, isElementOfType, isModifierKey, isNodeOfType, @@ -347,7 +347,7 @@ export class ImageEditPlugin implements ImageEditor, EditorPlugin { this.zoomScale = editor.getDOMHelper().calculateZoomScale(); editor.setEditorStyle('imageEdit', `outline-style:none!important;`, [ - `span:has(>img#${this.selectedImage.id})`, + `span:has(>img${getSafeIdSelector(this.selectedImage.id)})`, ]); } diff --git a/packages/roosterjs-content-model-plugins/test/imageEdit/ImageEditPluginTest.ts b/packages/roosterjs-content-model-plugins/test/imageEdit/ImageEditPluginTest.ts index 8ab5af40299..fd16083deef 100644 --- a/packages/roosterjs-content-model-plugins/test/imageEdit/ImageEditPluginTest.ts +++ b/packages/roosterjs-content-model-plugins/test/imageEdit/ImageEditPluginTest.ts @@ -298,4 +298,55 @@ describe('ImageEditPlugin', () => { expect(dataset).toBeTruthy(); plugin.dispose(); }); + + it('flip with unsupportedId', () => { + const modelWithUnsupportedId: ContentModelDocument = { + blockGroupType: 'Document', + blocks: [ + { + blockType: 'Paragraph', + segments: [ + { + segmentType: 'Image', + src: 'test', + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + id: '0', + maxWidth: '1800px', + }, + dataset: {}, + isSelectedAsImageSelection: true, + isSelected: true, + }, + ], + format: {}, + segmentFormat: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: 'rgb(0, 0, 0)', + }, + }, + ], + format: { + fontFamily: 'Calibri', + fontSize: '11pt', + textColor: '#000000', + }, + }; + const plugin = new ImageEditPlugin(); + const editor = initEditor('image_edit', [plugin], modelWithUnsupportedId); + spyOn(editor, 'setEditorStyle').and.callThrough(); + + plugin.initialize(editor); + plugin.flipImage('horizontal'); + plugin.dispose(); + + expect(editor.setEditorStyle).toHaveBeenCalledWith( + 'imageEdit', + 'outline-style:none!important;', + ['span:has(>img[id="0"])'] + ); + }); }); From f704988c8ee099062bd6d8edc796f6177de344f1 Mon Sep 17 00:00:00 2001 From: Bryan Valverde U Date: Thu, 18 Jul 2024 08:27:29 -0600 Subject: [PATCH 19/31] Add AriaLiveMode to announce to let consumers change the ariaLive attribute of the announce container. (#2749) * init * Add tests and remove unneeded change --- .../lib/coreApi/announce/announce.ts | 6 +- .../test/coreApi/announce/announceTest.ts | 58 ++++++++++++++++++- .../lib/parameter/AnnounceData.ts | 6 ++ 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/packages/roosterjs-content-model-core/lib/coreApi/announce/announce.ts b/packages/roosterjs-content-model-core/lib/coreApi/announce/announce.ts index ad92200bd39..59490603bba 100644 --- a/packages/roosterjs-content-model-core/lib/coreApi/announce/announce.ts +++ b/packages/roosterjs-content-model-core/lib/coreApi/announce/announce.ts @@ -10,7 +10,7 @@ const DOT_STRING = '.'; * @param announceData Data to announce */ export const announce: Announce = (core, announceData) => { - const { text, defaultStrings, formatStrings = [] } = announceData; + const { text, defaultStrings, formatStrings = [], ariaLiveMode = 'assertive' } = announceData; const { announcerStringGetter } = core.lifecycle; const template = defaultStrings && announcerStringGetter?.(defaultStrings); let textToAnnounce = formatString(template || text, formatStrings); @@ -21,6 +21,10 @@ export const announce: Announce = (core, announceData) => { if (textToAnnounce && core.lifecycle.announceContainer) { const { announceContainer } = core.lifecycle; + if (announceContainer.ariaLive != ariaLiveMode) { + announceContainer.ariaLive = ariaLiveMode; + } + if (textToAnnounce == announceContainer.textContent) { textToAnnounce += DOT_STRING; } diff --git a/packages/roosterjs-content-model-core/test/coreApi/announce/announceTest.ts b/packages/roosterjs-content-model-core/test/coreApi/announce/announceTest.ts index 53727233613..516a634b716 100644 --- a/packages/roosterjs-content-model-core/test/coreApi/announce/announceTest.ts +++ b/packages/roosterjs-content-model-core/test/coreApi/announce/announceTest.ts @@ -183,10 +183,64 @@ describe('announce', () => { parentElement: { removeChild: removeChildSpy, }, + ariaLive: 'assertive', + }); + }); + + it('already has div with same text 2', () => { + const mockedDiv = { + textContent: 'test', + } as any; + + core.lifecycle.announceContainer = mockedDiv; + + announce(core, { + text: 'test', + }); + + expect(mockedDiv).toEqual({ + textContent: 'test.', + ariaLive: 'assertive', + }); + }); + + it('Set AriaLive polite', () => { + const mockedDiv = { + textContent: 'test', + } as any; + + core.lifecycle.announceContainer = mockedDiv; + + announce(core, { + text: 'test', + ariaLiveMode: 'polite', + }); + + expect(mockedDiv).toEqual({ + textContent: 'test.', + ariaLive: 'polite', + }); + }); + + it('Set AriaLive off', () => { + const mockedDiv = { + textContent: 'test', + } as any; + + core.lifecycle.announceContainer = mockedDiv; + + announce(core, { + text: 'test', + ariaLiveMode: 'off', + }); + + expect(mockedDiv).toEqual({ + textContent: 'test.', + ariaLive: 'off', }); }); - it('already has div with same text', () => { + it('Set AriaLive off', () => { const mockedDiv = { textContent: 'test', } as any; @@ -195,10 +249,12 @@ describe('announce', () => { announce(core, { text: 'test', + ariaLiveMode: 'assertive', }); expect(mockedDiv).toEqual({ textContent: 'test.', + ariaLive: 'assertive', }); }); }); diff --git a/packages/roosterjs-content-model-types/lib/parameter/AnnounceData.ts b/packages/roosterjs-content-model-types/lib/parameter/AnnounceData.ts index 1b92e219e9b..55684269ad2 100644 --- a/packages/roosterjs-content-model-types/lib/parameter/AnnounceData.ts +++ b/packages/roosterjs-content-model-types/lib/parameter/AnnounceData.ts @@ -41,4 +41,10 @@ export interface AnnounceData { * @optional if provided, will attempt to replace {n} with each of the values inside of the array. */ formatStrings?: string[]; + + /** + * @optional if provided, will set the ariaLive property of the announce container element to the provided value. + * @see https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-live#values + */ + ariaLiveMode?: 'assertive' | 'polite' | 'off'; } From 84ac4284cde532cb3952a3af02e7a2afcc2a5ced Mon Sep 17 00:00:00 2001 From: Bryan Valverde U Date: Fri, 26 Jul 2024 08:57:25 -0600 Subject: [PATCH 20/31] Add PasteTypeOrGetter so we can update the default paste type based on the content and clipboard items of the Paste Event #2753 --- .../lib/command/paste/paste.ts | 10 ++++-- .../test/command/paste/pasteTest.ts | 31 ++++++++++++++++++ .../copyPaste/CopyPastePluginTest.ts | 32 +++++++++++++++++++ .../lib/editor/EditorOptions.ts | 6 ++-- .../lib/index.ts | 1 + .../lib/parameter/PasteTypeOrGetter.ts | 10 ++++++ .../lib/pluginState/CopyPastePluginState.ts | 4 +-- 7 files changed, 86 insertions(+), 8 deletions(-) create mode 100644 packages/roosterjs-content-model-types/lib/parameter/PasteTypeOrGetter.ts diff --git a/packages/roosterjs-content-model-core/lib/command/paste/paste.ts b/packages/roosterjs-content-model-core/lib/command/paste/paste.ts index c53748b3659..aca1944187a 100644 --- a/packages/roosterjs-content-model-core/lib/command/paste/paste.ts +++ b/packages/roosterjs-content-model-core/lib/command/paste/paste.ts @@ -4,7 +4,7 @@ import { createPasteFragment } from './createPasteFragment'; import { generatePasteOptionFromPlugins } from './generatePasteOptionFromPlugins'; import { retrieveHtmlInfo } from './retrieveHtmlInfo'; import type { - PasteType, + PasteTypeOrGetter, ClipboardData, TrustedHTMLHandler, IEditor, @@ -14,12 +14,12 @@ import type { * Paste into editor using a clipboardData object * @param editor The Editor object. * @param clipboardData Clipboard data retrieved from clipboard - * @param pasteType Type of content to paste. @default normal + * @param pasteTypeOrGetter Type of content to paste or function that returns the Paste Type to use based on the document and the clipboard Data. @default normal */ export function paste( editor: IEditor, clipboardData: ClipboardData, - pasteType: PasteType = 'normal' + pasteTypeOrGetter: PasteTypeOrGetter = 'normal' ) { editor.focus(); @@ -35,6 +35,10 @@ export function paste( // 1. Prepare variables const doc = createDOMFromHtml(clipboardData.rawHtml, trustedHTMLHandler); + const pasteType = + typeof pasteTypeOrGetter == 'function' + ? pasteTypeOrGetter(doc, clipboardData) + : pasteTypeOrGetter; // 2. Handle HTML from clipboard const htmlFromClipboard = retrieveHtmlInfo(doc, clipboardData); diff --git a/packages/roosterjs-content-model-core/test/command/paste/pasteTest.ts b/packages/roosterjs-content-model-core/test/command/paste/pasteTest.ts index 7a0f0cbe3dc..5fbed20a037 100644 --- a/packages/roosterjs-content-model-core/test/command/paste/pasteTest.ts +++ b/packages/roosterjs-content-model-core/test/command/paste/pasteTest.ts @@ -1,6 +1,8 @@ import * as addParserF from 'roosterjs-content-model-plugins/lib/paste/utils/addParser'; +import * as createPasteFragmentFile from '../../../lib/command/paste/createPasteFragment'; import * as domToContentModel from 'roosterjs-content-model-dom/lib/domToModel/domToContentModel'; import * as ExcelF from 'roosterjs-content-model-plugins/lib/paste/Excel/processPastedContentFromExcel'; +import * as generatePasteOptionFromPluginsFile from '../../../lib/command/paste/generatePasteOptionFromPlugins'; import * as getPasteSourceF from 'roosterjs-content-model-plugins/lib/paste/pasteSourceValidations/getPasteSource'; import * as getSelectedSegmentsF from 'roosterjs-content-model-dom/lib/modelApi/selection/collectSelections'; import * as mergeModelFile from 'roosterjs-content-model-dom/lib/modelApi/editing/mergeModel'; @@ -18,6 +20,7 @@ import { IEditor, BeforePasteEvent, PluginEvent, + PasteTypeOrGetter, } from 'roosterjs-content-model-types'; let clipboardData: ClipboardData; @@ -98,6 +101,34 @@ describe('Paste ', () => { expect(mockedModel).toEqual(mockedMergeModel); }); + + it('Execute | With callback to return paste type', () => { + spyOn(createPasteFragmentFile, 'createPasteFragment').and.callThrough(); + spyOn( + generatePasteOptionFromPluginsFile, + 'generatePasteOptionFromPlugins' + ).and.callThrough(); + + const cb: PasteTypeOrGetter = () => 'normal'; + paste(editor, clipboardData, cb); + + expect(mockedModel).toEqual(mockedMergeModel); + expect(createPasteFragmentFile.createPasteFragment).toHaveBeenCalledWith( + jasmine.anything(), + jasmine.anything(), + 'normal', + jasmine.anything() + ); + expect( + generatePasteOptionFromPluginsFile.generatePasteOptionFromPlugins + ).toHaveBeenCalledWith( + jasmine.anything(), + jasmine.anything(), + jasmine.anything(), + jasmine.anything(), + 'normal' + ); + }); }); describe('paste with content model & paste plugin', () => { diff --git a/packages/roosterjs-content-model-core/test/corePlugin/copyPaste/CopyPastePluginTest.ts b/packages/roosterjs-content-model-core/test/corePlugin/copyPaste/CopyPastePluginTest.ts index c5c3bc0e8d6..0ecfcf4a175 100644 --- a/packages/roosterjs-content-model-core/test/corePlugin/copyPaste/CopyPastePluginTest.ts +++ b/packages/roosterjs-content-model-core/test/corePlugin/copyPaste/CopyPastePluginTest.ts @@ -20,6 +20,7 @@ import { CopyPastePluginState, PluginWithState, DarkColorHandler, + PasteTypeOrGetter, } from 'roosterjs-content-model-types'; import { adjustSelectionForCopyCut, @@ -752,6 +753,37 @@ describe('CopyPastePlugin |', () => { ); expect(preventDefaultSpy).toHaveBeenCalledTimes(1); }); + + it('Handle with defaultPasteType as callback', () => { + const preventDefaultSpy = jasmine.createSpy('preventDefaultPaste'); + const cb: PasteTypeOrGetter = () => 'normal'; + plugin.getState().defaultPasteType = cb; + let clipboardEvent = { + clipboardData: ({ + items: [{}], + }), + preventDefault() { + preventDefaultSpy(); + }, + }; + spyOn(extractClipboardItemsFile, 'extractClipboardItems').and.returnValue(< + Promise + >{ + then: (cb: (value: ClipboardData) => void | PromiseLike) => { + cb(clipboardData); + }, + }); + isDisposed.and.returnValue(false); + + domEvents.paste.beforeDispatch?.(clipboardEvent); + + expect(pasteSpy).toHaveBeenCalledWith(editor, clipboardData, cb); + expect(extractClipboardItemsFile.extractClipboardItems).toHaveBeenCalledWith( + Array.from(clipboardEvent.clipboardData!.items), + allowedCustomPasteType + ); + expect(preventDefaultSpy).toHaveBeenCalledTimes(1); + }); }); it('onNodeCreated with table', () => { diff --git a/packages/roosterjs-content-model-types/lib/editor/EditorOptions.ts b/packages/roosterjs-content-model-types/lib/editor/EditorOptions.ts index a1aa222999f..4bc635e03f1 100644 --- a/packages/roosterjs-content-model-types/lib/editor/EditorOptions.ts +++ b/packages/roosterjs-content-model-types/lib/editor/EditorOptions.ts @@ -1,6 +1,6 @@ +import type { PasteTypeOrGetter } from '../parameter/PasteTypeOrGetter'; import type { ExperimentalFeature } from './ExperimentalFeature'; import type { KnownAnnounceStrings } from '../parameter/AnnounceData'; -import type { PasteType } from '../enum/PasteType'; import type { Colors, ColorTransformFunction } from '../context/DarkColorHandler'; import type { EditorPlugin } from './EditorPlugin'; import type { ContentModelSegmentFormat } from '../contentModel/format/ContentModelSegmentFormat'; @@ -100,9 +100,9 @@ export interface PasteOptions { allowedCustomPasteType?: string[]; /** - * Default paste type. By default will use the normal (as-is) paste type. + * Default paste type or function that returns the paste type. By default will use the normal (as-is) paste type. */ - defaultPasteType?: PasteType; + defaultPasteType?: PasteTypeOrGetter; } /** diff --git a/packages/roosterjs-content-model-types/lib/index.ts b/packages/roosterjs-content-model-types/lib/index.ts index a0fbcc00d7f..80df0987096 100644 --- a/packages/roosterjs-content-model-types/lib/index.ts +++ b/packages/roosterjs-content-model-types/lib/index.ts @@ -398,6 +398,7 @@ export { ContentModelFormatter, } from './parameter/FormatContentModelOptions'; export { ContentModelFormatState } from './parameter/ContentModelFormatState'; +export { PasteTypeOrGetter } from './parameter/PasteTypeOrGetter'; export { ImageFormatState } from './parameter/ImageFormatState'; export { Border } from './parameter/Border'; export { InsertEntityOptions } from './parameter/InsertEntityOptions'; diff --git a/packages/roosterjs-content-model-types/lib/parameter/PasteTypeOrGetter.ts b/packages/roosterjs-content-model-types/lib/parameter/PasteTypeOrGetter.ts new file mode 100644 index 00000000000..057ed9bf4a1 --- /dev/null +++ b/packages/roosterjs-content-model-types/lib/parameter/PasteTypeOrGetter.ts @@ -0,0 +1,10 @@ +import type { ClipboardData } from './ClipboardData'; +import type { PasteType } from '../enum/PasteType'; + +/** + * Represents the PasteType parameter used to set the paste type to use. + * It can be either the Paste Type value or a callback that retuns the Paste Type to use. + */ +export type PasteTypeOrGetter = + | PasteType + | ((document: Document | null, clipboardData: ClipboardData) => PasteType); diff --git a/packages/roosterjs-content-model-types/lib/pluginState/CopyPastePluginState.ts b/packages/roosterjs-content-model-types/lib/pluginState/CopyPastePluginState.ts index 04ba00a1968..fe75fd40c18 100644 --- a/packages/roosterjs-content-model-types/lib/pluginState/CopyPastePluginState.ts +++ b/packages/roosterjs-content-model-types/lib/pluginState/CopyPastePluginState.ts @@ -1,4 +1,4 @@ -import type { PasteType } from '../enum/PasteType'; +import type { PasteTypeOrGetter } from '../parameter/PasteTypeOrGetter'; /** * The state object for CopyPastePlugin @@ -18,5 +18,5 @@ export interface CopyPastePluginState { /** * Default paste type. By default will use the normal (as-is) paste type. */ - defaultPasteType?: PasteType; + defaultPasteType?: PasteTypeOrGetter; } From 64f449f3110ee1ad4c712a9985460417c1463a59 Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Tue, 6 Aug 2024 15:55:41 -0300 Subject: [PATCH 21/31] fix --- .../autoFormat/numbers/transformOrdinals.ts | 14 ++++- .../numbers/transformOrdinalsTest.ts | 56 +++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts b/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts index 49da2c899be..02d16b31463 100644 --- a/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts +++ b/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts @@ -16,8 +16,7 @@ const getOrdinal = (value: number) => { /** * @internal - */ -export function transformOrdinals( + */ export function transformOrdinals( previousSegment: ContentModelText, paragraph: ShallowMutableContentModelParagraph, context: FormatContentModelContext @@ -25,7 +24,7 @@ export function transformOrdinals( const value = previousSegment.text.split(' ').pop()?.trim(); if (value) { const ordinal = value.substring(value.length - 2); - const ordinalValue = parseInt(value); + const ordinalValue = getValue(value); if (ordinalValue && getOrdinal(ordinalValue) === ordinal) { const ordinalSegment = splitTextSegment( previousSegment, @@ -41,3 +40,12 @@ export function transformOrdinals( } return false; } + +function getValue(text: string) { + const number = text.substring(0, text.length - 2); + const isNumber = /^-?\d+$/.test(number); + if (isNumber) { + return parseInt(text); + } + return null; +} diff --git a/packages/roosterjs-content-model-plugins/test/autoFormat/numbers/transformOrdinalsTest.ts b/packages/roosterjs-content-model-plugins/test/autoFormat/numbers/transformOrdinalsTest.ts index 74ffb14f773..f5ffeda194d 100644 --- a/packages/roosterjs-content-model-plugins/test/autoFormat/numbers/transformOrdinalsTest.ts +++ b/packages/roosterjs-content-model-plugins/test/autoFormat/numbers/transformOrdinalsTest.ts @@ -127,4 +127,60 @@ describe('transformOrdinals', () => { }; runTest(segment, paragraph, { canUndoByBackspace: true } as any, false); }); + + it('word and th', () => { + const segment: ContentModelText = { + segmentType: 'Text', + text: '12-month', + format: {}, + }; + const paragraph: ContentModelParagraph = { + blockType: 'Paragraph', + segments: [segment], + format: {}, + }; + runTest(segment, paragraph, { canUndoByBackspace: true } as any, false); + }); + + it('word and rd', () => { + const segment: ContentModelText = { + segmentType: 'Text', + text: '13-rd', + format: {}, + }; + const paragraph: ContentModelParagraph = { + blockType: 'Paragraph', + segments: [segment], + format: {}, + }; + runTest(segment, paragraph, { canUndoByBackspace: true } as any, false); + }); + + it('word and nd', () => { + const segment: ContentModelText = { + segmentType: 'Text', + text: '14-second', + format: {}, + }; + const paragraph: ContentModelParagraph = { + blockType: 'Paragraph', + segments: [segment], + format: {}, + }; + runTest(segment, paragraph, { canUndoByBackspace: true } as any, false); + }); + + it('large number', () => { + const segment: ContentModelText = { + segmentType: 'Text', + text: '145th', + format: {}, + }; + const paragraph: ContentModelParagraph = { + blockType: 'Paragraph', + segments: [segment], + format: {}, + }; + runTest(segment, paragraph, { canUndoByBackspace: true } as any, true); + }); }); From d23a1607f7a842a3d24a809d10eac0d1304a8319 Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Thu, 8 Aug 2024 11:50:13 -0300 Subject: [PATCH 22/31] fixes --- .../lib/autoFormat/numbers/transformOrdinals.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts b/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts index 02d16b31463..101b0dca7f6 100644 --- a/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts +++ b/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts @@ -14,6 +14,11 @@ const getOrdinal = (value: number) => { return ORDINALS[value] || 'th'; }; +/** + * The two last characters of ordinal number (st, nd, rd, th) + */ +const ORDINAL_LENGTH = 2; + /** * @internal */ export function transformOrdinals( @@ -23,9 +28,9 @@ const getOrdinal = (value: number) => { ): boolean { const value = previousSegment.text.split(' ').pop()?.trim(); if (value) { - const ordinal = value.substring(value.length - 2); - const ordinalValue = getValue(value); - if (ordinalValue && getOrdinal(ordinalValue) === ordinal) { + const ordinal = value.substring(value.length - ORDINAL_LENGTH); // This value is equal st, nd, rd, th + const numericValue = getNumericValue(value, ordinal); //This is the numeric part. Ex: 10th, numeric value = 10 + if (numericValue && getOrdinal(numericValue) === ordinal) { const ordinalSegment = splitTextSegment( previousSegment, paragraph, @@ -41,8 +46,8 @@ const getOrdinal = (value: number) => { return false; } -function getValue(text: string) { - const number = text.substring(0, text.length - 2); +function getNumericValue(text: string, ordinal: string) { + const number = text.replace(ordinal, ''); const isNumber = /^-?\d+$/.test(number); if (isNumber) { return parseInt(text); From 3a3c09bc67d8562418468a5e51e7c1b4812e9620 Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Thu, 8 Aug 2024 12:12:29 -0300 Subject: [PATCH 23/31] fixes --- .../lib/autoFormat/numbers/transformOrdinals.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts b/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts index 101b0dca7f6..7726b5592b3 100644 --- a/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts +++ b/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts @@ -29,7 +29,7 @@ const ORDINAL_LENGTH = 2; const value = previousSegment.text.split(' ').pop()?.trim(); if (value) { const ordinal = value.substring(value.length - ORDINAL_LENGTH); // This value is equal st, nd, rd, th - const numericValue = getNumericValue(value, ordinal); //This is the numeric part. Ex: 10th, numeric value = 10 + const numericValue = getNumericValue(value); //This is the numeric part. Ex: 10th, numeric value = 10 if (numericValue && getOrdinal(numericValue) === ordinal) { const ordinalSegment = splitTextSegment( previousSegment, @@ -46,8 +46,8 @@ const ORDINAL_LENGTH = 2; return false; } -function getNumericValue(text: string, ordinal: string) { - const number = text.replace(ordinal, ''); +function getNumericValue(text: string) { + const number = text.substring(0, text.length - ORDINAL_LENGTH); const isNumber = /^-?\d+$/.test(number); if (isNumber) { return parseInt(text); From 3a285f33f724455b8e946313ad77a8b40863aa6f Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Thu, 8 Aug 2024 18:15:19 -0300 Subject: [PATCH 24/31] markdown fixes --- .../autoFormat/numbers/transformOrdinals.ts | 3 ++- .../lib/markdown/utils/setFormat.ts | 23 +++++++++++++--- .../test/markdown/utils/setFormatTest.ts | 27 +++++++++++++++++++ 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts b/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts index 7726b5592b3..64cd3c162d6 100644 --- a/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts +++ b/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts @@ -21,7 +21,8 @@ const ORDINAL_LENGTH = 2; /** * @internal - */ export function transformOrdinals( + */ +export function transformOrdinals( previousSegment: ContentModelText, paragraph: ShallowMutableContentModelParagraph, context: FormatContentModelContext diff --git a/packages/roosterjs-content-model-plugins/lib/markdown/utils/setFormat.ts b/packages/roosterjs-content-model-plugins/lib/markdown/utils/setFormat.ts index 3205c5ce8b0..dfafe429979 100644 --- a/packages/roosterjs-content-model-plugins/lib/markdown/utils/setFormat.ts +++ b/packages/roosterjs-content-model-plugins/lib/markdown/utils/setFormat.ts @@ -21,19 +21,25 @@ export function setFormat( editor, (_model, previousSegment, paragraph, markerFormat, context) => { if (previousSegment.text[previousSegment.text.length - 1] == character) { - const textBeforeMarker = previousSegment.text.slice(0, -1); + const textSegment = previousSegment.text; + const textBeforeMarker = textSegment.slice(0, -1); context.newPendingFormat = { ...markerFormat, strikethrough: !!markerFormat.strikethrough, italic: !!markerFormat.italic, fontWeight: markerFormat?.fontWeight ? 'bold' : undefined, }; + console.log(textBeforeMarker); if (textBeforeMarker.indexOf(character) > -1) { - const lastCharIndex = previousSegment.text.length; - const firstCharIndex = previousSegment.text + const lastCharIndex = textSegment.length; + const firstCharIndex = textSegment .substring(0, lastCharIndex - 1) .lastIndexOf(character); - if (lastCharIndex - firstCharIndex > 2) { + + if ( + hasSpaceBeforeFirstCharacter(textSegment, firstCharIndex) && + lastCharIndex - firstCharIndex > 2 + ) { const formattedText = splitTextSegment( previousSegment, paragraph, @@ -61,3 +67,12 @@ export function setFormat( } ); } + +/** + * The markdown should not be trigger inside a word, so whe check if exist a space before the trigger character + * Should trigger markdown example: _one two_ + * Should not trigger markdown example: one_two_ + */ +function hasSpaceBeforeFirstCharacter(text: string, index: number) { + return !text[index - 1] || text[index - 1].trim().length == 0; +} diff --git a/packages/roosterjs-content-model-plugins/test/markdown/utils/setFormatTest.ts b/packages/roosterjs-content-model-plugins/test/markdown/utils/setFormatTest.ts index fd7ed0add02..21b7dcf1d4b 100644 --- a/packages/roosterjs-content-model-plugins/test/markdown/utils/setFormatTest.ts +++ b/packages/roosterjs-content-model-plugins/test/markdown/utils/setFormatTest.ts @@ -492,4 +492,31 @@ describe('setFormat', () => { runTest(input, '*', { fontWeight: 'bold' }, input, false); }); + + it('should not set italic - one_two_', () => { + const input: ContentModelDocument = { + blockGroupType: 'Document', + blocks: [ + { + blockType: 'Paragraph', + segments: [ + { + segmentType: 'Text', + text: 'one_two_', + format: {}, + }, + { + segmentType: 'SelectionMarker', + format: {}, + isSelected: true, + }, + ], + format: {}, + }, + ], + format: {}, + }; + + runTest(input, '_', { italic: true }, input, false); + }); }); From 9fca23587fd5e989781fa857507846bd405012b0 Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Thu, 8 Aug 2024 18:22:51 -0300 Subject: [PATCH 25/31] markdown fixes --- .../lib/markdown/utils/setFormat.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/roosterjs-content-model-plugins/lib/markdown/utils/setFormat.ts b/packages/roosterjs-content-model-plugins/lib/markdown/utils/setFormat.ts index dfafe429979..4890ee6e221 100644 --- a/packages/roosterjs-content-model-plugins/lib/markdown/utils/setFormat.ts +++ b/packages/roosterjs-content-model-plugins/lib/markdown/utils/setFormat.ts @@ -29,7 +29,6 @@ export function setFormat( italic: !!markerFormat.italic, fontWeight: markerFormat?.fontWeight ? 'bold' : undefined, }; - console.log(textBeforeMarker); if (textBeforeMarker.indexOf(character) > -1) { const lastCharIndex = textSegment.length; const firstCharIndex = textSegment From 61ffa6300111e65597e3999d4a2436d9b2c8bfc7 Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Thu, 8 Aug 2024 18:25:05 -0300 Subject: [PATCH 26/31] markdown fixes --- .../lib/markdown/utils/setFormat.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/roosterjs-content-model-plugins/lib/markdown/utils/setFormat.ts b/packages/roosterjs-content-model-plugins/lib/markdown/utils/setFormat.ts index 4890ee6e221..600f7e369e4 100644 --- a/packages/roosterjs-content-model-plugins/lib/markdown/utils/setFormat.ts +++ b/packages/roosterjs-content-model-plugins/lib/markdown/utils/setFormat.ts @@ -68,7 +68,7 @@ export function setFormat( } /** - * The markdown should not be trigger inside a word, so whe check if exist a space before the trigger character + * The markdown should not be trigger inside a word, then check if exist a space before the trigger character * Should trigger markdown example: _one two_ * Should not trigger markdown example: one_two_ */ From acd7a5c71c62088ad2f1ec1df02f3f90c3fe09bc Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Thu, 8 Aug 2024 18:26:43 -0300 Subject: [PATCH 27/31] remove change --- .../lib/autoFormat/numbers/transformOrdinals.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts b/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts index 64cd3c162d6..7726b5592b3 100644 --- a/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts +++ b/packages/roosterjs-content-model-plugins/lib/autoFormat/numbers/transformOrdinals.ts @@ -21,8 +21,7 @@ const ORDINAL_LENGTH = 2; /** * @internal - */ -export function transformOrdinals( + */ export function transformOrdinals( previousSegment: ContentModelText, paragraph: ShallowMutableContentModelParagraph, context: FormatContentModelContext From 6370e593e21b69e1d5c71532526c7c8c63159a39 Mon Sep 17 00:00:00 2001 From: Rain-Zheng <67583056+Rain-Zheng@users.noreply.github.com> Date: Sat, 10 Aug 2024 02:20:38 +0800 Subject: [PATCH 28/31] remove character check for Android (#2762) Co-authored-by: Jiuqing Song --- .../lib/corePlugin/format/FormatPlugin.ts | 5 ++++- .../test/corePlugin/format/FormatPluginTest.ts | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/roosterjs-content-model-core/lib/corePlugin/format/FormatPlugin.ts b/packages/roosterjs-content-model-core/lib/corePlugin/format/FormatPlugin.ts index 973bc685c52..872a4dd1d12 100644 --- a/packages/roosterjs-content-model-core/lib/corePlugin/format/FormatPlugin.ts +++ b/packages/roosterjs-content-model-core/lib/corePlugin/format/FormatPlugin.ts @@ -21,6 +21,8 @@ import type { // During IME input, KeyDown event will have "Process" as key const ProcessKey = 'Process'; +// For some Android IME, KeyDown event will have "Unidentified" as key +const UnidentifiedKey = 'Unidentified'; const DefaultStyleKeyMap: Record< keyof (FontFamilyFormat & FontSizeFormat & TextColorFormat & BackgroundColorFormat), keyof CSSStyleDeclaration @@ -116,12 +118,13 @@ class FormatPlugin implements PluginWithState { break; case 'keyDown': + const isAndroidIME = this.editor.getEnvironment().isAndroid && event.rawEvent.key == UnidentifiedKey; if (isCursorMovingKey(event.rawEvent)) { this.clearPendingFormat(); this.lastCheckedNode = null; } else if ( this.defaultFormatKeys.size > 0 && - (isCharacterValue(event.rawEvent) || event.rawEvent.key == ProcessKey) && + (isAndroidIME || isCharacterValue(event.rawEvent) || event.rawEvent.key == ProcessKey) && this.shouldApplyDefaultFormat(this.editor) ) { applyDefaultFormat(this.editor, this.state.defaultFormat); diff --git a/packages/roosterjs-content-model-core/test/corePlugin/format/FormatPluginTest.ts b/packages/roosterjs-content-model-core/test/corePlugin/format/FormatPluginTest.ts index 8b5b73aa3b6..b947a73cffa 100644 --- a/packages/roosterjs-content-model-core/test/corePlugin/format/FormatPluginTest.ts +++ b/packages/roosterjs-content-model-core/test/corePlugin/format/FormatPluginTest.ts @@ -18,6 +18,7 @@ describe('FormatPlugin', () => { const editor = ({ cacheContentModel: () => {}, isDarkMode: () => false, + getEnvironment: () => ({}), } as any) as IEditor; const plugin = createFormatPlugin({}); plugin.initialize(editor); @@ -101,6 +102,7 @@ describe('FormatPlugin', () => { const editor = ({ createContentModel: () => model, cacheContentModel: () => {}, + getEnvironment: () => ({}), } as any) as IEditor; const plugin = createFormatPlugin({}); @@ -243,6 +245,7 @@ describe('FormatPlugin for default format', () => { cacheContentModel: cacheContentModelSpy, takeSnapshot: takeSnapshotSpy, formatContentModel: formatContentModelSpy, + getEnvironment: () => ({}), } as any) as IEditor; }); From 97e65dd72e81fc61979234b213f6cb9897c13114 Mon Sep 17 00:00:00 2001 From: Jiuqing Song Date: Wed, 14 Aug 2024 09:58:36 -0700 Subject: [PATCH 29/31] Do not increase list number that starts from 1 (#2766) --- .../list/listLevelThreadFormatHandler.ts | 5 ++-- .../processors/childProcessorTest.ts | 10 +++++++- .../processors/listProcessorTest.ts | 24 +++++++++++++------ .../test/endToEndTest.ts | 10 ++++---- .../list/listLevelThreadFormatHandlerTest.ts | 6 ++--- 5 files changed, 38 insertions(+), 17 deletions(-) diff --git a/packages/roosterjs-content-model-dom/lib/formatHandlers/list/listLevelThreadFormatHandler.ts b/packages/roosterjs-content-model-dom/lib/formatHandlers/list/listLevelThreadFormatHandler.ts index 91d7c739f59..5029aea159c 100644 --- a/packages/roosterjs-content-model-dom/lib/formatHandlers/list/listLevelThreadFormatHandler.ts +++ b/packages/roosterjs-content-model-dom/lib/formatHandlers/list/listLevelThreadFormatHandler.ts @@ -13,8 +13,9 @@ export const listLevelThreadFormatHandler: FormatHandler = { const depth = levels.length; if ( - typeof threadItemCounts[depth] === 'number' && - element.start != threadItemCounts[depth] + 1 + element.start == 1 || + (typeof threadItemCounts[depth] === 'number' && + element.start != threadItemCounts[depth] + 1) ) { format.startNumberOverride = element.start; } diff --git a/packages/roosterjs-content-model-dom/test/domToModel/processors/childProcessorTest.ts b/packages/roosterjs-content-model-dom/test/domToModel/processors/childProcessorTest.ts index dd4f8301fc7..5074f28b276 100644 --- a/packages/roosterjs-content-model-dom/test/domToModel/processors/childProcessorTest.ts +++ b/packages/roosterjs-content-model-dom/test/domToModel/processors/childProcessorTest.ts @@ -453,7 +453,15 @@ describe('childProcessor', () => { { blockType: 'BlockGroup', blockGroupType: 'ListItem', - levels: [{ listType: 'OL', format: {}, dataset: {} }], + levels: [ + { + listType: 'OL', + format: { + startNumberOverride: 1, + }, + dataset: {}, + }, + ], formatHolder: { segmentType: 'SelectionMarker', isSelected: false, format: {} }, blocks: [ { diff --git a/packages/roosterjs-content-model-dom/test/domToModel/processors/listProcessorTest.ts b/packages/roosterjs-content-model-dom/test/domToModel/processors/listProcessorTest.ts index 63bd2a1de78..db3fd845b16 100644 --- a/packages/roosterjs-content-model-dom/test/domToModel/processors/listProcessorTest.ts +++ b/packages/roosterjs-content-model-dom/test/domToModel/processors/listProcessorTest.ts @@ -56,7 +56,7 @@ describe('listProcessor', () => { childProcessor.and.callFake((group, parent, context) => { expect(context.listFormat.listParent).toBe(group); expect(context.listFormat.levels).toEqual([ - { listType: 'OL', format: {}, dataset: {} }, + { listType: 'OL', format: { startNumberOverride: 1 }, dataset: {} }, ]); expect(context.listFormat.threadItemCounts).toEqual([0]); expect(context.segmentFormat).toEqual({}); @@ -88,7 +88,13 @@ describe('listProcessor', () => { childProcessor.and.callFake((group, parent, context) => { expect(context.listFormat.listParent).toBe(group); expect(context.listFormat.levels).toEqual([ - { listType: 'OL', format: {}, dataset: {} }, + { + listType: 'OL', + format: { + startNumberOverride: 1, + }, + dataset: {}, + }, ]); expect(context.listFormat.threadItemCounts).toEqual([0]); expect(context.segmentFormat).toEqual({ @@ -212,6 +218,7 @@ describe('listProcessor', () => { paddingBottom: '2px', paddingLeft: '2px', listStylePosition: 'inside', + startNumberOverride: 1, }, dataset: {}, }, @@ -254,6 +261,7 @@ describe('listProcessor', () => { marginBottom: '0px', marginLeft: '0px', marginRight: '0px', + startNumberOverride: 1, }, dataset: {}, }, @@ -468,7 +476,7 @@ describe('listProcessor process metadata', () => { expect(context.listFormat.levels).toEqual([ { listType: 'OL', - format: {}, + format: { startNumberOverride: 1 }, dataset: {}, }, ]); @@ -492,7 +500,7 @@ describe('listProcessor process metadata', () => { expect(context.listFormat.levels).toEqual([ { listType: 'OL', - format: {}, + format: { startNumberOverride: 1 }, dataset: { editingInfo: JSON.stringify({ orderedStyleType: 1, unorderedStyleType: 2 }), }, @@ -520,7 +528,7 @@ describe('listProcessor process metadata', () => { expect(context.listFormat.levels).toEqual([ { listType: 'OL', - format: {}, + format: { startNumberOverride: 1 }, dataset: { editingInfo: metadata }, }, ]); @@ -544,7 +552,7 @@ describe('listProcessor process metadata', () => { expect(context.listFormat.levels).toEqual([ { listType: 'OL', - format: {}, + format: { startNumberOverride: 1 }, dataset: { editingInfo: JSON.stringify({ orderedStyleType: NumberingListType.Max, @@ -576,7 +584,7 @@ describe('listProcessor process metadata', () => { { listType: 'OL', dataset: { editingInfo }, - format: {}, + format: { startNumberOverride: 1 }, }, ]); }); @@ -601,6 +609,7 @@ describe('listProcessor process metadata', () => { listType: 'OL', format: { listStyleType: 'decimal', + startNumberOverride: 1, }, dataset: { editingInfo: JSON.stringify({ orderedStyleType: NumberingListType.Max }), @@ -640,6 +649,7 @@ describe('listProcessor process metadata', () => { dataset: {}, format: { direction: 'rtl', + startNumberOverride: 1, }, }, ], diff --git a/packages/roosterjs-content-model-dom/test/endToEndTest.ts b/packages/roosterjs-content-model-dom/test/endToEndTest.ts index be9b8fc582d..d282506eea8 100644 --- a/packages/roosterjs-content-model-dom/test/endToEndTest.ts +++ b/packages/roosterjs-content-model-dom/test/endToEndTest.ts @@ -207,7 +207,9 @@ describe('End to end test for DOM => Model => DOM/TEXT', () => { isImplicit: true, }, ], - levels: [{ listType: 'OL', format: {}, dataset: {} }], + levels: [ + { listType: 'OL', format: { startNumberOverride: 1 }, dataset: {} }, + ], formatHolder: { segmentType: 'SelectionMarker', isSelected: false, @@ -228,7 +230,7 @@ describe('End to end test for DOM => Model => DOM/TEXT', () => { ], levels: [ { listType: 'OL', format: {}, dataset: {} }, - { listType: 'OL', format: {}, dataset: {} }, + { listType: 'OL', format: { startNumberOverride: 1 }, dataset: {} }, ], formatHolder: { segmentType: 'SelectionMarker', @@ -2139,12 +2141,12 @@ describe('End to end test for DOM => Model => DOM/TEXT', () => { levels: [ { listType: 'OL', - format: {}, + format: { startNumberOverride: 1 }, dataset: {}, }, { listType: 'OL', - format: { listStyleType: '"1) "' }, + format: { listStyleType: '"1) "', startNumberOverride: 1 }, dataset: {}, }, ], diff --git a/packages/roosterjs-content-model-dom/test/formatHandlers/list/listLevelThreadFormatHandlerTest.ts b/packages/roosterjs-content-model-dom/test/formatHandlers/list/listLevelThreadFormatHandlerTest.ts index 210d5281342..ee7c4e606b1 100644 --- a/packages/roosterjs-content-model-dom/test/formatHandlers/list/listLevelThreadFormatHandlerTest.ts +++ b/packages/roosterjs-content-model-dom/test/formatHandlers/list/listLevelThreadFormatHandlerTest.ts @@ -22,7 +22,7 @@ describe('listLevelThreadFormatHandler.parse', () => { listLevelThreadFormatHandler.parse(format, ol, context, {}); - expect(format.startNumberOverride).toBeUndefined(); + expect(format.startNumberOverride).toBe(1); expect(context.listFormat).toEqual({ threadItemCounts: [0], levels: [], @@ -38,7 +38,7 @@ describe('listLevelThreadFormatHandler.parse', () => { listLevelThreadFormatHandler.parse(format, ol, context, {}); - expect(format.startNumberOverride).toBeUndefined(); + expect(format.startNumberOverride).toBe(1); expect(context.listFormat).toEqual({ threadItemCounts: [0], levels: [], @@ -91,7 +91,7 @@ describe('listLevelThreadFormatHandler.parse', () => { listLevelThreadFormatHandler.parse(format, ol, context, {}); - expect(format.startNumberOverride).toBeUndefined(); + expect(format.startNumberOverride).toBe(1); expect(context.listFormat).toEqual({ threadItemCounts: [2, 0], levels: [ From 024877f79ec1c3955be6796afe784fcb5149a396 Mon Sep 17 00:00:00 2001 From: Bryan Valverde U Date: Wed, 14 Aug 2024 18:12:02 -0600 Subject: [PATCH 30/31] Handle Up and Down in Table #2767 --- .../corePlugin/selection/SelectionPlugin.ts | 28 ++- .../selection/SelectionPluginTest.ts | 199 ++++++++++++++++++ 2 files changed, 224 insertions(+), 3 deletions(-) diff --git a/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts b/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts index d2aaf74b2be..a4ef7d713aa 100644 --- a/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts +++ b/packages/roosterjs-content-model-core/lib/corePlugin/selection/SelectionPlugin.ts @@ -467,6 +467,13 @@ class SelectionPlugin implements PluginWithState { key == Up ? td.childNodes.length : 0, this.editor ); + } else if (!td && (lastCo.row == -1 || lastCo.row <= parsedTable.length)) { + this.selectBeforeOrAfterElement( + this.editor, + table, + change == 1 /* after */, + change != 1 /* setSelectionInNextSiblingElement */ + ); } } else if (key == 'TabLeft' || key == 'TabRight') { const reverse = key == 'TabLeft'; @@ -568,15 +575,30 @@ class SelectionPlugin implements PluginWithState { } } - private selectBeforeOrAfterElement(editor: IEditor, element: HTMLElement, after?: boolean) { + private selectBeforeOrAfterElement( + editor: IEditor, + element: HTMLElement, + after?: boolean, + setSelectionInNextSiblingElement?: boolean + ) { const doc = editor.getDocument(); const parent = element.parentNode; const index = parent && toArray(parent.childNodes).indexOf(element); + let sibling: Element | undefined | null; if (parent && index !== null && index >= 0) { const range = doc.createRange(); - range.setStart(parent, index + (after ? 1 : 0)); - range.collapse(); + if ( + setSelectionInNextSiblingElement && + (sibling = after ? element.nextElementSibling : element.previousElementSibling) && + isNodeOfType(sibling, 'ELEMENT_NODE') + ) { + range.selectNodeContents(sibling); + range.collapse(false /* toStart */); + } else { + range.setStart(parent, index + (after ? 1 : 0)); + range.collapse(); + } this.setDOMSelection( { diff --git a/packages/roosterjs-content-model-core/test/corePlugin/selection/SelectionPluginTest.ts b/packages/roosterjs-content-model-core/test/corePlugin/selection/SelectionPluginTest.ts index e2584338b5d..d03704d44d7 100644 --- a/packages/roosterjs-content-model-core/test/corePlugin/selection/SelectionPluginTest.ts +++ b/packages/roosterjs-content-model-core/test/corePlugin/selection/SelectionPluginTest.ts @@ -1788,6 +1788,205 @@ describe('SelectionPlugin handle table selection', () => { }); }); + it('From Range, Press Down in the last row and move focus outside of table.', () => { + getDOMSelectionSpy.and.returnValue({ + type: 'range', + range: { + startContainer: td3, + startOffset: 0, + endContainer: td3, + endOffset: 0, + commonAncestorContainer: tr2, + }, + isReverted: false, + }); + + requestAnimationFrameSpy.and.callFake((func: Function) => { + getDOMSelectionSpy.and.returnValue({ + type: 'range', + range: { + startContainer: td4, + startOffset: 0, + endContainer: td4, + endOffset: 0, + commonAncestorContainer: tr2, + collapsed: true, + }, + isReverted: false, + }); + + func(); + }); + + const setStartSpy = jasmine.createSpy('setStart'); + const collapseSpy = jasmine.createSpy('collapse'); + const mockedRange = { + setStart: setStartSpy, + collapse: collapseSpy, + } as any; + + createRangeSpy.and.returnValue(mockedRange); + + plugin.onPluginEvent!({ + eventType: 'keyDown', + rawEvent: { + key: 'ArrowDown', + } as any, + }); + + expect(requestAnimationFrameSpy).toHaveBeenCalledTimes(1); + expect(plugin.getState()).toEqual({ + selection: null, + tableSelection: null, + imageSelectionBorderColor: DEFAULT_SELECTION_BORDER_COLOR, + imageSelectionBorderColorDark: DEFAULT_SELECTION_BORDER_COLOR, + tableCellSelectionBackgroundColor: DEFAULT_TABLE_CELL_SELECTION_BACKGROUND_COLOR, + tableCellSelectionBackgroundColorDark: DEFAULT_TABLE_CELL_SELECTION_BACKGROUND_COLOR, + }); + expect(setDOMSelectionSpy).toHaveBeenCalledTimes(1); + expect(setDOMSelectionSpy).toHaveBeenCalledWith({ + type: 'range', + range: mockedRange, + isReverted: false, + }); + expect(setStartSpy).toHaveBeenCalledWith(table.parentElement, 1); + }); + + it('From Range, Press Up in the first row and move focus outside of table, select before table as there are no elements before table.', () => { + getDOMSelectionSpy.and.returnValue({ + type: 'range', + range: { + startContainer: td2, + startOffset: 0, + endContainer: td2, + endOffset: 0, + commonAncestorContainer: tr1, + }, + isReverted: false, + }); + + requestAnimationFrameSpy.and.callFake((func: Function) => { + getDOMSelectionSpy.and.returnValue({ + type: 'range', + range: { + startContainer: td1, + startOffset: 0, + endContainer: td1, + endOffset: 0, + commonAncestorContainer: tr1, + collapsed: true, + }, + isReverted: false, + }); + + func(); + }); + + const setStartSpy = jasmine.createSpy('setStart'); + const collapseSpy = jasmine.createSpy('collapse'); + const mockedRange = { + setStart: setStartSpy, + collapse: collapseSpy, + } as any; + + createRangeSpy.and.returnValue(mockedRange); + + plugin.onPluginEvent!({ + eventType: 'keyDown', + rawEvent: { + key: 'ArrowUp', + } as any, + }); + + expect(requestAnimationFrameSpy).toHaveBeenCalledTimes(1); + expect(plugin.getState()).toEqual({ + selection: null, + tableSelection: null, + imageSelectionBorderColor: DEFAULT_SELECTION_BORDER_COLOR, + imageSelectionBorderColorDark: DEFAULT_SELECTION_BORDER_COLOR, + tableCellSelectionBackgroundColor: DEFAULT_TABLE_CELL_SELECTION_BACKGROUND_COLOR, + tableCellSelectionBackgroundColorDark: DEFAULT_TABLE_CELL_SELECTION_BACKGROUND_COLOR, + }); + expect(setDOMSelectionSpy).toHaveBeenCalledTimes(1); + expect(setDOMSelectionSpy).toHaveBeenCalledWith({ + type: 'range', + range: mockedRange, + isReverted: false, + }); + expect(setStartSpy).toHaveBeenCalledWith(table.parentElement, 0); + }); + + it('From Range, Press Up in the first row and move focus outside of table, select before table as there are no elements before table.', () => { + getDOMSelectionSpy.and.returnValue({ + type: 'range', + range: { + startContainer: td2, + startOffset: 0, + endContainer: td2, + endOffset: 0, + commonAncestorContainer: tr1, + }, + isReverted: false, + }); + + requestAnimationFrameSpy.and.callFake((func: Function) => { + getDOMSelectionSpy.and.returnValue({ + type: 'range', + range: { + startContainer: td1, + startOffset: 0, + endContainer: td1, + endOffset: 0, + commonAncestorContainer: tr1, + collapsed: true, + }, + isReverted: false, + }); + + func(); + }); + + const setStartSpy = jasmine.createSpy('setStart'); + const collapseSpy = jasmine.createSpy('collapse'); + const selectNodeContentsSpy = jasmine.createSpy('selectNodeContents'); + + const mockedRange = { + setStart: setStartSpy, + collapse: collapseSpy, + selectNodeContents: selectNodeContentsSpy, + } as any; + + const div = document.createElement('div'); + table.parentElement?.insertBefore(div, table); + createRangeSpy.and.returnValue(mockedRange); + + plugin.onPluginEvent!({ + eventType: 'keyDown', + rawEvent: { + key: 'ArrowUp', + } as any, + }); + + expect(requestAnimationFrameSpy).toHaveBeenCalledTimes(1); + expect(plugin.getState()).toEqual({ + selection: null, + tableSelection: null, + imageSelectionBorderColor: DEFAULT_SELECTION_BORDER_COLOR, + imageSelectionBorderColorDark: DEFAULT_SELECTION_BORDER_COLOR, + tableCellSelectionBackgroundColor: DEFAULT_TABLE_CELL_SELECTION_BACKGROUND_COLOR, + tableCellSelectionBackgroundColorDark: DEFAULT_TABLE_CELL_SELECTION_BACKGROUND_COLOR, + }); + expect(setDOMSelectionSpy).toHaveBeenCalledTimes(1); + expect(setDOMSelectionSpy).toHaveBeenCalledWith({ + type: 'range', + range: mockedRange, + isReverted: false, + }); + expect(setStartSpy).not.toHaveBeenCalledWith(table.parentElement, 0); + expect(selectNodeContentsSpy).toHaveBeenCalledWith(div); + expect(collapseSpy).toHaveBeenCalledWith(false); + }); + it('From Range, Press Shift+Up', () => { getDOMSelectionSpy.and.returnValue({ type: 'range', From 6fef89ef1e82da0c2709959f442d117a63a2e51d Mon Sep 17 00:00:00 2001 From: "Julia Roldi (from Dev Box)" Date: Fri, 16 Aug 2024 15:49:19 -0300 Subject: [PATCH 31/31] merge version --- versions.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/versions.json b/versions.json index 148574cb665..c4a78d6cd0d 100644 --- a/versions.json +++ b/versions.json @@ -1,5 +1,5 @@ { "react": "9.0.0", - "main": "9.9.0", + "main": "9.9.1", "legacyAdapter": "8.62.1" }