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.

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

Revert "Add a check for Experimental Feature DefaultFormatInSpan" #1471

Merged
merged 2 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import ApiPaneProps from '../ApiPaneProps';
import { ExperimentalFeatures, IEditor, PositionType, Region } from 'roosterjs-editor-types';
import { IEditor, PositionType, Region } from 'roosterjs-editor-types';
import {
createRange,
getSelectedBlockElementsInRegion,
Expand Down Expand Up @@ -55,11 +55,7 @@ export default class GetSelectedRegionsPane extends React.Component<

function Region({ region, editor, index }: { region: Region; editor: IEditor; index: number }) {
const selectRegion = React.useCallback(() => {
const blocks = getSelectedBlockElementsInRegion(
region,
undefined /* createBlockIfEmpty */,
editor.isFeatureEnabled(ExperimentalFeatures.DefaultFormatInSpan)
);
const blocks = getSelectedBlockElementsInRegion(region);
if (blocks.length > 0) {
const range = createRange(
blocks[0].getStartNode(),
Expand Down
9 changes: 2 additions & 7 deletions packages/roosterjs-editor-api/lib/format/clearFormat.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import applyListItemStyleWrap from '../utils/applyListItemWrap';
import blockFormat from '../utils/blockFormat';
import execCommand from '../utils/execCommand';
import formatUndoSnapshot from '../utils/formatUndoSnapshot';
Expand All @@ -13,7 +12,6 @@ import {
ChangeSource,
ClearFormatMode,
DocumentCommand,
ExperimentalFeatures,
IEditor,
QueryScope,
} from 'roosterjs-editor-types';
Expand All @@ -36,6 +34,7 @@ import {
wrap,
} from 'roosterjs-editor-dom';
import type { CompatibleClearFormatMode } from 'roosterjs-editor-types/lib/compatibleTypes';
import applyListItemStyleWrap from '../utils/applyListItemWrap';

const STYLES_TO_REMOVE = ['font', 'text-decoration', 'color', 'background'];
const TAGS_TO_UNWRAP = 'B,I,U,STRONG,EM,SUB,SUP,STRIKE,FONT,CENTER,H1,H2,H3,H4,H5,H6,UL,OL,LI,SPAN,P,BLOCKQUOTE,CODE,S,PRE'.split(
Expand Down Expand Up @@ -201,11 +200,7 @@ function clearBlockFormat(editor: IEditor) {
editor,
() => {
blockFormat(editor, region => {
const blocks = getSelectedBlockElementsInRegion(
region,
undefined /* createBlockIfEmpty */,
editor.isFeatureEnabled(ExperimentalFeatures.DefaultFormatInSpan)
);
const blocks = getSelectedBlockElementsInRegion(region);
let nodes = collapseNodesInRegion(region, blocks);

if (editor.contains(region.rootNode)) {
Expand Down
6 changes: 1 addition & 5 deletions packages/roosterjs-editor-api/lib/format/setAlignment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,7 @@ function alignList(editor: IEditor, alignment: Alignment | CompatibleAlignment)
blockFormat(
editor,
(region, start, end) => {
const blocks = getSelectedBlockElementsInRegion(
region,
undefined /* createBlockIfEmpty */,
editor.isFeatureEnabled(ExperimentalFeatures.DefaultFormatInSpan)
);
const blocks = getSelectedBlockElementsInRegion(region);
const startNode = blocks[0].getStartNode();
const vList = createVListFromRegion(region, true /*includeSiblingLists*/, startNode);
vList.setAlignment(start, end, alignment);
Expand Down
6 changes: 1 addition & 5 deletions packages/roosterjs-editor-api/lib/format/setIndentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,7 @@ export default function setIndentation(
blockFormat(
editor,
(region, start, end) => {
const blocks = getSelectedBlockElementsInRegion(
region,
true /*createBlockIfEmpty*/,
editor.isFeatureEnabled(ExperimentalFeatures.DefaultFormatInSpan)
);
const blocks = getSelectedBlockElementsInRegion(region, true /*createBlockIfEmpty*/);
const blockGroups: BlockElement[][] = [[]];

for (let i = 0; i < blocks.length; i++) {
Expand Down
8 changes: 2 additions & 6 deletions packages/roosterjs-editor-api/lib/utils/blockWrap.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import blockFormat from './blockFormat';
import { ExperimentalFeatures, IEditor } from 'roosterjs-editor-types';
import { IEditor } from 'roosterjs-editor-types';
import {
collapseNodesInRegion,
getSelectedBlockElementsInRegion,
Expand Down Expand Up @@ -28,11 +28,7 @@ export default function blockWrap(
blockFormat(
editor,
region => {
const blocks = getSelectedBlockElementsInRegion(
region,
true /*createBlockIfEmpty*/,
editor.isFeatureEnabled(ExperimentalFeatures.DefaultFormatInSpan)
);
const blocks = getSelectedBlockElementsInRegion(region, true /*createBlockIfEmpty*/);
let nodes = collapseNodesInRegion(region, blocks);
if (nodes.length > 0) {
if (nodes.length == 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,17 @@ describe('toggleBlockQuote', () => {
expect(result).toBe(expected);
}

it('Empty input, feature off', () => {
runTest('<!--{"start":[0],"end":[0]}-->', '<blockquote><div><br></div></blockquote>');
it('Empty input', () => {
runTest(
'<!--{"start":[0],"end":[0]}-->',
'<blockquote><div><span><br></span></div></blockquote>'
);
});

TestHelper.itFirefoxOnly('Empty DIV, feature off', () => {
TestHelper.itFirefoxOnly('Empty DIV', () => {
runTest(
'<div></div><!--{"start":[0],"end":[0]}-->',
'<blockquote><div><br></div></blockquote>'
'<blockquote><div><span><br></span></div></blockquote>'
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,18 @@ export const ensureTypeInContainer: EnsureTypeInContainer = (
// Only reason we don't get the selection block is that we have an empty content div
// which can happen when users removes everything (i.e. select all and DEL, or backspace from very end to begin)
// The fix is to add a DIV wrapping, apply default format and move cursor over
formatNode = createElement(
applyFormatToSpan
? KnownCreateElementDataIndex.EmptyLineFormatInSpan
: KnownCreateElementDataIndex.EmptyLine,
const newNode = createElement(
KnownCreateElementDataIndex.EmptyLine,
core.contentDiv.ownerDocument
) as HTMLElement;
core.api.insertNode(core, formatNode, {
core.api.insertNode(core, newNode, {
position: ContentPosition.End,
updateCursor: false,
replaceSelection: false,
insertOnNewLine: false,
});

formatNode = applyFormatToSpan ? (formatNode.firstChild as HTMLElement) : formatNode;
formatNode = newNode.firstChild as HTMLElement;

// element points to a wrapping node we added "<div><br></div>". We should move the selection left to <br>
position = new Position(formatNode, PositionType.Begin);
Expand Down
7 changes: 1 addition & 6 deletions packages/roosterjs-editor-core/lib/editor/Editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -976,12 +976,7 @@ export default class Editor implements IEditor {
*/
public ensureTypeInContainer(position: NodePosition, keyboardEvent?: KeyboardEvent) {
const core = this.getCore();
core.api.ensureTypeInContainer(
core,
position,
keyboardEvent,
this.isFeatureEnabled(ExperimentalFeatures.DefaultFormatInSpan)
);
core.api.ensureTypeInContainer(core, position, keyboardEvent);
}

//#endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,7 @@ describe('ensureTypeInContainer', () => {
}

it('empty', () => {
runTest('', undefined, undefined, true, '<div><span><br></span></div>');
});

it('empty, no format span', () => {
runTest('', undefined, undefined, false, '<div><br></div>');
runTest('', undefined, undefined, false, '<div><span><br></span></div>');
});

it('pure text', () => {
Expand All @@ -64,26 +60,14 @@ describe('ensureTypeInContainer', () => {
});

it('empty editor with format', () => {
runTest(
'',
{
fontSize: '10pt',
},
undefined,
true,
'<div><span style="font-size: 10pt;"><br></span></div>'
);
});

it('empty editor with format, no format span', () => {
runTest(
'',
{
fontSize: '10pt',
},
undefined,
false,
'<div style="font-size: 10pt;"><br></div>'
'<div><span style="font-size: 10pt;"><br></span></div>'
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,7 @@ export default function createVListFromRegion(
nodes.push(list);
}
} else {
const blocks = getSelectedBlockElementsInRegion(
region,
undefined,
true /* shouldApplyFormatToSpan */
);
const blocks = getSelectedBlockElementsInRegion(region);
blocks.forEach(block => {
const list = getRootListNode(region, ListSelector, block.getStartNode());

Expand All @@ -71,7 +67,7 @@ export default function createVListFromRegion(

if (nodes.length == 0 && !region.rootNode.firstChild) {
const newNode = createElement(
KnownCreateElementDataIndex.EmptyLineFormatInSpan,
KnownCreateElementDataIndex.EmptyLine,
region.rootNode.ownerDocument
)!;
region.rootNode.appendChild(newNode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ import { BlockElement, KnownCreateElementDataIndex, RegionBase } from 'roosterjs
*/
export default function getSelectedBlockElementsInRegion(
regionBase: RegionBase,
createBlockIfEmpty?: boolean,
shouldApplyFormatToSpan?: boolean
createBlockIfEmpty?: boolean
): BlockElement[] {
const range = getSelectionRangeInRegion(regionBase);
let blocks: BlockElement[] = [];
Expand Down Expand Up @@ -47,9 +46,7 @@ export default function getSelectedBlockElementsInRegion(

if (blocks.length == 0 && regionBase && !regionBase.rootNode.firstChild && createBlockIfEmpty) {
const newNode = createElement(
shouldApplyFormatToSpan
? KnownCreateElementDataIndex.EmptyLineFormatInSpan
: KnownCreateElementDataIndex.EmptyLine,
KnownCreateElementDataIndex.EmptyLine,
regionBase.rootNode.ownerDocument
);
regionBase.rootNode.appendChild(newNode!);
Expand Down
12 changes: 4 additions & 8 deletions packages/roosterjs-editor-dom/lib/utils/createElement.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import getObjectKeys from '../jsUtils/getObjectKeys';
import safeInstanceOf from './safeInstanceOf';
import { Browser } from './Browser';
import { CreateElementData, KnownCreateElementDataIndex } from 'roosterjs-editor-types';
import type { CompatibleKnownCreateElementDataIndex } from 'roosterjs-editor-types/lib/compatibleTypes';

Expand All @@ -12,9 +11,10 @@ export const KnownCreateElementData: Record<KnownCreateElementDataIndex, CreateE

// Edge can sometimes lose current format when Enter to new line.
// So here we add an extra SPAN for Edge to workaround this bug
[KnownCreateElementDataIndex.EmptyLine]: Browser.isEdge
? { tag: 'div', children: [{ tag: 'span', children: [{ tag: 'br' }] }] }
: { tag: 'div', children: [{ tag: 'br' }] },
[KnownCreateElementDataIndex.EmptyLine]: {
tag: 'div',
children: [{ tag: 'span', children: [{ tag: 'br' }] }],
},
[KnownCreateElementDataIndex.BlockquoteWrapper]: {
tag: 'blockquote',
style: 'margin-top:0;margin-bottom:0',
Expand Down Expand Up @@ -62,10 +62,6 @@ export const KnownCreateElementData: Record<KnownCreateElementDataIndex, CreateE
tag: 'div',
style: 'position: fixed; cursor: all-scroll; user-select: none; border: 1px solid #808080',
},
[KnownCreateElementDataIndex.EmptyLineFormatInSpan]: {
tag: 'div',
children: [{ tag: 'span', children: [{ tag: 'br' }] }],
},
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ describe('createElement', () => {
});

it('create by index', () => {
runTest(KnownCreateElementDataIndex.EmptyLine, '<div><br></div>');
});

it('create by index with span', () => {
runTest(KnownCreateElementDataIndex.EmptyLineFormatInSpan, '<div><span><br></span></div>');
runTest(KnownCreateElementDataIndex.EmptyLine, '<div><span><br></span></div>');
});

it('create by tag', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,4 @@ export const enum KnownCreateElementDataIndex {
* @deprecated
*/
TableSelector = 11,

/**
* An empty line without format with span inside of it.
*/
EmptyLineFormatInSpan = 12,
}