浏览代码

Merge pull request #75 from ckeditor/t/ckeditor5-ui/402

Feature: Used the switch button to toggle table headers  (see ckeditor/ckeditor5-ui#402).

Also:
* Aligned the `TableUI` to the new API of the `addListToDropdown` helper,
* Updated the tests to consider the `ListItemView` as simply a container for buttons.
Maciej 7 年之前
父节点
当前提交
ece32d41b9
共有 2 个文件被更改,包括 178 次插入91 次删除
  1. 122 48
      packages/ckeditor5-table/src/tableui.js
  2. 56 43
      packages/ckeditor5-table/tests/tableui.js

+ 122 - 48
packages/ckeditor5-table/src/tableui.js

@@ -74,11 +74,36 @@ export default class TableUI extends Plugin {
 
 		editor.ui.componentFactory.add( 'tableColumn', locale => {
 			const options = [
-				{ commandName: 'setTableColumnHeader', label: t( 'Header column' ), bindIsActive: true },
-				'|',
-				{ commandName: 'insertTableColumnBefore', label: t( 'Insert column before' ) },
-				{ commandName: 'insertTableColumnAfter', label: t( 'Insert column after' ) },
-				{ commandName: 'removeTableColumn', label: t( 'Delete column' ) }
+				{
+					type: 'switchbutton',
+					model: {
+						commandName: 'setTableColumnHeader',
+						label: t( 'Header column' ),
+						bindIsOn: true
+					}
+				},
+				{ type: 'separator' },
+				{
+					type: 'button',
+					model: {
+						commandName: 'insertTableColumnBefore',
+						label: t( 'Insert column before' )
+					}
+				},
+				{
+					type: 'button',
+					model: {
+						commandName: 'insertTableColumnAfter',
+						label: t( 'Insert column after' )
+					}
+				},
+				{
+					type: 'button',
+					model: {
+						commandName: 'removeTableColumn',
+						label: t( 'Delete column' )
+					}
+				}
 			];
 
 			return this._prepareDropdown( t( 'Column' ), tableColumnIcon, options, locale );
@@ -86,11 +111,36 @@ export default class TableUI extends Plugin {
 
 		editor.ui.componentFactory.add( 'tableRow', locale => {
 			const options = [
-				{ commandName: 'setTableRowHeader', label: t( 'Header row' ), bindIsActive: true },
-				'|',
-				{ commandName: 'insertTableRowBelow', label: t( 'Insert row below' ) },
-				{ commandName: 'insertTableRowAbove', label: t( 'Insert row above' ) },
-				{ commandName: 'removeTableRow', label: t( 'Delete row' ) }
+				{
+					type: 'switchbutton',
+					model: {
+						commandName: 'setTableRowHeader',
+						label: t( 'Header row' ),
+						bindIsOn: true
+					}
+				},
+				{ type: 'separator' },
+				{
+					type: 'button',
+					model: {
+						commandName: 'insertTableRowBelow',
+						label: t( 'Insert row below' )
+					}
+				},
+				{
+					type: 'button',
+					model: {
+						commandName: 'insertTableRowAbove',
+						label: t( 'Insert row above' )
+					}
+				},
+				{
+					type: 'button',
+					model: {
+						commandName: 'removeTableRow',
+						label: t( 'Delete row' )
+					}
+				}
 			];
 
 			return this._prepareDropdown( t( 'Row' ), tableRowIcon, options, locale );
@@ -98,13 +148,49 @@ export default class TableUI extends Plugin {
 
 		editor.ui.componentFactory.add( 'mergeTableCells', locale => {
 			const options = [
-				{ commandName: 'mergeTableCellUp', label: t( 'Merge cell up' ) },
-				{ commandName: 'mergeTableCellRight', label: t( 'Merge cell right' ) },
-				{ commandName: 'mergeTableCellDown', label: t( 'Merge cell down' ) },
-				{ commandName: 'mergeTableCellLeft', label: t( 'Merge cell left' ) },
-				'|',
-				{ commandName: 'splitTableCellVertically', label: t( 'Split cell vertically' ) },
-				{ commandName: 'splitTableCellHorizontally', label: t( 'Split cell horizontally' ) }
+				{
+					type: 'button',
+					model: {
+						commandName: 'mergeTableCellUp',
+						label: t( 'Merge cell up' )
+					}
+				},
+				{
+					type: 'button',
+					model: {
+						commandName: 'mergeTableCellRight',
+						label: t( 'Merge cell right' )
+					}
+				},
+				{
+					type: 'button',
+					model: {
+						commandName: 'mergeTableCellDown',
+						label: t( 'Merge cell down' )
+					}
+				},
+				{
+					type: 'button',
+					model: {
+						commandName: 'mergeTableCellLeft',
+						label: t( 'Merge cell left' )
+					}
+				},
+				{ type: 'separator' },
+				{
+					type: 'button',
+					model: {
+						commandName: 'splitTableCellVertically',
+						label: t( 'Split cell vertically' )
+					}
+				},
+				{
+					type: 'button',
+					model: {
+						commandName: 'splitTableCellHorizontally',
+						label: t( 'Split cell horizontally' )
+					}
+				}
 			];
 
 			return this._prepareDropdown( t( 'Merge cells' ), tableMergeCellIcon, options, locale );
@@ -117,7 +203,7 @@ export default class TableUI extends Plugin {
 	 * @private
 	 * @param {String} label The dropdown button label.
 	 * @param {String} icon An icon for the dropdown button.
-	 * @param {Array.<module:table/tableui~DropdownOption>} options The list of options for the dropdown.
+	 * @param {Array.<module:ui/dropdown/utils~ListDropdownItemDefinition>} options The list of options for the dropdown.
 	 * @param {module:utils/locale~Locale} locale
 	 * @returns {module:ui/dropdown/dropdownview~DropdownView}
 	 */
@@ -128,13 +214,13 @@ export default class TableUI extends Plugin {
 		const commands = [];
 
 		// Prepare dropdown list items for list dropdown.
-		const dropdownItems = new Collection();
+		const itemDefinitions = new Collection();
 
 		for ( const option of options ) {
-			addListOption( option, editor, commands, dropdownItems );
+			addListOption( option, editor, commands, itemDefinitions );
 		}
 
-		addListToDropdown( dropdownView, dropdownItems );
+		addListToDropdown( dropdownView, itemDefinitions );
 
 		// Decorate dropdown's button.
 		dropdownView.buttonView.set( {
@@ -162,41 +248,29 @@ export default class TableUI extends Plugin {
 // @param {module:table/tableui~DropdownOption} option Configuration option.
 // @param {module:core/editor/editor~Editor} editor
 // @param {Array.<module:core/command~Command>} 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 itemModel = new Model();
+// @param {Iterable.<module:ui/dropdown/utils~ListDropdownItemDefinition>} itemDefinitions
+// Collection of dropdown items to update with given option.
+function addListOption( option, editor, commands, itemDefinitions ) {
+	const model = option.model = new Model( option.model );
+	const { commandName, bindIsOn } = option.model;
 
-	if ( option === '|' ) {
-		itemModel.set( {
-			isSeparator: true
-		} );
-	} else {
-		const { commandName, label, bindIsActive } = option;
+	if ( option.type !== 'separator' ) {
 		const command = editor.commands.get( commandName );
 
 		commands.push( command );
 
-		itemModel.set( {
-			commandName,
-			label
-		} );
+		model.set( { commandName } );
 
-		itemModel.bind( 'isEnabled' ).to( command );
+		model.bind( 'isEnabled' ).to( command );
 
-		if ( bindIsActive ) {
-			itemModel.bind( 'isActive' ).to( command, 'value' );
+		if ( bindIsOn ) {
+			model.bind( 'isOn' ).to( command, 'value' );
 		}
 	}
 
-	dropdownItems.add( itemModel );
-}
+	model.set( {
+		withText: true
+	} );
 
-/**
- * An object describing the table dropdown 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`, it will bind the command's value to the `isActive` dropdown item property.
- */
+	itemDefinitions.add( option );
+}

+ 56 - 43
packages/ckeditor5-table/tests/tableui.js

@@ -11,6 +11,7 @@ import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 
 import TableEditing from '../src/tableediting';
 import TableUI from '../src/tableui';
+import SwitchButtonView from '@ckeditor/ckeditor5-ui/src/button/switchbuttonview';
 import DropdownView from '@ckeditor/ckeditor5-ui/src/dropdown/dropdownview';
 import ListSeparatorView from '@ckeditor/ckeditor5-ui/src/list/listseparatorview';
 
@@ -74,7 +75,7 @@ describe( 'TableUI', () => {
 		it( 'should execute insertTable command on button execute event', () => {
 			const executeSpy = testUtils.sinon.spy( editor, 'execute' );
 
-			const tableSizeView = insertTable.panelView.children.get( 0 );
+			const tableSizeView = insertTable.panelView.children.first;
 
 			tableSizeView.rows = 2;
 			tableSizeView.columns = 7;
@@ -86,7 +87,7 @@ describe( 'TableUI', () => {
 		} );
 
 		it( 'should reset rows & columns on dropdown open', () => {
-			const tableSizeView = insertTable.panelView.children.get( 0 );
+			const tableSizeView = insertTable.panelView.children.first;
 
 			expect( tableSizeView.rows ).to.equal( 0 );
 			expect( tableSizeView.columns ).to.equal( 0 );
@@ -122,7 +123,7 @@ describe( 'TableUI', () => {
 		it( 'should have proper items in panel', () => {
 			const listView = dropdown.listView;
 
-			const labels = listView.items.map( item => item instanceof ListSeparatorView ? '|' : item.label );
+			const labels = listView.items.map( item => item instanceof ListSeparatorView ? '|' : item.children.first.label );
 
 			expect( labels ).to.deep.equal( [ 'Header row', '|', 'Insert row below', 'Insert row above', 'Delete row' ] );
 		} );
@@ -140,36 +141,36 @@ describe( 'TableUI', () => {
 			insertRowAboveCommand.isEnabled = true;
 			removeRowCommand.isEnabled = true;
 
-			expect( items.get( 0 ).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.first.children.first.isEnabled ).to.be.true;
+			expect( items.get( 2 ).children.first.isEnabled ).to.be.true;
+			expect( items.get( 3 ).children.first.isEnabled ).to.be.true;
+			expect( items.get( 4 ).children.first.isEnabled ).to.be.true;
 			expect( dropdown.buttonView.isEnabled ).to.be.true;
 
 			setRowHeaderCommand.isEnabled = false;
 
-			expect( items.get( 0 ).isEnabled ).to.be.false;
+			expect( items.first.children.first.isEnabled ).to.be.false;
 			expect( dropdown.buttonView.isEnabled ).to.be.true;
 
 			insertRowBelowCommand.isEnabled = false;
 
-			expect( items.get( 2 ).isEnabled ).to.be.false;
+			expect( items.get( 2 ).children.first.isEnabled ).to.be.false;
 			expect( dropdown.buttonView.isEnabled ).to.be.true;
 
 			insertRowAboveCommand.isEnabled = false;
-			expect( items.get( 3 ).isEnabled ).to.be.false;
+			expect( items.get( 3 ).children.first.isEnabled ).to.be.false;
 			expect( dropdown.buttonView.isEnabled ).to.be.true;
 
 			removeRowCommand.isEnabled = false;
 
-			expect( items.get( 4 ).isEnabled ).to.be.false;
+			expect( items.get( 4 ).children.first.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' );
+			dropdown.listView.items.first.children.first.fire( 'execute' );
 
 			sinon.assert.calledOnce( focusSpy );
 		} );
@@ -177,22 +178,28 @@ describe( 'TableUI', () => {
 		it( 'executes command when it\'s executed', () => {
 			const spy = sinon.stub( editor, 'execute' );
 
-			dropdown.listView.items.get( 0 ).fire( 'execute' );
+			dropdown.listView.items.first.children.first.fire( 'execute' );
 
 			expect( spy.calledOnce ).to.be.true;
 			expect( spy.args[ 0 ][ 0 ] ).to.equal( 'setTableRowHeader' );
 		} );
 
+		it( 'should use a toggle switch for the setTableRowHeader item', () => {
+			const items = dropdown.listView.items;
+
+			expect( items.first.children.first ).to.be.instanceOf( SwitchButtonView );
+		} );
+
 		it( 'should bind set header row command value to dropdown item', () => {
 			const items = dropdown.listView.items;
 
 			const setRowHeaderCommand = editor.commands.get( 'setTableRowHeader' );
 
 			setRowHeaderCommand.value = false;
-			expect( items.get( 0 ).isActive ).to.be.false;
+			expect( items.first.children.first.isOn ).to.be.false;
 
 			setRowHeaderCommand.value = true;
-			expect( items.get( 0 ).isActive ).to.be.true;
+			expect( items.first.children.first.isOn ).to.be.true;
 		} );
 	} );
 
@@ -217,7 +224,7 @@ describe( 'TableUI', () => {
 		it( 'should have proper items in panel', () => {
 			const listView = dropdown.listView;
 
-			const labels = listView.items.map( item => item instanceof ListSeparatorView ? '|' : item.label );
+			const labels = listView.items.map( item => item instanceof ListSeparatorView ? '|' : item.children.first.label );
 
 			expect( labels ).to.deep.equal( [ 'Header column', '|', 'Insert column before', 'Insert column after', 'Delete column' ] );
 		} );
@@ -235,34 +242,34 @@ describe( 'TableUI', () => {
 			insertColumnAfterCommand.isEnabled = true;
 			removeColumnCommand.isEnabled = true;
 
-			expect( items.get( 0 ).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.first.children.first.isEnabled ).to.be.true;
+			expect( items.get( 2 ).children.first.isEnabled ).to.be.true;
+			expect( items.get( 3 ).children.first.isEnabled ).to.be.true;
+			expect( items.get( 4 ).children.first.isEnabled ).to.be.true;
 			expect( dropdown.buttonView.isEnabled ).to.be.true;
 
 			setColumnHeaderCommand.isEnabled = false;
 
-			expect( items.get( 0 ).isEnabled ).to.be.false;
+			expect( items.first.children.first.isEnabled ).to.be.false;
 			expect( dropdown.buttonView.isEnabled ).to.be.true;
 
 			insertColumnBeforeCommand.isEnabled = false;
 
-			expect( items.get( 2 ).isEnabled ).to.be.false;
+			expect( items.get( 2 ).children.first.isEnabled ).to.be.false;
 			expect( dropdown.buttonView.isEnabled ).to.be.true;
 
 			insertColumnAfterCommand.isEnabled = false;
-			expect( items.get( 3 ).isEnabled ).to.be.false;
+			expect( items.get( 3 ).children.first.isEnabled ).to.be.false;
 
 			removeColumnCommand.isEnabled = false;
-			expect( items.get( 4 ).isEnabled ).to.be.false;
+			expect( items.get( 4 ).children.first.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' );
+			dropdown.listView.items.first.children.first.fire( 'execute' );
 
 			sinon.assert.calledOnce( focusSpy );
 		} );
@@ -270,22 +277,28 @@ describe( 'TableUI', () => {
 		it( 'executes command when it\'s executed', () => {
 			const spy = sinon.stub( editor, 'execute' );
 
-			dropdown.listView.items.get( 0 ).fire( 'execute' );
+			dropdown.listView.items.first.children.first.fire( 'execute' );
 
 			expect( spy.calledOnce ).to.be.true;
 			expect( spy.args[ 0 ][ 0 ] ).to.equal( 'setTableColumnHeader' );
 		} );
 
+		it( 'should use a toggle switch for the setTableColumnHeader item', () => {
+			const items = dropdown.listView.items;
+
+			expect( items.first.children.first ).to.be.instanceOf( SwitchButtonView );
+		} );
+
 		it( 'should bind set header column command value to dropdown item', () => {
 			const items = dropdown.listView.items;
 
 			const setColumnHeaderCommand = editor.commands.get( 'setTableColumnHeader' );
 
 			setColumnHeaderCommand.value = false;
-			expect( items.get( 0 ).isActive ).to.be.false;
+			expect( items.first.children.first.isOn ).to.be.false;
 
 			setColumnHeaderCommand.value = true;
-			expect( items.get( 0 ).isActive ).to.be.true;
+			expect( items.first.children.first.isOn ).to.be.true;
 		} );
 	} );
 
@@ -310,7 +323,7 @@ describe( 'TableUI', () => {
 		it( 'should have proper items in panel', () => {
 			const listView = dropdown.listView;
 
-			const labels = listView.items.map( item => item instanceof ListSeparatorView ? '|' : item.label );
+			const labels = listView.items.map( item => item instanceof ListSeparatorView ? '|' : item.children.first.label );
 
 			expect( labels ).to.deep.equal( [
 				'Merge cell up',
@@ -340,35 +353,35 @@ describe( 'TableUI', () => {
 			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( 5 ).isEnabled ).to.be.true;
-			expect( items.get( 6 ).isEnabled ).to.be.true;
+			expect( items.first.children.first.isEnabled ).to.be.true;
+			expect( items.get( 1 ).children.first.isEnabled ).to.be.true;
+			expect( items.get( 2 ).children.first.isEnabled ).to.be.true;
+			expect( items.get( 3 ).children.first.isEnabled ).to.be.true;
+			expect( items.get( 5 ).children.first.isEnabled ).to.be.true;
+			expect( items.get( 6 ).children.first.isEnabled ).to.be.true;
 			expect( dropdown.buttonView.isEnabled ).to.be.true;
 
 			mergeCellUpCommand.isEnabled = false;
 
-			expect( items.get( 0 ).isEnabled ).to.be.false;
+			expect( items.first.children.first.isEnabled ).to.be.false;
 			expect( dropdown.buttonView.isEnabled ).to.be.true;
 
 			mergeCellRightCommand.isEnabled = false;
 
-			expect( items.get( 1 ).isEnabled ).to.be.false;
+			expect( items.get( 1 ).children.first.isEnabled ).to.be.false;
 			expect( dropdown.buttonView.isEnabled ).to.be.true;
 
 			mergeCellDownCommand.isEnabled = false;
-			expect( items.get( 2 ).isEnabled ).to.be.false;
+			expect( items.get( 2 ).children.first.isEnabled ).to.be.false;
 
 			mergeCellLeftCommand.isEnabled = false;
-			expect( items.get( 3 ).isEnabled ).to.be.false;
+			expect( items.get( 3 ).children.first.isEnabled ).to.be.false;
 
 			splitCellVerticallyCommand.isEnabled = false;
-			expect( items.get( 5 ).isEnabled ).to.be.false;
+			expect( items.get( 5 ).children.first.isEnabled ).to.be.false;
 
 			splitCellHorizontallyCommand.isEnabled = false;
-			expect( items.get( 6 ).isEnabled ).to.be.false;
+			expect( items.get( 6 ).children.first.isEnabled ).to.be.false;
 
 			expect( dropdown.buttonView.isEnabled ).to.be.false;
 		} );
@@ -376,7 +389,7 @@ describe( 'TableUI', () => {
 		it( 'should focus view after command execution', () => {
 			const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' );
 
-			dropdown.listView.items.get( 0 ).fire( 'execute' );
+			dropdown.listView.items.first.children.first.fire( 'execute' );
 
 			sinon.assert.calledOnce( focusSpy );
 		} );
@@ -384,7 +397,7 @@ describe( 'TableUI', () => {
 		it( 'executes command when it\'s executed', () => {
 			const spy = sinon.stub( editor, 'execute' );
 
-			dropdown.listView.items.get( 0 ).fire( 'execute' );
+			dropdown.listView.items.first.children.first.fire( 'execute' );
 
 			expect( spy.calledOnce ).to.be.true;
 			expect( spy.args[ 0 ][ 0 ] ).to.equal( 'mergeTableCellUp' );