Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

Commit

Permalink
Merge successive non-leaf blocks resulting from untab
Browse files Browse the repository at this point in the history
Summary:
When untab results in two consecutive non-leaf blocks of the same type, it causes various bugs & issues on certain further operations.

- Implement an operation to merge two consecutive blocks in the block map by adding all the children of the next block to the given (previous) block & removing the next block.
- Use this operation during untab for the case where children were moved up & there are two consecutive non-leaf blocks (see before & after videos in test plan)

Merge operation:
https://pxl.cl/hsS9

Untab resulting in merge:
https://pxl.cl/hsSg

(these two examples are implemented as tests )

Reviewed By: vdurmont

Differential Revision: D9834729

fbshipit-source-id: 5352763266e3b5fbb030b329015298bd669a4a4f
  • Loading branch information
niveditc authored and facebook-github-bot committed Sep 14, 2018
1 parent 0cb3804 commit 36e588a
Show file tree
Hide file tree
Showing 6 changed files with 618 additions and 0 deletions.
59 changes: 59 additions & 0 deletions src/model/modifier/exploration/DraftTreeOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -433,11 +433,70 @@ const moveChildUp = (blockMap: BlockMap, key: string): BlockMap => {
return newBlockMap;
};

/**
* This is a utility method to merge two non-leaf blocks into one. The next block's
* children are added to the provided block & the next block is deleted.
*
* This operation respects the tree data invariants - it expects and returns a
* valid tree.
*/
const mergeBlocks = (blockMap: BlockMap, key: string): BlockMap => {
verifyTree(blockMap);
// current block must be a non-leaf
let block = blockMap.get(key);
invariant(block !== null, 'block must exist in block map');
invariant(block.getChildKeys().count() > 0, 'block must be a non-leaf');
// next block must exist & be a non-leaf
const nextBlockKey = block.getNextSiblingKey();
invariant(nextBlockKey != null, 'block must have a next block');
const nextBlock = blockMap.get(nextBlockKey);
invariant(nextBlock != null, 'next block must exist in block map');
invariant(
nextBlock.getChildKeys().count() > 0,
'next block must be a non-leaf',
);

const childKeys = block.getChildKeys().concat(nextBlock.getChildKeys());
block = block.merge({
nextSibling: nextBlock.getNextSiblingKey(),
children: childKeys,
});
const nextChildren = childKeys.map((k, i) =>
blockMap.get(k).merge({
parent: key,
prevSibling: i - 1 < 0 ? null : childKeys.get(i - 1),
nextSibling: i + 1 === childKeys.count() ? null : childKeys.get(i + 1),
}),
);

const nextNextBlockKey = nextBlock.getNextSiblingKey();
const blocks = blockMap.toSeq();
const newBlockMap = blocks
.takeUntil(b => b.getKey() === key)
.concat(
[[block.getKey(), block]],
nextChildren.map(b => [b.getKey(), b]),
nextNextBlockKey != null
? [
[
nextNextBlockKey,
blockMap.get(nextNextBlockKey).merge({prevSibling: key}),
],
]
: [],
blocks.skipUntil(b => b.getKey() === nextNextBlockKey).rest(),
)
.toOrderedMap();
verifyTree(newBlockMap);
return newBlockMap;
};

