diff --git a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleBlock.ts b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleBlock.ts index 5da5754cfa6..be10bd282cf 100644 --- a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleBlock.ts +++ b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleBlock.ts @@ -12,43 +12,38 @@ export const handleBlock: ContentModelBlockHandler = ( context: ModelToDomContext, refNode: Node | null ) => { - function callHandler( - block: T, - handler: ContentModelBlockHandler - ) { - handler(doc, parent, block, context, refNode); - } - const handlers = context.modelHandlers; switch (block.blockType) { case 'Table': - callHandler(block, handlers.table); + refNode = handlers.table(doc, parent, block, context, refNode); break; case 'Paragraph': - callHandler(block, handlers.paragraph); + refNode = handlers.paragraph(doc, parent, block, context, refNode); break; case 'Entity': - callHandler(block, handlers.entity); + refNode = handlers.entity(doc, parent, block, context, refNode); break; case 'Divider': - callHandler(block, handlers.divider); + refNode = handlers.divider(doc, parent, block, context, refNode); break; case 'BlockGroup': switch (block.blockGroupType) { case 'General': - callHandler(block, handlers.general); + refNode = handlers.general(doc, parent, block, context, refNode); break; case 'Quote': - callHandler(block, handlers.quote); + refNode = handlers.quote(doc, parent, block, context, refNode); break; case 'ListItem': - callHandler(block, handlers.listItem); + refNode = handlers.listItem(doc, parent, block, context, refNode); break; } break; } + + return refNode; }; diff --git a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleBlockGroupChildren.ts b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleBlockGroupChildren.ts index e9116163470..71c7f30f6dc 100644 --- a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleBlockGroupChildren.ts +++ b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleBlockGroupChildren.ts @@ -1,11 +1,6 @@ -import { ContentModelBlock } from '../../publicTypes/block/ContentModelBlock'; import { ContentModelBlockGroup } from '../../publicTypes/group/ContentModelBlockGroup'; -import { ContentModelBlockWithCache } from '../../publicTypes/block/ContentModelBlockWithCache'; import { ContentModelHandler } from '../../publicTypes/context/ContentModelHandler'; -import { getEntityFromElement } from 'roosterjs-editor-dom'; -import { isNodeOfType } from '../../domUtils/isNodeOfType'; import { ModelToDomContext } from '../../publicTypes/context/ModelToDomContext'; -import { NodeType } from 'roosterjs-editor-types'; /** * @internal @@ -34,65 +29,17 @@ export const handleBlockGroupChildren: ContentModelHandler = ( context: ModelToDomContext, refNode: Node | null ) => { - const element = doc.createElement(divider.tagName); + const element = divider.cachedElement; - divider.cachedElement = element; - parent.insertBefore(element, refNode); + if (element) { + refNode = reuseCachedElement(parent, element, refNode); + } else { + const element = doc.createElement(divider.tagName); - applyFormat(element, context.formatAppliers.divider, divider.format, context); + divider.cachedElement = element; + parent.insertBefore(element, refNode); + + applyFormat(element, context.formatAppliers.divider, divider.format, context); + } + + return refNode; }; diff --git a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleEntity.ts b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleEntity.ts index 80710ad5de0..1c7ad22c547 100644 --- a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleEntity.ts +++ b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleEntity.ts @@ -3,6 +3,7 @@ import { ContentModelBlockHandler } from '../../publicTypes/context/ContentModel import { ContentModelEntity } from '../../publicTypes/entity/ContentModelEntity'; import { Entity } from 'roosterjs-editor-types'; import { ModelToDomContext } from '../../publicTypes/context/ModelToDomContext'; +import { reuseCachedElement } from '../utils/reuseCachedElement'; import { addDelimiters, commitEntity, @@ -38,7 +39,7 @@ export const handleEntity: ContentModelBlockHandler = ( commitEntity(wrapper, entity.type, entity.isReadonly, entity.id); } - parent.insertBefore(wrapper, refNode); + refNode = reuseCachedElement(parent, wrapper, refNode); if (isInlineEntity && getObjectKeys(format).length > 0) { const span = wrap(wrapper, 'span'); @@ -47,6 +48,10 @@ export const handleEntity: ContentModelBlockHandler = ( } if (context.addDelimiterForEntity && isInlineEntity && isReadonly) { - addDelimiters(wrapper); + const [after] = addDelimiters(wrapper); + + context.regularSelection.current.segment = after; } + + return refNode; }; diff --git a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleGeneralModel.ts b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleGeneralModel.ts index 18eee5c7dee..efb27b3d16b 100644 --- a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleGeneralModel.ts +++ b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleGeneralModel.ts @@ -5,6 +5,7 @@ import { ContentModelGeneralSegment } from '../../publicTypes/segment/ContentMod import { isNodeOfType } from '../../domUtils/isNodeOfType'; import { ModelToDomContext } from '../../publicTypes/context/ModelToDomContext'; import { NodeType } from 'roosterjs-editor-types'; +import { reuseCachedElement } from '../utils/reuseCachedElement'; /** * @internal @@ -16,9 +17,16 @@ export const handleGeneralModel: ContentModelBlockHandler { - const element = group.element.cloneNode(); + let element: Node = group.element; - parent.insertBefore(element, refNode); + if (refNode && element.parentNode == parent) { + refNode = reuseCachedElement(parent, element, refNode); + } else { + element = element.cloneNode(); + group.element = element as HTMLElement; + + parent.insertBefore(element, refNode); + } if (isGeneralSegment(group) && isNodeOfType(element, NodeType.Element)) { if (!group.element.firstChild) { @@ -31,6 +39,8 @@ export const handleGeneralModel: ContentModelBlockHandler = ( nodeStack.push({ node: newList, ...level }); } + + return refNode; }; function handleMetadata( diff --git a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleListItem.ts b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleListItem.ts index eb89ce4e49a..abaecbd8ad8 100644 --- a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleListItem.ts +++ b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleListItem.ts @@ -15,7 +15,7 @@ export const handleListItem: ContentModelBlockHandler = ( context: ModelToDomContext, refNode: Node | null ) => { - context.modelHandlers.list(doc, parent, listItem, context, refNode); + refNode = context.modelHandlers.list(doc, parent, listItem, context, refNode); const { nodeStack } = context.listFormat; @@ -42,4 +42,6 @@ export const handleListItem: ContentModelBlockHandler = ( unwrap(li); } + + return refNode; }; diff --git a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleParagraph.ts b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleParagraph.ts index 0e956a8cbbd..0642276909e 100644 --- a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleParagraph.ts +++ b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleParagraph.ts @@ -4,6 +4,7 @@ import { ContentModelParagraph } from '../../publicTypes/block/ContentModelParag import { CreateElementData } from 'roosterjs-editor-types'; import { getObjectKeys, unwrap, wrap } from 'roosterjs-editor-dom'; import { ModelToDomContext } from '../../publicTypes/context/ModelToDomContext'; +import { reuseCachedElement } from '../utils/reuseCachedElement'; import { stackFormat } from '../utils/stackFormat'; const DefaultParagraphTag = 'div'; @@ -22,51 +23,59 @@ export const handleParagraph: ContentModelBlockHandler = context: ModelToDomContext, refNode: Node | null ) => { - stackFormat(context, paragraph.decorator?.tagName || null, () => { - const needParagraphWrapper = - !paragraph.isImplicit || - !!paragraph.decorator || - (getObjectKeys(paragraph.format).length > 0 && - paragraph.segments.some(segment => segment.segmentType != 'SelectionMarker')); + const element = paragraph.cachedElement; - let container = doc.createElement(paragraph.decorator?.tagName || DefaultParagraphTag); + if (element) { + refNode = reuseCachedElement(parent, element, refNode); + } else { + stackFormat(context, paragraph.decorator?.tagName || null, () => { + const needParagraphWrapper = + !paragraph.isImplicit || + !!paragraph.decorator || + (getObjectKeys(paragraph.format).length > 0 && + paragraph.segments.some(segment => segment.segmentType != 'SelectionMarker')); - parent.insertBefore(container, refNode); + let container = doc.createElement(paragraph.decorator?.tagName || DefaultParagraphTag); - if (needParagraphWrapper) { - applyFormat(container, context.formatAppliers.block, paragraph.format, context); - } + parent.insertBefore(container, refNode); - if (paragraph.decorator) { - applyFormat( - container, - context.formatAppliers.segmentOnBlock, - paragraph.decorator.format, - context - ); - } + if (needParagraphWrapper) { + applyFormat(container, context.formatAppliers.block, paragraph.format, context); + } - let pre: HTMLElement | undefined; + if (paragraph.decorator) { + applyFormat( + container, + context.formatAppliers.segmentOnBlock, + paragraph.decorator.format, + context + ); + } - // Need some special handling for PRE tag in order to cache the correct element. - // TODO: Consider use decorator to handle PRE tag - if (paragraph.format.whiteSpace == 'pre') { - pre = wrap(container, Pre); - } + let pre: HTMLElement | undefined; - context.regularSelection.current = { - block: needParagraphWrapper ? container : container.parentNode, - segment: null, - }; + // Need some special handling for PRE tag in order to cache the correct element. + // TODO: Consider use decorator to handle PRE tag + if (paragraph.format.whiteSpace == 'pre') { + pre = wrap(container, Pre); + } - paragraph.segments.forEach(segment => { - context.modelHandlers.segment(doc, container, segment, context); + context.regularSelection.current = { + block: needParagraphWrapper ? container : container.parentNode, + segment: null, + }; + + paragraph.segments.forEach(segment => { + context.modelHandlers.segment(doc, container, segment, context); + }); + + if (needParagraphWrapper) { + paragraph.cachedElement = pre || container; + } else { + unwrap(container); + } }); + } - if (needParagraphWrapper) { - paragraph.cachedElement = pre || container; - } else { - unwrap(container); - } - }); + return refNode; }; diff --git a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleQuote.ts b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleQuote.ts index d5774de44c2..2d8021c58e4 100644 --- a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleQuote.ts +++ b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleQuote.ts @@ -3,6 +3,7 @@ import { ContentModelBlockHandler } from '../../publicTypes/context/ContentModel import { ContentModelQuote } from '../../publicTypes/group/ContentModelQuote'; import { isBlockGroupEmpty } from '../../modelApi/common/isEmpty'; import { ModelToDomContext } from '../../publicTypes/context/ModelToDomContext'; +import { reuseCachedElement } from '../utils/reuseCachedElement'; import { stackFormat } from '../utils/stackFormat'; const QuoteTagName = 'blockquote'; @@ -17,7 +18,13 @@ export const handleQuote: ContentModelBlockHandler = ( context: ModelToDomContext, refNode: Node | null ) => { - if (!isBlockGroupEmpty(quote)) { + let element = quote.cachedElement; + + if (element) { + refNode = reuseCachedElement(parent, element, refNode); + + context.modelHandlers.blockGroupChildren(doc, element, quote, context); + } else if (!isBlockGroupEmpty(quote)) { const blockQuote = doc.createElement(QuoteTagName); quote.cachedElement = blockQuote; @@ -35,4 +42,6 @@ export const handleQuote: ContentModelBlockHandler = ( context.modelHandlers.blockGroupChildren(doc, blockQuote, quote, context); } + + return refNode; }; diff --git a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleTable.ts b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleTable.ts index 3c56ae1074e..628a792e3c7 100644 --- a/packages/roosterjs-content-model/lib/modelToDom/handlers/handleTable.ts +++ b/packages/roosterjs-content-model/lib/modelToDom/handlers/handleTable.ts @@ -16,7 +16,7 @@ export const handleTable: ContentModelBlockHandler = ( ) => { if (isBlockEmpty(table)) { // Empty table, do not create TABLE element and just return - return; + return refNode; } const tableNode = doc.createElement('table'); @@ -89,4 +89,6 @@ export const handleTable: ContentModelBlockHandler = ( } } } + + return refNode; }; diff --git a/packages/roosterjs-content-model/lib/modelToDom/utils/reuseCachedElement.ts b/packages/roosterjs-content-model/lib/modelToDom/utils/reuseCachedElement.ts new file mode 100644 index 00000000000..6e1e7c56aae --- /dev/null +++ b/packages/roosterjs-content-model/lib/modelToDom/utils/reuseCachedElement.ts @@ -0,0 +1,44 @@ +import { getEntityFromElement } from 'roosterjs-editor-dom'; +import { isNodeOfType } from '../../domUtils/isNodeOfType'; +import { NodeType } from 'roosterjs-editor-types'; + +/** + * @internal + */ +export function reuseCachedElement(parent: Node, element: Node, refNode: Node | null): Node | null { + if (element.parentNode == parent) { + // Remove nodes before the one we are hitting since they don't appear in Content Model at this position. + // But we don't want to touch entity since it would better to keep entity at its place unless it is removed + // In that case we will remove it after we have handled all other nodes + while (refNode && refNode != element && !isEntity(refNode)) { + const next = refNode.nextSibling; + + refNode.parentNode?.removeChild(refNode); + refNode = next; + } + + if (refNode && refNode == element) { + refNode = refNode.nextSibling; + } else { + parent.insertBefore(element, refNode); + } + } else { + parent.insertBefore(element, refNode); + } + + return refNode; +} + +/** + * @internal + */ +export function removeNode(node: Node): Node | null { + const next = node.nextSibling; + node.parentNode?.removeChild(node); + + return next; +} + +function isEntity(node: Node) { + return isNodeOfType(node, NodeType.Element) && !!getEntityFromElement(node); +} diff --git a/packages/roosterjs-content-model/lib/publicTypes/block/ContentModelBlockWithCache.ts b/packages/roosterjs-content-model/lib/publicTypes/block/ContentModelBlockWithCache.ts index 4c1b0b9e337..5be09f5447c 100644 --- a/packages/roosterjs-content-model/lib/publicTypes/block/ContentModelBlockWithCache.ts +++ b/packages/roosterjs-content-model/lib/publicTypes/block/ContentModelBlockWithCache.ts @@ -1,9 +1,9 @@ /** * Represent a Content Model block with cached element */ -export interface ContentModelBlockWithCache { +export interface ContentModelBlockWithCache { /** * Cached element for reuse */ - cachedElement?: HTMLElement; + cachedElement?: TElement; } diff --git a/packages/roosterjs-content-model/lib/publicTypes/context/ContentModelHandler.ts b/packages/roosterjs-content-model/lib/publicTypes/context/ContentModelHandler.ts index f446807218d..74dbb5578c8 100644 --- a/packages/roosterjs-content-model/lib/publicTypes/context/ContentModelHandler.ts +++ b/packages/roosterjs-content-model/lib/publicTypes/context/ContentModelHandler.ts @@ -30,4 +30,4 @@ export type ContentModelBlockHandler void; +) => Node | null; diff --git a/packages/roosterjs-content-model/test/modelToDom/handlers/handleBlockGroupChildrenTest.ts b/packages/roosterjs-content-model/test/modelToDom/handlers/handleBlockGroupChildrenTest.ts index 13e935ddda8..94248bc9231 100644 --- a/packages/roosterjs-content-model/test/modelToDom/handlers/handleBlockGroupChildrenTest.ts +++ b/packages/roosterjs-content-model/test/modelToDom/handlers/handleBlockGroupChildrenTest.ts @@ -15,7 +15,7 @@ describe('handleBlockGroupChildren', () => { let parent: HTMLDivElement; beforeEach(() => { - handleBlock = jasmine.createSpy('handleBlock'); + handleBlock = jasmine.createSpy('handleBlock').and.callFake(originalHandleBlock); context = createModelToDomContext(undefined, { modelHandlerOverride: { block: handleBlock, @@ -42,7 +42,7 @@ describe('handleBlockGroupChildren', () => { handleBlockGroupChildren(document, parent, group, context); - expect(parent.outerHTML).toBe('
'); + expect(parent.outerHTML).toBe('
'); expect(context.listFormat.nodeStack).toEqual([]); expect(handleBlock).toHaveBeenCalledTimes(1); expect(handleBlock).toHaveBeenCalledWith(document, parent, paragraph, context, null); @@ -58,7 +58,7 @@ describe('handleBlockGroupChildren', () => { handleBlockGroupChildren(document, parent, group, context); - expect(parent.outerHTML).toBe('
'); + expect(parent.outerHTML).toBe('
'); expect(context.listFormat.nodeStack).toEqual([]); expect(handleBlock).toHaveBeenCalledTimes(2); expect(handleBlock).toHaveBeenCalledWith(document, parent, paragraph1, context, null); @@ -73,8 +73,9 @@ describe('handleBlockGroupChildren', () => { group.blocks.push(paragraph); context.listFormat.nodeStack = nodeStack; - handleBlock.and.callFake((doc, parent, child, context) => { + handleBlock.and.callFake((doc, parent, child, context, refNode) => { expect(context.listFormat.nodeStack).toEqual([]); + return refNode; }); handleBlockGroupChildren(document, parent, group, context); @@ -98,7 +99,7 @@ describe('handleBlockGroupChildren', () => { group.blocks.push(paragraph2); context.listFormat.nodeStack = nodeStack; - handleBlock.and.callFake((doc, parent, child, context) => { + handleBlock.and.callFake((doc, parent, child, context, refNode) => { if (child == paragraph1) { expect(context.listFormat.nodeStack).toEqual([]); context.listFormat.nodeStack.push({ c: 'd' } as any); @@ -109,6 +110,8 @@ describe('handleBlockGroupChildren', () => { } else { throw new Error('Should never run to here: ' + JSON.stringify(child)); } + + return refNode; }); handleBlockGroupChildren(document, parent, group, context); diff --git a/packages/roosterjs-content-model/test/modelToDom/handlers/handleBlockTest.ts b/packages/roosterjs-content-model/test/modelToDom/handlers/handleBlockTest.ts index 3dc3e60e2ce..dbb01c007df 100644 --- a/packages/roosterjs-content-model/test/modelToDom/handlers/handleBlockTest.ts +++ b/packages/roosterjs-content-model/test/modelToDom/handlers/handleBlockTest.ts @@ -205,9 +205,10 @@ describe('handleBlockGroup', () => { parent = document.createElement('div'); parent.appendChild(br); - handleBlock(document, parent, block, context, br); + const result = handleBlock(document, parent, block, context, br); expect(parent.innerHTML).toBe(expectedInnerHTML); + expect(result).toBe(br); } it('General block', () => { @@ -223,9 +224,10 @@ describe('handleBlockGroup', () => { expect(handleGeneralModel).toHaveBeenCalledTimes(1); expect(handleGeneralModel).toHaveBeenCalledWith(document, parent, group, context, null); - handleGeneralModel.and.callFake((doc, parent, model, context, refNode) => - parent.insertBefore(doc.createTextNode('test'), refNode) - ); + handleGeneralModel.and.callFake((doc, parent, model, context, refNode) => { + parent.insertBefore(doc.createTextNode('test'), refNode); + return refNode; + }); runTestWithRefNode(group, 'test
'); }); @@ -239,9 +241,10 @@ describe('handleBlockGroup', () => { expect(handleQuote).toHaveBeenCalledTimes(1); expect(handleQuote).toHaveBeenCalledWith(document, parent, group, context, null); - handleQuote.and.callFake((doc, parent, model, context, refNode) => - parent.insertBefore(doc.createTextNode('test'), refNode) - ); + handleQuote.and.callFake((doc, parent, model, context, refNode) => { + parent.insertBefore(doc.createTextNode('test'), refNode); + return refNode; + }); runTestWithRefNode(group, 'test
'); }); @@ -255,9 +258,10 @@ describe('handleBlockGroup', () => { expect(handleListItem).toHaveBeenCalledTimes(1); expect(handleListItem).toHaveBeenCalledWith(document, parent, group, context, null); - handleListItem.and.callFake((doc, parent, model, context, refNode) => - parent.insertBefore(doc.createTextNode('test'), refNode) - ); + handleListItem.and.callFake((doc, parent, model, context, refNode) => { + parent.insertBefore(doc.createTextNode('test'), refNode); + return refNode; + }); runTestWithRefNode(group, 'test
'); }); diff --git a/packages/roosterjs-content-model/test/modelToDom/handlers/handleDividerTest.ts b/packages/roosterjs-content-model/test/modelToDom/handlers/handleDividerTest.ts index b5fb2beb77a..29781c31df0 100644 --- a/packages/roosterjs-content-model/test/modelToDom/handlers/handleDividerTest.ts +++ b/packages/roosterjs-content-model/test/modelToDom/handlers/handleDividerTest.ts @@ -105,9 +105,33 @@ describe('handleDivider', () => { parent.appendChild(br); - handleDivider(document, parent, hr, context, br); + const result = handleDivider(document, parent, hr, context, br); expect(parent.innerHTML).toBe('

'); expect(hr.cachedElement).toBe(parent.firstChild as HTMLElement); + expect(result).toBe(br); + }); + + it('HR with refNode, already in target node', () => { + const hrNode = document.createElement('hr'); + const hr: ContentModelDivider = { + blockType: 'Divider', + tagName: 'hr', + format: {}, + cachedElement: hrNode, + }; + + const parent = document.createElement('div'); + const br = document.createElement('br'); + + parent.appendChild(hrNode); + parent.appendChild(br); + + const result = handleDivider(document, parent, hr, context, hrNode); + + expect(parent.innerHTML).toBe('

'); + expect(hr.cachedElement).toBe(hrNode); + expect(parent.firstChild).toBe(hrNode); + expect(result).toBe(br); }); }); diff --git a/packages/roosterjs-content-model/test/modelToDom/handlers/handleEntityTest.ts b/packages/roosterjs-content-model/test/modelToDom/handlers/handleEntityTest.ts index 55b17323044..fe872d564f4 100644 --- a/packages/roosterjs-content-model/test/modelToDom/handlers/handleEntityTest.ts +++ b/packages/roosterjs-content-model/test/modelToDom/handlers/handleEntityTest.ts @@ -9,7 +9,7 @@ describe('handleEntity', () => { beforeEach(() => { context = createModelToDomContext(); - spyOn(addDelimiters, 'default').and.callFake(() => []); + spyOn(addDelimiters, 'default').and.callThrough(); }); it('Simple block entity', () => { @@ -76,7 +76,7 @@ describe('handleEntity', () => { handleEntity(document, parent, entityModel, context, null); expect(parent.innerHTML).toBe( - '' + '' ); expect(span.outerHTML).toBe( '' @@ -102,7 +102,7 @@ describe('handleEntity', () => { const br = document.createElement('br'); parent.appendChild(br); - handleEntity(document, parent, entityModel, context, br); + const result = handleEntity(document, parent, entityModel, context, br); expect(parent.innerHTML).toBe( '
test

' @@ -110,5 +110,66 @@ describe('handleEntity', () => { expect(div.outerHTML).toBe( '
test
' ); + expect(result).toBe(br); + }); + + it('Entity is already there', () => { + const br = document.createElement('br'); + const insertBefore = jasmine.createSpy('insertBefore'); + const parent = ({ + insertBefore, + } as any) as HTMLElement; + const entityDiv = ({ + nextSibling: br, + parentNode: parent, + } as any) as HTMLElement; + const entityModel: ContentModelEntity = { + blockType: 'Entity', + segmentType: 'Entity', + format: {}, + id: 'entity_1', + type: 'entity', + isReadonly: true, + wrapper: entityDiv, + }; + + entityDiv.textContent = 'test'; + + const result = handleEntity(document, parent, entityModel, context, entityDiv); + + expect(insertBefore).not.toHaveBeenCalled(); + expect(result).toBe(br); + }); + + it('Entity with delimiter', () => { + const span = document.createElement('span'); + const entityModel: ContentModelEntity = { + blockType: 'Entity', + segmentType: 'Entity', + format: {}, + id: 'entity_1', + type: 'entity', + isReadonly: true, + wrapper: span, + }; + + span.textContent = 'test'; + + const parent = document.createElement('div'); + const br = document.createElement('br'); + parent.appendChild(br); + + context.addDelimiterForEntity = true; + + const result = handleEntity(document, parent, entityModel, context, br); + + expect(parent.innerHTML).toBe( + 'test
' + ); + expect(span.outerHTML).toBe( + 'test' + ); + expect(result).toBe(br); + expect(context.regularSelection.current.segment).toBe(span.nextSibling); }); }); diff --git a/packages/roosterjs-content-model/test/modelToDom/handlers/handleGeneralModelTest.ts b/packages/roosterjs-content-model/test/modelToDom/handlers/handleGeneralModelTest.ts index cd07d693115..47fc4a93540 100644 --- a/packages/roosterjs-content-model/test/modelToDom/handlers/handleGeneralModelTest.ts +++ b/packages/roosterjs-content-model/test/modelToDom/handlers/handleGeneralModelTest.ts @@ -1,7 +1,6 @@ import * as applyFormat from '../../../lib/modelToDom/utils/applyFormat'; import * as stackFormat from '../../../lib/modelToDom/utils/stackFormat'; import { ContentModelBlockGroup } from '../../../lib/publicTypes/group/ContentModelBlockGroup'; -import { ContentModelHandler } from '../../../lib/publicTypes/context/ContentModelHandler'; import { ContentModelListItem } from '../../../lib/publicTypes/group/ContentModelListItem'; import { ContentModelQuote } from '../../../lib/publicTypes/group/ContentModelQuote'; import { createGeneralBlock } from '../../../lib/modelApi/creators/createGeneralBlock'; @@ -9,13 +8,17 @@ import { createGeneralSegment } from '../../../lib/modelApi/creators/createGener import { createModelToDomContext } from '../../../lib/modelToDom/context/createModelToDomContext'; import { handleGeneralModel } from '../../../lib/modelToDom/handlers/handleGeneralModel'; import { ModelToDomContext } from '../../../lib/publicTypes/context/ModelToDomContext'; +import { + ContentModelBlockHandler, + ContentModelHandler, +} from '../../../lib/publicTypes/context/ContentModelHandler'; describe('handleBlockGroup', () => { let context: ModelToDomContext; let parent: HTMLDivElement; let handleBlockGroupChildren: jasmine.Spy>; - let handleListItem: jasmine.Spy>; - let handleQuote: jasmine.Spy>; + let handleListItem: jasmine.Spy>; + let handleQuote: jasmine.Spy>; beforeEach(() => { handleBlockGroupChildren = jasmine.createSpy('handleBlockGroupChildren'); @@ -96,7 +99,7 @@ describe('handleBlockGroup', () => { handleGeneralModel(document, parent, group, context, null); expect(parent.outerHTML).toBe('
'); - expect(context.regularSelection.current.segment).toBeNull(); + expect(context.regularSelection.current.segment).toBe(clonedChild); expect(typeof parent.firstChild).toBe('object'); expect(parent.firstChild).toBe(clonedChild); expect(context.listFormat.nodeStack).toEqual([]); @@ -131,7 +134,7 @@ describe('handleBlockGroup', () => { handleGeneralModel(document, parent, group, context, null); expect(parent.outerHTML).toBe('
'); - expect(context.regularSelection.current.segment).toBeNull(); + expect(context.regularSelection.current.segment).toBe(clonedChild); expect(typeof parent.firstChild).toBe('object'); expect(parent.firstChild).toBe(clonedChild.parentElement); expect(context.listFormat.nodeStack).toEqual([]); @@ -180,7 +183,7 @@ describe('handleBlockGroup', () => { const br = document.createElement('br'); parent.appendChild(br); - handleGeneralModel(document, parent, group, context, br); + const result = handleGeneralModel(document, parent, group, context, br); expect(parent.outerHTML).toBe('

'); expect(typeof parent.firstChild).toBe('object'); @@ -194,5 +197,26 @@ describe('handleBlockGroup', () => { context ); expect(applyFormat.applyFormat).not.toHaveBeenCalled(); + expect(result).toBe(br); + expect(group.element).toBe(clonedChild); + }); + + it('General block with refNode, already in target node', () => { + const node = document.createElement('span'); + const group = createGeneralBlock(node); + const br = document.createElement('br'); + + parent.appendChild(node); + parent.appendChild(br); + + const result = handleGeneralModel(document, parent, group, context, node); + + expect(parent.outerHTML).toBe('

'); + expect(parent.firstChild).toBe(node); + expect(context.listFormat.nodeStack).toEqual([]); + expect(handleBlockGroupChildren).toHaveBeenCalledTimes(1); + expect(handleBlockGroupChildren).toHaveBeenCalledWith(document, node, group, context); + expect(result).toBe(br); + expect(group.element).toBe(node); }); }); diff --git a/packages/roosterjs-content-model/test/modelToDom/handlers/handleListItemTest.ts b/packages/roosterjs-content-model/test/modelToDom/handlers/handleListItemTest.ts index 4a1b70f9d41..578c367f30d 100644 --- a/packages/roosterjs-content-model/test/modelToDom/handlers/handleListItemTest.ts +++ b/packages/roosterjs-content-model/test/modelToDom/handlers/handleListItemTest.ts @@ -395,4 +395,27 @@ describe('handleListItem without format handler', () => { context ); }); + + it('UL with refNode', () => { + const listItem = createListItem([{ listType: 'UL' }]); + const br = document.createElement('br'); + const parent = document.createElement('div'); + + parent.appendChild(br); + + const result = handleListItem(document, parent, listItem, context, br); + + expect(parent.outerHTML).toBe('

'); + expect(handleList).toHaveBeenCalledTimes(1); + expect(handleList).toHaveBeenCalledWith(document, parent, listItem, context, br); + expect(applyFormat.applyFormat).toHaveBeenCalled(); + expect(handleBlockGroupChildren).toHaveBeenCalledTimes(1); + expect(handleBlockGroupChildren).toHaveBeenCalledWith( + document, + parent.firstChild!.firstChild as HTMLElement, + listItem, + context + ); + expect(result).toBe(br); + }); }); diff --git a/packages/roosterjs-content-model/test/modelToDom/handlers/handleListTest.ts b/packages/roosterjs-content-model/test/modelToDom/handlers/handleListTest.ts index d427da4a625..712a9e002b1 100644 --- a/packages/roosterjs-content-model/test/modelToDom/handlers/handleListTest.ts +++ b/packages/roosterjs-content-model/test/modelToDom/handlers/handleListTest.ts @@ -829,7 +829,7 @@ describe('handleList handles metadata', () => { parent.appendChild(existingOL); parent.appendChild(br); - handleList(document, parent, listItem, context, br); + const result = handleList(document, parent, listItem, context, br); const possibleResults = [ '

    ', //Chrome @@ -853,5 +853,6 @@ describe('handleList handles metadata', () => { }, ], }); + expect(result).toBe(br); }); }); diff --git a/packages/roosterjs-content-model/test/modelToDom/handlers/handleParagraphTest.ts b/packages/roosterjs-content-model/test/modelToDom/handlers/handleParagraphTest.ts index 121bc64de30..faff84901d3 100644 --- a/packages/roosterjs-content-model/test/modelToDom/handlers/handleParagraphTest.ts +++ b/packages/roosterjs-content-model/test/modelToDom/handlers/handleParagraphTest.ts @@ -382,12 +382,13 @@ describe('handleParagraph', () => { }; const br = document.createElement('br'); - parent.appendChild(br); + const result = parent.appendChild(br); handleParagraph(document, parent, paragraph, context, br); expect(parent.innerHTML).toBe('

    '); expect(paragraph.cachedElement).toBe(parent.firstChild as HTMLElement); + expect(result).toBe(br); }); it('Handle paragraph with PRE', () => { diff --git a/packages/roosterjs-content-model/test/modelToDom/handlers/handleQuoteTest.ts b/packages/roosterjs-content-model/test/modelToDom/handlers/handleQuoteTest.ts index 94f6e3721c3..cc31b6005e8 100644 --- a/packages/roosterjs-content-model/test/modelToDom/handlers/handleQuoteTest.ts +++ b/packages/roosterjs-content-model/test/modelToDom/handlers/handleQuoteTest.ts @@ -69,7 +69,7 @@ describe('handleQuote', () => { handleBlockGroupChildren.and.callFake(originalHandleBlockGroupChildren); - handleQuote(document, parent, quote, context, br); + const result = handleQuote(document, parent, quote, context, br); expect(parent.outerHTML).toBe( '
    test

    ' @@ -82,5 +82,6 @@ describe('handleQuote', () => { context ); expect(quote.cachedElement).toBe(parent.firstChild as HTMLElement); + expect(result).toBe(br); }); }); diff --git a/packages/roosterjs-content-model/test/modelToDom/handlers/handleTableTest.ts b/packages/roosterjs-content-model/test/modelToDom/handlers/handleTableTest.ts index 4182d16dd13..2382ad6d6a6 100644 --- a/packages/roosterjs-content-model/test/modelToDom/handlers/handleTableTest.ts +++ b/packages/roosterjs-content-model/test/modelToDom/handlers/handleTableTest.ts @@ -235,7 +235,7 @@ describe('handleTable', () => { div.appendChild(br); - handleTable( + const result = handleTable( document, div, { @@ -251,5 +251,6 @@ describe('handleTable', () => { ); expect(div.innerHTML).toBe('

    '); + expect(result).toBe(br); }); }); diff --git a/packages/roosterjs-content-model/test/modelToDom/utils/reuseCachedElementTest.ts b/packages/roosterjs-content-model/test/modelToDom/utils/reuseCachedElementTest.ts new file mode 100644 index 00000000000..431a62caacb --- /dev/null +++ b/packages/roosterjs-content-model/test/modelToDom/utils/reuseCachedElementTest.ts @@ -0,0 +1,85 @@ +import { commitEntity } from 'roosterjs-editor-dom'; +import { reuseCachedElement } from '../../../lib/modelToDom/utils/reuseCachedElement'; + +describe('reuseCachedElement', () => { + it('No refNode', () => { + const parent = document.createElement('div'); + const element = document.createElement('span'); + + const result = reuseCachedElement(parent, element, null); + + expect(parent.outerHTML).toBe('
    '); + expect(parent.firstChild).toBe(element); + expect(result).toBe(null); + }); + + it('RefNode is not current element', () => { + const parent = document.createElement('div'); + const element = document.createElement('span'); + const refNode = document.createElement('br'); + + parent.appendChild(refNode); + + const result = reuseCachedElement(parent, element, refNode); + + expect(parent.outerHTML).toBe('

    '); + expect(parent.firstChild).toBe(element); + expect(result).toBe(refNode); + }); + + it('RefNode is current element', () => { + const parent = document.createElement('div'); + const element = document.createElement('span'); + const nextNode = document.createElement('br'); + + parent.appendChild(element); + parent.appendChild(nextNode); + + const result = reuseCachedElement(parent, element, element); + + expect(parent.outerHTML).toBe('

    '); + expect(parent.firstChild).toBe(element); + expect(parent.firstChild?.nextSibling).toBe(nextNode); + expect(result).toBe(nextNode); + }); + + it('RefNode is before current element', () => { + const parent = document.createElement('div'); + const refNode = document.createElement('hr'); + const element = document.createElement('span'); + const nextNode = document.createElement('br'); + + parent.appendChild(refNode); + parent.appendChild(element); + parent.appendChild(nextNode); + + const result = reuseCachedElement(parent, element, refNode); + + expect(parent.outerHTML).toBe('

    '); + expect(parent.firstChild).toBe(element); + expect(parent.firstChild?.nextSibling).toBe(nextNode); + expect(result).toBe(nextNode); + }); + + it('RefNode is entity', () => { + const parent = document.createElement('div'); + const refNode = document.createElement('div'); + const element = document.createElement('span'); + const nextNode = document.createElement('br'); + + parent.appendChild(refNode); + parent.appendChild(element); + parent.appendChild(nextNode); + + commitEntity(refNode, 'TestEntity', true); + + const result = reuseCachedElement(parent, element, refNode); + + expect(parent.outerHTML).toBe( + '

    ' + ); + expect(parent.firstChild).toBe(element); + expect(parent.firstChild?.nextSibling).toBe(refNode); + expect(result).toBe(refNode); + }); +}); diff --git a/packages/roosterjs-editor-api/lib/table/editTable.ts b/packages/roosterjs-editor-api/lib/table/editTable.ts index e42a9ad7fa3..09603fd5413 100644 --- a/packages/roosterjs-editor-api/lib/table/editTable.ts +++ b/packages/roosterjs-editor-api/lib/table/editTable.ts @@ -25,17 +25,24 @@ export default function editTable( editor.transformToDarkColor(vtable.table); editor.focus(); - let cellToSelect = calculateCellToSelect(operation, vtable.row, vtable.col); - editor.select( - vtable.getCell(cellToSelect.newRow, cellToSelect.newCol).td, - PositionType.Begin - ); + if (isUndefined(vtable.row) || isUndefined(vtable.col)) { + return; + } + let { newCol, newRow } = calculateCellToSelect(operation, vtable.row, vtable.col); + const newTd = vtable.getCell(newRow, newCol).td; + if (newTd) { + editor.select(newTd, PositionType.Begin); + } }, 'editTable' ); } } +function isUndefined(n: number | undefined): n is undefined { + return n == undefined; +} + function calculateCellToSelect( operation: TableOperation | CompatibleTableOperation, currentRow: number, @@ -69,6 +76,6 @@ function calculateCellToSelect( function saveTableSelection(editor: IEditor, vtable: VTable) { const selection = editor.getSelectionRangeEx(); if (selection && selection.type === SelectionRangeTypes.TableSelection) { - vtable.selection = selection.coordinates; + vtable.selection = selection.coordinates ?? null; } } diff --git a/packages/roosterjs-editor-api/lib/table/formatTable.ts b/packages/roosterjs-editor-api/lib/table/formatTable.ts index af64ae8a667..65fb1b22b4b 100644 --- a/packages/roosterjs-editor-api/lib/table/formatTable.ts +++ b/packages/roosterjs-editor-api/lib/table/formatTable.ts @@ -18,12 +18,18 @@ export default function formatTable( formatUndoSnapshot( editor, (start, end) => { + if (!table) { + return; + } + let vtable = new VTable(table); vtable.applyFormat(format); vtable.writeBack(); editor.transformToDarkColor(vtable.table); editor.focus(); - editor.select(start, end); + if (start && end) { + editor.select(start, end); + } }, 'formatTable' ); diff --git a/packages/roosterjs-editor-api/lib/table/insertTable.ts b/packages/roosterjs-editor-api/lib/table/insertTable.ts index 72142a67f98..6ba5c83acae 100644 --- a/packages/roosterjs-editor-api/lib/table/insertTable.ts +++ b/packages/roosterjs-editor-api/lib/table/insertTable.ts @@ -42,7 +42,7 @@ export default function insertTable( setBackgroundColor(editor, 'transparent'); } let vtable = new VTable(table); - vtable.applyFormat(format); + vtable.applyFormat(format || {}); vtable.writeBack(); editor.insertNode(table); editor.runAsync(editor => diff --git a/packages/roosterjs-editor-api/lib/table/tsconfig.child.json b/packages/roosterjs-editor-api/lib/table/tsconfig.child.json index 955870fefc8..a172cdfac39 100644 --- a/packages/roosterjs-editor-api/lib/table/tsconfig.child.json +++ b/packages/roosterjs-editor-api/lib/table/tsconfig.child.json @@ -1,6 +1,6 @@ { "compilerOptions": { - "strict": false + "strict": true }, "extends": "../../../tsconfig.json", "include": ["./**/*.ts"], diff --git a/packages/roosterjs-editor-api/lib/utils/applyInlineStyle.ts b/packages/roosterjs-editor-api/lib/utils/applyInlineStyle.ts index da6be94a523..b9b5e7f94bb 100644 --- a/packages/roosterjs-editor-api/lib/utils/applyInlineStyle.ts +++ b/packages/roosterjs-editor-api/lib/utils/applyInlineStyle.ts @@ -50,10 +50,13 @@ export default function applyInlineStyle( formatUndoSnapshot( editor, () => { - let firstNode: Node; - let lastNode: Node; + let firstNode: Node | undefined; + let lastNode: Node | undefined; selection.ranges.forEach(range => { let contentTraverser = editor.getSelectionTraverser(range); + if (!contentTraverser) { + return; + } let inlineElement = contentTraverser && contentTraverser.currentInlineElement; while (inlineElement) { let nextInlineElement = contentTraverser.getNextInlineElement(); diff --git a/packages/roosterjs-editor-api/lib/utils/blockFormat.ts b/packages/roosterjs-editor-api/lib/utils/blockFormat.ts index ba3fb1bd7c8..e7545e5086a 100644 --- a/packages/roosterjs-editor-api/lib/utils/blockFormat.ts +++ b/packages/roosterjs-editor-api/lib/utils/blockFormat.ts @@ -10,8 +10,8 @@ export default function blockFormat( editor: IEditor, callback: ( region: Region, - start: NodePosition, - end: NodePosition, + start: NodePosition | null, + end: NodePosition | null, chains: VListChain[] ) => void, beforeRunCallback?: () => boolean, @@ -31,7 +31,7 @@ export default function blockFormat( commitListChains(editor, chains); } } - if (selection.type == SelectionRangeTypes.Normal) { + if (selection.type == SelectionRangeTypes.Normal && start && end) { editor.select(start, end); } else { editor.select(selection); diff --git a/packages/roosterjs-editor-api/lib/utils/blockWrap.ts b/packages/roosterjs-editor-api/lib/utils/blockWrap.ts index 2b5a74635c1..5efe3da9081 100644 --- a/packages/roosterjs-editor-api/lib/utils/blockWrap.ts +++ b/packages/roosterjs-editor-api/lib/utils/blockWrap.ts @@ -46,10 +46,12 @@ export default function blockWrap( while ( nodes[0] && + nodes[0].parentNode && isNodeInRegion(region, nodes[0].parentNode) && nodes.some(node => getTagOfNode(node) == 'LI') ) { - nodes = [splitBalancedNodeRange(nodes)]; + const result = splitBalancedNodeRange(nodes); + nodes = result ? [result] : []; } wrapFunction(nodes); diff --git a/packages/roosterjs-editor-api/lib/utils/collapseSelectedBlocks.ts b/packages/roosterjs-editor-api/lib/utils/collapseSelectedBlocks.ts index 922779745a7..f3a538ecceb 100644 --- a/packages/roosterjs-editor-api/lib/utils/collapseSelectedBlocks.ts +++ b/packages/roosterjs-editor-api/lib/utils/collapseSelectedBlocks.ts @@ -12,7 +12,10 @@ export default function collapseSelectedBlocks( forEachCallback: (element: HTMLElement) => any ) { let traverser = editor.getSelectionTraverser(); - let block = traverser && traverser.currentBlockElement; + if (!traverser) { + return; + } + let block = traverser.currentBlockElement; let blocks: BlockElement[] = []; while (block) { if (!isEmptyBlockUnderTR(block)) { diff --git a/packages/roosterjs-editor-api/lib/utils/commitListChains.ts b/packages/roosterjs-editor-api/lib/utils/commitListChains.ts index 5a9dee6034e..d8cf4384fd3 100644 --- a/packages/roosterjs-editor-api/lib/utils/commitListChains.ts +++ b/packages/roosterjs-editor-api/lib/utils/commitListChains.ts @@ -15,7 +15,9 @@ export default function commitListChains(editor: IEditor, chains: VListChain[]) ExperimentalFeatures.ReuseAllAncestorListElements ); chains.forEach(chain => chain.commit(shouldReuseAllAncestorListElements)); - editor.select(start, end); + if (start && end) { + editor.select(start, end); + } } } diff --git a/packages/roosterjs-editor-api/lib/utils/execCommand.ts b/packages/roosterjs-editor-api/lib/utils/execCommand.ts index d016ecf29c4..b293e051fa6 100644 --- a/packages/roosterjs-editor-api/lib/utils/execCommand.ts +++ b/packages/roosterjs-editor-api/lib/utils/execCommand.ts @@ -25,7 +25,7 @@ export default function execCommand( ) { editor.focus(); - let formatter = () => editor.getDocument().execCommand(command, false, null); + let formatter = () => editor.getDocument().execCommand(command, false, undefined); let selection = editor.getSelectionRangeEx(); if (selection && selection.areAllCollapsed) { diff --git a/packages/roosterjs-editor-api/lib/utils/formatUndoSnapshot.ts b/packages/roosterjs-editor-api/lib/utils/formatUndoSnapshot.ts index 8764e6c5114..8ba3c760d01 100644 --- a/packages/roosterjs-editor-api/lib/utils/formatUndoSnapshot.ts +++ b/packages/roosterjs-editor-api/lib/utils/formatUndoSnapshot.ts @@ -10,15 +10,17 @@ import { ChangeSource, IEditor, NodePosition } from 'roosterjs-editor-types'; */ export default function formatUndoSnapshot( editor: IEditor, - callback?: (start: NodePosition, end: NodePosition) => any, + callback?: (start: NodePosition | null, end: NodePosition | null) => any, apiName?: string ) { editor.addUndoSnapshot( callback, ChangeSource.Format, undefined /* canUndoByBackspace */, - apiName && { - formatApiName: apiName, - } + apiName && apiName != '' + ? { + formatApiName: apiName, + } + : undefined ); } diff --git a/packages/roosterjs-editor-api/lib/utils/normalizeBlockquote.ts b/packages/roosterjs-editor-api/lib/utils/normalizeBlockquote.ts index e98575dff60..1ed4c712314 100644 --- a/packages/roosterjs-editor-api/lib/utils/normalizeBlockquote.ts +++ b/packages/roosterjs-editor-api/lib/utils/normalizeBlockquote.ts @@ -11,6 +11,9 @@ export default function normalizeBlockquote(node: Node, quotesHandled?: Node[]): const alignment = node.style.textAlign; let quote = findClosestElementAncestor(node, undefined /* root */, 'blockquote'); + if (!quote) { + return; + } const isNodeRTL = isRTL(node); if (quotesHandled) { @@ -23,15 +26,15 @@ export default function normalizeBlockquote(node: Node, quotesHandled?: Node[]): while (quote) { if (alignment == 'center') { if (isNodeRTL) { - delete quote.style.marginInlineEnd; + quote.style.removeProperty('marginInlineEnd'); quote.style.marginInlineStart = 'auto'; } else { - delete quote.style.marginInlineStart; + quote.style.removeProperty('marginInlineStart'); quote.style.marginInlineEnd = 'auto'; } } else { - delete quote.style.marginInlineStart; - delete quote.style.marginInlineEnd; + quote.style.removeProperty('marginInlineEnd'); + quote.style.removeProperty('marginInlineStart'); } quote = findClosestElementAncestor( diff --git a/packages/roosterjs-editor-api/lib/utils/toggleListType.ts b/packages/roosterjs-editor-api/lib/utils/toggleListType.ts index 57ba26945ea..626c1af9257 100644 --- a/packages/roosterjs-editor-api/lib/utils/toggleListType.ts +++ b/packages/roosterjs-editor-api/lib/utils/toggleListType.ts @@ -36,7 +36,7 @@ import type { export default function toggleListType( editor: IEditor, listType: ListType | CompatibleListType, - startNumber?: number, + startNumber: number = 0, includeSiblingLists: boolean = true, orderedStyle?: NumberingListType | CompatibleNumberingListType, unorderedStyle?: BulletListType | CompatibleBulletListType, @@ -47,21 +47,22 @@ export default function toggleListType( (region, start, end, chains) => { const chain = startNumber > 0 && chains.filter(chain => chain.canAppendAtCursor(startNumber))[0]; + const block = getBlockElementAtNode( + region.rootNode, + start?.node ?? null + )?.collapseToSingleElement(); + if (!block) { + return; + } const vList = - chain && start.equalTo(end) - ? chain.createVListAtBlock( - getBlockElementAtNode( - region.rootNode, - start.node - )?.collapseToSingleElement(), - startNumber - ) + chain && end && start?.equalTo(end) + ? chain.createVListAtBlock(block, startNumber) : createVListFromRegion( region, startNumber === 1 ? false : includeSiblingLists ); - if (vList) { + if (vList && start && end) { vList.changeListType(start, end, listType); if (editor.isFeatureEnabled(ExperimentalFeatures.AutoFormatList)) { vList.setListStyleType(orderedStyle, unorderedStyle); diff --git a/packages/roosterjs-editor-api/lib/utils/tsconfig.child.json b/packages/roosterjs-editor-api/lib/utils/tsconfig.child.json index 08c366e8552..4765b53a98e 100644 --- a/packages/roosterjs-editor-api/lib/utils/tsconfig.child.json +++ b/packages/roosterjs-editor-api/lib/utils/tsconfig.child.json @@ -1,6 +1,6 @@ { "compilerOptions": { - "strict": false + "strict": true }, "extends": "../../../tsconfig.json", "include": ["./**/*.ts"], diff --git a/packages/roosterjs-editor-api/test/format/clearFormatTest.ts b/packages/roosterjs-editor-api/test/format/clearFormatTest.ts index e7a093dbfd9..49955f5ddeb 100644 --- a/packages/roosterjs-editor-api/test/format/clearFormatTest.ts +++ b/packages/roosterjs-editor-api/test/format/clearFormatTest.ts @@ -26,7 +26,7 @@ describe('clearFormat()', () => { clearFormat(editor); expect(editor.addUndoSnapshot).toHaveBeenCalled(); - expect(document.execCommand).toHaveBeenCalledWith('removeFormat', false, null); + expect(document.execCommand).toHaveBeenCalledWith('removeFormat', false, undefined); }); TestHelper.itFirefoxOnly('removes the existing formats', () => { diff --git a/packages/roosterjs-editor-api/test/format/formatApiTest.ts b/packages/roosterjs-editor-api/test/format/formatApiTest.ts index 92b8021f66d..b4093c465df 100644 --- a/packages/roosterjs-editor-api/test/format/formatApiTest.ts +++ b/packages/roosterjs-editor-api/test/format/formatApiTest.ts @@ -28,7 +28,7 @@ describe('FormatUtils', () => { toggleStrikethrough(editor); - expect(document.execCommand).toHaveBeenCalledWith('strikeThrough', false, null); + expect(document.execCommand).toHaveBeenCalledWith('strikeThrough', false, undefined); }); it('toggleSuperscript() triggers the superscript command in document', () => { @@ -37,7 +37,7 @@ describe('FormatUtils', () => { toggleSuperscript(editor); - expect(document.execCommand).toHaveBeenCalledWith('superscript', false, null); + expect(document.execCommand).toHaveBeenCalledWith('superscript', false, undefined); }); it('toggleSubscript() triggers the subscript command in document', () => { @@ -46,7 +46,7 @@ describe('FormatUtils', () => { toggleSubscript(editor); - expect(document.execCommand).toHaveBeenCalledWith('subscript', false, null); + expect(document.execCommand).toHaveBeenCalledWith('subscript', false, undefined); }); it('setTextColor() triggers the applyInlineStyle method in editor', () => { diff --git a/packages/roosterjs-editor-api/test/format/setAlignmentTest.ts b/packages/roosterjs-editor-api/test/format/setAlignmentTest.ts index 33825ee47a8..da9fed7e046 100644 --- a/packages/roosterjs-editor-api/test/format/setAlignmentTest.ts +++ b/packages/roosterjs-editor-api/test/format/setAlignmentTest.ts @@ -101,7 +101,7 @@ describe('setAlignment()', () => { setAlignment(editor, alignment); expect(editor.addUndoSnapshot).toHaveBeenCalled(); - expect(document.execCommand).toHaveBeenCalledWith(command, false, null); + expect(document.execCommand).toHaveBeenCalledWith(command, false, undefined); } function runningTestInTable(alignment: Alignment, table: string) { diff --git a/packages/roosterjs-editor-api/test/format/toggleBoldTest.ts b/packages/roosterjs-editor-api/test/format/toggleBoldTest.ts index 1c67183d308..556ff8518d5 100644 --- a/packages/roosterjs-editor-api/test/format/toggleBoldTest.ts +++ b/packages/roosterjs-editor-api/test/format/toggleBoldTest.ts @@ -24,7 +24,7 @@ describe('toggleBold()', () => { toggleBold(editor); - expect(document.execCommand).toHaveBeenCalledWith('bold', false, null); + expect(document.execCommand).toHaveBeenCalledWith('bold', false, undefined); }); TestHelper.itFirefoxOnly( diff --git a/packages/roosterjs-editor-api/test/format/toggleItalicTest.ts b/packages/roosterjs-editor-api/test/format/toggleItalicTest.ts index aa12bfc52d8..752052c71f1 100644 --- a/packages/roosterjs-editor-api/test/format/toggleItalicTest.ts +++ b/packages/roosterjs-editor-api/test/format/toggleItalicTest.ts @@ -24,7 +24,7 @@ describe('toggleItalic()', () => { toggleItalic(editor); - expect(document.execCommand).toHaveBeenCalledWith('italic', false, null); + expect(document.execCommand).toHaveBeenCalledWith('italic', false, undefined); }); TestHelper.itFirefoxOnly( diff --git a/packages/roosterjs-editor-api/test/format/toggleUnderlineTest.ts b/packages/roosterjs-editor-api/test/format/toggleUnderlineTest.ts index 21395255a75..c6107044a5a 100644 --- a/packages/roosterjs-editor-api/test/format/toggleUnderlineTest.ts +++ b/packages/roosterjs-editor-api/test/format/toggleUnderlineTest.ts @@ -23,7 +23,7 @@ describe('toggleUnderline()', () => { toggleUnderline(editor); - expect(document.execCommand).toHaveBeenCalledWith('underline', false, null); + expect(document.execCommand).toHaveBeenCalledWith('underline', false, undefined); }); TestHelper.itFirefoxOnly( diff --git a/packages/roosterjs-editor-core/lib/corePlugins/utils/inlineEntityOnPluginEvent.ts b/packages/roosterjs-editor-core/lib/corePlugins/utils/inlineEntityOnPluginEvent.ts index 02c2d9d1e94..47352354b41 100644 --- a/packages/roosterjs-editor-core/lib/corePlugins/utils/inlineEntityOnPluginEvent.ts +++ b/packages/roosterjs-editor-core/lib/corePlugins/utils/inlineEntityOnPluginEvent.ts @@ -1,12 +1,13 @@ import { addDelimiters, - createElement, + arrayPush, createRange, getDelimiterFromElement, getEntityFromElement, getEntitySelector, isBlockElement, isCharacterValue, + matchesSelector, Position, safeInstanceOf, splitTextNode, @@ -29,7 +30,6 @@ const DELIMITER_SELECTOR = '.' + DelimiterClasses.DELIMITER_AFTER + ',.' + DelimiterClasses.DELIMITER_BEFORE; const ZERO_WIDTH_SPACE = '\u200B'; const INLINE_ENTITY_SELECTOR = 'span' + getEntitySelector(); -const NBSP = '\u00A0'; export function inlineEntityOnPluginEvent(event: PluginEvent, editor: IEditor) { switch (event.eventType) { @@ -43,7 +43,15 @@ export function inlineEntityOnPluginEvent(event: PluginEvent, editor: IEditor) { break; case PluginEventType.BeforePaste: - addDelimitersIfNeeded(event.fragment.querySelectorAll(INLINE_ENTITY_SELECTOR)); + const { fragment, sanitizingOption } = event; + addDelimitersIfNeeded(fragment.querySelectorAll(INLINE_ENTITY_SELECTOR)); + + if (sanitizingOption.additionalAllowedCssClasses) { + arrayPush(sanitizingOption.additionalAllowedCssClasses, [ + DelimiterClasses.DELIMITER_AFTER, + DelimiterClasses.DELIMITER_BEFORE, + ]); + } break; case PluginEventType.ExtractContentWithDom: @@ -98,13 +106,13 @@ export function normalizeDelimitersInEditor(editor: IEditor) { function addDelimitersIfNeeded(nodes: Element[] | NodeListOf) { nodes.forEach(node => { - if (tryGetEntityFromNode(node)) { + if (isEntityElement(node)) { addDelimiters(node); } }); } -function tryGetEntityFromNode(node: Element | null): node is HTMLElement { +function isEntityElement(node: Node | null): node is HTMLElement { return !!( node && safeInstanceOf(node, 'HTMLElement') && @@ -124,7 +132,7 @@ function isReadOnly(entity: Entity | null) { ); } -function removeInvalidDelimiters(nodes: Element[]) { +function removeInvalidDelimiters(nodes: Element[] | NodeListOf) { nodes.forEach(node => { if (getDelimiterFromElement(node)) { const sibling = node.classList.contains(DelimiterClasses.DELIMITER_BEFORE) @@ -139,14 +147,14 @@ function removeInvalidDelimiters(nodes: Element[]) { }); } -function removeDelimiterAttr(node: Element | undefined | null) { +function removeDelimiterAttr(node: Element | undefined | null, checkEntity: boolean = true) { if (!node) { return; } const isAfter = node.classList.contains(DelimiterClasses.DELIMITER_AFTER); const entitySibling = isAfter ? node.previousElementSibling : node.nextElementSibling; - if (entitySibling && tryGetEntityFromNode(entitySibling)) { + if (checkEntity && entitySibling && isEntityElement(entitySibling)) { return; } @@ -163,39 +171,34 @@ function removeDelimiterAttr(node: Element | undefined | null) { function handleCollapsedEnter(editor: IEditor, delimiter: HTMLElement) { const isAfter = delimiter.classList.contains(DelimiterClasses.DELIMITER_AFTER); - const sibling = isAfter ? delimiter.nextSibling : delimiter.previousSibling; - let positionToUse: Position | undefined; - let element: Element | null; - - if (sibling) { - positionToUse = new Position(sibling, isAfter ? PositionType.Begin : PositionType.End); - } else { - element = delimiter.insertAdjacentElement( - isAfter ? 'afterend' : 'beforebegin', - createElement( - { - tag: 'span', - children: [NBSP], - }, - editor.getDocument() - )! - ); + const entity = !isAfter ? delimiter.nextSibling : delimiter.previousSibling; + const block = editor.getBlockElementAtNode(delimiter)?.getStartNode(); - if (!element) { + editor.runAsync(() => { + if (!block) { return; } + const blockToCheck = isAfter ? block.nextSibling : block.previousSibling; + if (blockToCheck && safeInstanceOf(blockToCheck, 'HTMLElement')) { + const delimiters = blockToCheck.querySelectorAll(DELIMITER_SELECTOR); + // Check if the last or first delimiter still contain the delimiter class and remove it. + const delimiterToCheck = delimiters.item(isAfter ? 0 : delimiters.length - 1); + removeDelimiterAttr(delimiterToCheck); + } - positionToUse = new Position(element, PositionType.Begin); - } - - if (positionToUse) { - editor.select(positionToUse); - editor.runAsync(aEditor => { - const elAfter = aEditor.getElementAtCursor(); - removeDelimiterAttr(elAfter); - removeNode(element); - }); - } + if (isEntityElement(entity)) { + const { nextElementSibling, previousElementSibling } = entity; + [nextElementSibling, previousElementSibling].forEach(el => { + // Check if after Enter the ZWS got removed but we still have a element with the class + // Remove the attributes of the element if it is invalid now. + if (el && matchesSelector(el, DELIMITER_SELECTOR) && !getDelimiterFromElement(el)) { + removeDelimiterAttr(el, false /* checkEntity */); + } + }); + // Add delimiters to the entity if needed because on Enter we can sometimes lose the ZWS of the element. + addDelimiters(entity); + } + }); } const getPosition = (container: HTMLElement | null) => { diff --git a/packages/roosterjs-editor-core/test/corePlugins/inlineEntityOnPluginEventTest.ts b/packages/roosterjs-editor-core/test/corePlugins/inlineEntityOnPluginEventTest.ts index 30eefb4ddc2..063a134dd0e 100644 --- a/packages/roosterjs-editor-core/test/corePlugins/inlineEntityOnPluginEventTest.ts +++ b/packages/roosterjs-editor-core/test/corePlugins/inlineEntityOnPluginEventTest.ts @@ -2,24 +2,25 @@ import * as splitTextNode from 'roosterjs-editor-dom/lib/utils/splitTextNode'; import { inlineEntityOnPluginEvent } from '../../lib/corePlugins/utils/inlineEntityOnPluginEvent'; import { BeforeCutCopyEvent, - EditorReadyEvent, + BeforePasteEvent, + ChangeSource, + ContentChangedEvent, DelimiterClasses, + EditorReadyEvent, Entity, ExtractContentWithDomEvent, IEditor, NormalSelectionRange, - PluginEventType, - ContentChangedEvent, PluginEvent, - SelectionRangeTypes, - BeforePasteEvent, + PluginEventType, PluginKeyDownEvent, - ChangeSource, + SelectionRangeTypes, } from 'roosterjs-editor-types'; import { addDelimiters, commitEntity, findClosestElementAncestor, + getBlockElementAtNode, Position, } from 'roosterjs-editor-dom'; @@ -61,6 +62,7 @@ describe('Inline Entity On Plugin Event |', () => { }; }, select: selectSpy = jasmine.createSpy('select'), + getBlockElementAtNode: (node: Node) => getBlockElementAtNode(document.body, node), }); }); @@ -169,21 +171,16 @@ describe('Inline Entity On Plugin Event |', () => { expect(splitTextNode.default).not.toHaveBeenCalled(); }); - it('Enter on delimiter before, no previous sibling', () => { - arrangeAndAct(13 /* ENTER */, false /* addElementOnRunAsync */); - - expect(selectSpy).toHaveBeenCalled(); - expect(delimiterBefore?.previousElementSibling).toBeNull(); - }); + it('Enter on delimiter before, clear previous block delimiter', () => { + const div = document.createElement('div'); + testContainer.insertAdjacentElement('beforebegin', div); + div.appendChild(delimiterBefore!.cloneNode(true /* deep */)); - it('Enter on delimiter before, no previous sibling', () => { - const testElement = document.createElement('span'); - testElement.appendChild(document.createTextNode('Test')); - delimiterBefore?.parentElement?.insertBefore(testElement, delimiterBefore); arrangeAndAct(13 /* ENTER */, false /* addElementOnRunAsync */); - expect(selectSpy).toHaveBeenCalled(); - expect(delimiterBefore?.previousElementSibling).not.toBeNull(); + expect(div.firstElementChild?.className).toEqual(''); + expect(div.firstElementChild?.textContent).not.toEqual(ZERO_WIDTH_SPACE); + expect(delimiterBefore!.className).toEqual(DelimiterClasses.DELIMITER_BEFORE); }); it('Key press when selection is not collapsed, delimiter before is the endContainer', () => { @@ -343,21 +340,15 @@ describe('Inline Entity On Plugin Event |', () => { expect(splitTextNode.default).not.toHaveBeenCalled(); }); - it('Enter on delimiter after, no previous sibling', () => { - arrangeAndAct(13 /* ENTER */, false /* addElementOnRunAsync */); - - expect(selectSpy).toHaveBeenCalled(); - expect(delimiterAfter?.nextElementSibling).toBeNull(); - }); - - it('Enter on delimiter after, no previous sibling', () => { - const testElement = document.createElement('span'); - testElement.appendChild(document.createTextNode('Test')); - delimiterAfter?.parentElement?.insertBefore(testElement, null); + it('Enter on delimiter after, clear the previous sibling class', () => { + const div = document.createElement('div'); + testContainer.insertAdjacentElement('afterend', div); + div.appendChild(delimiterAfter!.cloneNode(true /* deep */)); arrangeAndAct(13 /* ENTER */, false /* addElementOnRunAsync */); - expect(selectSpy).toHaveBeenCalled(); - expect(delimiterAfter?.nextElementSibling).not.toBeNull(); + expect(div.firstElementChild?.className).toEqual(''); + expect(div.firstElementChild?.textContent).not.toEqual(ZERO_WIDTH_SPACE); + expect(delimiterAfter!.className).toEqual(DelimiterClasses.DELIMITER_AFTER); }); it('Key press when selection is not collapsed, delimiter after is the endContainer', () => { @@ -617,17 +608,21 @@ describe('Inline Entity On Plugin Event |', () => { if (elementToUse) { rootDiv.appendChild(elementToUse); } - + const additionalAllowedCssClasses: string[] = []; inlineEntityOnPluginEvent( ({ eventType: PluginEventType.BeforePaste, clipboardData: {}, fragment: rootDiv, + sanitizingOption: { + additionalAllowedCssClasses, + }, }), editor ); - expect(rootDiv.querySelectorAll(DELIMITER_SELECTOR).length).toBe(expectedDelimiters); + expect(additionalAllowedCssClasses).toContain(DelimiterClasses.DELIMITER_AFTER); + expect(additionalAllowedCssClasses).toContain(DelimiterClasses.DELIMITER_BEFORE); } it('Before Paste with Read only Inline Entity in content', () => { @@ -669,6 +664,7 @@ describe('Inline Entity On Plugin Event |', () => { }); }); }); + function addEntityBeforeEach(entity: Entity, wrapper: HTMLElement) { entity = { id: 'test', diff --git a/packages/roosterjs-editor-dom/lib/delimiter/addDelimiters.ts b/packages/roosterjs-editor-dom/lib/delimiter/addDelimiters.ts index 7200499b1ae..8fd5d885c0d 100644 --- a/packages/roosterjs-editor-dom/lib/delimiter/addDelimiters.ts +++ b/packages/roosterjs-editor-dom/lib/delimiter/addDelimiters.ts @@ -59,12 +59,12 @@ function insertDelimiter(element: Element, delimiterClass: DelimiterClasses) { children: [ZERO_WIDTH_SPACE], }, element.ownerDocument - ); + ) as HTMLElement; if (span) { const insertPosition: InsertPosition = delimiterClass == DelimiterClasses.DELIMITER_AFTER ? 'afterend' : 'beforebegin'; element.insertAdjacentElement(insertPosition, span); } - return element; + return span; } diff --git a/packages/roosterjs-editor-dom/test/delimiter/addDelimitersTest.ts b/packages/roosterjs-editor-dom/test/delimiter/addDelimitersTest.ts index df8db2e2694..fb65af55189 100644 --- a/packages/roosterjs-editor-dom/test/delimiter/addDelimitersTest.ts +++ b/packages/roosterjs-editor-dom/test/delimiter/addDelimitersTest.ts @@ -1,7 +1,7 @@ import addDelimiters from '../../lib/delimiter/addDelimiters'; import { DelimiterClasses } from 'roosterjs-editor-types'; -describe('addDelimiterstest', () => { +describe('addDelimitersTest', () => { afterAll(() => { document.body.childNodes.forEach(node => { document.body.removeChild(node); @@ -12,7 +12,7 @@ describe('addDelimiterstest', () => { const element = document.createElement('span'); document.body.append(element); - addDelimiters(element); + const [after, before] = addDelimiters(element); expect(element.nextElementSibling).toBeDefined(); expect(element.nextElementSibling?.className).toEqual(DelimiterClasses.DELIMITER_AFTER); @@ -20,6 +20,8 @@ describe('addDelimiterstest', () => { expect(element.previousElementSibling?.className).toEqual( DelimiterClasses.DELIMITER_BEFORE ); + expect(after.outerHTML).toBe(''); + expect(before.outerHTML).toBe(''); }); it('Add between other Entity with delimiters', () => { diff --git a/packages/roosterjs-editor-plugins/test/ContentEdit/features/shortcutFeatureTest.ts b/packages/roosterjs-editor-plugins/test/ContentEdit/features/shortcutFeatureTest.ts index f40b4779230..d05c5da51a1 100644 --- a/packages/roosterjs-editor-plugins/test/ContentEdit/features/shortcutFeatureTest.ts +++ b/packages/roosterjs-editor-plugins/test/ContentEdit/features/shortcutFeatureTest.ts @@ -91,7 +91,7 @@ parameters.forEach(({ description, key, command }) => { const shortCutFeature = ShortcutFeatures.defaultShortcut; const spya = spyOn(editor.getDocument(), 'execCommand'); shortCutFeature.handleEvent(event, editor); - expect(spya).toHaveBeenCalledWith(command, false, null); + expect(spya).toHaveBeenCalledWith(command, false, undefined); }); });