Skip to content

Commit

Permalink
Merge pull request #1471 from microsoft/revert-1469-u/bvalverde/useEx…
Browse files Browse the repository at this point in the history
…pFeature

Revert "Add a check for Experimental Feature DefaultFormatInSpan"
juliaroldi authored Dec 20, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
2 parents 80d1a01 + 8e5333a commit 35588ba
Showing 14 changed files with 31 additions and 92 deletions.
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,
@@ -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(),
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';
@@ -13,7 +12,6 @@ import {
ChangeSource,
ClearFormatMode,
DocumentCommand,
ExperimentalFeatures,
IEditor,
QueryScope,
} from 'roosterjs-editor-types';
@@ -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(
@@ -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)) {
6 changes: 1 addition & 5 deletions packages/roosterjs-editor-api/lib/format/setAlignment.ts
Original file line number Diff line number Diff line change
@@ -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);
6 changes: 1 addition & 5 deletions packages/roosterjs-editor-api/lib/format/setIndentation.ts
Original file line number Diff line number Diff line change
@@ -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++) {
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,
@@ -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) {
Original file line number Diff line number Diff line change
@@ -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>'
);
});

Original file line number Diff line number Diff line change
@@ -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);
7 changes: 1 addition & 6 deletions packages/roosterjs-editor-core/lib/editor/Editor.ts
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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', () => {
@@ -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>'
);
});

Original file line number Diff line number Diff line change
@@ -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());

@@ -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);
Original file line number Diff line number Diff line change
@@ -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[] = [];
@@ -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!);
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';

@@ -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',
@@ -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' }] }],
},
};

/**
Original file line number Diff line number Diff line change
@@ -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', () => {
Original file line number Diff line number Diff line change
@@ -61,9 +61,4 @@ export const enum KnownCreateElementDataIndex {
* @deprecated
*/
TableSelector = 11,

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

0 comments on commit 35588ba

Please sign in to comment.