Support for default table (and cells) properties #9393

pomek · 2021-03-31T09:40:57Z

Suggested merge commit message (convention)

Feature (table): The TablePropertiesEditing plugin allows specifying default properties for the new inserted tables. Closes #8502. Closes #9219.

Feature (table): The TableCellPropertiesEditing plugin allows specifying default properties of all cells when inserting the new table.

Other (table): The TableUtils#createTable() is now decorated and fires an event when executing the function. Read more about decorating object methods.

Other (table): The TableUtils#insertRows() and TableUtils#insertColumns() returns an array that contains the created model elements.

Mgsy · 2021-03-31T09:59:02Z

PR is ready for testing @ckeditor/qa-team.

FilipTokarski · 2021-03-31T13:41:00Z

Looks ok 👍

packages/ckeditor5-table/src/tablecellproperties.js

packages/ckeditor5-table/src/tableproperties.js

packages/ckeditor5-table/src/commands/inserttablecommand.js

packages/ckeditor5-table/src/commands/insertcolumncommand.js

packages/ckeditor5-table/src/tablecellproperties.js

packages/ckeditor5-table/src/tableproperties.js

packages/ckeditor5-table/src/tableutils.js

packages/ckeditor5-table/docs/features/table.md

jacekbogdanski

I'm getting some error when merging cells with default properties:

Screen.Recording.2021-04-06.at.14.42.19.mov

packages/ckeditor5-table/tests/_utils/utils.js

Co-authored-by: Jacek Bogdański <jacekbogd@gmail.com>

…rom table/cell PoV they are just "properties".

pomek · 2021-04-08T11:29:35Z

Today we discussed another approach to have default table/cell properties. Instead of modifying the current commands, let's try to decorate them and apply the properties.

pomek · 2021-04-08T12:28:39Z

Problem to solve: single undo step after creating a table with applied styles. As for now, we have three steps:

created table
applied table properties
applied cell properties

…ad of passing the attributes to TableUtils.

pomek · 2021-04-09T12:14:18Z

I'm getting some error when merging cells with default properties:

After redesigning the feature to decorate commands/plugins, the error does not occur anymore.

pomek · 2021-04-09T15:05:58Z

The PR is ready for review once again.

Now, the proposed solution uses decorated methods from the TableUtils plugin. Thanks to that, table commands do not have to know about table/cell properties. The TableProperties and TableCellProperties listen to proper events and modify the table before inserting it into the document.

Splitting a column does not inherit "the parent" properties but it should be resolved in another issue – #7246.

cc: @niegowski

niegowski

As commented below.

niegowski · 2021-04-12T12:13:39Z

packages/ckeditor5-table/src/commands/insertcolumncommand.js

