Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

T/1: Implement Table UI #23

Merged
merged 29 commits into from
Jun 5, 2018
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6fd5103
Added: Initial row & column dropdowns.
jodator May 10, 2018
c43ecb3
Added: Initial mergeCell dropdown.
jodator May 10, 2018
1026056
Tests: Fix TableUI tests.
jodator May 10, 2018
b21451f
Tests: Add missing tests for TableUI.
jodator May 10, 2018
e45ba7f
Added: Initial splitCell dropdown.
jodator May 10, 2018
9dde19b
Added: Initial TableToolbar implementation.
jodator May 15, 2018
b2919df
Added table icons: table, table-row, table-column, table-split-cell, …
oleq May 17, 2018
a8d9a5c
Changed: Use proper icons for buttons in UI.
jodator May 17, 2018
3d0ede9
Other: Refactored SetTableHeadersCommand to two SetHeaderColumnComman…
jodator May 25, 2018
9f8a1be
Changed: Bind dropdown item isActive to header commands.
jodator May 25, 2018
1404f22
Changed: Add insertTable dropdown.
jodator May 28, 2018
72c9709
Tests: Fix SetHeaderRowCommand.
jodator May 29, 2018
e797859
Other: Remove focus handling from InsertTableView.
jodator May 29, 2018
67244ca
Other: Update CSS classes to be more BEM & update color values.
jodator May 29, 2018
a5a3140
Docs: Update InsertTableView docs.
jodator May 29, 2018
5e6744b
Changed: Move splitCell dropdown items to mergeCell dropdown.
jodator May 29, 2018
710b8a6
Docs: Update ui/utils module docs.
jodator May 29, 2018
969347b
Docs: Update Table UI plugins docs.
jodator May 29, 2018
27e9222
Other: Add UI components label to translation service.
jodator May 29, 2018
91da813
T/1: Alternate icon set for table feature
dtwist May 30, 2018
dbb0a53
Merge branch 'dtwist-t/1' into t/1
oleq May 30, 2018
df93871
Removed unused icons. Refined row, column, table and merge icons.
oleq May 30, 2018
8d95244
Tests: Small adjustments to table styles in the manual test.
oleq May 30, 2018
0608377
Code style: Fixed some minor code style issues.
jodator Jun 4, 2018
4d7e2c7
Tests: Update table manual test with description and minor fixes.
jodator Jun 4, 2018
a50de97
Docs: Update table documentation.
jodator Jun 4, 2018
7e03fe5
Docs: Remove invalid link.
jodator Jun 4, 2018
c887db8
Docs: fixed typo.
jodator Jun 4, 2018
cf7f363
Merge branch 'master' into t/1
oleq Jun 5, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions src/commands/setheadercolumncommand.js
Original file line number Diff line number Diff line change
@@ -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 );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the value dependent on isEnabled? https://docs.ckeditor.com/ckeditor5/latest/api/module_core_command-Command.html#member-value does not mention that and I couldn't find another command which acts like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update this piece of code as it does not make sense to check if selection is in heading when there is no table.

}

/**
* @inheritDoc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure it does more than that. It must be explained (see: linkcommand.js)

*/
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;
Copy link
Member

@oleq oleq Jun 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ify it

let { column } = tableUtils.getCellLocation( tableCell );

if ( column !== currentHeadingColumns - 1 ) {
    column ++;
}


model.change( writer => {
updateNumericAttribute( 'headingColumns', columnsToSet, table, writer, 0 );
} );
}

/**
* Checks if table cell is in heading section.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a table cell
in the 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

/**
* @module table/commands/settableheaderscommand
* @module table/commands/setheaderrowcommand
*/

import Command from '@ckeditor/ckeditor5-core/src/command';
Expand All @@ -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
*/
Expand All @@ -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 );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment in SetHeaderColumnCommand.

}

/**
* @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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment in SetHeaderColumnCommand#execute.


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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/overlaping/overlapping

// 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 );
Expand All @@ -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.
Expand All @@ -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
Expand Down
5 changes: 3 additions & 2 deletions src/converters/downcast.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 );
Expand Down
8 changes: 5 additions & 3 deletions src/tableediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 ) );
Expand Down
Loading