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

Commit 36e588a

Browse files
niveditcfacebook-github-bot
authored andcommitted
Merge successive non-leaf blocks resulting from untab
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
1 parent 0cb3804 commit 36e588a

File tree

6 files changed

+618
-0
lines changed

6 files changed

+618
-0
lines changed

src/model/modifier/exploration/DraftTreeOperations.js

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,11 +433,70 @@ const moveChildUp = (blockMap: BlockMap, key: string): BlockMap => {
433433
return newBlockMap;
434434
};
435435

436+
/**
437+
* This is a utility method to merge two non-leaf blocks into one. The next block's
438+
* children are added to the provided block & the next block is deleted.
439+
*
440+
* This operation respects the tree data invariants - it expects and returns a
441+
* valid tree.
442+
*/
443+
const mergeBlocks = (blockMap: BlockMap, key: string): BlockMap => {
444+
verifyTree(blockMap);
445+
// current block must be a non-leaf
446+
let block = blockMap.get(key);
447+
invariant(block !== null, 'block must exist in block map');
448+
invariant(block.getChildKeys().count() > 0, 'block must be a non-leaf');
449+
// next block must exist & be a non-leaf
450+
const nextBlockKey = block.getNextSiblingKey();
451+
invariant(nextBlockKey != null, 'block must have a next block');
452+
const nextBlock = blockMap.get(nextBlockKey);
453+
invariant(nextBlock != null, 'next block must exist in block map');
454+
invariant(
455+
nextBlock.getChildKeys().count() > 0,
456+
'next block must be a non-leaf',
457+
);
458+
459+
const childKeys = block.getChildKeys().concat(nextBlock.getChildKeys());
460+
block = block.merge({
461+
nextSibling: nextBlock.getNextSiblingKey(),
462+
children: childKeys,
463+
});
464+
const nextChildren = childKeys.map((k, i) =>
465+
blockMap.get(k).merge({
466+
parent: key,
467+
prevSibling: i - 1 < 0 ? null : childKeys.get(i - 1),
468+
nextSibling: i + 1 === childKeys.count() ? null : childKeys.get(i + 1),
469+
}),
470+
);
471+
472+
const nextNextBlockKey = nextBlock.getNextSiblingKey();
473+
const blocks = blockMap.toSeq();
474+
const newBlockMap = blocks
475+
.takeUntil(b => b.getKey() === key)
476+
.concat(
477+
[[block.getKey(), block]],
478+
nextChildren.map(b => [b.getKey(), b]),
479+
nextNextBlockKey != null
480+
? [
481+
[
482+
nextNextBlockKey,
483+
blockMap.get(nextNextBlockKey).merge({prevSibling: key}),
484+
],
485+
]
486+
: [],
487+
blocks.skipUntil(b => b.getKey() === nextNextBlockKey).rest(),
488+
)
489+
.toOrderedMap();
490+
verifyTree(newBlockMap);
491+
return newBlockMap;
492+
};
493+
436494
module.exports = {
437495
updateParentChild,
438496
replaceParentChild,
439497
updateSibling,
440498
createNewParent,
441499
updateAsSiblingsChild,
442500
moveChildUp,
501+
mergeBlocks,
443502
};