@@ -78,6 +78,10 @@ export default class InsertColumnCommand extends Command {
 		const column = insertBefore ? columnIndexes.first : columnIndexes.last;
 		const table = affectedTableCells[ 0 ].findAncestor( 'table' );

-		tableUtils.insertColumns( table, { columns: 1, at: insertBefore ? column : column + 1 } );
+		// The `TableUtils` plugin fires the `#insertColumns` event. In order to having a single undo step,


Suggested change

// The `TableUtils` plugin fires the `#insertColumns` event. In order to having a single undo step,

// The `TableUtils` plugin fires the `#insertColumns` event. In order to have a single undo step,

niegowski · 2021-04-12T12:14:18Z

packages/ckeditor5-table/src/commands/insertrowcommand.js

@@ -77,6 +77,10 @@ export default class InsertRowCommand extends Command {
 		const row = insertAbove ? rowIndexes.first : rowIndexes.last;
 		const table = affectedTableCells[ 0 ].findAncestor( 'table' );

-		tableUtils.insertRows( table, { at: insertAbove ? row : row + 1, copyStructureFromAbove: !insertAbove } );
+		// The `TableUtils` plugin fires the `#insertRows` event. In order to having a single undo step,


Suggested change

// The `TableUtils` plugin fires the `#insertRows` event. In order to having a single undo step,

// The `TableUtils` plugin fires the `#insertRows` event. In order to have a single undo step,

niegowski · 2021-04-12T12:46:15Z

packages/ckeditor5-table/src/tableproperties/tablepropertiesediting.js

+		this.listenTo( tableUtils, 'createTable', ( evt, [ writer ] ) => {
+			const tableElement = evt.return;
+
+			writer.setAttributes( tableProperties, tableElement );
+		}, { priority: 'low' } );


Suggested change

this.listenTo( tableUtils, 'createTable', ( evt, [ writer ] ) => {

const tableElement = evt.return;

writer.setAttributes( tableProperties, tableElement );

}, { priority: 'low' } );

this.listenTo( tableUtils, 'createTable', evt => {

const tableElement = evt.return;

editor.model.change( writer => {

writer.setAttributes( tableProperties, tableElement );

} );

}, { priority: 'low' } );

Let's not use the writer from the event data.

niegowski · 2021-04-12T12:52:28Z

packages/ckeditor5-table/src/tablecellproperties/tablecellpropertiesediting.js

+
+		this._enableDefaultCellProperties();
+
+		injectTableCellDefaultPropertiesPostFixer( editor );


We should not pass the whole editor, we should pass only needed parameters, in this case, I would pass the model instance and the default cell attributes object.

niegowski · 2021-04-12T12:55:02Z

packages/ckeditor5-table/src/tablecellproperties/tablecellpropertiesediting.js

+		const tableUtils = editor.plugins.get( TableUtils );
+
+		// Apply default cell properties while creating a new table.
+		this.listenTo( tableUtils, 'createTable', ( evt, [ writer ] ) => {


Let's not use the writer from the event's data. We should have here a change block.

niegowski · 2021-04-12T16:45:53Z

packages/ckeditor5-table/tests/commands/insertrowcommand.js

@@ -14,14 +14,15 @@ import TableSelection from '../../src/tableselection';
 import { assertSelectedCells, modelTable } from '../_utils/utils';

 import InsertRowCommand from '../../src/commands/insertrowcommand';
+import { UndoEditing } from '@ckeditor/ckeditor5-undo';


Please fix the order of imports.

niegowski · 2021-04-12T16:46:06Z

packages/ckeditor5-table/tests/commands/insertrowcommand.js

@@ -308,6 +309,44 @@ describe( 'InsertRowCommand', () => {
 					[ '10', '11', '12' ]
 				] ) );
 			} );
+
+			it( 'should create a single undo step when inserting a column', () => {


Suggested change

it( 'should create a single undo step when inserting a column', () => {

it( 'should create a single undo step when inserting a row', () => {

niegowski · 2021-04-12T16:46:38Z

packages/ckeditor5-table/tests/converters/table-cell-default-properties-post-fixer.js

+import UndoEditing from '@ckeditor/ckeditor5-undo/src/undoediting';
+import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';


Please fix the order of imports.

niegowski · 2021-04-12T16:48:02Z

packages/ckeditor5-table/tests/tablecellproperties/tablecellpropertiesediting.js

+import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
-import { assertTableCellStyle, assertTRBLAttribute } from '../_utils/utils';
+import { assertTableCellStyle, assertTRBLAttribute, modelTable } from '../_utils/utils';
+import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor';
+import TableUtils from '../../src/tableutils';


Please fix the order of imports.

niegowski · 2021-04-12T16:55:38Z

packages/ckeditor5-table/tests/tableproperties/tablepropertiesediting.js

 import { setData as setModelData, getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
-import { assertTableStyle, assertTRBLAttribute } from '../_utils/utils';
+import { assertTableStyle, assertTRBLAttribute, modelTable } from '../_utils/utils';
+import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor';
+import TableUtils from '../../src/tableutils';


Please fix the order of imports.

pomek · 2021-04-21T17:12:01Z

See: #8502 (comment).

pomek · 2021-04-30T06:25:36Z

Closing in favor of #9555.

pomek added 13 commits March 26, 2021 11:55

An initial implementation of the default styles for the table/cell.

31041d1

Added tests for default table/cell properties.

445f60b

API Docs: Added default table/cell properties to docs.

24b747a

InsertColumnCommand: Support for TableCellProperties.

801dd24

InsertRowCommand: Support for TableCellProperties.

9aa0ad2

SplitCellCommand: Support for TableCellProperties.

1ddab6b

Support for default table properties when splitting a cell.

efa3e19

The table layout post-fixer uses the default cell properties.

149858d

Restored .travis.yml

3607d42

Updated the table properties manual tests (included default props).

072100d

Docs for the default table/cell properties.

eddf6af

Removed default values for attributes/options.

fa5f455

Merge branch 'master' into i/8502

0166366

pomek commented Mar 31, 2021

View reviewed changes

packages/ckeditor5-table/src/tablecellproperties.js Outdated Show resolved Hide resolved

packages/ckeditor5-table/src/tableproperties.js Outdated Show resolved Hide resolved

Fixed tabs in API docs.

912cb06

jacekbogdanski self-requested a review April 1, 2021 10:42

jacekbogdanski reviewed Apr 6, 2021

View reviewed changes

jacekbogdanski requested changes Apr 6, 2021

View reviewed changes

packages/ckeditor5-table/tests/_utils/utils.js Outdated Show resolved Hide resolved

pomek and others added 5 commits April 8, 2021 09:07

Merge branch 'master' into i/8502

5ad7484

Added the default table/cell properties to API docs.

0a1201d

Apply suggestions from code review

e245caf

Co-authored-by: Jacek Bogdański <jacekbogd@gmail.com>

Removed the "default" word from tableProperties and cellProperties. F…

c7b23f2

…rom table/cell PoV they are just "properties".

Extended available options/attributes for the modelTable() util.

88046d8

pomek added 3 commits April 9, 2021 14:02

Table and cells default properties decorate proper commands now inste…

bab6ca2

…ad of passing the attributes to TableUtils.

Reverted InsertTableCommand.

a1bf418

Reverted SplitCellCommand.

f3292f0

pomek added 2 commits April 9, 2021 14:19

Clean the code.

b00eba2

Added the table cell default properties post fixer.

7def432

pomek requested a review from niegowski April 9, 2021 14:59

pomek mentioned this pull request Apr 9, 2021

Should splitting cell copy all of its attributes? #7246

Closed

Final improvements.

6ae94d8

kuku711 mentioned this pull request Apr 11, 2021

[package - table]Option for default initial style (Solution provided) #9449

Closed

Simplified post-fixer.

977b69e

niegowski reviewed Apr 12, 2021

View reviewed changes

pomek mentioned this pull request Apr 21, 2021

No way to set default alignment for images or tables. #8502

Closed

pomek closed this Apr 30, 2021

pomek deleted the i/8502 branch April 30, 2021 06:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for default table (and cells) properties #9393

Support for default table (and cells) properties #9393

pomek commented Mar 31, 2021 •

edited

Loading

Mgsy commented Mar 31, 2021

FilipTokarski commented Mar 31, 2021

jacekbogdanski left a comment

pomek commented Apr 8, 2021

pomek commented Apr 8, 2021

pomek commented Apr 9, 2021

pomek commented Apr 9, 2021

niegowski left a comment

niegowski Apr 12, 2021

niegowski Apr 12, 2021

niegowski Apr 12, 2021

niegowski Apr 12, 2021

niegowski Apr 12, 2021

niegowski Apr 12, 2021

niegowski Apr 12, 2021

niegowski Apr 12, 2021

niegowski Apr 12, 2021

niegowski Apr 12, 2021

pomek commented Apr 21, 2021

pomek commented Apr 30, 2021

	// The `TableUtils` plugin fires the `#insertColumns` event. In order to having a single undo step,
	// The `TableUtils` plugin fires the `#insertColumns` event. In order to have a single undo step,


		this._enableDefaultCellProperties();

		injectTableCellDefaultPropertiesPostFixer( editor );

	it( 'should create a single undo step when inserting a column', () => {
	it( 'should create a single undo step when inserting a row', () => {

		import UndoEditing from '@ckeditor/ckeditor5-undo/src/undoediting';
		import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';

Support for default table (and cells) properties #9393

Support for default table (and cells) properties #9393

Conversation

pomek commented Mar 31, 2021 • edited Loading

Suggested merge commit message (convention)

Mgsy commented Mar 31, 2021

FilipTokarski commented Mar 31, 2021

jacekbogdanski left a comment

Choose a reason for hiding this comment

pomek commented Apr 8, 2021

pomek commented Apr 8, 2021

pomek commented Apr 9, 2021

pomek commented Apr 9, 2021

niegowski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pomek commented Apr 21, 2021

pomek commented Apr 30, 2021

pomek commented Mar 31, 2021 •

edited

Loading