Skip to content

Commit

Permalink
feat(merge): blocks of different types can be merged (#2671)
Browse files Browse the repository at this point in the history
* feature: possibilities to merge blocks of different types

* fix: remove scope change

* feat: use convert config instead of defined property

* chore:: use built-in function for type check

* fix: remove console.log

* chore: remove styling added by mistakes

* test: add testing for different blocks types merging

* fix: remove unused import

* fix: remove type argument

* fix: use existing functions for data export

* chore: update changelog

* fix: re put await

* fix: remove unnecessary check

* fix: typo in test name

* fix: re-add condition for merge

* test: add caret position test

* fix caret issues, add sanitize

* make if-else statement more clear

* upgrade cypress

* Update cypress.yml

* upd cypress to 13

* make sanitize test simpler

* patch rc version

---------

Co-authored-by: GuillaumeOnepilot <guillaume@onepilot.co>
Co-authored-by: Guillaume Leon <97881811+GuillaumeOnepilot@users.noreply.github.com>
  • Loading branch information
3 people authored Apr 1, 2024
1 parent b355f16 commit 1320b04
Show file tree
Hide file tree
Showing 12 changed files with 571 additions and 140 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/cypress.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ jobs:
steps:
- uses: actions/setup-node@v3
with:
node-version: 16
- uses: actions/checkout@v3
- uses: cypress-io/github-action@v5
node-version: 18
- uses: actions/checkout@v4
- uses: cypress-io/github-action@v6
with:
config: video=false
browser: ${{ matrix.browser }}
Expand Down
1 change: 1 addition & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

### 2.30.0

- `Improvement` — The ability to merge blocks of different types (if both tools provide the conversionConfig)
- `Fix``onChange` will be called when removing the entire text within a descendant element of a block.
- `Fix` - Unexpected new line on Enter press with selected block without caret
- `Fix` - Search input autofocus loosing after Block Tunes opening
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@editorjs/editorjs",
"version": "2.30.0-rc.1",
"version": "2.30.0-rc.2",
"description": "Editor.js — Native JS, based on API and Open Source",
"main": "dist/editorjs.umd.js",
"module": "dist/editorjs.mjs",
Expand Down Expand Up @@ -45,14 +45,14 @@
"@editorjs/code": "^2.7.0",
"@editorjs/delimiter": "^1.2.0",
"@editorjs/header": "^2.7.0",
"@editorjs/paragraph": "^2.11.3",
"@editorjs/paragraph": "^2.11.4",
"@editorjs/simple-image": "^1.4.1",
"@types/node": "^18.15.11",
"chai-subset": "^1.6.0",
"codex-notifier": "^1.1.2",
"codex-tooltip": "^1.0.5",
"core-js": "3.30.0",
"cypress": "^12.9.0",
"cypress": "^13.7.1",
"cypress-intellij-reporter": "^0.0.7",
"cypress-plugin-tab": "^1.0.5",
"cypress-terminal-report": "^5.3.2",
Expand Down
2 changes: 1 addition & 1 deletion src/components/block/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ export default class Block extends EventsDispatcher<BlockEvents> {
*
* @returns {object}
*/
public async save(): Promise<void | SavedData> {
public async save(): Promise<undefined | SavedData> {
const extractedBlock = await this.toolInstance.save(this.pluginsContent as HTMLElement);
const tunesData: { [name: string]: BlockTuneData } = this.unavailableTunesData;

Expand Down
9 changes: 3 additions & 6 deletions src/components/modules/blockEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -493,12 +493,9 @@ export default class BlockEvents extends Module {
BlockManager
.mergeBlocks(targetBlock, blockToMerge)
.then(() => {
window.requestAnimationFrame(() => {
/** Restore caret position after merge */
Caret.restoreCaret(targetBlock.pluginsContent as HTMLElement);
targetBlock.pluginsContent.normalize();
Toolbar.close();
});
/** Restore caret position after merge */
Caret.restoreCaret(targetBlock.pluginsContent as HTMLElement);
Toolbar.close();
});
}

Expand Down
40 changes: 34 additions & 6 deletions src/components/modules/blockManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import { BlockAddedMutationType } from '../../../types/events/block/BlockAdded';
import { BlockMovedMutationType } from '../../../types/events/block/BlockMoved';
import { BlockChangedMutationType } from '../../../types/events/block/BlockChanged';
import { BlockChanged } from '../events';
import { clean } from '../utils/sanitizer';
import { convertStringToBlockData } from '../utils/blocks';
import { clean, sanitizeBlocks } from '../utils/sanitizer';
import { convertStringToBlockData, isBlockConvertable } from '../utils/blocks';
import PromiseQueue from '../utils/promise-queue';

/**
Expand Down Expand Up @@ -69,7 +69,7 @@ export default class BlockManager extends Module {
*
* @returns {Block}
*/
public get currentBlock(): Block {
public get currentBlock(): Block | undefined {
return this._blocks[this.currentBlockIndex];
}

Expand Down Expand Up @@ -471,12 +471,40 @@ export default class BlockManager extends Module {
* @returns {Promise} - the sequence that can be continued
*/
public async mergeBlocks(targetBlock: Block, blockToMerge: Block): Promise<void> {
const blockToMergeData = await blockToMerge.data;
let blockToMergeData: BlockToolData | undefined;

if (!_.isEmpty(blockToMergeData)) {
await targetBlock.mergeWith(blockToMergeData);
/**
* We can merge:
* 1) Blocks with the same Tool if tool provides merge method
*/
if (targetBlock.name === blockToMerge.name && targetBlock.mergeable) {
const blockToMergeDataRaw = await blockToMerge.data;

if (_.isEmpty(blockToMergeDataRaw)) {
console.error('Could not merge Block. Failed to extract original Block data.');

return;
}

const [ cleanData ] = sanitizeBlocks([ blockToMergeDataRaw ], targetBlock.tool.sanitizeConfig);

blockToMergeData = cleanData;

/**
* 2) Blocks with different Tools if they provides conversionConfig
*/
} else if (targetBlock.mergeable && isBlockConvertable(blockToMerge, 'export') && isBlockConvertable(targetBlock, 'import')) {
const blockToMergeDataStringified = await blockToMerge.exportDataAsString();
const cleanData = clean(blockToMergeDataStringified, targetBlock.tool.sanitizeConfig);

blockToMergeData = convertStringToBlockData(cleanData, targetBlock.tool.conversionConfig);
}

if (blockToMergeData === undefined) {
return;
}

await targetBlock.mergeWith(blockToMergeData);
this.removeBlock(blockToMerge);
this.currentBlockIndex = this._blocks.indexOf(targetBlock);
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/modules/caret.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export default class Caret extends Module {
/**
* If Block does not contain inputs, treat caret as "at start"
*/
if (!currentBlock.focusable) {
if (!currentBlock?.focusable) {
return true;
}

Expand Down
38 changes: 37 additions & 1 deletion src/components/utils/blocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,54 @@ import type { BlockToolData } from '../../../types/tools/block-tool-data';
import type Block from '../block';
import { isFunction, isString, log } from '../utils';

/**
* Check if block has valid conversion config for export or import.
*
* @param block - block to check
* @param direction - export for block to merge from, import for block to merge to
*/
export function isBlockConvertable(block: Block, direction: 'export' | 'import'): boolean {
if (!block.tool.conversionConfig) {
return false;
}

const conversionProp = block.tool.conversionConfig[direction];

return isFunction(conversionProp) || isString(conversionProp);
}

/**
* Check if two blocks could be merged.
*
* We can merge two blocks if:
* - they have the same type
* - they have a merge function (.mergeable = true)
* - If they have valid conversions config
*
* @param targetBlock - block to merge to
* @param blockToMerge - block to merge from
*/
export function areBlocksMergeable(targetBlock: Block, blockToMerge: Block): boolean {
return targetBlock.mergeable && targetBlock.name === blockToMerge.name;
/**
* If target block has not 'merge' method, we can't merge blocks.
*
* Technically we can (through the conversion) but it will lead a target block delete and recreation, which is unexpected behavior.
*/
if (!targetBlock.mergeable) {
return false;
}

/**
* Tool knows how to merge own data format
*/
if (targetBlock.name === blockToMerge.name) {
return true;
}

/**
* We can merge blocks if they have valid conversion config
*/
return isBlockConvertable(blockToMerge, 'export') && isBlockConvertable(targetBlock, 'import');
}

/**
Expand Down
90 changes: 90 additions & 0 deletions test/cypress/fixtures/tools/SimpleHeader.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import {
BaseTool,
BlockToolConstructorOptions,
BlockToolData,
ConversionConfig
} from '../../../../types';

/**
* Simplified Header for testing
*/
export class SimpleHeader implements BaseTool {
private _data: BlockToolData;
private element: HTMLHeadingElement;

/**
*
* @param options - constructor options
*/
constructor({ data }: BlockToolConstructorOptions) {
this._data = data;
}

/**
* Return Tool's view
*
* @returns {HTMLHeadingElement}
* @public
*/
public render(): HTMLHeadingElement {
this.element = document.createElement('h1');

this.element.contentEditable = 'true';
this.element.innerHTML = this._data.text;

return this.element;
}

/**
* @param data - saved data to merger with current block
*/
public merge(data: BlockToolData): void {
this.data = {
text: this.data.text + data.text,
level: this.data.level,
};
}

/**
* Extract Tool's data from the view
*
* @param toolsContent - Text tools rendered view
*/
public save(toolsContent: HTMLHeadingElement): BlockToolData {
return {
text: toolsContent.innerHTML,
level: 1,
};
}

/**
* Allow Header to be converted to/from other blocks
*/
public static get conversionConfig(): ConversionConfig {
return {
export: 'text', // use 'text' property for other blocks
import: 'text', // fill 'text' property from other block's export string
};
}

/**
* Data getter
*/
private get data(): BlockToolData {
this._data.text = this.element.innerHTML;
this._data.level = 1;

return this._data;
}

/**
* Data setter
*/
private set data(data: BlockToolData) {
this._data = data;

if (data.text !== undefined) {
this.element.innerHTML = this._data.text || '';
}
}
}
Loading

0 comments on commit 1320b04

Please sign in to comment.