From 6fd5103ae6f1c5db94b950b000b08b58a621c0f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Thu, 10 May 2018 16:10:09 +0200 Subject: [PATCH 01/27] Added: Initial row & column dropdowns. --- src/tableui.js | 109 +++++++++++++++++++++++++----------- tests/manual/table.js | 2 +- tests/tableui.js | 127 +++++++++++++++++++++++++++++------------- 3 files changed, 166 insertions(+), 72 deletions(-) diff --git a/src/tableui.js b/src/tableui.js index 6ce4f570..70571c48 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -9,10 +9,11 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; +import { addListToDropdown, createDropdown } from '@ckeditor/ckeditor5-ui/src/dropdown/utils'; +import Model from '@ckeditor/ckeditor5-ui/src/model'; +import Collection from '@ckeditor/ckeditor5-utils/src/collection'; import icon from '@ckeditor/ckeditor5-core/theme/icons/object-center.svg'; -import insertRowIcon from '@ckeditor/ckeditor5-core/theme/icons/object-left.svg'; -import insertColumnIcon from '@ckeditor/ckeditor5-core/theme/icons/object-right.svg'; /** * The table UI plugin. @@ -46,44 +47,90 @@ export default class TableUI extends Plugin { return buttonView; } ); - editor.ui.componentFactory.add( 'insertRowBelow', locale => { - const command = editor.commands.get( 'insertRowBelow' ); - const buttonView = new ButtonView( locale ); - - buttonView.bind( 'isEnabled' ).to( command ); + editor.ui.componentFactory.add( 'tableColumn', locale => { + const options = [ + { command: 'insertColumnBefore', label: 'Insert column before' }, + { command: 'insertColumnAfter', label: 'Insert column after' }, + { command: 'removeColumn', label: 'Delete column' } + ]; - buttonView.set( { - icon: insertRowIcon, - label: 'Insert row', - tooltip: true - } ); + return this._prepareDropdown( 'Column', icon, options, locale ); + } ); - buttonView.on( 'execute', () => { - editor.execute( 'insertRowBelow' ); - editor.editing.view.focus(); - } ); + editor.ui.componentFactory.add( 'tableRow', locale => { + const options = [ + { command: 'insertRowBelow', label: 'Insert row below' }, + { command: 'insertRowAbove', label: 'Insert row above' }, + { command: 'removeRow', label: 'Delete row' } + ]; - return buttonView; + return this._prepareDropdown( 'Row', icon, options, locale ); } ); + } - editor.ui.componentFactory.add( 'insertColumnAfter', locale => { - const command = editor.commands.get( 'insertColumnAfter' ); - const buttonView = new ButtonView( locale ); + /** + * Common method that prepares dropdown. + * + * @private + * @param {String} buttonName + * @param {String} icon + * @param {Array.} options + * @param locale + * @returns {module:ui/dropdown/dropdownview~DropdownView} + */ + _prepareDropdown( buttonName, icon, options, locale ) { + const editor = this.editor; - buttonView.bind( 'isEnabled' ).to( command ); + const dropdownView = createDropdown( locale ); + const commands = []; - buttonView.set( { - icon: insertColumnIcon, - label: 'Insert column', - tooltip: true - } ); + const dropdownItems = new Collection(); - buttonView.on( 'execute', () => { - editor.execute( 'insertColumnAfter' ); - editor.editing.view.focus(); - } ); + for ( const { command, label } of options ) { + addListOption( command, label, editor, commands, dropdownItems ); + } - return buttonView; + addListToDropdown( dropdownView, dropdownItems ); + + dropdownView.buttonView.set( { + label: buttonName, + icon, + tooltip: true } ); + + dropdownView.bind( 'isEnabled' ).toMany( commands, 'isEnabled', ( ...areEnabled ) => { + return areEnabled.some( isEnabled => isEnabled ); + } ); + + this.listenTo( dropdownView, 'execute', evt => { + editor.execute( evt.source.commandName ); + editor.editing.view.focus(); + } ); + + return dropdownView; } } + +/** + * Adds an option to a list view. + * + * @param {String} commandName + * @param {String} label + * @param {module:core/editor/editor~Editor} editor + * @param {Array.} commands + * @param {module:utils/collection~Collection} dropdownItems + */ +function addListOption( commandName, label, editor, commands, dropdownItems ) { + const command = editor.commands.get( commandName ); + + commands.push( command ); + + const itemModel = new Model( { + commandName, + label + } ); + + itemModel.bind( 'isEnabled' ).to( command ); + + dropdownItems.add( itemModel ); +} diff --git a/tests/manual/table.js b/tests/manual/table.js index d2d72728..17df7e90 100644 --- a/tests/manual/table.js +++ b/tests/manual/table.js @@ -13,7 +13,7 @@ ClassicEditor .create( document.querySelector( '#editor' ), { plugins: [ ArticlePluginSet, Table ], toolbar: [ - 'heading', '|', 'insertTable', 'insertRowBelow', 'insertColumnAfter', + 'heading', '|', 'insertTable', '|', 'tableColumn', 'tableRow', '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' ] } ) diff --git a/tests/tableui.js b/tests/tableui.js index c2de62fb..71406011 100644 --- a/tests/tableui.js +++ b/tests/tableui.js @@ -12,6 +12,7 @@ import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import TableEditing from '../src/tableediting'; import TableUI from '../src/tableui'; +import DropdownView from '@ckeditor/ckeditor5-ui/src/dropdown/dropdownview'; testUtils.createSinonSandbox(); @@ -81,73 +82,119 @@ describe( 'TableUI', () => { } ); } ); - describe( 'insertRowBelow button', () => { - let insertRow; + describe( 'tableRow dropdown', () => { + let dropdown; beforeEach( () => { - insertRow = editor.ui.componentFactory.create( 'insertRowBelow' ); + dropdown = editor.ui.componentFactory.create( 'tableRow' ); } ); - it( 'should register insertRowBelow button', () => { - expect( insertRow ).to.be.instanceOf( ButtonView ); - expect( insertRow.isOn ).to.be.false; - expect( insertRow.label ).to.equal( 'Insert row' ); - expect( insertRow.icon ).to.match( / { + expect( dropdown ).to.be.instanceOf( DropdownView ); + + const button = dropdown.buttonView; + + expect( button.isOn ).to.be.false; + expect( button.tooltip ).to.be.true; + expect( button.label ).to.equal( 'Column' ); + expect( button.icon ).to.match( / { - const command = editor.commands.get( 'insertRowBelow' ); + it( 'should have proper items in panel', () => { + const listView = dropdown.listView; - command.isEnabled = true; - expect( insertRow.isOn ).to.be.false; - expect( insertRow.isEnabled ).to.be.true; + const labels = listView.items.map( ( { label } ) => label ); - command.isEnabled = false; - expect( insertRow.isEnabled ).to.be.false; + expect( labels ).to.deep.equal( [ 'Insert row below', 'Insert row above', 'Delete row' ] ); } ); - it( 'should execute insertRow command on button execute event', () => { - const executeSpy = testUtils.sinon.spy( editor, 'execute' ); + it( 'should bind items in panel to proper commands', () => { + const items = dropdown.listView.items; - insertRow.fire( 'execute' ); + const insertRowBelowCommand = editor.commands.get( 'insertRowBelow' ); + const insertRowAboveCommand = editor.commands.get( 'insertRowAbove' ); + const removeRowCommand = editor.commands.get( 'removeRow' ); - sinon.assert.calledOnce( executeSpy ); - sinon.assert.calledWithExactly( executeSpy, 'insertRowBelow' ); + insertRowBelowCommand.isEnabled = true; + insertRowAboveCommand.isEnabled = true; + removeRowCommand.isEnabled = true; + + expect( items.get( 0 ).isEnabled ).to.be.true; + expect( items.get( 1 ).isEnabled ).to.be.true; + expect( items.get( 2 ).isEnabled ).to.be.true; + expect( dropdown.buttonView.isEnabled ).to.be.true; + + insertRowBelowCommand.isEnabled = false; + + expect( items.get( 0 ).isEnabled ).to.be.false; + expect( dropdown.buttonView.isEnabled ).to.be.true; + + insertRowAboveCommand.isEnabled = false; + + expect( items.get( 1 ).isEnabled ).to.be.false; + expect( dropdown.buttonView.isEnabled ).to.be.true; + + removeRowCommand.isEnabled = false; + expect( items.get( 2 ).isEnabled ).to.be.false; + expect( dropdown.buttonView.isEnabled ).to.be.false; } ); } ); - describe( 'insertColumnAfter button', () => { - let insertColumn; + describe.only( 'tableColumn button', () => { + let dropdown; beforeEach( () => { - insertColumn = editor.ui.componentFactory.create( 'insertColumnAfter' ); + dropdown = editor.ui.componentFactory.create( 'tableColumn' ); } ); - it( 'should register insertColumn buton', () => { - expect( insertColumn ).to.be.instanceOf( ButtonView ); - expect( insertColumn.isOn ).to.be.false; - expect( insertColumn.label ).to.equal( 'Insert column' ); - expect( insertColumn.icon ).to.match( / { + expect( dropdown ).to.be.instanceOf( DropdownView ); + + const button = dropdown.buttonView; + + expect( button.isOn ).to.be.false; + expect( button.tooltip ).to.be.true; + expect( button.label ).to.equal( 'Row' ); + expect( button.icon ).to.match( / { - const command = editor.commands.get( 'insertColumnAfter' ); + it( 'should have proper items in panel', () => { + const listView = dropdown.listView; - command.isEnabled = true; - expect( insertColumn.isOn ).to.be.false; - expect( insertColumn.isEnabled ).to.be.true; + const labels = listView.items.map( ( { label } ) => label ); - command.isEnabled = false; - expect( insertColumn.isEnabled ).to.be.false; + expect( labels ).to.deep.equal( [ 'Insert column before', 'Insert column after', 'Delete column' ] ); } ); - it( 'should execute insertColumn command on button execute event', () => { - const executeSpy = testUtils.sinon.spy( editor, 'execute' ); + it( 'should bind items in panel to proper commands', () => { + const items = dropdown.listView.items; - insertColumn.fire( 'execute' ); + const insertColumnBeforeCommand = editor.commands.get( 'insertColumnBefore' ); + const insertColumnAfterCommand = editor.commands.get( 'insertColumnAfter' ); + const removeColumnCommand = editor.commands.get( 'removeColumn' ); - sinon.assert.calledOnce( executeSpy ); - sinon.assert.calledWithExactly( executeSpy, 'insertColumnAfter' ); + insertColumnBeforeCommand.isEnabled = true; + insertColumnAfterCommand.isEnabled = true; + removeColumnCommand.isEnabled = true; + + expect( items.get( 0 ).isEnabled ).to.be.true; + expect( items.get( 1 ).isEnabled ).to.be.true; + expect( items.get( 2 ).isEnabled ).to.be.true; + expect( dropdown.buttonView.isEnabled ).to.be.true; + + insertColumnBeforeCommand.isEnabled = false; + + expect( items.get( 0 ).isEnabled ).to.be.false; + expect( dropdown.buttonView.isEnabled ).to.be.true; + + insertColumnAfterCommand.isEnabled = false; + + expect( items.get( 1 ).isEnabled ).to.be.false; + expect( dropdown.buttonView.isEnabled ).to.be.true; + + removeColumnCommand.isEnabled = false; + expect( items.get( 2 ).isEnabled ).to.be.false; + expect( dropdown.buttonView.isEnabled ).to.be.false; } ); } ); } ); From c43ecb34b969d7d431922d758b26411d5b74a43c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Thu, 10 May 2018 17:17:41 +0200 Subject: [PATCH 02/27] Added: Initial mergeCell dropdown. --- src/tableui.js | 11 +++++++++++ tests/manual/table.js | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/tableui.js b/src/tableui.js index 70571c48..57aed16c 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -66,6 +66,17 @@ export default class TableUI extends Plugin { return this._prepareDropdown( 'Row', icon, options, locale ); } ); + + editor.ui.componentFactory.add( 'mergeCell', locale => { + const options = [ + { command: 'mergeCellUp', label: 'Merge cell up' }, + { command: 'mergeCellRight', label: 'Merge cell right' }, + { command: 'mergeCellDown', label: 'Merge cell down' }, + { command: 'mergeCellLeft', label: 'Merge cell left' } + ]; + + return this._prepareDropdown( 'Merge cell', icon, options, locale ); + } ); } /** diff --git a/tests/manual/table.js b/tests/manual/table.js index 17df7e90..9629cba2 100644 --- a/tests/manual/table.js +++ b/tests/manual/table.js @@ -13,7 +13,7 @@ ClassicEditor .create( document.querySelector( '#editor' ), { plugins: [ ArticlePluginSet, Table ], toolbar: [ - 'heading', '|', 'insertTable', '|', 'tableColumn', 'tableRow', + 'heading', '|', 'insertTable', '|', 'tableColumn', 'tableRow', 'mergeCell', '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' ] } ) From 1026056508fc3f6355ee2bcef91ff824f3e34d6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Thu, 10 May 2018 17:24:21 +0200 Subject: [PATCH 03/27] Tests: Fix TableUI tests. --- tests/tableui.js | 71 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 3 deletions(-) diff --git a/tests/tableui.js b/tests/tableui.js index 71406011..92196599 100644 --- a/tests/tableui.js +++ b/tests/tableui.js @@ -96,7 +96,7 @@ describe( 'TableUI', () => { expect( button.isOn ).to.be.false; expect( button.tooltip ).to.be.true; - expect( button.label ).to.equal( 'Column' ); + expect( button.label ).to.equal( 'Row' ); expect( button.icon ).to.match( / { } ); } ); - describe.only( 'tableColumn button', () => { + describe( 'tableColumn button', () => { let dropdown; beforeEach( () => { @@ -154,7 +154,7 @@ describe( 'TableUI', () => { expect( button.isOn ).to.be.false; expect( button.tooltip ).to.be.true; - expect( button.label ).to.equal( 'Row' ); + expect( button.label ).to.equal( 'Column' ); expect( button.icon ).to.match( / { expect( dropdown.buttonView.isEnabled ).to.be.false; } ); } ); + + describe( 'mergeCell button', () => { + let dropdown; + + beforeEach( () => { + dropdown = editor.ui.componentFactory.create( 'mergeCell' ); + } ); + + it( 'have button with proper properties set', () => { + expect( dropdown ).to.be.instanceOf( DropdownView ); + + const button = dropdown.buttonView; + + expect( button.isOn ).to.be.false; + expect( button.tooltip ).to.be.true; + expect( button.label ).to.equal( 'Merge cell' ); + expect( button.icon ).to.match( / { + const listView = dropdown.listView; + + const labels = listView.items.map( ( { label } ) => label ); + + expect( labels ).to.deep.equal( [ 'Merge cell up', 'Merge cell right', 'Merge cell down', 'Merge cell left' ] ); + } ); + + it( 'should bind items in panel to proper commands', () => { + const items = dropdown.listView.items; + + const mergeCellUpCommand = editor.commands.get( 'mergeCellUp' ); + const mergeCellRightCommand = editor.commands.get( 'mergeCellRight' ); + const mergeCellDownCommand = editor.commands.get( 'mergeCellDown' ); + const mergeCellLeftCommand = editor.commands.get( 'mergeCellLeft' ); + + mergeCellUpCommand.isEnabled = true; + mergeCellRightCommand.isEnabled = true; + mergeCellDownCommand.isEnabled = true; + mergeCellLeftCommand.isEnabled = true; + + expect( items.get( 0 ).isEnabled ).to.be.true; + expect( items.get( 1 ).isEnabled ).to.be.true; + expect( items.get( 2 ).isEnabled ).to.be.true; + expect( items.get( 3 ).isEnabled ).to.be.true; + expect( dropdown.buttonView.isEnabled ).to.be.true; + + mergeCellUpCommand.isEnabled = false; + + expect( items.get( 0 ).isEnabled ).to.be.false; + expect( dropdown.buttonView.isEnabled ).to.be.true; + + mergeCellRightCommand.isEnabled = false; + + expect( items.get( 1 ).isEnabled ).to.be.false; + expect( dropdown.buttonView.isEnabled ).to.be.true; + + mergeCellDownCommand.isEnabled = false; + expect( items.get( 2 ).isEnabled ).to.be.false; + + mergeCellLeftCommand.isEnabled = false; + expect( items.get( 3 ).isEnabled ).to.be.false; + + expect( dropdown.buttonView.isEnabled ).to.be.false; + } ); + } ); } ); From b21451ffe0c772a4fe7295f92b5d193bf851ed7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Thu, 10 May 2018 17:28:50 +0200 Subject: [PATCH 04/27] Tests: Add missing tests for TableUI. --- tests/tableui.js | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/tableui.js b/tests/tableui.js index 92196599..103de4be 100644 --- a/tests/tableui.js +++ b/tests/tableui.js @@ -138,6 +138,23 @@ describe( 'TableUI', () => { expect( items.get( 2 ).isEnabled ).to.be.false; expect( dropdown.buttonView.isEnabled ).to.be.false; } ); + + it( 'should focus view after command execution', () => { + const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' ); + + dropdown.listView.items.get( 0 ).fire( 'execute' ); + + sinon.assert.calledOnce( focusSpy ); + } ); + + it( 'executes command when it\'s executed', () => { + const spy = sinon.stub( editor, 'execute' ); + + dropdown.listView.items.get( 0 ).fire( 'execute' ); + + expect( spy.calledOnce ).to.be.true; + expect( spy.args[ 0 ][ 0 ] ).to.equal( 'insertRowBelow' ); + } ); } ); describe( 'tableColumn button', () => { @@ -196,6 +213,23 @@ describe( 'TableUI', () => { expect( items.get( 2 ).isEnabled ).to.be.false; expect( dropdown.buttonView.isEnabled ).to.be.false; } ); + + it( 'should focus view after command execution', () => { + const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' ); + + dropdown.listView.items.get( 0 ).fire( 'execute' ); + + sinon.assert.calledOnce( focusSpy ); + } ); + + it( 'executes command when it\'s executed', () => { + const spy = sinon.stub( editor, 'execute' ); + + dropdown.listView.items.get( 0 ).fire( 'execute' ); + + expect( spy.calledOnce ).to.be.true; + expect( spy.args[ 0 ][ 0 ] ).to.equal( 'insertColumnBefore' ); + } ); } ); describe( 'mergeCell button', () => { @@ -261,5 +295,22 @@ describe( 'TableUI', () => { expect( dropdown.buttonView.isEnabled ).to.be.false; } ); + + it( 'should focus view after command execution', () => { + const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' ); + + dropdown.listView.items.get( 0 ).fire( 'execute' ); + + sinon.assert.calledOnce( focusSpy ); + } ); + + it( 'executes command when it\'s executed', () => { + const spy = sinon.stub( editor, 'execute' ); + + dropdown.listView.items.get( 0 ).fire( 'execute' ); + + expect( spy.calledOnce ).to.be.true; + expect( spy.args[ 0 ][ 0 ] ).to.equal( 'mergeCellUp' ); + } ); } ); } ); From e45ba7f064333a9cd35fc0ed2eaa59bedbdeabb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Thu, 10 May 2018 18:23:42 +0200 Subject: [PATCH 05/27] Added: Initial splitCell dropdown. --- src/tableui.js | 9 ++++++ tests/manual/table.js | 2 +- tests/tableui.js | 74 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/tableui.js b/src/tableui.js index 57aed16c..3ae09de9 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -77,6 +77,15 @@ export default class TableUI extends Plugin { return this._prepareDropdown( 'Merge cell', icon, options, locale ); } ); + + editor.ui.componentFactory.add( 'splitCell', locale => { + const options = [ + { command: 'splitCellVertically', label: 'Split cell vertically' }, + { command: 'splitCellHorizontally', label: 'Split cell horizontally' } + ]; + + return this._prepareDropdown( 'Split cell', icon, options, locale ); + } ); } /** diff --git a/tests/manual/table.js b/tests/manual/table.js index 9629cba2..20800a8e 100644 --- a/tests/manual/table.js +++ b/tests/manual/table.js @@ -13,7 +13,7 @@ ClassicEditor .create( document.querySelector( '#editor' ), { plugins: [ ArticlePluginSet, Table ], toolbar: [ - 'heading', '|', 'insertTable', '|', 'tableColumn', 'tableRow', 'mergeCell', + 'heading', '|', 'insertTable', '|', 'tableColumn', 'tableRow', 'mergeCell', 'splitCell', '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' ] } ) diff --git a/tests/tableui.js b/tests/tableui.js index 103de4be..d579b6f5 100644 --- a/tests/tableui.js +++ b/tests/tableui.js @@ -47,7 +47,7 @@ describe( 'TableUI', () => { return editor.destroy(); } ); - describe( 'insertTable button', () => { + describe( 'insertTable dropdown', () => { let insertTable; beforeEach( () => { @@ -157,7 +157,7 @@ describe( 'TableUI', () => { } ); } ); - describe( 'tableColumn button', () => { + describe( 'tableColumn dropdown', () => { let dropdown; beforeEach( () => { @@ -232,7 +232,7 @@ describe( 'TableUI', () => { } ); } ); - describe( 'mergeCell button', () => { + describe( 'mergeCell dropdown', () => { let dropdown; beforeEach( () => { @@ -313,4 +313,72 @@ describe( 'TableUI', () => { expect( spy.args[ 0 ][ 0 ] ).to.equal( 'mergeCellUp' ); } ); } ); + + describe( 'splitCell dropdown', () => { + let dropdown; + + beforeEach( () => { + dropdown = editor.ui.componentFactory.create( 'splitCell' ); + } ); + + it( 'have button with proper properties set', () => { + expect( dropdown ).to.be.instanceOf( DropdownView ); + + const button = dropdown.buttonView; + + expect( button.isOn ).to.be.false; + expect( button.tooltip ).to.be.true; + expect( button.label ).to.equal( 'Split cell' ); + expect( button.icon ).to.match( / { + const listView = dropdown.listView; + + const labels = listView.items.map( ( { label } ) => label ); + + expect( labels ).to.deep.equal( [ 'Split cell vertically', 'Split cell horizontally' ] ); + } ); + + it( 'should bind items in panel to proper commands', () => { + const items = dropdown.listView.items; + + const splitCellVerticallyCommand = editor.commands.get( 'splitCellVertically' ); + const splitCellHorizontallyCommand = editor.commands.get( 'splitCellHorizontally' ); + + splitCellVerticallyCommand.isEnabled = true; + splitCellHorizontallyCommand.isEnabled = true; + + expect( items.get( 0 ).isEnabled ).to.be.true; + expect( items.get( 1 ).isEnabled ).to.be.true; + expect( dropdown.buttonView.isEnabled ).to.be.true; + + splitCellVerticallyCommand.isEnabled = false; + + expect( items.get( 0 ).isEnabled ).to.be.false; + expect( dropdown.buttonView.isEnabled ).to.be.true; + + splitCellHorizontallyCommand.isEnabled = false; + + expect( items.get( 1 ).isEnabled ).to.be.false; + expect( dropdown.buttonView.isEnabled ).to.be.false; + } ); + + it( 'should focus view after command execution', () => { + const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' ); + + dropdown.listView.items.get( 0 ).fire( 'execute' ); + + sinon.assert.calledOnce( focusSpy ); + } ); + + it( 'executes command when it\'s executed', () => { + const spy = sinon.stub( editor, 'execute' ); + + dropdown.listView.items.get( 0 ).fire( 'execute' ); + + expect( spy.calledOnce ).to.be.true; + expect( spy.args[ 0 ][ 0 ] ).to.equal( 'splitCellVertically' ); + } ); + } ); } ); From 9dde19b5500d7ccd9a9ae97709090097081b8537 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 15 May 2018 17:18:01 +0200 Subject: [PATCH 06/27] Added: Initial TableToolbar implementation. --- src/converters/downcast.js | 5 +- src/tabletoolbar.js | 194 +++++++++++++++++++++++++++++++++++++ src/ui/utils.js | 51 ++++++++++ src/utils.js | 51 ++++++++++ tests/integration.js | 86 ++++++++++++++++ tests/manual/table.js | 11 ++- tests/tabletoolbar.js | 192 ++++++++++++++++++++++++++++++++++++ 7 files changed, 584 insertions(+), 6 deletions(-) create mode 100644 src/tabletoolbar.js create mode 100644 src/ui/utils.js create mode 100644 src/utils.js create mode 100644 tests/integration.js create mode 100644 tests/tabletoolbar.js diff --git a/src/converters/downcast.js b/src/converters/downcast.js index 5dd87760..b976a40f 100644 --- a/src/converters/downcast.js +++ b/src/converters/downcast.js @@ -10,7 +10,8 @@ import ViewPosition from '@ckeditor/ckeditor5-engine/src/view/position'; import ViewRange from '@ckeditor/ckeditor5-engine/src/view/range'; import TableWalker from './../tablewalker'; -import { toWidget, toWidgetEditable } from '@ckeditor/ckeditor5-widget/src/utils'; +import { toWidgetEditable } from '@ckeditor/ckeditor5-widget/src/utils'; +import { toTableWidget } from '../utils'; /** * Model table element to view table element conversion helper. @@ -40,7 +41,7 @@ export function downcastInsertTable( options = {} ) { let tableWidget; if ( asWidget ) { - tableWidget = toWidget( tableElement, conversionApi.writer ); + tableWidget = toTableWidget( tableElement, conversionApi.writer ); } const tableWalker = new TableWalker( table ); diff --git a/src/tabletoolbar.js b/src/tabletoolbar.js new file mode 100644 index 00000000..27f947a3 --- /dev/null +++ b/src/tabletoolbar.js @@ -0,0 +1,194 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module table/tabletoolbar + */ + +import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; +import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview'; +import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon'; +import { isTableWidgetSelected } from './utils'; +import { repositionContextualBalloon, getBalloonPositionData } from './ui/utils'; + +const balloonClassName = 'ck-toolbar-container'; + +/** + * The table toolbar class. Creates an table toolbar that shows up when the table widget is selected. + * + * Toolbar components are created using the editor {@link module:ui/componentfactory~ComponentFactory ComponentFactory} + * based on the {@link module:core/editor/editor~Editor#config configuration} stored under `table.toolbar`. + * + * The toolbar uses the {@link module:ui/panel/balloon/contextualballoon~ContextualBalloon}. + * + * For a detailed overview, check the {@glink features/table#table-contextual-toolbar table contextual toolbar} documentation. + * + * @extends module:core/plugin~Plugin + */ +export default class TableToolbar extends Plugin { + /** + * @inheritDoc + */ + static get requires() { + return [ ContextualBalloon ]; + } + + /** + * @inheritDoc + */ + static get pluginName() { + return 'TableToolbar'; + } + + /** + * @inheritDoc + */ + init() { + const editor = this.editor; + const balloonToolbar = editor.plugins.get( 'BalloonToolbar' ); + + // If the `BalloonToolbar` plugin is loaded, it should be disabled for tables + // which have their own toolbar to avoid duplication. + // https://github.com/ckeditor/ckeditor5-image/issues/110 + if ( balloonToolbar ) { + this.listenTo( balloonToolbar, 'show', evt => { + if ( isTableWidgetSelected( editor.editing.view.document.selection ) ) { + evt.stop(); + } + }, { priority: 'high' } ); + } + } + + /** + * @inheritDoc + */ + afterInit() { + const editor = this.editor; + const toolbarConfig = editor.config.get( 'table.toolbar' ); + + // Don't add the toolbar if there is no configuration. + if ( !toolbarConfig || !toolbarConfig.length ) { + return; + } + + /** + * A contextual balloon plugin instance. + * + * @private + * @member {module:ui/panel/balloon/contextualballoon~ContextualBalloon} + */ + this._balloon = this.editor.plugins.get( 'ContextualBalloon' ); + + /** + * A `ToolbarView` instance used to display the buttons specific for table editing. + * + * @protected + * @type {module:ui/toolbar/toolbarview~ToolbarView} + */ + this._toolbar = new ToolbarView(); + + // Add buttons to the toolbar. + this._toolbar.fillFromConfig( toolbarConfig, editor.ui.componentFactory ); + + // Show balloon panel each time table widget is selected. + this.listenTo( editor.editing.view, 'render', () => { + this._checkIsVisible(); + } ); + + // There is no render method after focus is back in editor, we need to check if balloon panel should be visible. + this.listenTo( editor.ui.focusTracker, 'change:isFocused', () => { + this._checkIsVisible(); + }, { priority: 'low' } ); + } + + /** + * Checks whether the toolbar should show up or hide depending on the current selection. + * + * @private + */ + _checkIsVisible() { + const editor = this.editor; + + if ( !editor.ui.focusTracker.isFocused ) { + this._hideToolbar(); + } else { + if ( isTableWidgetSelected( editor.editing.view.document.selection ) ) { + this._showToolbar(); + } else { + this._hideToolbar(); + } + } + } + + /** + * Shows the {@link #_toolbar} in the {@link #_balloon}. + * + * @private + */ + _showToolbar() { + const editor = this.editor; + + if ( this._isVisible ) { + repositionContextualBalloon( editor ); + } else { + if ( !this._balloon.hasView( this._toolbar ) ) { + this._balloon.add( { + view: this._toolbar, + position: getBalloonPositionData( editor ), + balloonClassName + } ); + } + } + } + + /** + * Removes the {@link #_toolbar} from the {@link #_balloon}. + * + * @private + */ + _hideToolbar() { + if ( !this._isVisible ) { + return; + } + + this._balloon.remove( this._toolbar ); + } + + /** + * Returns `true` when the {@link #_toolbar} is the visible view in the {@link #_balloon}. + * + * @private + * @type {Boolean} + */ + get _isVisible() { + return this._balloon.visibleView == this._toolbar; + } +} + +/** + * Items to be placed in the table toolbar. + * This option is used by the {@link module:table/tabletoolbar~TableToolbar} feature. + * + * Assuming that you use the {@link module:table/tableui~TableUI} feature below toolbar items will be available + * in {@link module:ui/componentfactory~ComponentFactory}: + * + * * `'tableRow'`, + * * `'tableColumn'`, + * * `'mergeCell'`, + * * `'splitCell'`, + * + * so you can configure the toolbar like this: + * + * const tableConfig = { + * toolbar: [ 'tableRow', 'tableColumn', 'mergeCell', 'splitCell' ] + * }; + * + * Of course, the same buttons can also be used in the + * {@link module:core/editor/editorconfig~EditorConfig#toolbar main editor toolbar}. + * + * Read more about configuring toolbar in {@link module:core/editor/editorconfig~EditorConfig#toolbar}. + * + * @member {Array.} module:table~TableConfig#toolbar + */ diff --git a/src/ui/utils.js b/src/ui/utils.js new file mode 100644 index 00000000..9cc75a49 --- /dev/null +++ b/src/ui/utils.js @@ -0,0 +1,51 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module image/image/ui/utils + */ + +import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview'; +import { getParentTable } from '../commands/utils'; + +/** + * A helper utility that positions the + * {@link module:ui/panel/balloon/contextualballoon~ContextualBalloon contextual balloon} instance + * with respect to the image in the editor content, if one is selected. + * + * @param {module:core/editor/editor~Editor} editor The editor instance. + */ +export function repositionContextualBalloon( editor ) { + const balloon = editor.plugins.get( 'ContextualBalloon' ); + + balloon.updatePosition( getBalloonPositionData( editor ) ); +} + +/** + * Returns the positioning options that control the geometry of the + * {@link module:ui/panel/balloon/contextualballoon~ContextualBalloon contextual balloon} with respect + * to the selected element in the editor content. + * + * @param {module:core/editor/editor~Editor} editor The editor instance. + * @returns {module:utils/dom/position~Options} + */ +export function getBalloonPositionData( editor ) { + const editingView = editor.editing.view; + const defaultPositions = BalloonPanelView.defaultPositions; + + const parentTable = getParentTable( editingView.document.selection.getFirstPosition() ); + + return { + target: editingView.domConverter.viewToDom( parentTable ), + positions: [ + defaultPositions.northArrowSouth, + defaultPositions.northArrowSouthWest, + defaultPositions.northArrowSouthEast, + defaultPositions.southArrowNorth, + defaultPositions.southArrowNorthWest, + defaultPositions.southArrowNorthEast + ] + }; +} diff --git a/src/utils.js b/src/utils.js new file mode 100644 index 00000000..da09883a --- /dev/null +++ b/src/utils.js @@ -0,0 +1,51 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module table/utils + */ + +import { toWidget, isWidget } from '@ckeditor/ckeditor5-widget/src/utils'; +import { getParentTable } from './commands/utils'; + +const tableSymbol = Symbol( 'isImage' ); + +/** + * Converts a given {@link module:engine/view/element~Element} to an table widget: + * * Adds a {@link module:engine/view/element~Element#_setCustomProperty custom property} allowing to recognize the table widget element. + * * Calls the {@link module:widget/utils~toWidget} function with the proper element's label creator. + * + * @param {module:engine/view/element~Element} viewElement + * @param {module:engine/view/writer~Writer} writer An instance of the view writer. + * @param {String} label The element's label. It will be concatenated with the table `alt` attribute if one is present. + * @returns {module:engine/view/element~Element} + */ +export function toTableWidget( viewElement, writer ) { + writer.setCustomProperty( tableSymbol, true, viewElement ); + + return toWidget( viewElement, writer ); +} + +/** + * Checks if a given view element is an table widget. + * + * @param {module:engine/view/element~Element} viewElement + * @returns {Boolean} + */ +export function isTableWidget( viewElement ) { + return !!viewElement.getCustomProperty( tableSymbol ) && isWidget( viewElement ); +} + +/** + * Checks if an table widget is the only selected element. + * + * @param {module:engine/view/selection~Selection|module:engine/view/documentselection~DocumentSelection} selection + * @returns {Boolean} + */ +export function isTableWidgetSelected( selection ) { + const parentTable = getParentTable( selection.getFirstPosition() ); + + return !!( parentTable && isTableWidget( parentTable ) ); +} diff --git a/tests/integration.js b/tests/integration.js new file mode 100644 index 00000000..5f0d9edb --- /dev/null +++ b/tests/integration.js @@ -0,0 +1,86 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; +import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import BalloonToolbar from '@ckeditor/ckeditor5-ui/src/toolbar/balloon/balloontoolbar'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import global from '@ckeditor/ckeditor5-utils/src/dom/global'; +import Table from '../src/table'; +import TableToolbar from '../src/tabletoolbar'; +import View from '@ckeditor/ckeditor5-ui/src/view'; + +describe( 'TableToolbar integration', () => { + describe( 'with the BalloonToolbar', () => { + let balloon, balloonToolbar, newEditor, editorElement; + + beforeEach( () => { + editorElement = global.document.createElement( 'div' ); + global.document.body.appendChild( editorElement ); + + return ClassicTestEditor + .create( editorElement, { + plugins: [ Table, TableToolbar, BalloonToolbar, Paragraph ] + } ) + .then( editor => { + newEditor = editor; + balloon = newEditor.plugins.get( 'ContextualBalloon' ); + balloonToolbar = newEditor.plugins.get( 'BalloonToolbar' ); + const button = new View(); + + button.element = global.document.createElement( 'div' ); + + // There must be at least one toolbar items which is not disabled to show it. + // https://github.com/ckeditor/ckeditor5-ui/issues/269 + balloonToolbar.toolbarView.items.add( button ); + + newEditor.editing.view.isFocused = true; + newEditor.editing.view.getDomRoot().focus(); + } ); + } ); + + afterEach( () => { + editorElement.remove(); + return newEditor.destroy(); + } ); + + it( 'should prevent the BalloonToolbar from being displayed when an table is selected', () => { + // When table is selected along with text. + setModelData( newEditor.model, 'fo[o
]' ); + + balloonToolbar.show(); + + // BalloonToolbar should be visible. + expect( balloon.visibleView ).to.equal( balloonToolbar.toolbarView ); + + // When only table is selected. + setModelData( newEditor.model, 'foo[]
' ); + + balloonToolbar.show(); + + // BalloonToolbar should not be visible. + expect( balloon.visibleView ).to.be.null; + } ); + + it( 'should listen to BalloonToolbar#show event with the high priority', () => { + const highestPrioritySpy = sinon.spy(); + const highPrioritySpy = sinon.spy(); + const normalPrioritySpy = sinon.spy(); + + // Select an table + setModelData( newEditor.model, 'foo[]
' ); + + newEditor.listenTo( balloonToolbar, 'show', highestPrioritySpy, { priority: 'highest' } ); + newEditor.listenTo( balloonToolbar, 'show', highPrioritySpy, { priority: 'high' } ); + newEditor.listenTo( balloonToolbar, 'show', normalPrioritySpy, { priority: 'normal' } ); + + balloonToolbar.show(); + + sinon.assert.calledOnce( highestPrioritySpy ); + sinon.assert.notCalled( highPrioritySpy ); + sinon.assert.notCalled( normalPrioritySpy ); + } ); + } ); +} ); diff --git a/tests/manual/table.js b/tests/manual/table.js index 20800a8e..0ffe84a8 100644 --- a/tests/manual/table.js +++ b/tests/manual/table.js @@ -8,14 +8,17 @@ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset'; import Table from '../../src/table'; +import TableToolbar from '../../src/tabletoolbar'; ClassicEditor .create( document.querySelector( '#editor' ), { - plugins: [ ArticlePluginSet, Table ], + plugins: [ ArticlePluginSet, Table, TableToolbar ], toolbar: [ - 'heading', '|', 'insertTable', '|', 'tableColumn', 'tableRow', 'mergeCell', 'splitCell', - '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' - ] + 'heading', '|', 'insertTable', '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' + ], + table: { + toolbar: [ 'tableColumn', 'tableRow', 'mergeCell', 'splitCell' ] + } } ) .then( editor => { window.editor = editor; diff --git a/tests/tabletoolbar.js b/tests/tabletoolbar.js new file mode 100644 index 00000000..676aa71b --- /dev/null +++ b/tests/tabletoolbar.js @@ -0,0 +1,192 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* global document */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; +import TableToolbar from '../src/tabletoolbar'; +import Table from '../src/table'; +import global from '@ckeditor/ckeditor5-utils/src/dom/global'; +import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; +import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Range from '@ckeditor/ckeditor5-engine/src/model/range'; +import View from '@ckeditor/ckeditor5-ui/src/view'; +import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; + +describe( 'TableToolbar', () => { + let editor, model, doc, editingView, plugin, toolbar, balloon, editorElement; + + beforeEach( () => { + editorElement = global.document.createElement( 'div' ); + global.document.body.appendChild( editorElement ); + + return ClassicEditor + .create( editorElement, { + plugins: [ Paragraph, Table, TableToolbar, FakeButton ], + table: { + toolbar: [ 'fake_button' ] + } + } ) + .then( newEditor => { + editor = newEditor; + model = newEditor.model; + doc = model.document; + plugin = editor.plugins.get( TableToolbar ); + toolbar = plugin._toolbar; + editingView = editor.editing.view; + balloon = editor.plugins.get( 'ContextualBalloon' ); + } ); + } ); + + afterEach( () => { + editorElement.remove(); + + return editor.destroy(); + } ); + + it( 'should be loaded', () => { + expect( editor.plugins.get( TableToolbar ) ).to.be.instanceOf( TableToolbar ); + } ); + + it( 'should not initialize if there is no configuration', () => { + const editorElement = global.document.createElement( 'div' ); + global.document.body.appendChild( editorElement ); + + return ClassicEditor.create( editorElement, { + plugins: [ TableToolbar ] + } ) + .then( editor => { + expect( editor.plugins.get( TableToolbar )._toolbar ).to.be.undefined; + + editorElement.remove(); + return editor.destroy(); + } ); + } ); + + describe( 'toolbar', () => { + it( 'should use the config.table.toolbar to create items', () => { + expect( toolbar.items ).to.have.length( 1 ); + expect( toolbar.items.get( 0 ).label ).to.equal( 'fake button' ); + } ); + + it( 'should set proper CSS classes', () => { + const spy = sinon.spy( balloon, 'add' ); + + editor.ui.focusTracker.isFocused = true; + + setData( model, '[]
' ); + + sinon.assert.calledWithMatch( spy, { + view: toolbar, + balloonClassName: 'ck-toolbar-container' + } ); + } ); + } ); + + describe( 'integration with the editor focus', () => { + it( 'should show the toolbar when the editor gains focus and the table is selected', () => { + editor.ui.focusTracker.isFocused = true; + + setData( model, '[]
' ); + + editor.ui.focusTracker.isFocused = false; + expect( balloon.visibleView ).to.be.null; + + editor.ui.focusTracker.isFocused = true; + expect( balloon.visibleView ).to.equal( toolbar ); + } ); + + it( 'should hide the toolbar when the editor loses focus and the table is selected', () => { + editor.ui.focusTracker.isFocused = false; + + setData( model, '[]
' ); + + editor.ui.focusTracker.isFocused = true; + expect( balloon.visibleView ).to.equal( toolbar ); + + editor.ui.focusTracker.isFocused = false; + expect( balloon.visibleView ).to.be.null; + } ); + } ); + + describe( 'integration with the editor selection (#change event)', () => { + beforeEach( () => { + editor.ui.focusTracker.isFocused = true; + } ); + + it( 'should show the toolbar on render when the table is selected', () => { + setData( model, '[foo]
' ); + + expect( balloon.visibleView ).to.be.null; + + editingView.change( () => {} ); + expect( balloon.visibleView ).to.be.null; + + model.change( writer => { + // Select the [
] + writer.setSelection( + Range.createOn( doc.getRoot().getChild( 1 ).getChild( 0 ).getChild( 0 ) ) + ); + } ); + + expect( balloon.visibleView ).to.equal( toolbar ); + + // Make sure successive change does not throw, e.g. attempting + // to insert the toolbar twice. + editingView.change( () => {} ); + expect( balloon.visibleView ).to.equal( toolbar ); + } ); + + it( 'should not engage when the toolbar is in the balloon yet invisible', () => { + setData( model, '[]
' ); + expect( balloon.visibleView ).to.equal( toolbar ); + + const lastView = new View(); + lastView.element = document.createElement( 'div' ); + + balloon.add( { view: lastView } ); + expect( balloon.visibleView ).to.equal( lastView ); + + editingView.change( () => {} ); + expect( balloon.visibleView ).to.equal( lastView ); + } ); + + it( 'should hide the toolbar on render if the table is de–selected', () => { + setData( model, 'foo[]
' ); + + expect( balloon.visibleView ).to.equal( toolbar ); + + model.change( writer => { + // Select the [...] + writer.setSelection( + Range.createIn( doc.getRoot().getChild( 0 ) ) + ); + } ); + + expect( balloon.visibleView ).to.be.null; + + // Make sure successive change does not throw, e.g. attempting + // to remove the toolbar twice. + editingView.change( () => {} ); + expect( balloon.visibleView ).to.be.null; + } ); + } ); + + // Plugin that adds fake_button to editor's component factory. + class FakeButton extends Plugin { + init() { + this.editor.ui.componentFactory.add( 'fake_button', locale => { + const view = new ButtonView( locale ); + + view.set( { + label: 'fake button' + } ); + + return view; + } ); + } + } +} ); From b2919df150f64a877735cd7306749b5b93c89a85 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 17 May 2018 12:31:57 +0200 Subject: [PATCH 07/27] Added table icons: table, table-row, table-column, table-split-cell, table-merge-cell, and table-headers. --- theme/icons/table-column.svg | 1 + theme/icons/table-headers.svg | 1 + theme/icons/table-merge-cell.svg | 1 + theme/icons/table-row.svg | 1 + theme/icons/table-split-cell.svg | 1 + theme/icons/table.svg | 1 + 6 files changed, 6 insertions(+) create mode 100644 theme/icons/table-column.svg create mode 100644 theme/icons/table-headers.svg create mode 100644 theme/icons/table-merge-cell.svg create mode 100644 theme/icons/table-row.svg create mode 100644 theme/icons/table-split-cell.svg create mode 100644 theme/icons/table.svg diff --git a/theme/icons/table-column.svg b/theme/icons/table-column.svg new file mode 100644 index 00000000..c02f487f --- /dev/null +++ b/theme/icons/table-column.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/theme/icons/table-headers.svg b/theme/icons/table-headers.svg new file mode 100644 index 00000000..c6f28bec --- /dev/null +++ b/theme/icons/table-headers.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/theme/icons/table-merge-cell.svg b/theme/icons/table-merge-cell.svg new file mode 100644 index 00000000..cf01b01b --- /dev/null +++ b/theme/icons/table-merge-cell.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/theme/icons/table-row.svg b/theme/icons/table-row.svg new file mode 100644 index 00000000..7c6aa14e --- /dev/null +++ b/theme/icons/table-row.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/theme/icons/table-split-cell.svg b/theme/icons/table-split-cell.svg new file mode 100644 index 00000000..a31ea32d --- /dev/null +++ b/theme/icons/table-split-cell.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/theme/icons/table.svg b/theme/icons/table.svg new file mode 100644 index 00000000..5b6fc40f --- /dev/null +++ b/theme/icons/table.svg @@ -0,0 +1 @@ + \ No newline at end of file From a8d9a5cf6b2a04cbf88855d40b487260fd1a471d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Thu, 17 May 2018 13:03:10 +0200 Subject: [PATCH 08/27] Changed: Use proper icons for buttons in UI. --- src/tableui.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/tableui.js b/src/tableui.js index 3ae09de9..5e0367cd 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -13,7 +13,11 @@ import { addListToDropdown, createDropdown } from '@ckeditor/ckeditor5-ui/src/dr import Model from '@ckeditor/ckeditor5-ui/src/model'; import Collection from '@ckeditor/ckeditor5-utils/src/collection'; -import icon from '@ckeditor/ckeditor5-core/theme/icons/object-center.svg'; +import tableIcon from './../theme/icons/table.svg'; +import tableColumnIcon from './../theme/icons/table-column.svg'; +import tableRowIcon from './../theme/icons/table-row.svg'; +import tableMergeCellIcon from './../theme/icons/table-merge-cell.svg'; +import tableSplitCellIcon from './../theme/icons/table-split-cell.svg'; /** * The table UI plugin. @@ -34,7 +38,7 @@ export default class TableUI extends Plugin { buttonView.bind( 'isEnabled' ).to( command ); buttonView.set( { - icon, + icon: tableIcon, label: 'Insert table', tooltip: true } ); @@ -54,7 +58,7 @@ export default class TableUI extends Plugin { { command: 'removeColumn', label: 'Delete column' } ]; - return this._prepareDropdown( 'Column', icon, options, locale ); + return this._prepareDropdown( 'Column', tableColumnIcon, options, locale ); } ); editor.ui.componentFactory.add( 'tableRow', locale => { @@ -64,7 +68,7 @@ export default class TableUI extends Plugin { { command: 'removeRow', label: 'Delete row' } ]; - return this._prepareDropdown( 'Row', icon, options, locale ); + return this._prepareDropdown( 'Row', tableRowIcon, options, locale ); } ); editor.ui.componentFactory.add( 'mergeCell', locale => { @@ -75,7 +79,7 @@ export default class TableUI extends Plugin { { command: 'mergeCellLeft', label: 'Merge cell left' } ]; - return this._prepareDropdown( 'Merge cell', icon, options, locale ); + return this._prepareDropdown( 'Merge cell', tableMergeCellIcon, options, locale ); } ); editor.ui.componentFactory.add( 'splitCell', locale => { @@ -84,7 +88,7 @@ export default class TableUI extends Plugin { { command: 'splitCellHorizontally', label: 'Split cell horizontally' } ]; - return this._prepareDropdown( 'Split cell', icon, options, locale ); + return this._prepareDropdown( 'Split cell', tableSplitCellIcon, options, locale ); } ); } From 3d0ede9a5e08bbea152504b63baee194598e8193 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 25 May 2018 11:18:43 +0200 Subject: [PATCH 09/27] Other: Refactored SetTableHeadersCommand to two SetHeaderColumnCommand and SetHeaderRowCommand. --- src/commands/setheadercolumncommand.js | 78 +++++++ ...aderscommand.js => setheaderrowcommand.js} | 54 +++-- src/tableediting.js | 8 +- src/tableui.js | 2 + tests/commands/setheadercolumncommand.js | 191 ++++++++++++++++++ ...aderscommand.js => setheaderrowcommand.js} | 185 +++++++++-------- tests/tableediting.js | 11 +- tests/tableui.js | 36 +++- 8 files changed, 437 insertions(+), 128 deletions(-) create mode 100644 src/commands/setheadercolumncommand.js rename src/commands/{settableheaderscommand.js => setheaderrowcommand.js} (74%) create mode 100644 tests/commands/setheadercolumncommand.js rename tests/commands/{settableheaderscommand.js => setheaderrowcommand.js} (53%) diff --git a/src/commands/setheadercolumncommand.js b/src/commands/setheadercolumncommand.js new file mode 100644 index 00000000..3222f790 --- /dev/null +++ b/src/commands/setheadercolumncommand.js @@ -0,0 +1,78 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module table/commands/setheadercolumncommand + */ + +import Command from '@ckeditor/ckeditor5-core/src/command'; + +import { getParentTable, updateNumericAttribute } from './utils'; + +/** + * The header coloumn command. + * + * @extends module:core/command~Command + */ +export default class SetHeaderColumnCommand extends Command { + /** + * @inheritDoc + */ + refresh() { + const model = this.editor.model; + const doc = model.document; + const selection = doc.selection; + + const position = selection.getFirstPosition(); + const tableParent = getParentTable( position ); + + this.isEnabled = !!tableParent; + + this.value = this.isEnabled && this._isInHeading( position.parent, tableParent ); + } + + /** + * @inheritDoc + */ + execute() { + const model = this.editor.model; + const doc = model.document; + const selection = doc.selection; + const tableUtils = this.editor.plugins.get( 'TableUtils' ); + + const position = selection.getFirstPosition(); + const tableCell = position.parent; + const tableRow = tableCell.parent; + const table = tableRow.parent; + + const currentHeadingColumns = parseInt( table.getAttribute( 'headingColumns' ) || 0 ); + + const { column } = tableUtils.getCellLocation( tableCell ); + + const columnsToSet = column + 1 !== currentHeadingColumns ? column + 1 : column; + + model.change( writer => { + updateNumericAttribute( 'headingColumns', columnsToSet, table, writer, 0 ); + } ); + } + + /** + * Checks if table cell is in heading section. + * + * @param {module:engine/model/element~Element} tableCell + * @param {module:engine/model/element~Element} table + * @returns {Boolean} + * @private + */ + _isInHeading( tableCell, table ) { + const headingColumns = parseInt( table.getAttribute( 'headingColumns' ) || 0 ); + + const tableUtils = this.editor.plugins.get( 'TableUtils' ); + + const { column } = tableUtils.getCellLocation( tableCell ); + + return !!headingColumns && column < headingColumns; + } +} diff --git a/src/commands/settableheaderscommand.js b/src/commands/setheaderrowcommand.js similarity index 74% rename from src/commands/settableheaderscommand.js rename to src/commands/setheaderrowcommand.js index 176a14e9..ba936c53 100644 --- a/src/commands/settableheaderscommand.js +++ b/src/commands/setheaderrowcommand.js @@ -4,7 +4,7 @@ */ /** - * @module table/commands/settableheaderscommand + * @module table/commands/setheaderrowcommand */ import Command from '@ckeditor/ckeditor5-core/src/command'; @@ -14,11 +14,11 @@ import { getParentTable, updateNumericAttribute } from './utils'; import TableWalker from '../tablewalker'; /** - * The set table headers command. + * The header row command. * * @extends module:core/command~Command */ -export default class SetTableHeadersCommand extends Command { +export default class SetHeaderRowCommand extends Command { /** * @inheritDoc */ @@ -27,27 +27,34 @@ export default class SetTableHeadersCommand extends Command { const doc = model.document; const selection = doc.selection; - const tableParent = getParentTable( selection.getFirstPosition() ); + const position = selection.getFirstPosition(); + const tableParent = getParentTable( position ); this.isEnabled = !!tableParent; + + this.value = this.isEnabled && this._isInHeading( position.parent, tableParent ); } /** * @inheritDoc */ - execute( options = {} ) { + execute() { const model = this.editor.model; const doc = model.document; const selection = doc.selection; - const rowsToSet = parseInt( options.rows ) || 0; + const position = selection.getFirstPosition(); + const tableCell = position.parent; + const tableRow = tableCell.parent; + const table = tableRow.parent; - const table = getParentTable( selection.getFirstPosition() ); + const currentHeadingRows = table.getAttribute( 'headingRows' ) || 0; + const rowIndex = tableRow.index; - model.change( writer => { - const currentHeadingRows = table.getAttribute( 'headingRows' ) || 0; + const rowsToSet = rowIndex + 1 !== currentHeadingRows ? rowIndex + 1 : rowIndex; - if ( currentHeadingRows !== rowsToSet && rowsToSet > 0 ) { + model.change( writer => { + if ( rowsToSet ) { // Changing heading rows requires to check if any of a heading cell is overlaping vertically the table head. // Any table cell that has a rowspan attribute > 1 will not exceed the table head so we need to fix it in rows below. const cellsToSplit = getOverlappingCells( table, rowsToSet, currentHeadingRows ); @@ -57,11 +64,23 @@ export default class SetTableHeadersCommand extends Command { } } - const columnsToSet = parseInt( options.columns ) || 0; - updateTableAttribute( table, 'headingColumns', columnsToSet, writer ); - updateTableAttribute( table, 'headingRows', rowsToSet, writer ); + updateNumericAttribute( 'headingRows', rowsToSet, table, writer, 0 ); } ); } + + /** + * Checks if table cell is in heading section. + * + * @param {module:engine/model/element~Element} tableCell + * @param {module:engine/model/element~Element} table + * @returns {Boolean} + * @private + */ + _isInHeading( tableCell, table ) { + const headingRows = parseInt( table.getAttribute( 'headingRows' ) || 0 ); + + return headingRows && tableCell.parent.index < headingRows; + } } // Returns cells that span beyond new heading section. @@ -86,15 +105,6 @@ function getOverlappingCells( table, headingRowsToSet, currentHeadingRows ) { return cellsToSplit; } -// @private -function updateTableAttribute( table, attributeName, newValue, writer ) { - const currentValue = table.getAttribute( attributeName ) || 0; - - if ( newValue !== currentValue ) { - updateNumericAttribute( attributeName, newValue, table, writer, 0 ); - } -} - // Splits table cell horizontally. // // @param {module:engine/model/element~Element} tableCell diff --git a/src/tableediting.js b/src/tableediting.js index 35d6e63e..f69ebeb1 100644 --- a/src/tableediting.js +++ b/src/tableediting.js @@ -28,11 +28,12 @@ import SplitCellCommand from './commands/splitcellcommand'; import MergeCellCommand from './commands/mergecellcommand'; import RemoveRowCommand from './commands/removerowcommand'; import RemoveColumnCommand from './commands/removecolumncommand'; -import SetTableHeadersCommand from './commands/settableheaderscommand'; +import SetHeaderRowCommand from './commands/setheaderrowcommand'; +import SetHeaderColumnCommand from './commands/setheadercolumncommand'; import { getParentTable } from './commands/utils'; +import TableUtils from './tableutils'; import './../theme/table.css'; -import TableUtils from './tableutils'; /** * The table editing feature. @@ -111,7 +112,8 @@ export default class TableEditing extends Plugin { editor.commands.add( 'mergeCellDown', new MergeCellCommand( editor, { direction: 'down' } ) ); editor.commands.add( 'mergeCellUp', new MergeCellCommand( editor, { direction: 'up' } ) ); - editor.commands.add( 'setTableHeaders', new SetTableHeadersCommand( editor ) ); + editor.commands.add( 'setColumnHeader', new SetHeaderColumnCommand( editor ) ); + editor.commands.add( 'setRowHeader', new SetHeaderRowCommand( editor ) ); // Handle tab key navigation. this.listenTo( editor.editing.view.document, 'keydown', ( ...args ) => this._handleTabOnSelectedTable( ...args ) ); diff --git a/src/tableui.js b/src/tableui.js index 5e0367cd..98816ce7 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -53,6 +53,7 @@ export default class TableUI extends Plugin { editor.ui.componentFactory.add( 'tableColumn', locale => { const options = [ + { command: 'setColumnHeader', label: 'Header column' }, { command: 'insertColumnBefore', label: 'Insert column before' }, { command: 'insertColumnAfter', label: 'Insert column after' }, { command: 'removeColumn', label: 'Delete column' } @@ -63,6 +64,7 @@ export default class TableUI extends Plugin { editor.ui.componentFactory.add( 'tableRow', locale => { const options = [ + { command: 'setRowHeader', label: 'Header row' }, { command: 'insertRowBelow', label: 'Insert row below' }, { command: 'insertRowAbove', label: 'Insert row above' }, { command: 'removeRow', label: 'Delete row' } diff --git a/tests/commands/setheadercolumncommand.js b/tests/commands/setheadercolumncommand.js new file mode 100644 index 00000000..fa60c77a --- /dev/null +++ b/tests/commands/setheadercolumncommand.js @@ -0,0 +1,191 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor'; +import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters'; + +import SetHeaderColumnCommand from '../../src/commands/setheadercolumncommand'; +import { + downcastInsertCell, + downcastInsertRow, + downcastInsertTable, + downcastRemoveRow, + downcastTableHeadingColumnsChange, + downcastTableHeadingRowsChange +} from '../../src/converters/downcast'; +import upcastTable from '../../src/converters/upcasttable'; +import { formatTable, formattedModelTable, modelTable } from '../_utils/utils'; +import TableUtils from '../../src/tableutils'; + +describe( 'HeaderColumnCommand', () => { + let editor, model, command; + + beforeEach( () => { + return ModelTestEditor.create( { + plugins: [ TableUtils ] + } ).then( newEditor => { + editor = newEditor; + model = editor.model; + command = new SetHeaderColumnCommand( editor ); + + const conversion = editor.conversion; + const schema = model.schema; + + schema.register( 'table', { + allowWhere: '$block', + allowAttributes: [ 'headingRows' ], + isBlock: true, + isObject: true + } ); + + schema.register( 'tableRow', { + allowIn: 'table', + allowAttributes: [], + isBlock: true, + isLimit: true + } ); + + schema.register( 'tableCell', { + allowIn: 'tableRow', + allowContentOf: '$block', + allowAttributes: [ 'colspan', 'rowspan' ], + isBlock: true, + isLimit: true + } ); + + model.schema.register( 'p', { inheritAllFrom: '$block' } ); + + // Table conversion. + conversion.for( 'upcast' ).add( upcastTable() ); + conversion.for( 'downcast' ).add( downcastInsertTable() ); + + // Insert row conversion. + conversion.for( 'downcast' ).add( downcastInsertRow() ); + + // Remove row conversion. + conversion.for( 'downcast' ).add( downcastRemoveRow() ); + + // Table cell conversion. + conversion.for( 'downcast' ).add( downcastInsertCell() ); + + conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) ); + conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) ); + + // Table attributes conversion. + conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } ); + conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } ); + + conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() ); + conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() ); + } ); + } ); + + afterEach( () => { + return editor.destroy(); + } ); + + describe( 'isEnabled', () => { + it( 'should be false if selection is not in a table', () => { + setData( model, '

foo[]

' ); + expect( command.isEnabled ).to.be.false; + } ); + + it( 'should be true if selection is in table', () => { + setData( model, 'foo[]
' ); + expect( command.isEnabled ).to.be.true; + } ); + } ); + + describe( 'value', () => { + it( 'should be false if selection is not in a heading column', () => { + setData( model, modelTable( [ + [ '01', '02' ], + [ '11', '12[]' ] + ], { headingColumns: 1 } ) ); + + expect( command.value ).to.be.false; + } ); + + it( 'should be true if selection is in a heading column', () => { + setData( model, modelTable( [ + [ '01[]', '02' ], + [ '11', '12' ] + ], { headingColumns: 1 } ) ); + + expect( command.value ).to.be.true; + } ); + + it( 'should be false if selection is in a heading row', () => { + setData( model, modelTable( [ + [ '01', '02[]' ], + [ '11', '12' ] + ], { headingRows: 1, headingColumns: 1 } ) ); + + expect( command.value ).to.be.false; + } ); + } ); + + describe( 'execute()', () => { + it( 'should set heading columns attribute that cover column in which is selection', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '[]10', '11' ], + [ '20', '21' ] + ] ) ); + + command.execute(); + + expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ + [ '00', '01' ], + [ '[]10', '11' ], + [ '20', '21' ] + ], { headingColumns: 1 } ) ); + } ); + + it( + 'should set heading columns attribute if currently selected column is a heading so the heading section is before this column', + () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '[]10', '11' ], + [ '20', '21' ] + ], { headingColumns: 2 } ) ); + + command.execute(); + + expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ + [ '00', '01' ], + [ '[]10', '11' ], + [ '20', '21' ] + ], { headingColumns: 1 } ) ); + } + ); + + it( 'should toggle of selected column', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11[]' ], + [ '20', '21' ] + ], { headingColumns: 2 } ) ); + + command.execute(); + + expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ + [ '00', '01' ], + [ '10', '11[]' ], + [ '20', '21' ] + ], { headingColumns: 1 } ) ); + + command.execute(); + + expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ + [ '00', '01' ], + [ '10', '11[]' ], + [ '20', '21' ] + ], { headingColumns: 2 } ) ); + } ); + } ); +} ); diff --git a/tests/commands/settableheaderscommand.js b/tests/commands/setheaderrowcommand.js similarity index 53% rename from tests/commands/settableheaderscommand.js rename to tests/commands/setheaderrowcommand.js index 8b717437..1553a3ff 100644 --- a/tests/commands/settableheaderscommand.js +++ b/tests/commands/setheaderrowcommand.js @@ -6,8 +6,8 @@ import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor'; import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters'; +import SetHeaderRowCommand from '../../src/commands/setheaderrowcommand'; -import SetTableHeadersCommand from '../../src/commands/settableheaderscommand'; import { downcastInsertCell, downcastInsertRow, @@ -18,60 +18,62 @@ import { } from '../../src/converters/downcast'; import upcastTable from '../../src/converters/upcasttable'; import { formatTable, formattedModelTable, modelTable } from '../_utils/utils'; +import TableUtils from '../../src/tableutils'; -describe( 'SetTableHeadersCommand', () => { +describe( 'HeaderRowCommand', () => { let editor, model, command; beforeEach( () => { - return ModelTestEditor.create() - .then( newEditor => { - editor = newEditor; - model = editor.model; - command = new SetTableHeadersCommand( editor ); - - const conversion = editor.conversion; - const schema = model.schema; - - schema.register( 'table', { - allowWhere: '$block', - allowAttributes: [ 'headingRows' ], - isObject: true - } ); + return ModelTestEditor.create( { + plugins: [ TableUtils ] + } ).then( newEditor => { + editor = newEditor; + model = editor.model; + command = new SetHeaderRowCommand( editor ); + + const conversion = editor.conversion; + const schema = model.schema; + + schema.register( 'table', { + allowWhere: '$block', + allowAttributes: [ 'headingRows' ], + isObject: true + } ); - schema.register( 'tableRow', { allowIn: 'table' } ); + schema.register( 'tableRow', { allowIn: 'table' } ); - schema.register( 'tableCell', { - allowIn: 'tableRow', - allowContentOf: '$block', - allowAttributes: [ 'colspan', 'rowspan' ], - isLimit: true - } ); + schema.register( 'tableCell', { + allowIn: 'tableRow', + allowContentOf: '$block', + allowAttributes: [ 'colspan', 'rowspan' ], + isLimit: true + } ); - model.schema.register( 'p', { inheritAllFrom: '$block' } ); + model.schema.register( 'p', { inheritAllFrom: '$block' } ); - // Table conversion. - conversion.for( 'upcast' ).add( upcastTable() ); - conversion.for( 'downcast' ).add( downcastInsertTable() ); + // Table conversion. + conversion.for( 'upcast' ).add( upcastTable() ); + conversion.for( 'downcast' ).add( downcastInsertTable() ); - // Insert row conversion. - conversion.for( 'downcast' ).add( downcastInsertRow() ); + // Insert row conversion. + conversion.for( 'downcast' ).add( downcastInsertRow() ); - // Remove row conversion. - conversion.for( 'downcast' ).add( downcastRemoveRow() ); + // Remove row conversion. + conversion.for( 'downcast' ).add( downcastRemoveRow() ); - // Table cell conversion. - conversion.for( 'downcast' ).add( downcastInsertCell() ); + // Table cell conversion. + conversion.for( 'downcast' ).add( downcastInsertCell() ); - conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) ); - conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) ); + conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) ); + conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) ); - // Table attributes conversion. - conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } ); - conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } ); + // Table attributes conversion. + conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } ); + conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } ); - conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() ); - conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() ); - } ); + conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() ); + conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() ); + } ); } ); afterEach( () => { @@ -79,88 +81,93 @@ describe( 'SetTableHeadersCommand', () => { } ); describe( 'isEnabled', () => { - it( 'should be false if not in a table', () => { + it( 'should be false if selection is not in a table', () => { setData( model, '

foo[]

' ); expect( command.isEnabled ).to.be.false; } ); - it( 'should be true if in table', () => { + it( 'should be true if selection is in table', () => { setData( model, 'foo[]
' ); expect( command.isEnabled ).to.be.true; } ); } ); - describe( 'execute()', () => { - it( 'should set heading rows attribute', () => { + describe( 'value', () => { + it( 'should be false if selection is not in a heading row', () => { setData( model, modelTable( [ - [ '00', '01' ], - [ '[]10', '11' ], - [ '20', '21' ] - ] ) ); - - command.execute( { rows: 2 } ); + [ '01', '02' ], + [ '11', '12[]' ] + ], { headingRows: 1 } ) ); - expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ - [ '00', '01' ], - [ '[]10', '11' ], - [ '20', '21' ] - ], { headingRows: 2 } ) ); + expect( command.value ).to.be.false; } ); - it( 'should remove heading rows attribute', () => { + it( 'should be true if selection is in a heading row', () => { setData( model, modelTable( [ - [ '00', '01' ], - [ '[]10', '11' ], - [ '20', '21' ] - ], { headingRows: 2 } ) ); + [ '01[]', '02' ], + [ '11', '12' ] + ], { headingRows: 1 } ) ); - command.execute( { rows: 0 } ); + expect( command.value ).to.be.true; + } ); - expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ - [ '00', '01' ], - [ '[]10', '11' ], - [ '20', '21' ] - ] ) ); + it( 'should be false if selection is in a heading column', () => { + setData( model, modelTable( [ + [ '01', '02' ], + [ '11[]', '12' ] + ], { headingRows: 1, headingColumns: 1 } ) ); + + expect( command.value ).to.be.false; } ); + } ); - it( 'should set heading columns attribute', () => { + describe( 'execute()', () => { + it( 'should set heading rows attribute that cover row in which is selection', () => { setData( model, modelTable( [ [ '00', '01' ], [ '[]10', '11' ], [ '20', '21' ] ] ) ); - command.execute( { columns: 2 } ); + command.execute(); expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ [ '00', '01' ], [ '[]10', '11' ], [ '20', '21' ] - ], { headingColumns: 2 } ) ); + ], { headingRows: 2 } ) ); } ); - it( 'should remove heading columns attribute', () => { + it( 'should toggle heading rows attribute', () => { setData( model, modelTable( [ - [ '00', '01' ], - [ '[]10', '11' ], + [ '[]00', '01' ], + [ '10', '11' ], [ '20', '21' ] - ], { headingColumns: 2 } ) ); + ] ) ); - command.execute( { columns: 0 } ); + command.execute(); expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ - [ '00', '01' ], - [ '[]10', '11' ], + [ '[]00', '01' ], + [ '10', '11' ], + [ '20', '21' ] + ], { headingRows: 1 } ) ); + + command.execute(); + + setData( model, modelTable( [ + [ '[]00', '01' ], + [ '10', '11' ], [ '20', '21' ] ] ) ); } ); - it( 'should remove heading columns & heading rows attributes', () => { + it( 'should set heading rows attribute if currently selected row is a heading so the heading section is below this row', () => { setData( model, modelTable( [ [ '00', '01' ], [ '[]10', '11' ], [ '20', '21' ] - ], { headingColumns: 2, headingRows: 2 } ) ); + ], { headingRows: 2 } ) ); command.execute(); @@ -168,7 +175,7 @@ describe( 'SetTableHeadersCommand', () => { [ '00', '01' ], [ '[]10', '11' ], [ '20', '21' ] - ] ) ); + ], { headingRows: 1 } ) ); } ); it( 'should fix rowspaned cells on the edge of an table head section', () => { @@ -178,7 +185,7 @@ describe( 'SetTableHeadersCommand', () => { [ '22' ] ], { headingColumns: 2, headingRows: 1 } ) ); - command.execute( { rows: 2, columns: 2 } ); + command.execute(); expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ [ '00', '01', '02' ], @@ -190,19 +197,19 @@ describe( 'SetTableHeadersCommand', () => { it( 'should split to at most 2 table cells when fixing rowspaned cells on the edge of an table head section', () => { setData( model, modelTable( [ [ '00', '01', '02' ], - [ { colspan: 2, rowspan: 5, contents: '10[]' }, '12' ], - [ '22' ], + [ { colspan: 2, rowspan: 5, contents: '10' }, '12' ], + [ '22[]' ], [ '32' ], [ '42' ], [ '52' ] ], { headingColumns: 2, headingRows: 1 } ) ); - command.execute( { rows: 3, columns: 2 } ); + command.execute(); expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ [ '00', '01', '02' ], - [ { colspan: 2, rowspan: 2, contents: '10[]' }, '12' ], - [ '22' ], + [ { colspan: 2, rowspan: 2, contents: '10' }, '12' ], + [ '22[]' ], [ { colspan: 2, rowspan: 3, contents: '' }, '32' ], [ '42' ], [ '52' ] @@ -215,11 +222,11 @@ describe( 'SetTableHeadersCommand', () => { [ '11' ] ], { headingRows: 2 } ) ); - command.execute( { rows: 1 } ); + command.execute(); expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ [ '[]00', '01' ], - [ '', '11' ], + [ '', '11' ] ], { headingRows: 1 } ) ); } ); @@ -229,7 +236,7 @@ describe( 'SetTableHeadersCommand', () => { [ '10' ] ], { headingRows: 2 } ) ); - command.execute( { rows: 1 } ); + command.execute(); expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ [ '00', '[]01' ], diff --git a/tests/tableediting.js b/tests/tableediting.js index 75204d22..758e3804 100644 --- a/tests/tableediting.js +++ b/tests/tableediting.js @@ -17,7 +17,8 @@ import RemoveRowCommand from '../src/commands/removerowcommand'; import RemoveColumnCommand from '../src/commands/removecolumncommand'; import SplitCellCommand from '../src/commands/splitcellcommand'; import MergeCellCommand from '../src/commands/mergecellcommand'; -import SetTableHeadersCommand from '../src/commands/settableheaderscommand'; +import SetHeaderRowCommand from '../src/commands/setheaderrowcommand'; +import SetHeaderColumnCommand from '../src/commands/setheadercolumncommand'; describe( 'TableEditing', () => { let editor, model; @@ -93,8 +94,12 @@ describe( 'TableEditing', () => { expect( editor.commands.get( 'mergeCellUp' ) ).to.be.instanceOf( MergeCellCommand ); } ); - it( 'adds setTableHeaders command', () => { - expect( editor.commands.get( 'setTableHeaders' ) ).to.be.instanceOf( SetTableHeadersCommand ); + it( 'adds setColumnHeader command', () => { + expect( editor.commands.get( 'setColumnHeader' ) ).to.be.instanceOf( SetHeaderColumnCommand ); + } ); + + it( 'adds setRowHeader command', () => { + expect( editor.commands.get( 'setRowHeader' ) ).to.be.instanceOf( SetHeaderRowCommand ); } ); describe( 'conversion in data pipeline', () => { diff --git a/tests/tableui.js b/tests/tableui.js index d579b6f5..157a4e45 100644 --- a/tests/tableui.js +++ b/tests/tableui.js @@ -105,16 +105,18 @@ describe( 'TableUI', () => { const labels = listView.items.map( ( { label } ) => label ); - expect( labels ).to.deep.equal( [ 'Insert row below', 'Insert row above', 'Delete row' ] ); + expect( labels ).to.deep.equal( [ 'Header row', 'Insert row below', 'Insert row above', 'Delete row' ] ); } ); it( 'should bind items in panel to proper commands', () => { const items = dropdown.listView.items; + const setRowHeaderCommand = editor.commands.get( 'setRowHeader' ); const insertRowBelowCommand = editor.commands.get( 'insertRowBelow' ); const insertRowAboveCommand = editor.commands.get( 'insertRowAbove' ); const removeRowCommand = editor.commands.get( 'removeRow' ); + setRowHeaderCommand.isEnabled = true; insertRowBelowCommand.isEnabled = true; insertRowAboveCommand.isEnabled = true; removeRowCommand.isEnabled = true; @@ -122,20 +124,26 @@ describe( 'TableUI', () => { expect( items.get( 0 ).isEnabled ).to.be.true; expect( items.get( 1 ).isEnabled ).to.be.true; expect( items.get( 2 ).isEnabled ).to.be.true; + expect( items.get( 3 ).isEnabled ).to.be.true; expect( dropdown.buttonView.isEnabled ).to.be.true; - insertRowBelowCommand.isEnabled = false; + setRowHeaderCommand.isEnabled = false; expect( items.get( 0 ).isEnabled ).to.be.false; expect( dropdown.buttonView.isEnabled ).to.be.true; - insertRowAboveCommand.isEnabled = false; + insertRowBelowCommand.isEnabled = false; expect( items.get( 1 ).isEnabled ).to.be.false; expect( dropdown.buttonView.isEnabled ).to.be.true; - removeRowCommand.isEnabled = false; + insertRowAboveCommand.isEnabled = false; expect( items.get( 2 ).isEnabled ).to.be.false; + expect( dropdown.buttonView.isEnabled ).to.be.true; + + removeRowCommand.isEnabled = false; + + expect( items.get( 3 ).isEnabled ).to.be.false; expect( dropdown.buttonView.isEnabled ).to.be.false; } ); @@ -153,7 +161,7 @@ describe( 'TableUI', () => { dropdown.listView.items.get( 0 ).fire( 'execute' ); expect( spy.calledOnce ).to.be.true; - expect( spy.args[ 0 ][ 0 ] ).to.equal( 'insertRowBelow' ); + expect( spy.args[ 0 ][ 0 ] ).to.equal( 'setRowHeader' ); } ); } ); @@ -180,16 +188,18 @@ describe( 'TableUI', () => { const labels = listView.items.map( ( { label } ) => label ); - expect( labels ).to.deep.equal( [ 'Insert column before', 'Insert column after', 'Delete column' ] ); + expect( labels ).to.deep.equal( [ 'Header column', 'Insert column before', 'Insert column after', 'Delete column' ] ); } ); it( 'should bind items in panel to proper commands', () => { const items = dropdown.listView.items; + const setColumnHeaderCommand = editor.commands.get( 'setColumnHeader' ); const insertColumnBeforeCommand = editor.commands.get( 'insertColumnBefore' ); const insertColumnAfterCommand = editor.commands.get( 'insertColumnAfter' ); const removeColumnCommand = editor.commands.get( 'removeColumn' ); + setColumnHeaderCommand.isEnabled = true; insertColumnBeforeCommand.isEnabled = true; insertColumnAfterCommand.isEnabled = true; removeColumnCommand.isEnabled = true; @@ -197,24 +207,28 @@ describe( 'TableUI', () => { expect( items.get( 0 ).isEnabled ).to.be.true; expect( items.get( 1 ).isEnabled ).to.be.true; expect( items.get( 2 ).isEnabled ).to.be.true; + expect( items.get( 3 ).isEnabled ).to.be.true; expect( dropdown.buttonView.isEnabled ).to.be.true; - insertColumnBeforeCommand.isEnabled = false; + setColumnHeaderCommand.isEnabled = false; expect( items.get( 0 ).isEnabled ).to.be.false; expect( dropdown.buttonView.isEnabled ).to.be.true; - insertColumnAfterCommand.isEnabled = false; + insertColumnBeforeCommand.isEnabled = false; expect( items.get( 1 ).isEnabled ).to.be.false; expect( dropdown.buttonView.isEnabled ).to.be.true; - removeColumnCommand.isEnabled = false; + insertColumnAfterCommand.isEnabled = false; expect( items.get( 2 ).isEnabled ).to.be.false; + + removeColumnCommand.isEnabled = false; + expect( items.get( 3 ).isEnabled ).to.be.false; expect( dropdown.buttonView.isEnabled ).to.be.false; } ); - it( 'should focus view after command execution', () => { + it( 'should focus view a+fter command execution', () => { const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' ); dropdown.listView.items.get( 0 ).fire( 'execute' ); @@ -228,7 +242,7 @@ describe( 'TableUI', () => { dropdown.listView.items.get( 0 ).fire( 'execute' ); expect( spy.calledOnce ).to.be.true; - expect( spy.args[ 0 ][ 0 ] ).to.equal( 'insertColumnBefore' ); + expect( spy.args[ 0 ][ 0 ] ).to.equal( 'setColumnHeader' ); } ); } ); From 9f8a1be00cb4575eb2e9d9505f39763a2bdc1a9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 25 May 2018 12:36:16 +0200 Subject: [PATCH 10/27] Changed: Bind dropdown item isActive to header commands. --- src/tableui.js | 55 +++++++++++++++++++++++++----------------------- tests/tableui.js | 26 ++++++++++++++++++++++- 2 files changed, 54 insertions(+), 27 deletions(-) diff --git a/src/tableui.js b/src/tableui.js index 98816ce7..420f82af 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -53,10 +53,10 @@ export default class TableUI extends Plugin { editor.ui.componentFactory.add( 'tableColumn', locale => { const options = [ - { command: 'setColumnHeader', label: 'Header column' }, - { command: 'insertColumnBefore', label: 'Insert column before' }, - { command: 'insertColumnAfter', label: 'Insert column after' }, - { command: 'removeColumn', label: 'Delete column' } + { commandName: 'setColumnHeader', label: 'Header column', bindIsActive: true }, + { commandName: 'insertColumnBefore', label: 'Insert column before' }, + { commandName: 'insertColumnAfter', label: 'Insert column after' }, + { commandName: 'removeColumn', label: 'Delete column' } ]; return this._prepareDropdown( 'Column', tableColumnIcon, options, locale ); @@ -64,10 +64,10 @@ export default class TableUI extends Plugin { editor.ui.componentFactory.add( 'tableRow', locale => { const options = [ - { command: 'setRowHeader', label: 'Header row' }, - { command: 'insertRowBelow', label: 'Insert row below' }, - { command: 'insertRowAbove', label: 'Insert row above' }, - { command: 'removeRow', label: 'Delete row' } + { commandName: 'setRowHeader', label: 'Header row', bindIsActive: true }, + { commandName: 'insertRowBelow', label: 'Insert row below' }, + { commandName: 'insertRowAbove', label: 'Insert row above' }, + { commandName: 'removeRow', label: 'Delete row' } ]; return this._prepareDropdown( 'Row', tableRowIcon, options, locale ); @@ -75,10 +75,10 @@ export default class TableUI extends Plugin { editor.ui.componentFactory.add( 'mergeCell', locale => { const options = [ - { command: 'mergeCellUp', label: 'Merge cell up' }, - { command: 'mergeCellRight', label: 'Merge cell right' }, - { command: 'mergeCellDown', label: 'Merge cell down' }, - { command: 'mergeCellLeft', label: 'Merge cell left' } + { commandName: 'mergeCellUp', label: 'Merge cell up' }, + { commandName: 'mergeCellRight', label: 'Merge cell right' }, + { commandName: 'mergeCellDown', label: 'Merge cell down' }, + { commandName: 'mergeCellLeft', label: 'Merge cell left' } ]; return this._prepareDropdown( 'Merge cell', tableMergeCellIcon, options, locale ); @@ -86,8 +86,8 @@ export default class TableUI extends Plugin { editor.ui.componentFactory.add( 'splitCell', locale => { const options = [ - { command: 'splitCellVertically', label: 'Split cell vertically' }, - { command: 'splitCellHorizontally', label: 'Split cell horizontally' } + { commandName: 'splitCellVertically', label: 'Split cell vertically' }, + { commandName: 'splitCellHorizontally', label: 'Split cell horizontally' } ]; return this._prepareDropdown( 'Split cell', tableSplitCellIcon, options, locale ); @@ -112,8 +112,8 @@ export default class TableUI extends Plugin { const dropdownItems = new Collection(); - for ( const { command, label } of options ) { - addListOption( command, label, editor, commands, dropdownItems ); + for ( const option of options ) { + addListOption( option, editor, commands, dropdownItems ); } addListToDropdown( dropdownView, dropdownItems ); @@ -137,16 +137,15 @@ export default class TableUI extends Plugin { } } -/** - * Adds an option to a list view. - * - * @param {String} commandName - * @param {String} label - * @param {module:core/editor/editor~Editor} editor - * @param {Array.} commands - * @param {module:utils/collection~Collection} dropdownItems - */ -function addListOption( commandName, label, editor, commands, dropdownItems ) { +// Adds an option to a list view. +// +// @param {Object} commandName +// @param {String} label +// @param {module:core/editor/editor~Editor} editor +// @param {Array.} commands +// @param {module:utils/collection~Collection} dropdownItems +function addListOption( option, editor, commands, dropdownItems ) { + const { commandName, label, bindIsActive } = option; const command = editor.commands.get( commandName ); commands.push( command ); @@ -158,5 +157,9 @@ function addListOption( commandName, label, editor, commands, dropdownItems ) { itemModel.bind( 'isEnabled' ).to( command ); + if ( bindIsActive ) { + itemModel.bind( 'isActive' ).to( command, 'value' ); + } + dropdownItems.add( itemModel ); } diff --git a/tests/tableui.js b/tests/tableui.js index 157a4e45..e97ce721 100644 --- a/tests/tableui.js +++ b/tests/tableui.js @@ -163,6 +163,18 @@ describe( 'TableUI', () => { expect( spy.calledOnce ).to.be.true; expect( spy.args[ 0 ][ 0 ] ).to.equal( 'setRowHeader' ); } ); + + it( 'should bind set header row command value to dropdown item', () => { + const items = dropdown.listView.items; + + const setRowHeaderCommand = editor.commands.get( 'setRowHeader' ); + + setRowHeaderCommand.value = false; + expect( items.get( 0 ).isActive ).to.be.false; + + setRowHeaderCommand.value = true; + expect( items.get( 0 ).isActive ).to.be.true; + } ); } ); describe( 'tableColumn dropdown', () => { @@ -228,7 +240,7 @@ describe( 'TableUI', () => { expect( dropdown.buttonView.isEnabled ).to.be.false; } ); - it( 'should focus view a+fter command execution', () => { + it( 'should focus view after command execution', () => { const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' ); dropdown.listView.items.get( 0 ).fire( 'execute' ); @@ -244,6 +256,18 @@ describe( 'TableUI', () => { expect( spy.calledOnce ).to.be.true; expect( spy.args[ 0 ][ 0 ] ).to.equal( 'setColumnHeader' ); } ); + + it( 'should bind set header column command value to dropdown item', () => { + const items = dropdown.listView.items; + + const setColumnHeaderCommand = editor.commands.get( 'setColumnHeader' ); + + setColumnHeaderCommand.value = false; + expect( items.get( 0 ).isActive ).to.be.false; + + setColumnHeaderCommand.value = true; + expect( items.get( 0 ).isActive ).to.be.true; + } ); } ); describe( 'mergeCell dropdown', () => { From 1404f22d590326408ffa63c4d130352597ad0e24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Mon, 28 May 2018 18:19:19 +0200 Subject: [PATCH 11/27] Changed: Add insertTable dropdown. --- src/tableui.js | 28 ++++-- src/ui/inserttableview.js | 193 ++++++++++++++++++++++++++++++++++++ tests/tableui.js | 38 +++++-- tests/ui/inserttableview.js | 131 ++++++++++++++++++++++++ theme/inserttable.css | 42 ++++++++ 5 files changed, 415 insertions(+), 17 deletions(-) create mode 100644 src/ui/inserttableview.js create mode 100644 tests/ui/inserttableview.js create mode 100644 theme/inserttable.css diff --git a/src/tableui.js b/src/tableui.js index 420f82af..ef9b340c 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -8,11 +8,12 @@ */ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; -import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; import { addListToDropdown, createDropdown } from '@ckeditor/ckeditor5-ui/src/dropdown/utils'; import Model from '@ckeditor/ckeditor5-ui/src/model'; import Collection from '@ckeditor/ckeditor5-utils/src/collection'; +import InsertTableView from './ui/inserttableview'; + import tableIcon from './../theme/icons/table.svg'; import tableColumnIcon from './../theme/icons/table-column.svg'; import tableRowIcon from './../theme/icons/table-row.svg'; @@ -33,22 +34,35 @@ export default class TableUI extends Plugin { editor.ui.componentFactory.add( 'insertTable', locale => { const command = editor.commands.get( 'insertTable' ); - const buttonView = new ButtonView( locale ); + const dropdownView = createDropdown( locale ); - buttonView.bind( 'isEnabled' ).to( command ); + dropdownView.bind( 'isEnabled' ).to( command ); - buttonView.set( { + dropdownView.buttonView.set( { icon: tableIcon, label: 'Insert table', tooltip: true } ); - buttonView.on( 'execute', () => { - editor.execute( 'insertTable' ); + const insertTableView = new InsertTableView( locale ); + + insertTableView.delegate( 'execute' ).to( dropdownView ); + + dropdownView.buttonView.on( 'open', () => { + // Reset the chooser before showing it to the user. + insertTableView.rows = 0; + insertTableView.columns = 0; + } ); + + dropdownView.on( 'execute', () => { + editor.execute( 'insertTable', { rows: insertTableView.rows, columns: insertTableView.columns } ); + editor.editing.view.focus(); } ); - return buttonView; + dropdownView.panelView.children.add( insertTableView ); + + return dropdownView; } ); editor.ui.componentFactory.add( 'tableColumn', locale => { diff --git a/src/ui/inserttableview.js b/src/ui/inserttableview.js new file mode 100644 index 00000000..39753fd0 --- /dev/null +++ b/src/ui/inserttableview.js @@ -0,0 +1,193 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module table/ui/inserttableview + */ + +import View from '@ckeditor/ckeditor5-ui/src/view'; +import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; +import FocusCycler from '@ckeditor/ckeditor5-ui/src/focuscycler'; + +import './../../theme/inserttable.css'; + +/** + * The table size view. + * + * It renders a 10x10 grid to choose inserted table size. + * + * @extends module:ui/view~View + */ +export default class InsertTableView extends View { + /** + * @inheritDoc + */ + constructor( locale ) { + super( locale ); + + const bind = this.bindTemplate; + + /** + * Collection of the table size box items. + * + * @readonly + * @member {module:ui/viewcollection~ViewCollection} + */ + this.items = this.createCollection(); + + /** + * Tracks information about DOM focus in the list. + * + * @readonly + * @member {module:utils/focustracker~FocusTracker} + */ + this.focusTracker = new FocusTracker(); + + /** + * Helps cycling over focusable {@link #items} in the toolbar. + * + * @readonly + * @protected + * @member {module:ui/focuscycler~FocusCycler} + */ + this._focusCycler = new FocusCycler( { + focusables: this.items, + focusTracker: this.focusTracker, + keystrokeHandler: this.keystrokes, + actions: { + // Navigate toolbar items backwards using the arrow[left,up] keys. + focusPrevious: [ 'arrowleft', 'arrowup' ], + + // Navigate toolbar items forwards using the arrow[right,down] keys. + focusNext: [ 'arrowright', 'arrowdown' ] + } + } ); + + /** + * Currently selected number of rows of a new table. + * + * @observable + * @member {Number} #rows + */ + this.set( 'rows', 0 ); + + /** + * Currently selected number of columns of a new table. + * + * @observable + * @member {Number} #columns + */ + this.set( 'columns', 0 ); + + /** + * The label text displayed under the boxes. + * + * @observable + * @member {String} #label + */ + this.bind( 'label' ) + .to( this, 'columns', this, 'rows', ( columns, rows ) => `${ rows } x ${ columns }` ); + + this.setTemplate( { + tag: 'div', + attributes: { + class: [ 'ck' ] + }, + + children: [ + { + tag: 'div', + attributes: { + class: [ 'ck-table-size-choose-box-container' ] + }, + children: this.items + }, + { + tag: 'div', + attributes: { + class: [ 'ck-table-size-label' ] + }, + children: [ + { + text: bind.to( 'label' ) + } + ] + } + ], + + on: { + click: bind.to( () => { + this.fire( 'execute' ); + } ) + } + } ); + + for ( let i = 0; i < 100; i++ ) { + const view = new TableSizeChooser(); + + view.on( 'over', () => { + const row = parseInt( i / 10 ); + const column = i % 10; + + this.set( 'rows', row + 1 ); + this.set( 'columns', column + 1 ); + } ); + + this.items.add( view ); + } + + this.on( 'change:columns', () => { + this._updateItems(); + } ); + + this.on( 'change:rows', () => { + this._updateItems(); + } ); + } + + /** + * Enables current table size selection depending on rows & columns set. + * + * @private + */ + _updateItems() { + const row = this.rows - 1; + const column = this.columns - 1; + + this.items.map( ( item, index ) => { + const itemRow = parseInt( index / 10 ); + const itemColumn = index % 10; + + if ( itemRow <= row && itemColumn <= column ) { + item.set( 'isOn', true ); + } else { + item.set( 'isOn', false ); + } + } ); + } +} + +class TableSizeChooser extends View { + constructor( locale ) { + super( locale ); + + const bind = this.bindTemplate; + + this.set( 'isOn', false ); + + this.setTemplate( { + tag: 'div', + attributes: { + class: [ + 'ck-table-size-choose-box', + bind.if( 'isOn', 'ck-on' ) + ] + }, + on: { + mouseover: bind.to( 'over' ) + } + } ); + } +} diff --git a/tests/tableui.js b/tests/tableui.js index e97ce721..240ac00b 100644 --- a/tests/tableui.js +++ b/tests/tableui.js @@ -7,7 +7,6 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; import { _clear as clearTranslations, add as addTranslations } from '@ckeditor/ckeditor5-utils/src/translation-service'; -import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import TableEditing from '../src/tableediting'; @@ -54,31 +53,50 @@ describe( 'TableUI', () => { insertTable = editor.ui.componentFactory.create( 'insertTable' ); } ); - it( 'should register insertTable buton', () => { - expect( insertTable ).to.be.instanceOf( ButtonView ); - expect( insertTable.isOn ).to.be.false; - expect( insertTable.label ).to.equal( 'Insert table' ); - expect( insertTable.icon ).to.match( / { + expect( insertTable ).to.be.instanceOf( DropdownView ); + expect( insertTable.buttonView.label ).to.equal( 'Insert table' ); + expect( insertTable.buttonView.icon ).to.match( / { const command = editor.commands.get( 'insertTable' ); command.isEnabled = true; - expect( insertTable.isOn ).to.be.false; - expect( insertTable.isEnabled ).to.be.true; + expect( insertTable.buttonView.isOn ).to.be.false; + expect( insertTable.buttonView.isEnabled ).to.be.true; command.isEnabled = false; - expect( insertTable.isEnabled ).to.be.false; + expect( insertTable.buttonView.isEnabled ).to.be.false; } ); it( 'should execute insertTable command on button execute event', () => { const executeSpy = testUtils.sinon.spy( editor, 'execute' ); + const tableSizeView = insertTable.panelView.children.get( 0 ); + + tableSizeView.rows = 2; + tableSizeView.columns = 7; + insertTable.fire( 'execute' ); sinon.assert.calledOnce( executeSpy ); - sinon.assert.calledWithExactly( executeSpy, 'insertTable' ); + sinon.assert.calledWithExactly( executeSpy, 'insertTable', { rows: 2, columns: 7 } ); + } ); + + it( 'should reset rows & columns on dropdown open', () => { + const tableSizeView = insertTable.panelView.children.get( 0 ); + + expect( tableSizeView.rows ).to.equal( 0 ); + expect( tableSizeView.columns ).to.equal( 0 ); + + tableSizeView.rows = 2; + tableSizeView.columns = 2; + + insertTable.buttonView.fire( 'open' ); + + expect( tableSizeView.rows ).to.equal( 0 ); + expect( tableSizeView.columns ).to.equal( 0 ); } ); } ); diff --git a/tests/ui/inserttableview.js b/tests/ui/inserttableview.js new file mode 100644 index 00000000..9a840e48 --- /dev/null +++ b/tests/ui/inserttableview.js @@ -0,0 +1,131 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* globals document, Event */ + +import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; +import ViewCollection from '@ckeditor/ckeditor5-ui/src/viewcollection'; + +import InsertTableView from '../../src/ui/inserttableview'; + +describe( 'InsertTableView', () => { + let view, locale; + + beforeEach( () => { + locale = { t() {} }; + + view = new InsertTableView( locale ); + view.render(); + + document.body.appendChild( view.element ); + } ); + + afterEach( () => { + view.element.remove(); + } ); + + describe( 'constructor()', () => { + it( 'sets view#locale', () => { + expect( view.locale ).to.equal( locale ); + } ); + + it( 'sets view#rows to 0', () => { + expect( view.rows ).to.equal( 0 ); + } ); + + it( 'sets view#columns to 0', () => { + expect( view.columns ).to.equal( 0 ); + } ); + + it( 'sets #label to default rows & columns', () => { + expect( view.label ).to.equal( '0 x 0' ); + } ); + + it( 'creates #focusTracker instance', () => { + expect( view.focusTracker ).to.be.instanceOf( FocusTracker ); + } ); + + it( 'creates #element from template', () => { + expect( view.element.classList.contains( 'ck' ) ).to.be.true; + expect( view.element.children ).to.have.length( 2 ); + expect( view.element.children[ 0 ].classList.contains( 'ck-table-size-choose-box-container' ) ).to.be.true; + expect( view.element.children[ 1 ].classList.contains( 'ck-table-size-label' ) ).to.be.true; + } ); + + it( 'creates view#items collection', () => { + expect( view.items ).to.be.instanceOf( ViewCollection ); + expect( view.items ).to.have.length( 100 ); + } ); + + describe( 'view#items bindings', () => { + it( 'updates view#height & view#width on "over" event', () => { + const boxView = view.items.get( 0 ); + + expect( boxView.isOn ).to.be.false; + + boxView.fire( 'over' ); + + expect( boxView.isOn ).to.be.true; + + expect( view.rows ).to.equal( 1 ); + expect( view.columns ).to.equal( 1 ); + + const boxViewB = view.items.get( 22 ); + + boxViewB.fire( 'over' ); + + expect( view.rows ).to.equal( 3 ); + expect( view.columns ).to.equal( 3 ); + } ); + } ); + + describe( 'bindings', () => { + it( 'binds #label to rows & columns', () => { + view.rows = 3; + + expect( view.label ).to.equal( '3 x 0' ); + + view.columns = 7; + + expect( view.label ).to.equal( '3 x 7' ); + } ); + + describe( 'DOM', () => { + it( 'fires execute on "click" event', () => { + const spy = sinon.spy(); + + view.on( 'execute', spy ); + + dispatchEvent( view.element, 'click' ); + + sinon.assert.calledOnce( spy ); + } ); + + describe( 'view#items mousemove event', () => { + it( 'fires "over" event', () => { + const boxView = view.items.get( 0 ); + const spy = sinon.spy(); + + boxView.on( 'over', spy ); + + dispatchEvent( boxView.element, 'mouseover' ); + + sinon.assert.calledOnce( spy ); + } ); + } ); + } ); + } ); + } ); +} ); + +function dispatchEvent( el, domEvtName ) { + if ( !el.parentNode ) { + throw new Error( 'To dispatch an event, element must be in DOM. Otherwise #target is null.' ); + } + + el.dispatchEvent( new Event( domEvtName, { + bubbles: true + } ) ); +} diff --git a/theme/inserttable.css b/theme/inserttable.css new file mode 100644 index 00000000..5712a566 --- /dev/null +++ b/theme/inserttable.css @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +:root { + /* TODO: does it need to be a variable? */ + --ck-insert-table-box-height: 11px; + --ck-insert-table-box-width: 12px; + --ck-insert-table-box-margin: 1px; + --ck-insert-table-box-border: 1px; + --ck-insert-table-box-border-color: #aaa; + --ck-insert-table-box-border-color__active: #5fa6ff; + --ck-insert-table-box-background__active: #c9e9ff; + --ck-insert-table-padding: 10px; +} + +.ck .ck-table-size-choose-box-container { + display: flex; + flex-direction: row; + flex-wrap: wrap; + /* The width of a container should match 10 items in a row so there will be a 10x10 grid. */ + width: calc(var(--ck-insert-table-box-width) * 10 + var(--ck-insert-table-box-margin) * 20 + var(--ck-insert-table-padding) * 2); + padding: var(--ck-insert-table-padding) var(--ck-insert-table-padding) 0; +} + +.ck .ck-table-size-choose-box { + width: var(--ck-insert-table-box-width); + height: var(--ck-insert-table-box-height); + margin: var(--ck-insert-table-box-margin); + border: var(--ck-insert-table-box-border) solid var(--ck-insert-table-box-border-color); + border-radius: 1px; + + &.ck-on { + border-color: var(--ck-insert-table-box-border-color__active); + background: var(--ck-insert-table-box-background__active); + } +} + +.ck .ck-table-size-label { + text-align: center; +} From 72c970979b1909a9cf30ebd200db192a525240d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 29 May 2018 09:27:30 +0200 Subject: [PATCH 12/27] Tests: Fix SetHeaderRowCommand. --- src/commands/setheaderrowcommand.js | 2 +- tests/commands/setheaderrowcommand.js | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/commands/setheaderrowcommand.js b/src/commands/setheaderrowcommand.js index ba936c53..d31b6290 100644 --- a/src/commands/setheaderrowcommand.js +++ b/src/commands/setheaderrowcommand.js @@ -79,7 +79,7 @@ export default class SetHeaderRowCommand extends Command { _isInHeading( tableCell, table ) { const headingRows = parseInt( table.getAttribute( 'headingRows' ) || 0 ); - return headingRows && tableCell.parent.index < headingRows; + return !!headingRows && tableCell.parent.index < headingRows; } } diff --git a/tests/commands/setheaderrowcommand.js b/tests/commands/setheaderrowcommand.js index 1553a3ff..bbfdbed7 100644 --- a/tests/commands/setheaderrowcommand.js +++ b/tests/commands/setheaderrowcommand.js @@ -93,6 +93,15 @@ describe( 'HeaderRowCommand', () => { } ); describe( 'value', () => { + it( 'should be false if selection is not in a table without heading row', () => { + setData( model, modelTable( [ + [ '01[]', '02' ], + [ '11', '12' ] + ] ) ); + + expect( command.value ).to.be.false; + } ); + it( 'should be false if selection is not in a heading row', () => { setData( model, modelTable( [ [ '01', '02' ], From e7978598cc5432805cb14594b11ba1065723b72f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 29 May 2018 10:28:03 +0200 Subject: [PATCH 13/27] Other: Remove focus handling from InsertTableView. --- src/ui/inserttableview.js | 30 ------------------------------ tests/ui/inserttableview.js | 5 ----- 2 files changed, 35 deletions(-) diff --git a/src/ui/inserttableview.js b/src/ui/inserttableview.js index 39753fd0..44a2c2ad 100644 --- a/src/ui/inserttableview.js +++ b/src/ui/inserttableview.js @@ -8,8 +8,6 @@ */ import View from '@ckeditor/ckeditor5-ui/src/view'; -import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; -import FocusCycler from '@ckeditor/ckeditor5-ui/src/focuscycler'; import './../../theme/inserttable.css'; @@ -37,34 +35,6 @@ export default class InsertTableView extends View { */ this.items = this.createCollection(); - /** - * Tracks information about DOM focus in the list. - * - * @readonly - * @member {module:utils/focustracker~FocusTracker} - */ - this.focusTracker = new FocusTracker(); - - /** - * Helps cycling over focusable {@link #items} in the toolbar. - * - * @readonly - * @protected - * @member {module:ui/focuscycler~FocusCycler} - */ - this._focusCycler = new FocusCycler( { - focusables: this.items, - focusTracker: this.focusTracker, - keystrokeHandler: this.keystrokes, - actions: { - // Navigate toolbar items backwards using the arrow[left,up] keys. - focusPrevious: [ 'arrowleft', 'arrowup' ], - - // Navigate toolbar items forwards using the arrow[right,down] keys. - focusNext: [ 'arrowright', 'arrowdown' ] - } - } ); - /** * Currently selected number of rows of a new table. * diff --git a/tests/ui/inserttableview.js b/tests/ui/inserttableview.js index 9a840e48..040fb3c6 100644 --- a/tests/ui/inserttableview.js +++ b/tests/ui/inserttableview.js @@ -5,7 +5,6 @@ /* globals document, Event */ -import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; import ViewCollection from '@ckeditor/ckeditor5-ui/src/viewcollection'; import InsertTableView from '../../src/ui/inserttableview'; @@ -43,10 +42,6 @@ describe( 'InsertTableView', () => { expect( view.label ).to.equal( '0 x 0' ); } ); - it( 'creates #focusTracker instance', () => { - expect( view.focusTracker ).to.be.instanceOf( FocusTracker ); - } ); - it( 'creates #element from template', () => { expect( view.element.classList.contains( 'ck' ) ).to.be.true; expect( view.element.children ).to.have.length( 2 ); From 67244ca7e2fc8f9d15490709cb8399ab276f70e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 29 May 2018 10:51:31 +0200 Subject: [PATCH 14/27] Other: Update CSS classes to be more BEM & update color values. --- src/ui/inserttableview.js | 6 +++--- tests/ui/inserttableview.js | 4 ++-- theme/inserttable.css | 43 ++++++++++++++++++------------------- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/ui/inserttableview.js b/src/ui/inserttableview.js index 44a2c2ad..94fdcdcf 100644 --- a/src/ui/inserttableview.js +++ b/src/ui/inserttableview.js @@ -70,14 +70,14 @@ export default class InsertTableView extends View { { tag: 'div', attributes: { - class: [ 'ck-table-size-choose-box-container' ] + class: [ 'ck-insert-table-dropdown__grid' ] }, children: this.items }, { tag: 'div', attributes: { - class: [ 'ck-table-size-label' ] + class: [ 'ck-insert-table-dropdown__label' ] }, children: [ { @@ -151,7 +151,7 @@ class TableSizeChooser extends View { tag: 'div', attributes: { class: [ - 'ck-table-size-choose-box', + 'ck-insert-table-dropdown-grid-box', bind.if( 'isOn', 'ck-on' ) ] }, diff --git a/tests/ui/inserttableview.js b/tests/ui/inserttableview.js index 040fb3c6..78fef2c2 100644 --- a/tests/ui/inserttableview.js +++ b/tests/ui/inserttableview.js @@ -45,8 +45,8 @@ describe( 'InsertTableView', () => { it( 'creates #element from template', () => { expect( view.element.classList.contains( 'ck' ) ).to.be.true; expect( view.element.children ).to.have.length( 2 ); - expect( view.element.children[ 0 ].classList.contains( 'ck-table-size-choose-box-container' ) ).to.be.true; - expect( view.element.children[ 1 ].classList.contains( 'ck-table-size-label' ) ).to.be.true; + expect( view.element.children[ 0 ].classList.contains( 'ck-insert-table-dropdown__grid' ) ).to.be.true; + expect( view.element.children[ 1 ].classList.contains( 'ck-insert-table-dropdown__label' ) ).to.be.true; } ); it( 'creates view#items collection', () => { diff --git a/theme/inserttable.css b/theme/inserttable.css index 5712a566..d2e963be 100644 --- a/theme/inserttable.css +++ b/theme/inserttable.css @@ -4,39 +4,38 @@ */ :root { - /* TODO: does it need to be a variable? */ - --ck-insert-table-box-height: 11px; - --ck-insert-table-box-width: 12px; - --ck-insert-table-box-margin: 1px; - --ck-insert-table-box-border: 1px; - --ck-insert-table-box-border-color: #aaa; - --ck-insert-table-box-border-color__active: #5fa6ff; - --ck-insert-table-box-background__active: #c9e9ff; - --ck-insert-table-padding: 10px; + --ck-insert-table-dropdown-padding: 10px; + --ck-insert-table-dropdown-box-height: 11px; + --ck-insert-table-dropdown-box-width: 12px; + --ck-insert-table-dropdown-box-margin: 1px; + --ck-insert-table-dropdown-box-border-color: hsl(0, 0%, 75%); + --ck-insert-table-dropdown-box-border-active-color: hsl(208, 73%, 61%); + --ck-insert-table-dropdown-box-active-background: hsl(208, 100%, 89%); } -.ck .ck-table-size-choose-box-container { +.ck .ck-insert-table-dropdown__grid { display: flex; flex-direction: row; flex-wrap: wrap; /* The width of a container should match 10 items in a row so there will be a 10x10 grid. */ - width: calc(var(--ck-insert-table-box-width) * 10 + var(--ck-insert-table-box-margin) * 20 + var(--ck-insert-table-padding) * 2); - padding: var(--ck-insert-table-padding) var(--ck-insert-table-padding) 0; + width: calc(var(--ck-insert-table-dropdown-box-width) * 10 + var(--ck-insert-table-dropdown-box-margin) * 20 + var(--ck-insert-table-dropdown-padding) * 2); + padding: var(--ck-insert-table-dropdown-padding) var(--ck-insert-table-dropdown-padding) 0; } -.ck .ck-table-size-choose-box { - width: var(--ck-insert-table-box-width); - height: var(--ck-insert-table-box-height); - margin: var(--ck-insert-table-box-margin); - border: var(--ck-insert-table-box-border) solid var(--ck-insert-table-box-border-color); +.ck .ck-insert-table-dropdown__label { + text-align: center; +} + +.ck .ck-insert-table-dropdown-grid-box { + width: var(--ck-insert-table-dropdown-box-width); + height: var(--ck-insert-table-dropdown-box-height); + margin: var(--ck-insert-table-dropdown-box-margin); + border: 1px solid var(--ck-insert-table-dropdown-box-border-color); border-radius: 1px; &.ck-on { - border-color: var(--ck-insert-table-box-border-color__active); - background: var(--ck-insert-table-box-background__active); + border-color: var(--ck-insert-table-dropdown-box-border-active-color); + background: var(--ck-insert-table-dropdown-box-active-background); } } -.ck .ck-table-size-label { - text-align: center; -} From a5a3140a24a3effbfa516f44cf6aaf33796390e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 29 May 2018 11:39:09 +0200 Subject: [PATCH 15/27] Docs: Update InsertTableView docs. --- src/ui/inserttableview.js | 56 ++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/src/ui/inserttableview.js b/src/ui/inserttableview.js index 94fdcdcf..f9a417db 100644 --- a/src/ui/inserttableview.js +++ b/src/ui/inserttableview.js @@ -94,57 +94,75 @@ export default class InsertTableView extends View { } } ); - for ( let i = 0; i < 100; i++ ) { - const view = new TableSizeChooser(); + // Add grid boxes to table selection view. + for ( let index = 0; index < 100; index++ ) { + const boxView = new TableSizeGridBoxView(); - view.on( 'over', () => { - const row = parseInt( i / 10 ); - const column = i % 10; + // Listen to box view 'over' event which indicates that mouse is over this box. + boxView.on( 'over', () => { + // Translate box index to the row & column index. + const row = parseInt( index / 10 ); + const column = index % 10; + // As row & column indexes are zero-based transform it to number of selected rows & columns. this.set( 'rows', row + 1 ); this.set( 'columns', column + 1 ); } ); - this.items.add( view ); + this.items.add( boxView ); } this.on( 'change:columns', () => { - this._updateItems(); + this._highlightGridBoxes(); } ); this.on( 'change:rows', () => { - this._updateItems(); + this._highlightGridBoxes(); } ); } /** - * Enables current table size selection depending on rows & columns set. + * Highlights grid boxes depending on rows & columns selected. * * @private */ - _updateItems() { - const row = this.rows - 1; - const column = this.columns - 1; + _highlightGridBoxes() { + const rows = this.rows; + const columns = this.columns; - this.items.map( ( item, index ) => { + this.items.map( ( boxView, index ) => { + // Translate box index to the row & column index. const itemRow = parseInt( index / 10 ); const itemColumn = index % 10; - if ( itemRow <= row && itemColumn <= column ) { - item.set( 'isOn', true ); - } else { - item.set( 'isOn', false ); - } + // Grid box is highlighted when its row & column index belongs to selected number of rows & columns. + const isOn = itemRow < rows && itemColumn < columns; + + boxView.set( 'isOn', isOn ); } ); } } -class TableSizeChooser extends View { +/** + * Single grid box view element. + * + * @private + */ +class TableSizeGridBoxView extends View { + /** + * @inheritDoc + */ constructor( locale ) { super( locale ); const bind = this.bindTemplate; + /** + * Controls whether the grid box view is "on". + * + * @observable + * @member {Boolean} #isOn + */ this.set( 'isOn', false ); this.setTemplate( { From 5e6744b139d44cecb95983b1fd2910d841b52079 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 29 May 2018 11:52:45 +0200 Subject: [PATCH 16/27] Changed: Move splitCell dropdown items to mergeCell dropdown. --- src/tableui.js | 12 ++----- tests/tableui.js | 83 ++++++++++-------------------------------------- 2 files changed, 19 insertions(+), 76 deletions(-) diff --git a/src/tableui.js b/src/tableui.js index ef9b340c..66b37ed4 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -18,7 +18,6 @@ import tableIcon from './../theme/icons/table.svg'; import tableColumnIcon from './../theme/icons/table-column.svg'; import tableRowIcon from './../theme/icons/table-row.svg'; import tableMergeCellIcon from './../theme/icons/table-merge-cell.svg'; -import tableSplitCellIcon from './../theme/icons/table-split-cell.svg'; /** * The table UI plugin. @@ -92,19 +91,12 @@ export default class TableUI extends Plugin { { commandName: 'mergeCellUp', label: 'Merge cell up' }, { commandName: 'mergeCellRight', label: 'Merge cell right' }, { commandName: 'mergeCellDown', label: 'Merge cell down' }, - { commandName: 'mergeCellLeft', label: 'Merge cell left' } - ]; - - return this._prepareDropdown( 'Merge cell', tableMergeCellIcon, options, locale ); - } ); - - editor.ui.componentFactory.add( 'splitCell', locale => { - const options = [ + { commandName: 'mergeCellLeft', label: 'Merge cell left' }, { commandName: 'splitCellVertically', label: 'Split cell vertically' }, { commandName: 'splitCellHorizontally', label: 'Split cell horizontally' } ]; - return this._prepareDropdown( 'Split cell', tableSplitCellIcon, options, locale ); + return this._prepareDropdown( 'Merge cell', tableMergeCellIcon, options, locale ); } ); } diff --git a/tests/tableui.js b/tests/tableui.js index 240ac00b..e1e475ed 100644 --- a/tests/tableui.js +++ b/tests/tableui.js @@ -311,7 +311,14 @@ describe( 'TableUI', () => { const labels = listView.items.map( ( { label } ) => label ); - expect( labels ).to.deep.equal( [ 'Merge cell up', 'Merge cell right', 'Merge cell down', 'Merge cell left' ] ); + expect( labels ).to.deep.equal( [ + 'Merge cell up', + 'Merge cell right', + 'Merge cell down', + 'Merge cell left', + 'Split cell vertically', + 'Split cell horizontally' + ] ); } ); it( 'should bind items in panel to proper commands', () => { @@ -321,16 +328,22 @@ describe( 'TableUI', () => { const mergeCellRightCommand = editor.commands.get( 'mergeCellRight' ); const mergeCellDownCommand = editor.commands.get( 'mergeCellDown' ); const mergeCellLeftCommand = editor.commands.get( 'mergeCellLeft' ); + const splitCellVerticallyCommand = editor.commands.get( 'splitCellVertically' ); + const splitCellHorizontallyCommand = editor.commands.get( 'splitCellHorizontally' ); mergeCellUpCommand.isEnabled = true; mergeCellRightCommand.isEnabled = true; mergeCellDownCommand.isEnabled = true; mergeCellLeftCommand.isEnabled = true; + splitCellVerticallyCommand.isEnabled = true; + splitCellHorizontallyCommand.isEnabled = true; expect( items.get( 0 ).isEnabled ).to.be.true; expect( items.get( 1 ).isEnabled ).to.be.true; expect( items.get( 2 ).isEnabled ).to.be.true; expect( items.get( 3 ).isEnabled ).to.be.true; + expect( items.get( 4 ).isEnabled ).to.be.true; + expect( items.get( 5 ).isEnabled ).to.be.true; expect( dropdown.buttonView.isEnabled ).to.be.true; mergeCellUpCommand.isEnabled = false; @@ -349,74 +362,12 @@ describe( 'TableUI', () => { mergeCellLeftCommand.isEnabled = false; expect( items.get( 3 ).isEnabled ).to.be.false; - expect( dropdown.buttonView.isEnabled ).to.be.false; - } ); - - it( 'should focus view after command execution', () => { - const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' ); - - dropdown.listView.items.get( 0 ).fire( 'execute' ); - - sinon.assert.calledOnce( focusSpy ); - } ); - - it( 'executes command when it\'s executed', () => { - const spy = sinon.stub( editor, 'execute' ); - - dropdown.listView.items.get( 0 ).fire( 'execute' ); - - expect( spy.calledOnce ).to.be.true; - expect( spy.args[ 0 ][ 0 ] ).to.equal( 'mergeCellUp' ); - } ); - } ); - - describe( 'splitCell dropdown', () => { - let dropdown; - - beforeEach( () => { - dropdown = editor.ui.componentFactory.create( 'splitCell' ); - } ); - - it( 'have button with proper properties set', () => { - expect( dropdown ).to.be.instanceOf( DropdownView ); - - const button = dropdown.buttonView; - - expect( button.isOn ).to.be.false; - expect( button.tooltip ).to.be.true; - expect( button.label ).to.equal( 'Split cell' ); - expect( button.icon ).to.match( / { - const listView = dropdown.listView; - - const labels = listView.items.map( ( { label } ) => label ); - - expect( labels ).to.deep.equal( [ 'Split cell vertically', 'Split cell horizontally' ] ); - } ); - - it( 'should bind items in panel to proper commands', () => { - const items = dropdown.listView.items; - - const splitCellVerticallyCommand = editor.commands.get( 'splitCellVertically' ); - const splitCellHorizontallyCommand = editor.commands.get( 'splitCellHorizontally' ); - - splitCellVerticallyCommand.isEnabled = true; - splitCellHorizontallyCommand.isEnabled = true; - - expect( items.get( 0 ).isEnabled ).to.be.true; - expect( items.get( 1 ).isEnabled ).to.be.true; - expect( dropdown.buttonView.isEnabled ).to.be.true; - splitCellVerticallyCommand.isEnabled = false; - - expect( items.get( 0 ).isEnabled ).to.be.false; - expect( dropdown.buttonView.isEnabled ).to.be.true; + expect( items.get( 4 ).isEnabled ).to.be.false; splitCellHorizontallyCommand.isEnabled = false; + expect( items.get( 5 ).isEnabled ).to.be.false; - expect( items.get( 1 ).isEnabled ).to.be.false; expect( dropdown.buttonView.isEnabled ).to.be.false; } ); @@ -434,7 +385,7 @@ describe( 'TableUI', () => { dropdown.listView.items.get( 0 ).fire( 'execute' ); expect( spy.calledOnce ).to.be.true; - expect( spy.args[ 0 ][ 0 ] ).to.equal( 'splitCellVertically' ); + expect( spy.args[ 0 ][ 0 ] ).to.equal( 'mergeCellUp' ); } ); } ); } ); From 710b8a6e2253ae94ad3fec3fab5f6ea9f60ef78e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 29 May 2018 12:53:16 +0200 Subject: [PATCH 17/27] Docs: Update ui/utils module docs. --- src/ui/utils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ui/utils.js b/src/ui/utils.js index 9cc75a49..82bad1a2 100644 --- a/src/ui/utils.js +++ b/src/ui/utils.js @@ -4,7 +4,7 @@ */ /** - * @module image/image/ui/utils + * @module table/ui/utils */ import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview'; @@ -13,7 +13,7 @@ import { getParentTable } from '../commands/utils'; /** * A helper utility that positions the * {@link module:ui/panel/balloon/contextualballoon~ContextualBalloon contextual balloon} instance - * with respect to the image in the editor content, if one is selected. + * with respect to the table in the editor content, if one is selected. * * @param {module:core/editor/editor~Editor} editor The editor instance. */ From 969347b2f2e1e27df26bdb2b2015572c761e22f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 29 May 2018 13:32:25 +0200 Subject: [PATCH 18/27] Docs: Update Table UI plugins docs. --- src/tableui.js | 45 ++++++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/src/tableui.js b/src/tableui.js index 66b37ed4..a5219cd7 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -20,7 +20,14 @@ import tableRowIcon from './../theme/icons/table-row.svg'; import tableMergeCellIcon from './../theme/icons/table-merge-cell.svg'; /** - * The table UI plugin. + * The table UI plugin. It introduces: + * + * * The `'insertTable'` dropdown, + * * The `'tableColumn'` dropdown, + * * The `'tableRow'` dropdown, + * * The `'mergeCell'` dropdown. + * + * The `'tableColumn'`, `'tableRow'`, `'mergeCell'` are best to be used in conjunction with {@link module:table/tabletoolbar~TableToolbar}. * * @extends module:core/plugin~Plugin */ @@ -37,13 +44,16 @@ export default class TableUI extends Plugin { dropdownView.bind( 'isEnabled' ).to( command ); + // Decorate dropdown's button. dropdownView.buttonView.set( { icon: tableIcon, label: 'Insert table', tooltip: true } ); + // Prepare custom view for dropdown's panel. const insertTableView = new InsertTableView( locale ); + dropdownView.panelView.children.add( insertTableView ); insertTableView.delegate( 'execute' ).to( dropdownView ); @@ -55,12 +65,9 @@ export default class TableUI extends Plugin { dropdownView.on( 'execute', () => { editor.execute( 'insertTable', { rows: insertTableView.rows, columns: insertTableView.columns } ); - editor.editing.view.focus(); } ); - dropdownView.panelView.children.add( insertTableView ); - return dropdownView; } ); @@ -101,13 +108,13 @@ export default class TableUI extends Plugin { } /** - * Common method that prepares dropdown. + * Creates dropdown view from set of options. * * @private - * @param {String} buttonName - * @param {String} icon - * @param {Array.} options - * @param locale + * @param {String} buttonName Dropdown button name. + * @param {String} icon Icon for dropdown button. + * @param {Array.} options List of options for dropdown. + * @param {module:utils/locale~Locale} locale * @returns {module:ui/dropdown/dropdownview~DropdownView} */ _prepareDropdown( buttonName, icon, options, locale ) { @@ -116,6 +123,7 @@ export default class TableUI extends Plugin { const dropdownView = createDropdown( locale ); const commands = []; + // Prepare dropdown list items for list dropdown. const dropdownItems = new Collection(); for ( const option of options ) { @@ -124,12 +132,14 @@ export default class TableUI extends Plugin { addListToDropdown( dropdownView, dropdownItems ); + // Decorate dropdown's button. dropdownView.buttonView.set( { label: buttonName, icon, tooltip: true } ); + // Make dropdown button disabled when all options are disabled. dropdownView.bind( 'isEnabled' ).toMany( commands, 'isEnabled', ( ...areEnabled ) => { return areEnabled.some( isEnabled => isEnabled ); } ); @@ -145,11 +155,10 @@ export default class TableUI extends Plugin { // Adds an option to a list view. // -// @param {Object} commandName -// @param {String} label +// @param {module:table/tableui~DropdownOption} option Configuration option. // @param {module:core/editor/editor~Editor} editor -// @param {Array.} commands -// @param {module:utils/collection~Collection} dropdownItems +// @param {Array.} commands List of commands to update. +// @param {module:utils/collection~Collection} dropdownItems Collection of dropdown items to update with given option. function addListOption( option, editor, commands, dropdownItems ) { const { commandName, label, bindIsActive } = option; const command = editor.commands.get( commandName ); @@ -169,3 +178,13 @@ function addListOption( option, editor, commands, dropdownItems ) { dropdownItems.add( itemModel ); } + +/** + * Object describing table dropdowns' items. + * + * @typedef {Object} module:table/tableui~DropdownOption + * @private + * @property {String} commandName A command name to execute for that option. + * @property {String} label A dropdown item label. + * @property {Boolean} bindIsActive If `true` will bind command's value to `isActive` dropdown item property. + */ From 27e9222f0bf127f93c7a9b0fb258d0d018956ef5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 29 May 2018 16:14:40 +0200 Subject: [PATCH 19/27] Other: Add UI components label to translation service. --- src/tableui.js | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/tableui.js b/src/tableui.js index a5219cd7..71566fc0 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -37,6 +37,7 @@ export default class TableUI extends Plugin { */ init() { const editor = this.editor; + const t = this.editor.t; editor.ui.componentFactory.add( 'insertTable', locale => { const command = editor.commands.get( 'insertTable' ); @@ -47,7 +48,7 @@ export default class TableUI extends Plugin { // Decorate dropdown's button. dropdownView.buttonView.set( { icon: tableIcon, - label: 'Insert table', + label: t( 'Insert table' ), tooltip: true } ); @@ -73,10 +74,10 @@ export default class TableUI extends Plugin { editor.ui.componentFactory.add( 'tableColumn', locale => { const options = [ - { commandName: 'setColumnHeader', label: 'Header column', bindIsActive: true }, - { commandName: 'insertColumnBefore', label: 'Insert column before' }, - { commandName: 'insertColumnAfter', label: 'Insert column after' }, - { commandName: 'removeColumn', label: 'Delete column' } + { commandName: 'setColumnHeader', label: t( 'Header column' ), bindIsActive: true }, + { commandName: 'insertColumnBefore', label: t( 'Insert column before' ) }, + { commandName: 'insertColumnAfter', label: t( 'Insert column after' ) }, + { commandName: 'removeColumn', label: t( 'Delete column' ) } ]; return this._prepareDropdown( 'Column', tableColumnIcon, options, locale ); @@ -84,10 +85,10 @@ export default class TableUI extends Plugin { editor.ui.componentFactory.add( 'tableRow', locale => { const options = [ - { commandName: 'setRowHeader', label: 'Header row', bindIsActive: true }, - { commandName: 'insertRowBelow', label: 'Insert row below' }, - { commandName: 'insertRowAbove', label: 'Insert row above' }, - { commandName: 'removeRow', label: 'Delete row' } + { commandName: 'setRowHeader', label: t( 'Header row' ), bindIsActive: true }, + { commandName: 'insertRowBelow', label: t( 'Insert row below' ) }, + { commandName: 'insertRowAbove', label: t( 'Insert row above' ) }, + { commandName: 'removeRow', label: t( 'Delete row' ) } ]; return this._prepareDropdown( 'Row', tableRowIcon, options, locale ); @@ -95,12 +96,12 @@ export default class TableUI extends Plugin { editor.ui.componentFactory.add( 'mergeCell', locale => { const options = [ - { commandName: 'mergeCellUp', label: 'Merge cell up' }, - { commandName: 'mergeCellRight', label: 'Merge cell right' }, - { commandName: 'mergeCellDown', label: 'Merge cell down' }, - { commandName: 'mergeCellLeft', label: 'Merge cell left' }, - { commandName: 'splitCellVertically', label: 'Split cell vertically' }, - { commandName: 'splitCellHorizontally', label: 'Split cell horizontally' } + { commandName: 'mergeCellUp', label: t( 'Merge cell up' ) }, + { commandName: 'mergeCellRight', label: t( 'Merge cell right' ) }, + { commandName: 'mergeCellDown', label: t( 'Merge cell down' ) }, + { commandName: 'mergeCellLeft', label: t( 'Merge cell left' ) }, + { commandName: 'splitCellVertically', label: t( 'Split cell vertically' ) }, + { commandName: 'splitCellHorizontally', label: t( 'Split cell horizontally' ) } ]; return this._prepareDropdown( 'Merge cell', tableMergeCellIcon, options, locale ); From 91da813f3236c806d40c6aa61aed6e270b096bbc Mon Sep 17 00:00:00 2001 From: David Twist Date: Wed, 30 May 2018 04:48:33 -0700 Subject: [PATCH 20/27] T/1: Alternate icon set for table feature Includes: - Row, Col, Cell icons from https://github.com/ckeditor/ckeditor5-table/issues/1 - Alternate Table icon with 3 x 3 grid to match the row/col/cell icons above - Merge/Unmerge toggle states in case they are wanted instead of cell drop-down once multi-cell selection is working --- theme/icons/table-cell-merged.svg | 1 + theme/icons/table-cell.svg | 1 + theme/icons/table-cells-merge.svg | 1 + theme/icons/table-cells-unmerge.svg | 1 + theme/icons/table-column.svg | 2 +- theme/icons/table-row.svg | 2 +- theme/icons/table.svg | 2 +- 7 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 theme/icons/table-cell-merged.svg create mode 100644 theme/icons/table-cell.svg create mode 100644 theme/icons/table-cells-merge.svg create mode 100644 theme/icons/table-cells-unmerge.svg diff --git a/theme/icons/table-cell-merged.svg b/theme/icons/table-cell-merged.svg new file mode 100644 index 00000000..c89994ef --- /dev/null +++ b/theme/icons/table-cell-merged.svg @@ -0,0 +1 @@ + diff --git a/theme/icons/table-cell.svg b/theme/icons/table-cell.svg new file mode 100644 index 00000000..21042809 --- /dev/null +++ b/theme/icons/table-cell.svg @@ -0,0 +1 @@ + diff --git a/theme/icons/table-cells-merge.svg b/theme/icons/table-cells-merge.svg new file mode 100644 index 00000000..375b9a5d --- /dev/null +++ b/theme/icons/table-cells-merge.svg @@ -0,0 +1 @@ + diff --git a/theme/icons/table-cells-unmerge.svg b/theme/icons/table-cells-unmerge.svg new file mode 100644 index 00000000..e175008a --- /dev/null +++ b/theme/icons/table-cells-unmerge.svg @@ -0,0 +1 @@ + diff --git a/theme/icons/table-column.svg b/theme/icons/table-column.svg index c02f487f..68849053 100644 --- a/theme/icons/table-column.svg +++ b/theme/icons/table-column.svg @@ -1 +1 @@ - \ No newline at end of file + diff --git a/theme/icons/table-row.svg b/theme/icons/table-row.svg index 7c6aa14e..68995b95 100644 --- a/theme/icons/table-row.svg +++ b/theme/icons/table-row.svg @@ -1 +1 @@ - \ No newline at end of file + diff --git a/theme/icons/table.svg b/theme/icons/table.svg index 5b6fc40f..13b6cc45 100644 --- a/theme/icons/table.svg +++ b/theme/icons/table.svg @@ -1 +1 @@ - \ No newline at end of file + From df938717f336eeea192a88c37f4ce1960b3ca7e3 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 30 May 2018 15:57:27 +0200 Subject: [PATCH 21/27] Removed unused icons. Refined row, column, table and merge icons. --- theme/icons/table-cell-merged.svg | 1 - theme/icons/table-cell.svg | 1 - theme/icons/table-cells-merge.svg | 1 - theme/icons/table-cells-unmerge.svg | 1 - theme/icons/table-column.svg | 2 +- theme/icons/table-headers.svg | 1 - theme/icons/table-merge-cell.svg | 2 +- theme/icons/table-row.svg | 2 +- theme/icons/table-split-cell.svg | 1 - theme/icons/table.svg | 2 +- 10 files changed, 4 insertions(+), 10 deletions(-) delete mode 100644 theme/icons/table-cell-merged.svg delete mode 100644 theme/icons/table-cell.svg delete mode 100644 theme/icons/table-cells-merge.svg delete mode 100644 theme/icons/table-cells-unmerge.svg delete mode 100644 theme/icons/table-headers.svg delete mode 100644 theme/icons/table-split-cell.svg diff --git a/theme/icons/table-cell-merged.svg b/theme/icons/table-cell-merged.svg deleted file mode 100644 index c89994ef..00000000 --- a/theme/icons/table-cell-merged.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/theme/icons/table-cell.svg b/theme/icons/table-cell.svg deleted file mode 100644 index 21042809..00000000 --- a/theme/icons/table-cell.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/theme/icons/table-cells-merge.svg b/theme/icons/table-cells-merge.svg deleted file mode 100644 index 375b9a5d..00000000 --- a/theme/icons/table-cells-merge.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/theme/icons/table-cells-unmerge.svg b/theme/icons/table-cells-unmerge.svg deleted file mode 100644 index e175008a..00000000 --- a/theme/icons/table-cells-unmerge.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/theme/icons/table-column.svg b/theme/icons/table-column.svg index 68849053..e119509d 100644 --- a/theme/icons/table-column.svg +++ b/theme/icons/table-column.svg @@ -1 +1 @@ - + \ No newline at end of file diff --git a/theme/icons/table-headers.svg b/theme/icons/table-headers.svg deleted file mode 100644 index c6f28bec..00000000 --- a/theme/icons/table-headers.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/theme/icons/table-merge-cell.svg b/theme/icons/table-merge-cell.svg index cf01b01b..debe4729 100644 --- a/theme/icons/table-merge-cell.svg +++ b/theme/icons/table-merge-cell.svg @@ -1 +1 @@ - \ No newline at end of file + \ No newline at end of file diff --git a/theme/icons/table-row.svg b/theme/icons/table-row.svg index 68995b95..3b83ec46 100644 --- a/theme/icons/table-row.svg +++ b/theme/icons/table-row.svg @@ -1 +1 @@ - + \ No newline at end of file diff --git a/theme/icons/table-split-cell.svg b/theme/icons/table-split-cell.svg deleted file mode 100644 index a31ea32d..00000000 --- a/theme/icons/table-split-cell.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/theme/icons/table.svg b/theme/icons/table.svg index 13b6cc45..7e89f07d 100644 --- a/theme/icons/table.svg +++ b/theme/icons/table.svg @@ -1 +1 @@ - + \ No newline at end of file From 8d952447de2e99c6e55fb54dab75bccace792675 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 30 May 2018 16:18:33 +0200 Subject: [PATCH 22/27] Tests: Small adjustments to table styles in the manual test. --- tests/manual/table.html | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/manual/table.html b/tests/manual/table.html index ef126a9d..b819b609 100644 --- a/tests/manual/table.html +++ b/tests/manual/table.html @@ -3,12 +3,12 @@ table { border-collapse: collapse; border-spacing: 0; - border-color: #000000; + border-color: hsl(0,0%,87%); } table, th, td { padding: 5px; - border: 1px inset #000000; + border: 1px inset hsl(0,0%,87%); } table th, @@ -17,13 +17,12 @@ } table th { - background: #dadada; - font-weight: normal; + background: hsl(0,0%,96%); + font-weight: bold; } table thead th { - color: #ffffff; - background: #666666; + background: hsl(0,0%,90%); } From 06083779a30113f641979a5110d039a26c40bebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Mon, 4 Jun 2018 14:48:20 +0200 Subject: [PATCH 23/27] Code style: Fixed some minor code style issues. --- src/commands/setheadercolumncommand.js | 24 ++++++++++++++----- src/commands/setheaderrowcommand.js | 32 ++++++++++++++++++-------- src/ui/inserttableview.js | 4 ++-- 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/src/commands/setheadercolumncommand.js b/src/commands/setheadercolumncommand.js index 3222f790..c1cd7ff6 100644 --- a/src/commands/setheadercolumncommand.js +++ b/src/commands/setheadercolumncommand.js @@ -28,9 +28,19 @@ export default class SetHeaderColumnCommand extends Command { const position = selection.getFirstPosition(); const tableParent = getParentTable( position ); - this.isEnabled = !!tableParent; - - this.value = this.isEnabled && this._isInHeading( position.parent, tableParent ); + const isInTable = !!tableParent; + + this.isEnabled = isInTable; + + /** + * Flag indicating whether the command is active. The command is active when the + * {@link module:engine/model/selection~Selection} is in a header column. + * + * @observable + * @readonly + * @member {Boolean} #value + */ + this.value = isInTable && this._isInHeading( position.parent, tableParent ); } /** @@ -49,12 +59,14 @@ export default class SetHeaderColumnCommand extends Command { const currentHeadingColumns = parseInt( table.getAttribute( 'headingColumns' ) || 0 ); - const { column } = tableUtils.getCellLocation( tableCell ); + let { column } = tableUtils.getCellLocation( tableCell ); - const columnsToSet = column + 1 !== currentHeadingColumns ? column + 1 : column; + if ( column + 1 !== currentHeadingColumns ) { + column++; + } model.change( writer => { - updateNumericAttribute( 'headingColumns', columnsToSet, table, writer, 0 ); + updateNumericAttribute( 'headingColumns', column, table, writer, 0 ); } ); } diff --git a/src/commands/setheaderrowcommand.js b/src/commands/setheaderrowcommand.js index d31b6290..b8cca4af 100644 --- a/src/commands/setheaderrowcommand.js +++ b/src/commands/setheaderrowcommand.js @@ -30,9 +30,19 @@ export default class SetHeaderRowCommand extends Command { const position = selection.getFirstPosition(); const tableParent = getParentTable( position ); - this.isEnabled = !!tableParent; - - this.value = this.isEnabled && this._isInHeading( position.parent, tableParent ); + const isInTable = !!tableParent; + + this.isEnabled = isInTable; + + /** + * Flag indicating whether the command is active. The command is active when the + * {@link module:engine/model/selection~Selection} is in a header row. + * + * @observable + * @readonly + * @member {Boolean} #value + */ + this.value = isInTable && this._isInHeading( position.parent, tableParent ); } /** @@ -49,22 +59,24 @@ export default class SetHeaderRowCommand extends Command { const table = tableRow.parent; const currentHeadingRows = table.getAttribute( 'headingRows' ) || 0; - const rowIndex = tableRow.index; + let rowIndex = tableRow.index; - const rowsToSet = rowIndex + 1 !== currentHeadingRows ? rowIndex + 1 : rowIndex; + if ( rowIndex + 1 !== currentHeadingRows ) { + rowIndex++; + } model.change( writer => { - if ( rowsToSet ) { - // Changing heading rows requires to check if any of a heading cell is overlaping vertically the table head. + if ( rowIndex ) { + // Changing heading rows requires to check if any of a heading cell is overlapping vertically the table head. // Any table cell that has a rowspan attribute > 1 will not exceed the table head so we need to fix it in rows below. - const cellsToSplit = getOverlappingCells( table, rowsToSet, currentHeadingRows ); + const cellsToSplit = getOverlappingCells( table, rowIndex, currentHeadingRows ); for ( const cell of cellsToSplit ) { - splitHorizontally( cell, rowsToSet, writer ); + splitHorizontally( cell, rowIndex, writer ); } } - updateNumericAttribute( 'headingRows', rowsToSet, table, writer, 0 ); + updateNumericAttribute( 'headingRows', rowIndex, table, writer, 0 ); } ); } diff --git a/src/ui/inserttableview.js b/src/ui/inserttableview.js index f9a417db..5df0bbbb 100644 --- a/src/ui/inserttableview.js +++ b/src/ui/inserttableview.js @@ -101,7 +101,7 @@ export default class InsertTableView extends View { // Listen to box view 'over' event which indicates that mouse is over this box. boxView.on( 'over', () => { // Translate box index to the row & column index. - const row = parseInt( index / 10 ); + const row = Math.floor( index / 10 ); const column = index % 10; // As row & column indexes are zero-based transform it to number of selected rows & columns. @@ -132,7 +132,7 @@ export default class InsertTableView extends View { this.items.map( ( boxView, index ) => { // Translate box index to the row & column index. - const itemRow = parseInt( index / 10 ); + const itemRow = Math.floor( index / 10 ); const itemColumn = index % 10; // Grid box is highlighted when its row & column index belongs to selected number of rows & columns. From 4d7e2c71ec6d49113916350022cefbb389bca571 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Mon, 4 Jun 2018 15:10:43 +0200 Subject: [PATCH 24/27] Tests: Update table manual test with description and minor fixes. --- tests/manual/table.html | 4 +-- tests/manual/table.js | 2 +- tests/manual/table.md | 64 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 3 deletions(-) diff --git a/tests/manual/table.html b/tests/manual/table.html index b819b609..aec8eb00 100644 --- a/tests/manual/table.html +++ b/tests/manual/table.html @@ -29,7 +29,7 @@

Table with everything:

- + @@ -157,7 +157,7 @@ - +
Data about the planets of our solar system (Planetary facts taken from Nasa's Planetary Fact Sheet - Metric.Data about the planets of our solar system (Planetary facts taken from Nasa's Planetary Fact Sheet - Metric.
 5906.4 -225 5Declassified as a planet in 2006, but this remains controversial.Declassified as a planet in 2006, but this remains controversial.
diff --git a/tests/manual/table.js b/tests/manual/table.js index 0ffe84a8..7de94d0e 100644 --- a/tests/manual/table.js +++ b/tests/manual/table.js @@ -17,7 +17,7 @@ ClassicEditor 'heading', '|', 'insertTable', '|', 'bold', 'italic', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' ], table: { - toolbar: [ 'tableColumn', 'tableRow', 'mergeCell', 'splitCell' ] + toolbar: [ 'tableColumn', 'tableRow', 'mergeCell' ] } } ) .then( editor => { diff --git a/tests/manual/table.md b/tests/manual/table.md index 84a3f322..3ef738d6 100644 --- a/tests/manual/table.md +++ b/tests/manual/table.md @@ -1,3 +1,67 @@ ### Loading +1. The data should be loaded with: + * a complex table with: + - one heading row, + - two heading columns, + - merged cells in heading columns section, + * a table with 2 tbody sections in the DOM - should be rendered as a table with one tbody. + * a table with no tbody sections in the DOM - should be rendered as a table with one tbody. + * a table with a thead section between two tbody sections in dom - should be rendered as a table with one thead and on tbody section in proper order: 1, 2, 3. + +2. Main toolbar should have insert table dropdown. + +3. While the table widget is selected there should be a toolbar attached to the table with 3 dropdowns: + * column dropdown with items: + - header column, + - insert column before, + - insert column after, + - delete column. + * row dropdown with items: + - header row, + - insert row below, + - insert row above, + - delete row. + * merge cell dropdown with items: + - merge cell up, + - merge cell right, + - merge cell down, + - merge cell left, + - split cell vertically, + - split cell horizontally, + ### Testing + +Inserting table: + +1. Insert table of chosen size - the inserted table should have number columns & rows as selected in dropdown. +2. Re-opening dropdown should reset selected table size. +3. Table cannot be inserted into other table. + +Column manipulation: + +1. Insert column in table heading section. +2. Insert column in merged cell. +3. Insert column at the end/beginning of a table. +4. Remove column from table heading section. +5. Remove column of merged cell. +6. Change column heading status. + +Column manipulation: + +1. Insert row in table heading section. +2. Insert row in merged cell. +3. Insert row at the end/beginning of a table. +4. Remove row from table heading section. +5. Remove row of merged cell. +6. Change row heading status. + +Merging cells: + +1. Merge cell on the left/right/top/bottom of current cell. +2. Merge cell on the left/right/top/bottom touching another table section (mergin a table cell from header row with a cell from body should not be possible). +3. Merge cells that are already merged. + +Splitting cells: +1. Split not merged cell vertically/horizontally. +2. Split already merged cell vertically/horizontally. From a50de970485388fab64af03db85988668795dfcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Mon, 4 Jun 2018 17:09:25 +0200 Subject: [PATCH 25/27] Docs: Update table documentation. --- src/commands/insertcolumncommand.js | 6 +++++- src/commands/insertrowcommand.js | 6 +++++- src/commands/inserttablecommand.js | 9 ++++++++- src/commands/mergecellcommand.js | 6 +++++- src/commands/removecolumncommand.js | 2 +- src/commands/setheadercolumncommand.js | 10 ++++++++-- src/commands/setheaderrowcommand.js | 8 +++++++- src/table.js | 15 +++++++++++++++ src/tabletoolbar.js | 4 ++-- src/tableui.js | 2 +- src/ui/inserttableview.js | 4 +++- src/ui/utils.js | 2 +- src/utils.js | 4 ++-- 13 files changed, 63 insertions(+), 15 deletions(-) diff --git a/src/commands/insertcolumncommand.js b/src/commands/insertcolumncommand.js index c80a1e6b..1e4d33c1 100644 --- a/src/commands/insertcolumncommand.js +++ b/src/commands/insertcolumncommand.js @@ -49,7 +49,11 @@ export default class InsertColumnCommand extends Command { } /** - * @inheritDoc + * Executes the command. + * + * Depending on command's {@link #order} value it inserts a column `'before'` or `'after'` the column in which selection is set. + * + * @fires execute */ execute() { const editor = this.editor; diff --git a/src/commands/insertrowcommand.js b/src/commands/insertrowcommand.js index e939dc5b..40bbbeb7 100644 --- a/src/commands/insertrowcommand.js +++ b/src/commands/insertrowcommand.js @@ -49,7 +49,11 @@ export default class InsertRowCommand extends Command { } /** - * @inheritDoc + * Executes the command. + * + * Depending on command's {@link #order} value it inserts a row `'below'` or `'above'` the row in which selection is set. + * + * @fires execute */ execute() { const editor = this.editor; diff --git a/src/commands/inserttablecommand.js b/src/commands/inserttablecommand.js index c69d8ec4..d984b6e7 100644 --- a/src/commands/inserttablecommand.js +++ b/src/commands/inserttablecommand.js @@ -31,7 +31,14 @@ export default class InsertTableCommand extends Command { } /** - * @inheritDoc + * Executes the command. + * + * Inserts table of given rows and columns into the editor. + * + * @param {Object} options + * @param {Number} [options.rows=2] Number of rows to create in inserted table. + * @param {Number} [options.columns=2] Number of columns to create in inserted table. + * @fires execute */ execute( options = {} ) { const model = this.editor.model; diff --git a/src/commands/mergecellcommand.js b/src/commands/mergecellcommand.js index 032428e3..90f93583 100644 --- a/src/commands/mergecellcommand.js +++ b/src/commands/mergecellcommand.js @@ -59,7 +59,11 @@ export default class MergeCellCommand extends Command { } /** - * @inheritDoc + * Executes the command. + * + * Depending on command's {@link #direction} value it will merge a cell that is to the `'left'`, `'right'`, `'up'` or `'down'`. + * + * @fires execute */ execute() { const model = this.editor.model; diff --git a/src/commands/removecolumncommand.js b/src/commands/removecolumncommand.js index 46f30012..ab01454a 100644 --- a/src/commands/removecolumncommand.js +++ b/src/commands/removecolumncommand.js @@ -14,7 +14,7 @@ import TableUtils from '../tableutils'; import { updateNumericAttribute } from './utils'; /** - * The split cell command. + * The remove column command. * * @extends module:core/command~Command */ diff --git a/src/commands/setheadercolumncommand.js b/src/commands/setheadercolumncommand.js index c1cd7ff6..c6664845 100644 --- a/src/commands/setheadercolumncommand.js +++ b/src/commands/setheadercolumncommand.js @@ -44,7 +44,13 @@ export default class SetHeaderColumnCommand extends Command { } /** - * @inheritDoc + * Executes the command. + * + * When the selection is non-header column, the command will set `headingColumns` table's attribute to cover that column. + * + * When selection is already in a header column then it will set `headingColumns` so the heading section will end before that column. + * + * @fires execute */ execute() { const model = this.editor.model; @@ -71,7 +77,7 @@ export default class SetHeaderColumnCommand extends Command { } /** - * Checks if table cell is in heading section. + * Checks if a table cell is in the heading section. * * @param {module:engine/model/element~Element} tableCell * @param {module:engine/model/element~Element} table diff --git a/src/commands/setheaderrowcommand.js b/src/commands/setheaderrowcommand.js index b8cca4af..e8ec6be2 100644 --- a/src/commands/setheaderrowcommand.js +++ b/src/commands/setheaderrowcommand.js @@ -46,7 +46,13 @@ export default class SetHeaderRowCommand extends Command { } /** - * @inheritDoc + * Executes the command. + * + * When the selection is non-header row, the command will set `headingRows` table's attribute to cover that row. + * + * When selection is already in a header row then it will set `headingRows` so the heading section will end before that row. + * + * @fires execute */ execute() { const model = this.editor.model; diff --git a/src/table.js b/src/table.js index 83f11331..09e9ddcb 100644 --- a/src/table.js +++ b/src/table.js @@ -33,3 +33,18 @@ export default class Table extends Plugin { return 'Table'; } } + +/** + * The configuration of the table features. Used by the table features in the `@ckeditor/ckeditor5-table` package. + * + * ClassicEditor + * .create( editorElement, { + * table: ... // Table feature options. + * } ) + * .then( ... ) + * .catch( ... ); + * + * See {@link module:core/editor/editorconfig~EditorConfig all editor options}. + * + * @interface TableConfig + */ diff --git a/src/tabletoolbar.js b/src/tabletoolbar.js index 27f947a3..c9cb73a2 100644 --- a/src/tabletoolbar.js +++ b/src/tabletoolbar.js @@ -16,7 +16,7 @@ import { repositionContextualBalloon, getBalloonPositionData } from './ui/utils' const balloonClassName = 'ck-toolbar-container'; /** - * The table toolbar class. Creates an table toolbar that shows up when the table widget is selected. + * The table toolbar class. Creates a table toolbar that shows up when the table widget is selected. * * Toolbar components are created using the editor {@link module:ui/componentfactory~ComponentFactory ComponentFactory} * based on the {@link module:core/editor/editor~Editor#config configuration} stored under `table.toolbar`. @@ -171,7 +171,7 @@ export default class TableToolbar extends Plugin { * Items to be placed in the table toolbar. * This option is used by the {@link module:table/tabletoolbar~TableToolbar} feature. * - * Assuming that you use the {@link module:table/tableui~TableUI} feature below toolbar items will be available + * Assuming that you use the {@link module:table/tableui~TableUI} feature the following toolbar items will be available * in {@link module:ui/componentfactory~ComponentFactory}: * * * `'tableRow'`, diff --git a/src/tableui.js b/src/tableui.js index 71566fc0..3940f2e2 100644 --- a/src/tableui.js +++ b/src/tableui.js @@ -27,7 +27,7 @@ import tableMergeCellIcon from './../theme/icons/table-merge-cell.svg'; * * The `'tableRow'` dropdown, * * The `'mergeCell'` dropdown. * - * The `'tableColumn'`, `'tableRow'`, `'mergeCell'` are best to be used in conjunction with {@link module:table/tabletoolbar~TableToolbar}. + * The `'tableColumn'`, `'tableRow'`, `'mergeCell'` work best with {@link module:table/tabletoolbar~TableToolbar}. * * @extends module:core/plugin~Plugin */ diff --git a/src/ui/inserttableview.js b/src/ui/inserttableview.js index 5df0bbbb..7661bcf9 100644 --- a/src/ui/inserttableview.js +++ b/src/ui/inserttableview.js @@ -144,7 +144,9 @@ export default class InsertTableView extends View { } /** - * Single grid box view element. + * A single grid box view element. + * + * This class is used to render table size selection grid in {@link module:table/ui/inserttableview~InsertTableView} * * @private */ diff --git a/src/ui/utils.js b/src/ui/utils.js index 82bad1a2..0655b3b7 100644 --- a/src/ui/utils.js +++ b/src/ui/utils.js @@ -24,7 +24,7 @@ export function repositionContextualBalloon( editor ) { } /** - * Returns the positioning options that control the geometry of the + * Returns the positioning options that controls the geometry of the * {@link module:ui/panel/balloon/contextualballoon~ContextualBalloon contextual balloon} with respect * to the selected element in the editor content. * diff --git a/src/utils.js b/src/utils.js index da09883a..8b030461 100644 --- a/src/utils.js +++ b/src/utils.js @@ -29,7 +29,7 @@ export function toTableWidget( viewElement, writer ) { } /** - * Checks if a given view element is an table widget. + * Checks if a given view element is a table widget. * * @param {module:engine/view/element~Element} viewElement * @returns {Boolean} @@ -39,7 +39,7 @@ export function isTableWidget( viewElement ) { } /** - * Checks if an table widget is the only selected element. + * Checks if a table widget is the only selected element. * * @param {module:engine/view/selection~Selection|module:engine/view/documentselection~DocumentSelection} selection * @returns {Boolean} From 7e03fe500e151b554d4cda52d45026318cecba03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Mon, 4 Jun 2018 17:15:23 +0200 Subject: [PATCH 26/27] Docs: Remove invalid link. --- src/tabletoolbar.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tabletoolbar.js b/src/tabletoolbar.js index c9cb73a2..f85b4067 100644 --- a/src/tabletoolbar.js +++ b/src/tabletoolbar.js @@ -23,8 +23,6 @@ const balloonClassName = 'ck-toolbar-container'; * * The toolbar uses the {@link module:ui/panel/balloon/contextualballoon~ContextualBalloon}. * - * For a detailed overview, check the {@glink features/table#table-contextual-toolbar table contextual toolbar} documentation. - * * @extends module:core/plugin~Plugin */ export default class TableToolbar extends Plugin { From c887db893e410a31886357de3c9e668dc1ffdbd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Mon, 4 Jun 2018 17:19:51 +0200 Subject: [PATCH 27/27] Docs: fixed typo. --- src/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.js b/src/utils.js index 8b030461..c0d22081 100644 --- a/src/utils.js +++ b/src/utils.js @@ -13,7 +13,7 @@ import { getParentTable } from './commands/utils'; const tableSymbol = Symbol( 'isImage' ); /** - * Converts a given {@link module:engine/view/element~Element} to an table widget: + * Converts a given {@link module:engine/view/element~Element} to a table widget: * * Adds a {@link module:engine/view/element~Element#_setCustomProperty custom property} allowing to recognize the table widget element. * * Calls the {@link module:widget/utils~toWidget} function with the proper element's label creator. *