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

Fixed display of conversion menu for blocks without export rule #2799

Merged
merged 4 commits into from
Sep 13, 2024
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.30.6

– `Fix` – Fix the display of ‘Convert To’ near blocks that do not have the ‘conversionConfig.export’ rule specified

### 2.30.5

– `Fix` – Fix exported types
Expand Down
22 changes: 19 additions & 3 deletions src/components/utils/blocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,15 @@ export async function getConvertibleToolsForBlock(block: BlockAPI, allBlockTools
const savedData = await block.save() as SavedData;
const blockData = savedData.data;

/**
* Checking that the block's tool has an «export» rule
*/
const blockTool = allBlockTools.find((tool) => tool.name === block.name);

if (blockTool !== undefined && !isToolConvertable(blockTool, 'export')) {
return [];
}

return allBlockTools.reduce((result, tool) => {
/**
* Skip tools without «import» rule specified
Expand All @@ -59,12 +68,19 @@ export async function getConvertibleToolsForBlock(block: BlockAPI, allBlockTools
return result;
}

/**
* Skip tools that does not specify toolbox
*/
if (tool.toolbox === undefined) {
return result;
}

/** Filter out invalid toolbox entries */
const actualToolboxItems = tool.toolbox.filter((toolboxItem) => {
/**
* Skip items that don't pass 'toolbox' property or do not have an icon
*/
if (isEmpty(toolboxItem) || !toolboxItem.icon) {
if (isEmpty(toolboxItem) || toolboxItem.icon === undefined) {
return false;
}

Expand All @@ -86,10 +102,10 @@ export async function getConvertibleToolsForBlock(block: BlockAPI, allBlockTools
result.push({
...tool,
toolbox: actualToolboxItems,
});
} as BlockToolAdapter);

return result;
}, []);
}, [] as BlockToolAdapter[]);
}


Expand Down
23 changes: 23 additions & 0 deletions test/cypress/fixtures/tools/ToolWithoutConversionExport.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import type { ConversionConfig } from '@/types/configs/conversion-config';
import ToolMock from './ToolMock';

/**
* This tool has a conversionConfig, but it doesn't have export property.
*
* That means that tool can be created from string, but can't be converted to string.
*/
export class ToolWithoutConversionExport extends ToolMock {
/**
* Rules specified how our Tool can be converted to/from other Tool.
*/
public static get conversionConfig(): ConversionConfig {
return {
import: 'text', // this tool can be created from string

/**
* Here is no "export" property, so this tool can't be converted to string
*/
// export: (data) => data.text,
};
}
}
40 changes: 37 additions & 3 deletions test/cypress/tests/ui/BlockTunes.cy.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { selectionChangeDebounceTimeout } from '../../../../src/components/constants';
import Header from '@editorjs/header';
import type { ToolboxConfig } from '../../../../types';
import type { ConversionConfig, ToolboxConfig } from '../../../../types';
import type { MenuConfig } from '../../../../types/tools';

import { ToolWithoutConversionExport } from '../../fixtures/tools/ToolWithoutConversionExport';

describe('BlockTunes', function () {
describe('Search', () => {
Expand Down Expand Up @@ -185,6 +185,39 @@ describe('BlockTunes', function () {
.should('not.exist');
});

it('should not display the ConvertTo control if block has no conversionConfig.export specified', () => {
cy.createEditor({
tools: {
testTool: ToolWithoutConversionExport,
},
data: {
blocks: [
{
type: 'testTool',
data: {
text: 'Some text',
},
},
],
},
}).as('editorInstance');

cy.get('@editorInstance')
.get('[data-cy=editorjs]')
.find('.ce-block')
.click();

cy.get('@editorInstance')
.get('[data-cy=editorjs]')
.find('.ce-toolbar__settings-btn')
.click();

cy.get('@editorInstance')
.get('[data-cy=editorjs]')
.find('.ce-popover-item[data-item-name=convert-to]')
.should('not.exist');
});

it('should not display tool with the same data in "Convert to" menu', () => {
/**
* Tool with several toolbox entries configured
Expand All @@ -193,9 +226,10 @@ describe('BlockTunes', function () {
/**
* Tool is convertable
*/
public static get conversionConfig(): { import: string } {
public static get conversionConfig(): ConversionConfig {
return {
import: 'text',
export: 'text',
};
}

Expand Down
4 changes: 2 additions & 2 deletions test/cypress/tests/utils/flipper.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ describe('Flipper', () => {
.trigger('keydown', { keyCode: ARROW_DOWN_KEY_CODE });

/**
* Check whether we focus the Move Up Tune or not
* Check whether we focus the Delete Tune or not
*/
cy.get('[data-item-name="move-up"]')
cy.get('[data-item-name="delete"]')
.should('have.class', 'ce-popover-item--focused');

cy.get('[data-cy=editorjs]')
Expand Down
6 changes: 2 additions & 4 deletions types/tools/block-tool.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ConversionConfig, PasteConfig, SanitizerConfig } from '../configs';
import { BlockToolData } from './block-tool-data';
import { BaseTool, BaseToolConstructable } from './tool';
import { BaseTool, BaseToolConstructable, BaseToolConstructorOptions } from './tool';
import { ToolConfig } from './tool-config';
import { API, BlockAPI, ToolboxConfig } from '../index';
import { PasteEvent } from './paste-events';
Expand Down Expand Up @@ -83,10 +83,8 @@ export interface BlockTool extends BaseTool {
/**
* Describe constructor parameters
*/
export interface BlockToolConstructorOptions<D extends object = any, C extends object = any> {
api: API;
export interface BlockToolConstructorOptions<D extends object = any, C extends object = any> extends BaseToolConstructorOptions<C> {
data: BlockToolData<D>;
config: ToolConfig<C>;
block: BlockAPI;
readOnly: boolean;
}
Expand Down
23 changes: 15 additions & 8 deletions types/tools/tool.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,27 @@ import {MenuConfig} from './menu-config';
export interface BaseTool<RenderReturnType = HTMLElement> {
/**
* Tool`s render method
*
* For Inline Tools may return either HTMLElement (deprecated) or {@link MenuConfig}
*
* For Inline Tools may return either HTMLElement (deprecated) or {@link MenuConfig}
* @see https://editorjs.io/menu-config
*
*
* For Block Tools returns tool`s wrapper html element
*/
render(): RenderReturnType | Promise<RenderReturnType>;
}

export interface BaseToolConstructorOptions<C extends object = any> {
/**
* Editor.js API
*/
api: API;

/**
* Tool configuration
*/
config?: ToolConfig<C>;
}

export interface BaseToolConstructable {
/**
* Define Tool type as Inline
Expand All @@ -35,11 +47,6 @@ export interface BaseToolConstructable {
*/
title?: string;

/**
* Describe constructor parameters
*/
new (config: {api: API, config?: ToolConfig}): BaseTool;

/**
* Tool`s prepare method. Can be async
* @param data
Expand Down
Loading