module.exports = {
updateParentChild,
replaceParentChild,
updateSibling,
createNewParent,
updateAsSiblingsChild,
moveChildUp,
mergeBlocks,
};
21 changes: 21 additions & 0 deletions src/model/modifier/exploration/NestedRichTextEditorUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ const NestedRichTextEditorUtil: RichTextUtils = {

// on untab, we also want to unnest any sibling blocks that become two levels deep
// ensure that block's old parent does not have a non-leaf as its first child.
let childWasUntabbed = false;
if (parentKey != null) {
let parent = blockMap.get(parentKey);
while (parent != null) {
Expand All @@ -437,6 +438,26 @@ const NestedRichTextEditorUtil: RichTextUtils = {
} else {
blockMap = DraftTreeOperations.moveChildUp(blockMap, firstChildKey);
parent = blockMap.get(parentKey);
childWasUntabbed = true;
}
}
}

// now, we may be in a state with two non-leaf blocks of the same type
// next to each other
if (childWasUntabbed && parentKey != null) {
const parent = blockMap.get(parentKey);
const prevSiblingKey =
parent != null // parent may have been deleted
? parent.getPrevSiblingKey()
: null;
if (prevSiblingKey != null && parent.getChildKeys().count() > 0) {
const prevSibling = blockMap.get(prevSiblingKey);
if (prevSibling != null && prevSibling.getChildKeys().count() > 0) {
blockMap = DraftTreeOperations.mergeBlocks(
blockMap,
prevSiblingKey,
);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -544,3 +544,66 @@ const blockMap10 = Immutable.OrderedMap({
test('test moving only child up deletes parent 3', () => {
expect(DraftTreeOperations.moveChildUp(blockMap10, 'B')).toMatchSnapshot();
});

const blockMap11 = Immutable.OrderedMap({
A: new ContentBlockNode({
key: 'A',
parent: null,
text: 'alpha',
children: Immutable.List([]),
prevSibling: null,
nextSibling: 'X',
}),
X: new ContentBlockNode({
key: 'X',
parent: null,
text: '',
children: Immutable.List(['B', 'C']),
prevSibling: 'A',
nextSibling: 'Y',
}),
B: new ContentBlockNode({
key: 'B',
parent: 'X',
text: 'beta',
children: Immutable.List([]),
prevSibling: null,
nextSibling: 'C',
}),
C: new ContentBlockNode({
key: 'C',
parent: 'X',
text: 'charlie',
children: Immutable.List([]),
prevSibling: 'B',
nextSibling: null,
}),
Y: new ContentBlockNode({
key: 'Y',
parent: null,
text: '',
children: Immutable.List(['D', 'E']),
prevSibling: 'X',
nextSibling: null,
}),
D: new ContentBlockNode({
key: 'D',
parent: 'Y',
text: 'delta',
children: Immutable.List([]),
prevSibling: null,
nextSibling: 'E',
}),
E: new ContentBlockNode({
key: 'E',
parent: 'Y',
text: 'epsilon',
children: Immutable.List([]),
prevSibling: 'D',
nextSibling: null,
}),
});

test('test merging blocks', () => {
expect(DraftTreeOperations.mergeBlocks(blockMap11, 'X')).toMatchSnapshot();
});
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,84 @@ test('onTab (untab) unnests non-leaf next sibling', () => {
);
});

const contentBlockNodes5 = [
new ContentBlockNode({
key: 'A',
parent: null,
text: 'alpha',
children: Immutable.List([]),
prevSibling: null,
nextSibling: 'X',
type: 'ordered-list-item',
}),
new ContentBlockNode({
key: 'X',
parent: null,
text: '',
children: Immutable.List(['B', 'Y', 'E']),
prevSibling: 'A',
nextSibling: null,
type: 'ordered-list-item',
}),
new ContentBlockNode({
key: 'B',
parent: 'X',
text: 'beta',
children: Immutable.List([]),
prevSibling: null,
nextSibling: 'Y',
type: 'ordered-list-item',
}),
new ContentBlockNode({
key: 'Y',
parent: 'X',
text: '',
children: Immutable.List(['C', 'D']),
prevSibling: 'B',
nextSibling: 'E',
type: 'ordered-list-item',
}),
new ContentBlockNode({
key: 'C',
parent: 'Y',
text: 'charlie',
children: Immutable.List([]),
prevSibling: null,
nextSibling: 'D',
type: 'ordered-list-item',
}),
new ContentBlockNode({
key: 'D',
parent: 'Y',
text: 'delta',
children: Immutable.List([]),
prevSibling: 'C',
nextSibling: null,
type: 'ordered-list-item',
}),
new ContentBlockNode({
key: 'E',
parent: 'X',
text: 'epsilon',
children: Immutable.List([]),
prevSibling: 'Y',
nextSibling: null,
type: 'ordered-list-item',
}),
];

test('onTab (untab) merges adjacent non-leaf blocks', () => {
assertNestedUtilOperation(
editorState =>
onTab({preventDefault: () => {}, shiftKey: true}, editorState, 2),
{
anchorKey: 'B',
focusKey: 'B',
},
contentBlockNodes5,
);
});

// TODO (T32099101)
test('onSplitParent must split a nested block retaining parent', () => {
expect(true).toBe(true);
Expand Down
Loading

0 comments on commit 36e588a

Please sign in to comment.