From b6ad856b82e4a85ff5dd055247edb5925eecbcc0 Mon Sep 17 00:00:00 2001 From: "SOUTHAMERICA\\bvalverde" Date: Tue, 14 Nov 2023 11:53:53 -0600 Subject: [PATCH 1/2] init --- .../ContentEdit/features/entityFeatures.ts | 8 +- .../features/inlineEntityFeatureTest.ts | 114 +++++++++++++++++- 2 files changed, 118 insertions(+), 4 deletions(-) diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ContentEdit/features/entityFeatures.ts b/packages/roosterjs-editor-plugins/lib/plugins/ContentEdit/features/entityFeatures.ts index f62bafe4763..f81d766f0c6 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ContentEdit/features/entityFeatures.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ContentEdit/features/entityFeatures.ts @@ -463,8 +463,8 @@ function cacheGetCheckBefore(event: PluginKeyboardEvent, checkBefore?: boolean): } function getRelatedElements(delimiter: HTMLElement, checkBefore: boolean, editor: IEditor) { - let entity: Element | null = null; - let delimiterPair: Element | null = null; + let entity: HTMLElement | null = null; + let delimiterPair: HTMLElement | null = null; const traverser = getBlockTraverser(editor, delimiter); if (!traverser) { return { delimiterPair, entity }; @@ -492,6 +492,10 @@ function getRelatedElements(delimiter: HTMLElement, checkBefore: boolean, editor delimiterPair = null; break; } + if (entity && delimiterPair && !delimiterPair.isContentEditable) { + delimiterPair = null; + } + current = traverseFn(traverser); } diff --git a/packages/roosterjs-editor-plugins/test/ContentEdit/features/inlineEntityFeatureTest.ts b/packages/roosterjs-editor-plugins/test/ContentEdit/features/inlineEntityFeatureTest.ts index cec8afcfa65..554eb0df6bb 100644 --- a/packages/roosterjs-editor-plugins/test/ContentEdit/features/inlineEntityFeatureTest.ts +++ b/packages/roosterjs-editor-plugins/test/ContentEdit/features/inlineEntityFeatureTest.ts @@ -1,5 +1,6 @@ import * as addDelimiters from 'roosterjs-editor-dom/lib/delimiter/addDelimiters'; import * as getComputedStyles from 'roosterjs-editor-dom/lib/utils/getComputedStyles'; +import { BlockElement, Entity, IEditor, Keys, PluginKeyDownEvent } from 'roosterjs-editor-types'; import { EntityFeatures } from '../../../lib/plugins/ContentEdit/features/entityFeatures'; import { commitEntity, @@ -9,7 +10,6 @@ import { Position, PositionContentSearcher, } from 'roosterjs-editor-dom'; -import { Entity, IEditor, Keys, PluginKeyDownEvent, BlockElement } from 'roosterjs-editor-types'; describe('Content Edit Features |', () => { const { moveBetweenDelimitersFeature, removeEntityBetweenDelimiters } = EntityFeatures; @@ -37,6 +37,7 @@ describe('Content Edit Features |', () => { cleanUp(); defaultEvent = {}; testContainer = document.createElement('div'); + testContainer.setAttribute('contenteditable', 'true'); document.body.appendChild(testContainer); wrapper = document.createElement('span'); @@ -387,10 +388,44 @@ describe('Content Edit Features |', () => { restoreSelection(); }); + it('DelimiterBefore, should handle and handle, nested entity no shiftKey', () => { + setupNestedEntityScenario(entity, delimiterBefore, delimiterAfter); + + event = runTest(delimiterBefore, true /* expected */, event); + + spyOnSelection(); + moveBetweenDelimitersFeature.handleEvent(event, editor); + + expect(select).toHaveBeenCalledWith(new Position(delimiterAfter!, 1)); + expect(preventDefaultSpy).toHaveBeenCalledTimes(1); + expect(extendSpy).toHaveBeenCalledTimes(0); + + restoreSelection(); + }); + it('DelimiterBefore, should handle and handle, no shiftKey elements wrapped in B', () => { wrapElementInB(delimiterBefore); wrapElementInB(entity.wrapper); wrapElementInB(delimiterAfter); + + event = runTest(delimiterBefore, true /* expected */, event); + + spyOnSelection(); + moveBetweenDelimitersFeature.handleEvent(event, editor); + + expect(select).toHaveBeenCalledWith(new Position(delimiterAfter!, 1)); + expect(preventDefaultSpy).toHaveBeenCalledTimes(1); + expect(extendSpy).toHaveBeenCalledTimes(0); + + restoreSelection(); + }); + + it('DelimiterBefore, should handle and handle, no shiftKey elements wrapped in B and nestedEntity', () => { + wrapElementInB(delimiterBefore); + wrapElementInB(entity.wrapper); + wrapElementInB(delimiterAfter); + setupNestedEntityScenario(entity, delimiterBefore, delimiterAfter); + event = runTest(delimiterBefore, true /* expected */, event); spyOnSelection(); @@ -424,7 +459,29 @@ describe('Content Edit Features |', () => { restoreSelection(); }); - it('DelimiterBefore, should handle and handle, with shiftKey, elements wrapped in B', () => { + it('DelimiterBefore, should handle and handle, with shiftKey and nested entity', () => { + event = { + ...event, + rawEvent: { + ...event.rawEvent, + shiftKey: true, + }, + }; + setupNestedEntityScenario(entity, delimiterBefore, delimiterAfter); + + event = runTest(delimiterBefore, true /* expected */, event); + + spyOnSelection(); + + moveBetweenDelimitersFeature.handleEvent(event, editor); + + expect(extendSpy).toHaveBeenCalledWith(testContainer, 3); + expect(preventDefaultSpy).toHaveBeenCalledTimes(1); + + restoreSelection(); + }); + + it('DelimiterBefore, should handle and handle, with shiftKey, elements wrapped in B and nested entity', () => { event = { ...event, rawEvent: { @@ -436,6 +493,8 @@ describe('Content Edit Features |', () => { wrapElementInB(delimiterBefore); wrapElementInB(entity.wrapper); wrapElementInB(delimiterAfter); + setupNestedEntityScenario(entity, delimiterBefore, delimiterAfter); + event = runTest(delimiterBefore, true /* expected */, event); spyOnSelection(); @@ -518,6 +577,25 @@ describe('Content Edit Features |', () => { restoreSelection(); }); + it('DelimiterBefore, shouldHandle and Handle, cursor at end of element before delimiter before and nested entity', () => { + const bold = document.createElement('b'); + bold.append(document.createTextNode('Bold')); + testContainer.insertBefore(bold, delimiterBefore); + setupNestedEntityScenario(entity, delimiterBefore, delimiterAfter); + + event = runTest(new Position(bold.firstChild!, 4), true /* expected */, event); + + spyOnSelection(); + + moveBetweenDelimitersFeature.handleEvent(event, editor); + + expect(select).toHaveBeenCalledWith(new Position(delimiterAfter!, 1)); + expect(preventDefaultSpy).toHaveBeenCalledTimes(1); + expect(extendSpy).toHaveBeenCalledTimes(0); + + restoreSelection(); + }); + it('DelimiterBefore, should not Handle, cursor is not not at the start of the element after delimiter after', () => { const bold = document.createElement('b'); bold.append(document.createTextNode('Bold')); @@ -547,6 +625,21 @@ describe('Content Edit Features |', () => { runTest(delimiterBefore, true /* expected */, event); }); + it('DelimiterBefore, Inline Readonly Entity with multiple Inline Elements and nested scenario', () => { + const b = document.createElement('b'); + b.appendChild(document.createTextNode('Bold')); + + entity.wrapper.appendChild(b); + entity.wrapper.appendChild(b.cloneNode(true)); + + wrapElementInB(delimiterBefore); + wrapElementInB(entity.wrapper); + wrapElementInB(delimiterAfter); + setupNestedEntityScenario(entity, delimiterBefore, delimiterAfter); + + runTest(delimiterBefore, true /* expected */, event); + }); + it('DelimiterBefore, should not Handle, getBlockElementAtCursor returned inline', () => { const div = document.createElement('div'); div.appendChild(document.createTextNode('New block')); @@ -790,6 +883,22 @@ describe('Content Edit Features |', () => { } }); +function setupNestedEntityScenario( + entity: Entity, + delimiterBefore: Element | null, + delimiterAfter: Element | null +) { + const wrapperClone = entity.wrapper.cloneNode(true /* deep */); + while (entity.wrapper.firstChild) { + entity.wrapper.removeChild(entity.wrapper.firstChild); + } + entity.wrapper.append( + delimiterBefore!.cloneNode(true /* deep */), + wrapperClone, + delimiterAfter!.cloneNode(true) + ); +} + function wrapElementInB(delimiterBefore: Element | null) { const element = delimiterBefore?.insertAdjacentElement( 'beforebegin', @@ -823,6 +932,7 @@ function addEntityBeforeEach(entity: Entity, wrapper: HTMLElement) { type: 'Test', wrapper, }; + wrapper.setAttribute('contenteditable', 'false'); commitEntity(wrapper, 'test', true, 'test'); addDelimiters.default(wrapper); From 609d77cc6321217ff409f25544654ac8c1d84852 Mon Sep 17 00:00:00 2001 From: "SOUTHAMERICA\\bvalverde" Date: Tue, 14 Nov 2023 12:17:43 -0600 Subject: [PATCH 2/2] comment --- .../ContentEdit/features/entityFeatures.ts | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/roosterjs-editor-plugins/lib/plugins/ContentEdit/features/entityFeatures.ts b/packages/roosterjs-editor-plugins/lib/plugins/ContentEdit/features/entityFeatures.ts index f81d766f0c6..ba16265f6f6 100644 --- a/packages/roosterjs-editor-plugins/lib/plugins/ContentEdit/features/entityFeatures.ts +++ b/packages/roosterjs-editor-plugins/lib/plugins/ContentEdit/features/entityFeatures.ts @@ -486,16 +486,19 @@ function getRelatedElements(delimiter: HTMLElement, checkBefore: boolean, editor entity = entity || getElementFromInline(current, entitySelector); delimiterPair = delimiterPair || getElementFromInline(current, selector); - // If we found the entity but the next inline after the entity is not a delimiter, - // it means that the delimiter pair got removed or is invalid, return null instead. - if (entity && !delimiterPair && !getElementFromInline(current, entitySelector)) { - delimiterPair = null; - break; - } - if (entity && delimiterPair && !delimiterPair.isContentEditable) { - delimiterPair = null; + if (entity) { + // If we found the entity but the next inline after the entity is not a delimiter, + // it means that the delimiter pair got removed or is invalid, return null instead. + if (!delimiterPair && !getElementFromInline(current, entitySelector)) { + delimiterPair = null; + break; + } + // If the delimiter is not editable keep looking for a editable one, by setting the value as null, + // in case the entity is wrapping another inline readonly entity + if (delimiterPair && !delimiterPair.isContentEditable) { + delimiterPair = null; + } } - current = traverseFn(traverser); }