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

fix(renderer): handle empty array as data.blocks #2454

Merged
merged 2 commits into from
Sep 5, 2023
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
4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

### 2.29.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you want to publish it as a patch version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe. But I don't want to include it in 2.28.0 since it has already been tested via RC.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not so critical since it has workaround

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the workaround? @neSpecc


- `Fix` — Passing an empty array via initial data or `blocks.render()` won't break the editor

### 2.28.0

- `New` - Block ids now displayed in DOM via a data-id attribute. Could be useful for plugins that want to access a Block's element by id.
Expand Down
2 changes: 1 addition & 1 deletion src/components/blocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ export default class Blocks {
* @param {number} index — Block index
* @returns {Block}
*/
public get(index: number): Block {
public get(index: number): Block | undefined {
return this.blocks[index];
}

Expand Down
17 changes: 16 additions & 1 deletion src/components/modules/blockManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -587,13 +587,28 @@ export default class BlockManager extends Module {
return this.insert({ data });
}

/**
* Returns Block by passed index
*
* If we pass -1 as index, the last block will be returned
* There shouldn't be a case when there is no blocks at all — at least one always should exist
*/
public getBlockByIndex(index: -1): Block;

/**
* Returns Block by passed index.
*
* Could return undefined if there is no block with such index
*/
public getBlockByIndex(index: number): Block | undefined;

/**
* Returns Block by passed index
*
* @param {number} index - index to get. -1 to get last
* @returns {Block}
*/
public getBlockByIndex(index): Block {
public getBlockByIndex(index: number): Block | undefined {
if (index === -1) {
index = this._blocks.length - 1;
}
Expand Down
84 changes: 44 additions & 40 deletions src/components/modules/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,53 +18,57 @@ export default class Renderer extends Module {
return new Promise((resolve) => {
const { Tools, BlockManager } = this.Editor;

/**
* Create Blocks instances
*/
const blocks = blocksData.map(({ type: tool, data, tunes, id }) => {
if (Tools.available.has(tool) === false) {
_.logLabeled(`Tool «${tool}» is not found. Check 'tools' property at the Editor.js config.`, 'warn');
if (blocksData.length === 0) {
BlockManager.insert();
} else {
/**
* Create Blocks instances
*/
const blocks = blocksData.map(({ type: tool, data, tunes, id }) => {
if (Tools.available.has(tool) === false) {
_.logLabeled(`Tool «${tool}» is not found. Check 'tools' property at the Editor.js config.`, 'warn');

data = this.composeStubDataForTool(tool, data, id);
tool = Tools.stubTool;
}
data = this.composeStubDataForTool(tool, data, id);
tool = Tools.stubTool;
}

let block: Block;
let block: Block;

try {
block = BlockManager.composeBlock({
id,
tool,
data,
tunes,
});
} catch (error) {
_.log(`Block «${tool}» skipped because of plugins error`, 'error', {
data,
error,
});
try {
block = BlockManager.composeBlock({
id,
tool,
data,
tunes,
});
} catch (error) {
_.log(`Block «${tool}» skipped because of plugins error`, 'error', {
data,
error,
});

/**
* If tool throws an error during render, we should render stub instead of it
*/
data = this.composeStubDataForTool(tool, data, id);
tool = Tools.stubTool;
/**
* If tool throws an error during render, we should render stub instead of it
*/
data = this.composeStubDataForTool(tool, data, id);
tool = Tools.stubTool;

block = BlockManager.composeBlock({
id,
tool,
data,
tunes,
});
}
block = BlockManager.composeBlock({
id,
tool,
data,
tunes,
});
}

return block;
});
return block;
});

/**
* Insert batch of Blocks
*/
BlockManager.insertMany(blocks);
/**
* Insert batch of Blocks
*/
BlockManager.insertMany(blocks);
}

/**
* Wait till browser will render inserted Blocks and resolve a promise
Expand Down
27 changes: 17 additions & 10 deletions src/components/modules/ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -694,17 +694,10 @@ export default class UI extends Module<UINodes> {
* - otherwise, add a new empty Block and set a Caret to that
*/
private redactorClicked(event: MouseEvent): void {
const { BlockSelection } = this.Editor;

if (!Selection.isCollapsed) {
return;
}

const stopPropagation = (): void => {
event.stopImmediatePropagation();
event.stopPropagation();
};

/**
* case when user clicks on anchor element
* if it is clicked via ctrl key, then we open new window with url
Expand All @@ -713,7 +706,8 @@ export default class UI extends Module<UINodes> {
const ctrlKey = event.metaKey || event.ctrlKey;

if ($.isAnchor(element) && ctrlKey) {
stopPropagation();
event.stopImmediatePropagation();
event.stopPropagation();

const href = element.getAttribute('href');
const validUrl = _.getValidUrl(href);
Expand All @@ -723,10 +717,22 @@ export default class UI extends Module<UINodes> {
return;
}

this.processBottomZoneClick(event);
}

/**
* Check if user clicks on the Editor's bottom zone:
* - set caret to the last block
* - or add new empty block
*
* @param event - click event
*/
private processBottomZoneClick(event: MouseEvent): void {
const lastBlock = this.Editor.BlockManager.getBlockByIndex(-1);

const lastBlockBottomCoord = $.offset(lastBlock.holder).bottom;
const clickedCoord = event.pageY;

const { BlockSelection } = this.Editor;
const isClickedBottom = event.target instanceof Element &&
event.target.isEqualNode(this.nodes.redactor) &&
/**
Expand All @@ -740,7 +746,8 @@ export default class UI extends Module<UINodes> {
lastBlockBottomCoord < clickedCoord;

if (isClickedBottom) {
stopPropagation();
event.stopImmediatePropagation();
event.stopPropagation();

const { BlockManager, Caret, Toolbar } = this.Editor;

Expand Down
30 changes: 30 additions & 0 deletions test/cypress/tests/modules/Renderer.cy.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import ToolMock from '../../fixtures/tools/ToolMock';
import type EditorJS from '../../../../types/index';

describe('Renderer module', function () {
it('should not cause onChange firing during initial rendering', function () {
Expand Down Expand Up @@ -146,4 +147,33 @@ describe('Renderer module', function () {
}
});
});

it('should insert default empty block when [] passed as data.blocks', function () {
cy.createEditor({
data: {
blocks: [],
},
})
.as('editorInstance');

cy.get('[data-cy=editorjs]')
.find('.ce-block')
.should('have.length', 1);
});

it('should insert default empty block when [] passed via blocks.render() API', function () {
cy.createEditor({})
.as('editorInstance');

cy.get<EditorJS>('@editorInstance')
.then((editor) => {
editor.blocks.render({
blocks: [],
});
});

cy.get('[data-cy=editorjs]')
.find('.ce-block')
.should('have.length', 1);
});
});
Loading