Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Delimiter Nested Entity Scenario #2207

Merged
merged 4 commits into from
Nov 15, 2023
Merged

Fix Delimiter Nested Entity Scenario #2207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b6ad856
init
BryanValverdeU Nov 14, 2023
90e1634
Merge branch 'master' into u/bvalverde/fixNestedEntityDelimiterIssue
BryanValverdeU Nov 14, 2023
609d77c
comment
BryanValverdeU Nov 14, 2023
94b78ab
Merge branch 'u/bvalverde/fixNestedEntityDelimiterIssue' of https://g…
BryanValverdeU Nov 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions packages/roosterjs-editor-plugins/lib/plugins/ContentEdit/features/entityFeatures.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand All @@ -486,11 +486,18 @@ 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) {
// 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);
}
Expand Down
114 changes: 112 additions & 2 deletions packages/roosterjs-editor-plugins/test/ContentEdit/features/inlineEntityFeatureTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -37,6 +37,7 @@ describe('Content Edit Features |', () => {
cleanUp();
defaultEvent = <PluginKeyDownEvent>{};
testContainer = document.createElement('div');
testContainer.setAttribute('contenteditable', 'true');
document.body.appendChild(testContainer);

wrapper = document.createElement('span');
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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: <KeyboardEvent>{
...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: <KeyboardEvent>{
Expand All @@ -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();
Expand Down Expand Up @@ -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'));
Expand Down Expand Up @@ -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'));
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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);
Expand Down
Loading