From 2786d8b5fc86341d98b28d5c6e404abc62654f1d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 7 Apr 2020 11:21:21 +0200 Subject: [PATCH 1/2] Feature: Implemented a button that merges table cells directly from the table toolbar. --- src/tableui.js | 95 ++++++++++++++++++++++++++++++++++++++---------- tests/tableui.js | 53 +++++++++++++++++---------- 2 files changed, 109 insertions(+), 39 deletions(-) diff --git a/src/tableui.js b/src/tableui.js index b3bc1a1d..7b2c569d 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -9,6 +9,7 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import { addListToDropdown, createDropdown } from '@ckeditor/ckeditor5-ui/src/dropdown/utils'; +import SplitButtonView from '@ckeditor/ckeditor5-ui/src/dropdown/button/splitbuttonview'; import Model from '@ckeditor/ckeditor5-ui/src/model'; import Collection from '@ckeditor/ckeditor5-utils/src/collection'; @@ -201,14 +202,6 @@ export default class TableUI extends Plugin { } }, { type: 'separator' }, - { - type: 'button', - model: { - commandName: 'mergeTableCells', - label: t( 'Merge cells' ) - } - }, - { type: 'separator' }, { type: 'button', model: { @@ -225,7 +218,7 @@ export default class TableUI extends Plugin { } ]; - return this._prepareDropdown( t( 'Merge cells' ), tableMergeCellIcon, options, locale ); + return this._prepareMergeSplitButtonDropdown( t( 'Merge cells' ), tableMergeCellIcon, options, locale ); } ); } @@ -241,18 +234,8 @@ export default class TableUI extends Plugin { */ _prepareDropdown( label, icon, options, locale ) { const editor = this.editor; - const dropdownView = createDropdown( locale ); - const commands = []; - - // Prepare dropdown list items for list dropdown. - const itemDefinitions = new Collection(); - - for ( const option of options ) { - addListOption( option, editor, commands, itemDefinitions ); - } - - addListToDropdown( dropdownView, itemDefinitions, editor.ui.componentFactory ); + const commands = this._fillDropdownWithListOptions( dropdownView, options ); // Decorate dropdown's button. dropdownView.buttonView.set( { @@ -273,6 +256,78 @@ export default class TableUI extends Plugin { return dropdownView; } + + /** + * Creates a dropdown view with a {@link module:ui/dropdown/button/splitbuttonview~SplitButtonView} for + * merge (and split)–related commands. + * + * @private + * @param {String} label The dropdown button label. + * @param {String} icon An icon for the dropdown button. + * @param {Array.} options The list of options for the dropdown. + * @param {module:utils/locale~Locale} locale + * @returns {module:ui/dropdown/dropdownview~DropdownView} + */ + _prepareMergeSplitButtonDropdown( label, icon, options, locale ) { + const editor = this.editor; + const dropdownView = createDropdown( locale, SplitButtonView ); + const mergeCommandName = 'mergeTableCells'; + + this._fillDropdownWithListOptions( dropdownView, options ); + + dropdownView.buttonView.set( { + label, + icon, + tooltip: true + } ); + + // The main part of the split button is bound to the "mergeTableCells" command only. + dropdownView.bind( 'isEnabled' ).to( editor.commands.get( mergeCommandName ) ); + + // The split button dropdown must be **always** enabled and ready to open no matter the state + // of the "mergeTableCells" command. You may not be able to merge multiple cells but you may want + // to split them. This is also about mobile devices where multi–cell selection will never work + // (that's why "Merge cell right", "Merge cell down", etc. are still there in the first place). + dropdownView.buttonView.arrowView.unbind( 'isEnabled' ); + dropdownView.buttonView.arrowView.isEnabled = true; + + // Merge selected table cells when the main part of the split button is clicked. + this.listenTo( dropdownView.buttonView, 'execute', () => { + editor.execute( mergeCommandName ); + editor.editing.view.focus(); + } ); + + // Execute commands for events coming from the list in the dropdown panel. + this.listenTo( dropdownView, 'execute', evt => { + editor.execute( evt.source.commandName ); + editor.editing.view.focus(); + } ); + + return dropdownView; + } + + /** + * Injects a {@link module:ui/list/listview~ListView} into the passed dropdown with buttons + * which execute editor commands as configured in passed options. + * + * @private + * @param {module:ui/dropdown/dropdownview~DropdownView} dropdownView + * @param {Array.} options The list of options for the dropdown. + * @returns {Array.} Commands the list options are interacting with. + */ + _fillDropdownWithListOptions( dropdownView, options ) { + const editor = this.editor; + const commands = []; + const itemDefinitions = new Collection(); + + for ( const option of options ) { + addListOption( option, editor, commands, itemDefinitions ); + } + + addListToDropdown( dropdownView, itemDefinitions, editor.ui.componentFactory ); + + return commands; + } } // Adds an option to a list view. diff --git a/tests/tableui.js b/tests/tableui.js index 3b2d8316..512d0211 100644 --- a/tests/tableui.js +++ b/tests/tableui.js @@ -15,6 +15,7 @@ import InsertTableView from '../src/ui/inserttableview'; import SwitchButtonView from '@ckeditor/ckeditor5-ui/src/button/switchbuttonview'; import DropdownView from '@ckeditor/ckeditor5-ui/src/dropdown/dropdownview'; import ListSeparatorView from '@ckeditor/ckeditor5-ui/src/list/listseparatorview'; +import SplitButtonView from '@ckeditor/ckeditor5-ui/src/dropdown/button/splitbuttonview'; describe( 'TableUI', () => { let editor, element; @@ -363,10 +364,11 @@ describe( 'TableUI', () => { } ); describe( 'mergeTableCell dropdown', () => { - let dropdown; + let dropdown, command; beforeEach( () => { dropdown = editor.ui.componentFactory.create( 'mergeTableCells' ); + command = editor.commands.get( 'mergeTableCells' ); } ); it( 'have button with proper properties set', () => { @@ -380,6 +382,35 @@ describe( 'TableUI', () => { expect( button.icon ).to.match( / { + expect( dropdown.buttonView ).to.be.instanceOf( SplitButtonView ); + } ); + + it( 'should bind #isEnabled to the "mergeTableCells" command', () => { + command.isEnabled = false; + expect( dropdown.isEnabled ).to.be.false; + + command.isEnabled = true; + expect( dropdown.isEnabled ).to.be.true; + } ); + + it( 'should execute the "mergeTableCells" command when the main part of the split button is clicked', () => { + const spy = sinon.stub( editor, 'execute' ); + + dropdown.buttonView.fire( 'execute' ); + + sinon.assert.calledOnce( spy ); + sinon.assert.calledWithExactly( spy, 'mergeTableCells' ); + } ); + + it( 'should have the dropdown part of the split button always enabled no matter the "mergeTableCells" command state', () => { + command.isEnabled = true; + expect( dropdown.buttonView.arrowView.isEnabled ).to.be.true; + + command.isEnabled = false; + expect( dropdown.buttonView.arrowView.isEnabled ).to.be.true; + } ); + it( 'should have proper items in panel', () => { const listView = dropdown.listView; @@ -391,8 +422,6 @@ describe( 'TableUI', () => { 'Merge cell down', 'Merge cell left', '|', - 'Merge cells', - '|', 'Split cell vertically', 'Split cell horizontally' ] ); @@ -405,7 +434,6 @@ describe( 'TableUI', () => { const mergeCellRightCommand = editor.commands.get( 'mergeTableCellRight' ); const mergeCellDownCommand = editor.commands.get( 'mergeTableCellDown' ); const mergeCellLeftCommand = editor.commands.get( 'mergeTableCellLeft' ); - const mergeCellsCommand = editor.commands.get( 'mergeTableCells' ); const splitCellVerticallyCommand = editor.commands.get( 'splitTableCellVertically' ); const splitCellHorizontallyCommand = editor.commands.get( 'splitTableCellHorizontally' ); @@ -413,7 +441,6 @@ describe( 'TableUI', () => { mergeCellRightCommand.isEnabled = true; mergeCellDownCommand.isEnabled = true; mergeCellLeftCommand.isEnabled = true; - mergeCellsCommand.isEnabled = true; splitCellVerticallyCommand.isEnabled = true; splitCellHorizontallyCommand.isEnabled = true; @@ -422,28 +449,21 @@ describe( 'TableUI', () => { const mergeCellDownButton = items.get( 2 ); const mergeCellLeftButton = items.get( 3 ); // separator - const mergeCellsButton = items.get( 5 ); - // separator - const splitVerticallyButton = items.get( 7 ); - const splitHorizontallyButton = items.get( 8 ); + const splitVerticallyButton = items.get( 5 ); + const splitHorizontallyButton = items.get( 6 ); expect( mergeCellUpButton.children.first.isEnabled ).to.be.true; expect( mergeCellRightButton.children.first.isEnabled ).to.be.true; expect( mergeCellDownButton.children.first.isEnabled ).to.be.true; expect( mergeCellLeftButton.children.first.isEnabled ).to.be.true; - expect( mergeCellsButton.children.first.isEnabled ).to.be.true; expect( splitVerticallyButton.children.first.isEnabled ).to.be.true; expect( splitHorizontallyButton.children.first.isEnabled ).to.be.true; - expect( dropdown.buttonView.isEnabled ).to.be.true; mergeCellUpCommand.isEnabled = false; expect( mergeCellUpButton.children.first.isEnabled ).to.be.false; - expect( dropdown.buttonView.isEnabled ).to.be.true; mergeCellRightCommand.isEnabled = false; - expect( mergeCellRightButton.children.first.isEnabled ).to.be.false; - expect( dropdown.buttonView.isEnabled ).to.be.true; mergeCellDownCommand.isEnabled = false; expect( mergeCellDownButton.children.first.isEnabled ).to.be.false; @@ -451,16 +471,11 @@ describe( 'TableUI', () => { mergeCellLeftCommand.isEnabled = false; expect( mergeCellLeftButton.children.first.isEnabled ).to.be.false; - mergeCellsCommand.isEnabled = false; - expect( mergeCellsButton.children.first.isEnabled ).to.be.false; - splitCellVerticallyCommand.isEnabled = false; expect( splitVerticallyButton.children.first.isEnabled ).to.be.false; splitCellHorizontallyCommand.isEnabled = false; expect( splitHorizontallyButton.children.first.isEnabled ).to.be.false; - - expect( dropdown.buttonView.isEnabled ).to.be.false; } ); it( 'should bind items in panel to proper commands (RTL content)', () => { From e4cfd412bf3f7ce3b9723ea956a40fd70678ff99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 7 Apr 2020 16:05:21 +0200 Subject: [PATCH 2/2] Rename merge table cells dropdown to a split button in the docs. --- docs/features/table.md | 4 ++-- src/tableui.js | 2 +- tests/tableui.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/features/table.md b/docs/features/table.md index 8420a630..cf59acdb 100644 --- a/docs/features/table.md +++ b/docs/features/table.md @@ -335,7 +335,7 @@ The table plugins register the following UI components: The 'tableRow' dropdown - The 'mergeTableCells' dropdown + The 'mergeTableCells' split button The 'tableProperties' button @@ -351,7 +351,7 @@ The table plugins register the following UI components: #### Toolbars The {@link module:table/tabletoolbar~TableToolbar} plugin introduces two balloon toolbars for tables. -* The content toolbar shows up when a table cell is selected and it is anchored to the table. It is possible to {@link module:table/table~TableConfig#contentToolbar configure} its content. Normally, the toolbar contains the table-related tools such as `'tableColumn'`, `'tableRow'`, and `'mergeTableCells'` dropdowns. +* The content toolbar shows up when a table cell is selected and it is anchored to the table. It is possible to {@link module:table/table~TableConfig#contentToolbar configure} its content. Normally, the toolbar contains the table-related tools such as `'tableColumn'` and `'tableRow'` dropdowns and `'mergeTableCells'` split button. * The table toolbar shows up when the whole table is selected, for instance using the widget handler. It is possible to {@link module:table/table~TableConfig#tableToolbar configure} its content. ### Editor commands diff --git a/src/tableui.js b/src/tableui.js index 7b2c569d..6708f968 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -26,7 +26,7 @@ import tableMergeCellIcon from './../theme/icons/table-merge-cell.svg'; * * The `'insertTable'` dropdown, * * The `'tableColumn'` dropdown, * * The `'tableRow'` dropdown, - * * The `'mergeTableCells'` dropdown. + * * The `'mergeTableCells'` split button. * * The `'tableColumn'`, `'tableRow'` and `'mergeTableCells'` dropdowns work best with {@link module:table/tabletoolbar~TableToolbar}. * diff --git a/tests/tableui.js b/tests/tableui.js index 512d0211..ff8fe9f9 100644 --- a/tests/tableui.js +++ b/tests/tableui.js @@ -363,7 +363,7 @@ describe( 'TableUI', () => { } ); } ); - describe( 'mergeTableCell dropdown', () => { + describe( 'mergeTableCell split button', () => { let dropdown, command; beforeEach( () => {