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

Chore (toolbox): improved shortcuts visibility when tool exports array of toolbox items #2846

Merged
merged 10 commits into from
Oct 16, 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
1 change: 1 addition & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- `New` - Inline tools (those with `isReadOnlySupported` specified) can now be used in read-only mode
- `Fix` - Fix selection of first block in read-only initialization with "autofocus=true"
- `Fix` - Incorrect caret position after blocks merging in Safari
- `Fix` - Several toolbox items exported by the one tool have the same shortcut displayed in toolbox

### 2.30.6

Expand Down
8 changes: 4 additions & 4 deletions src/components/ui/toolbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,23 +308,23 @@ export default class Toolbox extends EventsDispatcher<ToolboxEventMap> {
/**
* Maps tool data to popover item structure
*/
const toPopoverItem = (toolboxItem: ToolboxConfigEntry, tool: BlockToolAdapter): PopoverItemParams => {
const toPopoverItem = (toolboxItem: ToolboxConfigEntry, tool: BlockToolAdapter, displaySecondaryLabel = true): PopoverItemParams => {
return {
icon: toolboxItem.icon,
title: I18n.t(I18nInternalNS.toolNames, toolboxItem.title || _.capitalize(tool.name)),
name: tool.name,
onActivate: (): void => {
this.toolButtonActivated(tool.name, toolboxItem.data);
},
secondaryLabel: tool.shortcut ? _.beautifyShortcut(tool.shortcut) : '',
secondaryLabel: (tool.shortcut && displaySecondaryLabel) ? _.beautifyShortcut(tool.shortcut) : '',
};
};

return this.toolsToBeDisplayed
.reduce<PopoverItemParams[]>((result, tool) => {
if (Array.isArray(tool.toolbox)) {
tool.toolbox.forEach(item => {
result.push(toPopoverItem(item, tool));
tool.toolbox.forEach((item, index) => {
result.push(toPopoverItem(item, tool, index === 0));
});
} else if (tool.toolbox !== undefined) {
result.push(toPopoverItem(tool.toolbox, tool));
neSpecc marked this conversation as resolved.
Show resolved Hide resolved
neSpecc marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
100 changes: 100 additions & 0 deletions test/cypress/tests/ui/toolbox.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,5 +114,105 @@ describe('Toolbox', function () {
expect(blocks[1].type).to.eq('nonConvertableTool');
});
});

it('should display shortcut only for the first toolbox item if tool exports toolbox with several items', function () {
/**
* Mock of Tool with conversionConfig
*/
class ToolWithSeveralToolboxItems extends ToolMock {
/**
* Specify toolbox with several items related to one tool
*/
public static get toolbox(): ToolboxConfig {
return [
{
icon: '',
title: 'first tool',
},
{
icon: '',
title: 'second tool',
},
];
}
}

cy.createEditor({
tools: {
severalToolboxItemsTool: {
class: ToolWithSeveralToolboxItems,
shortcut: 'CMD+SHIFT+L',
},
},
});

cy.get('[data-cy=editorjs]')
.find('.ce-paragraph')
.click()
.type('Some text')
.type('/'); // call a shortcut for toolbox

/**
* Secondary title (shortcut) should exist for first toolbox item of the tool
*/
/* eslint-disable-next-line cypress/require-data-selectors */
cy.get('.ce-popover')
.find('.ce-popover-item[data-item-name="severalToolboxItemsTool"]')
.first()
.find('.ce-popover-item__secondary-title')
.should('exist');

/**
* Secondary title (shortcut) should not exist for second toolbox item of the same tool
*/
/* eslint-disable-next-line cypress/require-data-selectors */
cy.get('.ce-popover')
.find('.ce-popover-item[data-item-name="severalToolboxItemsTool"]')
.eq(1)
.find('.ce-popover-item__secondary-title')
.should('not.exist');
});

it('should display shortcut for the item if tool exports toolbox as an one item object', function () {
/**
* Mock of Tool with conversionConfig
*/
class ToolWithOneToolboxItems extends ToolMock {
/**
* Specify toolbox with several items related to one tool
*/
public static get toolbox(): ToolboxConfig {
return {
icon: '',
title: 'tool',
};
}
}

cy.createEditor({
tools: {
oneToolboxItemTool: {
class: ToolWithOneToolboxItems,
shortcut: 'CMD+SHIFT+L',
},
},
});

cy.get('[data-cy=editorjs]')
.find('.ce-paragraph')
.click()
.type('Some text')
.type('/'); // call a shortcut for toolbox

/**
* Secondary title (shortcut) should exist for toolbox item of the tool
*/
/* eslint-disable-next-line cypress/require-data-selectors */
cy.get('.ce-popover')
.find('.ce-popover-item[data-item-name="oneToolboxItemTool"]')
.first()
.find('.ce-popover-item__secondary-title')
.should('exist');
});
});
});
2 changes: 1 addition & 1 deletion types/tools/tool-settings.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export interface ToolboxConfigEntry {
icon?: string;

/**
* May contain overrides for tool default config
* May contain overrides for tool default data
*/
data?: BlockToolData
}
Expand Down
Loading