Skip to content

Commit

Permalink
Merge pull request #2609 from microsoft/u/juliaroldi/image-span
Browse files Browse the repository at this point in the history 
Do not remove image span
  • Loading branch information
@juliaroldi
juliaroldi authored May 1, 2024
2 parents 657542f + ca86446 commit 7ca640e
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 35 deletions.
36 changes: 31 additions & 5 deletions packages/roosterjs-content-model-core/lib/coreApi/setDOMSelection/setDOMSelection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@ import { addRangeToSelection } from './addRangeToSelection';
import { ensureUniqueId } from '../setEditorStyle/ensureUniqueId';
import { findLastedCoInMergedCell } from './findLastedCoInMergedCell';
import { findTableCellElement } from './findTableCellElement';
import { isNodeOfType, parseTableCells, toArray } from 'roosterjs-content-model-dom';
import {
isElementOfType,
isNodeOfType,
parseTableCells,
toArray,
wrap,
} from 'roosterjs-content-model-dom';
import type {
ParsedTable,
SelectionChangedEvent,
Expand All @@ -18,7 +24,7 @@ const TABLE_ID = 'table';
const DEFAULT_SELECTION_BORDER_COLOR = '#DB626C';
const TABLE_CSS_RULE = 'background-color:#C6C6C6!important;';
const CARET_CSS_RULE = 'caret-color: transparent';
const TRANSPARENT_SELECTION_CSS_RULE = 'background-color: transparent !important';
const TRANSPARENT_SELECTION_CSS_RULE = 'background-color: transparent !important;';
const SELECTION_SELECTOR = '*::selection';

/**
Expand All @@ -39,16 +45,19 @@ export const setDOMSelection: SetDOMSelection = (core, selection, skipSelectionC
try {
switch (selection?.type) {
case 'image':
const image = selection.image;
const image = ensureImageHasSpanParent(selection.image);

core.selection.selection = selection;
core.selection.selection = {
type: 'image',
image,
};
core.api.setEditorStyle(
core,
DOM_SELECTION_CSS_KEY,
`outline-style:auto!important; outline-color:${
core.selection.imageSelectionBorderColor || DEFAULT_SELECTION_BORDER_COLOR
}!important;`,
[`#${ensureUniqueId(image, IMAGE_ID)}`]
[`span:has(>img#${ensureUniqueId(image, IMAGE_ID)})`]
);
core.api.setEditorStyle(
core,
Expand Down Expand Up @@ -231,3 +240,20 @@ function setRangeSelection(doc: Document, element: HTMLElement | undefined, coll
addRangeToSelection(doc, range, isReverted);
}
}

function ensureImageHasSpanParent(image: HTMLImageElement): HTMLImageElement {
const parent = image.parentElement;

if (
parent &&
isNodeOfType(parent, 'ELEMENT_NODE') &&
isElementOfType(parent, 'span') &&
parent.firstChild == image &&
parent.lastChild == image
) {
return image;
}

wrap(image.ownerDocument, image, 'span');
return image;
}
83 changes: 54 additions & 29 deletions packages/roosterjs-content-model-core/test/coreApi/setDOMSelection/setDOMSelectionTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ describe('setDOMSelection', () => {
let doc: Document;
let contentDiv: HTMLDivElement;
let mockedRange = 'RANGE' as any;
let createElementSpy: jasmine.Spy;
let appendChildSpy: jasmine.Spy;

beforeEach(() => {
querySelectorAllSpy = jasmine.createSpy('querySelectorAll');
Expand All @@ -27,11 +29,16 @@ describe('setDOMSelection', () => {
createRangeSpy = jasmine.createSpy('createRange');
setEditorStyleSpy = jasmine.createSpy('setEditorStyle');
containsSpy = jasmine.createSpy('contains').and.returnValue(true);
appendChildSpy = jasmine.createSpy('appendChild');
createElementSpy = jasmine.createSpy('createElement').and.returnValue({
appendChild: appendChildSpy,
});

doc = {
querySelectorAll: querySelectorAllSpy,
createRange: createRangeSpy,
contains: containsSpy,
createElement: createElementSpy,
} as any;
contentDiv = {
ownerDocument: doc,
Expand Down Expand Up @@ -221,9 +228,14 @@ describe('setDOMSelection', () => {

describe('Image selection', () => {
let mockedImage: HTMLImageElement;

beforeEach(() => {
mockedImage = {
parentElement: {
ownerDocument: doc,
firstElementChild: mockedImage,
lastElementChild: mockedImage,
appendChild: appendChildSpy,
},
ownerDocument: doc,
} as any;
});
Expand Down Expand Up @@ -265,6 +277,7 @@ describe('setDOMSelection', () => {
expect(mockedImage.id).toBe('image_0');
expect(setEditorStyleSpy).toHaveBeenCalledTimes(5);
expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelection', null);
expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelectionHideCursor', null);
expect(setEditorStyleSpy).toHaveBeenCalledWith(
core,
'_DOMSelectionHideSelection',
Expand All @@ -274,11 +287,17 @@ describe('setDOMSelection', () => {
core,
'_DOMSelection',
'outline-style:auto!important; outline-color:#DB626C!important;',
['#image_0']
['span:has(>img#image_0)']
);
expect(setEditorStyleSpy).toHaveBeenCalledWith(
core,
'_DOMSelectionHideSelection',
'background-color: transparent !important;',
['*::selection']
);
});

it('image selection with duplicated id', () => {
it('image selection with customized selection border color', () => {
const mockedSelection = {
type: 'image',
image: mockedImage,
Expand All @@ -290,19 +309,19 @@ describe('setDOMSelection', () => {
collapse: collapseSpy,
};

mockedImage.id = 'image_0';
core.selection.imageSelectionBorderColor = 'red';

createRangeSpy.and.returnValue(mockedRange);

querySelectorAllSpy.and.callFake(selector => {
return selector == '#image_0' ? ['', ''] : [''];
});
querySelectorAllSpy.and.returnValue([]);
hasFocusSpy.and.returnValue(false);

setDOMSelection(core, mockedSelection);

expect(core.selection).toEqual({
skipReselectOnFocus: undefined,
selection: mockedSelection,
imageSelectionBorderColor: 'red',
} as any);
expect(triggerEventSpy).toHaveBeenCalledWith(
core,
Expand All @@ -317,6 +336,7 @@ describe('setDOMSelection', () => {
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',
Expand All @@ -325,18 +345,18 @@ describe('setDOMSelection', () => {
expect(setEditorStyleSpy).toHaveBeenCalledWith(
core,
'_DOMSelection',
'outline-style:auto!important; outline-color:#DB626C!important;',
['#image_0_0']
'outline-style:auto!important; outline-color:red!important;',
['span:has(>img#image_0)']
);
expect(setEditorStyleSpy).toHaveBeenCalledWith(
core,
'_DOMSelectionHideSelection',
'background-color: transparent !important',
'background-color: transparent !important;',
['*::selection']
);
});

it('image selection with customized selection border color', () => {
it('do not select if node is out of document', () => {
const mockedSelection = {
type: 'image',
image: mockedImage,
Expand All @@ -348,7 +368,7 @@ describe('setDOMSelection', () => {
collapse: collapseSpy,
};

core.selection.imageSelectionBorderColor = 'red';
doc.contains = () => false;

createRangeSpy.and.returnValue(mockedRange);

Expand All @@ -360,7 +380,6 @@ describe('setDOMSelection', () => {
expect(core.selection).toEqual({
skipReselectOnFocus: undefined,
selection: mockedSelection,
imageSelectionBorderColor: 'red',
} as any);
expect(triggerEventSpy).toHaveBeenCalledWith(
core,
Expand All @@ -370,9 +389,10 @@ describe('setDOMSelection', () => {
},
true
);
expect(selectNodeSpy).toHaveBeenCalledWith(mockedImage);
expect(collapseSpy).not.toHaveBeenCalledWith();
expect(addRangeToSelectionSpy).toHaveBeenCalledWith(doc, mockedRange, undefined);
expect(selectNodeSpy).not.toHaveBeenCalled();
expect(collapseSpy).not.toHaveBeenCalled();
expect(addRangeToSelectionSpy).not.toHaveBeenCalled();
expect(mockedImage.id).toBe('image_0');
expect(setEditorStyleSpy).toHaveBeenCalledTimes(5);
expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelection', null);
expect(setEditorStyleSpy).toHaveBeenCalledWith(core, '_DOMSelectionHideCursor', null);
Expand All @@ -384,18 +404,18 @@ describe('setDOMSelection', () => {
expect(setEditorStyleSpy).toHaveBeenCalledWith(
core,
'_DOMSelection',
'outline-style:auto!important; outline-color:red!important;',
['#image_0']
'outline-style:auto!important; outline-color:#DB626C!important;',
['span:has(>img#image_0)']
);
expect(setEditorStyleSpy).toHaveBeenCalledWith(
core,
'_DOMSelectionHideSelection',
'background-color: transparent !important',
'background-color: transparent !important;',
['*::selection']
);
});

it('do not select if node is out of document', () => {
it('image selection with duplicated id', () => {
const mockedSelection = {
type: 'image',
image: mockedImage,
Expand All @@ -407,11 +427,12 @@ describe('setDOMSelection', () => {
collapse: collapseSpy,
};

doc.contains = () => false;

mockedImage.id = 'image_0';
createRangeSpy.and.returnValue(mockedRange);

querySelectorAllSpy.and.returnValue([]);
querySelectorAllSpy.and.callFake(selector => {
return selector == '#image_0' ? ['', ''] : [''];
});
hasFocusSpy.and.returnValue(false);

setDOMSelection(core, mockedSelection);
Expand All @@ -428,23 +449,27 @@ describe('setDOMSelection', () => {
},
true
);
expect(selectNodeSpy).not.toHaveBeenCalled();
expect(collapseSpy).not.toHaveBeenCalled();
expect(addRangeToSelectionSpy).not.toHaveBeenCalled();
expect(mockedImage.id).toBe('image_0');
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:auto!important; outline-color:#DB626C!important;',
['#image_0']
['span:has(>img#image_0_0)']
);
expect(setEditorStyleSpy).toHaveBeenCalledWith(
core,
'_DOMSelectionHideSelection',
'background-color: transparent !important',
'background-color: transparent !important;',
['*::selection']
);
});
Expand Down
11 changes: 10 additions & 1 deletion packages/roosterjs-content-model-dom/lib/modelToDom/optimizers/removeUnnecessarySpan.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ export function removeUnnecessarySpan(root: Node) {
if (
isNodeOfType(child, 'ELEMENT_NODE') &&
child.tagName == 'SPAN' &&
child.attributes.length == 0
child.attributes.length == 0 &&
!isImageSpan(child)
) {
const node = child;
let refNode = child.nextSibling;
Expand All @@ -26,3 +27,11 @@ export function removeUnnecessarySpan(root: Node) {
}
}
}

const isImageSpan = (child: HTMLElement) => {
return (
isNodeOfType(child.firstChild, 'ELEMENT_NODE') &&
child.firstChild.tagName == 'IMG' &&
child.firstChild == child.lastChild
);
};
9 changes: 9 additions & 0 deletions packages/roosterjs-content-model-dom/test/modelToDom/optimizers/removeUnnecessarySpanTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,13 @@ describe('removeUnnecessarySpan', () => {

expect(div.innerHTML).toBe('test1<span id="a">test2</span><b>test3</b>');
});

it('Do not remove image span', () => {
const div = document.createElement('div');
div.innerHTML = '<span><img src="test" /></span>';

removeUnnecessarySpan(div);

expect(div.innerHTML).toBe('<span><img src="test"></span>');
});
});

0 comments on commit 7ca640e

Please sign in to comment.