src/model/modifier/exploration/NestedRichTextEditorUtil.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,7 @@ const NestedRichTextEditorUtil: RichTextUtils = {
422422

423423
// on untab, we also want to unnest any sibling blocks that become two levels deep
424424
// ensure that block's old parent does not have a non-leaf as its first child.
425+
let childWasUntabbed = false;
425426
if (parentKey != null) {
426427
let parent = blockMap.get(parentKey);
427428
while (parent != null) {
@@ -437,6 +438,26 @@ const NestedRichTextEditorUtil: RichTextUtils = {
437438
} else {
438439
blockMap = DraftTreeOperations.moveChildUp(blockMap, firstChildKey);
439440
parent = blockMap.get(parentKey);
441+
childWasUntabbed = true;
442+
}
443+
}
444+
}
445+
446+
// now, we may be in a state with two non-leaf blocks of the same type
447+
// next to each other
448+
if (childWasUntabbed && parentKey != null) {
449+
const parent = blockMap.get(parentKey);
450+
const prevSiblingKey =
451+
parent != null // parent may have been deleted
452+
? parent.getPrevSiblingKey()
453+
: null;
454+
if (prevSiblingKey != null && parent.getChildKeys().count() > 0) {
455+
const prevSibling = blockMap.get(prevSiblingKey);
456+
if (prevSibling != null && prevSibling.getChildKeys().count() > 0) {
457+
blockMap = DraftTreeOperations.mergeBlocks(
458+
blockMap,
459+
prevSiblingKey,
460+
);
440461
}
441462
}
442463
}

src/model/modifier/exploration/__tests__/DraftTreeOperations-test.js

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -544,3 +544,66 @@ const blockMap10 = Immutable.OrderedMap({
544544
test('test moving only child up deletes parent 3', () => {
545545
expect(DraftTreeOperations.moveChildUp(blockMap10, 'B')).toMatchSnapshot();
546546
});
547+
548+
const blockMap11 = Immutable.OrderedMap({
549+
A: new ContentBlockNode({
550+
key: 'A',
551+
parent: null,
552+
text: 'alpha',
553+
children: Immutable.List([]),
554+
prevSibling: null,
555+
nextSibling: 'X',
556+
}),
557+
X: new ContentBlockNode({
558+
key: 'X',
559+
parent: null,
560+
text: '',
561+
children: Immutable.List(['B', 'C']),
562+
prevSibling: 'A',
563+
nextSibling: 'Y',
564+
}),
565+
B: new ContentBlockNode({
566+
key: 'B',
567+
parent: 'X',
568+
text: 'beta',
569+
children: Immutable.List([]),
570+
prevSibling: null,
571+
nextSibling: 'C',
572+
}),
573+
C: new ContentBlockNode({
574+
key: 'C',
575+
parent: 'X',
576+
text: 'charlie',
577+
children: Immutable.List([]),
578+
prevSibling: 'B',
579+
nextSibling: null,
580+
}),
581+
Y: new ContentBlockNode({
582+
key: 'Y',
583+
parent: null,
584+
text: '',
585+
children: Immutable.List(['D', 'E']),
586+
prevSibling: 'X',
587+
nextSibling: null,
588+
}),
589+
D: new ContentBlockNode({
590+
key: 'D',
591+
parent: 'Y',
592+
text: 'delta',
593+
children: Immutable.List([]),
594+
prevSibling: null,
595+
nextSibling: 'E',
596+
}),
597+
E: new ContentBlockNode({
598+
key: 'E',
599+
parent: 'Y',
600+
text: 'epsilon',
601+
children: Immutable.List([]),
602+
prevSibling: 'D',
603+
nextSibling: null,
604+
}),
605+
});
606+
607+
test('test merging blocks', () => {
608+
expect(DraftTreeOperations.mergeBlocks(blockMap11, 'X')).toMatchSnapshot();
609+
});

src/model/modifier/exploration/__tests__/NestedRichTextEditorUtil-test.js

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,84 @@ test('onTab (untab) unnests non-leaf next sibling', () => {
645645
);
646646
});
647647

648+
const contentBlockNodes5 = [
649+
new ContentBlockNode({
650+
key: 'A',
651+
parent: null,
652+
text: 'alpha',
653+
children: Immutable.List([]),
654+
prevSibling: null,
655+
nextSibling: 'X',
656+
type: 'ordered-list-item',
657+
}),
658+
new ContentBlockNode({
659+
key: 'X',
660+
parent: null,
661+
text: '',
662+
children: Immutable.List(['B', 'Y', 'E']),
663+
prevSibling: 'A',
664+
nextSibling: null,
665+
type: 'ordered-list-item',
666+
}),
667+
new ContentBlockNode({
668+
key: 'B',
669+
parent: 'X',
670+
text: 'beta',
671+
children: Immutable.List([]),
672+
prevSibling: null,
673+
nextSibling: 'Y',
674+
type: 'ordered-list-item',
675+
}),
676+
new ContentBlockNode({
677+
key: 'Y',
678+
parent: 'X',
679+
text: '',
680+
children: Immutable.List(['C', 'D']),
681+
prevSibling: 'B',
682+
nextSibling: 'E',
683+
type: 'ordered-list-item',
684+
}),
685+
new ContentBlockNode({
686+
key: 'C',
687+
parent: 'Y',
688+
text: 'charlie',
689+
children: Immutable.List([]),
690+
prevSibling: null,
691+
nextSibling: 'D',
692+
type: 'ordered-list-item',
693+
}),
694+
new ContentBlockNode({
695+
key: 'D',
696+
parent: 'Y',
697+
text: 'delta',
698+
children: Immutable.List([]),
699+
prevSibling: 'C',
700+
nextSibling: null,
701+
type: 'ordered-list-item',
702+
}),
703+
new ContentBlockNode({
704+
key: 'E',
705+
parent: 'X',
706+
text: 'epsilon',
707+
children: Immutable.List([]),
708+
prevSibling: 'Y',
709+
nextSibling: null,
710+
type: 'ordered-list-item',
711+
}),
712+
];
713+
714+
test('onTab (untab) merges adjacent non-leaf blocks', () => {
715+
assertNestedUtilOperation(
716+
editorState =>
717+
onTab({preventDefault: () => {}, shiftKey: true}, editorState, 2),
718+
{
719+
anchorKey: 'B',
720+
focusKey: 'B',
721+
},
722+
contentBlockNodes5,
723+
);
724+
});
725+
648726
// TODO (T32099101)
649727
test('onSplitParent must split a nested block retaining parent', () => {
650728
expect(true).toBe(true);

0 commit comments

Comments
 (0)