Merge pull request #9555 from ckeditor/i/8502-ui

Feature (table): Support for the default table properties. Read more about [the feature in the documentation](https://ckeditor.com/docs/ckeditor5/latest/features/table.html). Closes #8502. Closes #9219. Fix (engine): The conversion upcast `elementToAttribute()` and `attributeToAttribute()` functions should not call the `model.value()` callback if the element will not be converted. Closes #9536. MINOR BREAKING CHANGE (table): Clases `TableAlignmentCommand`, `TableBackgroundColorCommand`, `TableBorderColorCommand`, `TableBorderStyleCommand`, `TableBorderWidthCommand`, `TableHeightCommand`, `TablePropertyCommand`, `TableWidthCommand` requires the second argument called `defaultValue` which is the default value for the command. MINOR BREAKING CHANGE (table): The `TablePropertiesView` class requires additional property in the object as the second constructor argument (`options.defaultTableProperties`).
ckeditor · Apr 29, 2021 · efe5e57 · efe5e57
2 parents bfd899d + b7a9b5c
commit efe5e57
Show file tree

Hide file tree

Showing 42 changed files with 1,751 additions and 185 deletions.
diff --git a/packages/ckeditor5-core/lang/contexts.json b/packages/ckeditor5-core/lang/contexts.json
@@ -1,5 +1,6 @@
 {
 	"Save": "Label for the Save button.",
 	"Cancel": "Label for the Cancel button.",
-	"Remove color": "The label used by a button next to the color palette in the color picker that removes the color (resets it to an empty value, example usages in font color or table properties)."
+	"Remove color": "The label used by a button next to the color palette in the color picker that removes the color (resets it to an empty value, example usages in font color or table properties).",
+	"Restore default": "The label used by a button next to the color palette in the color picker that restores the default value if the default table properties are specified."
 }
diff --git a/packages/ckeditor5-engine/src/conversion/upcasthelpers.js b/packages/ckeditor5-engine/src/conversion/upcasthelpers.js
@@ -868,15 +868,6 @@ function prepareToAttributeConverter( config, shallow ) {
 			return;
 		}
 
-		const modelKey = config.model.key;
-		const modelValue = typeof config.model.value == 'function' ?
-			config.model.value( data.viewItem, conversionApi ) : config.model.value;
-
-		// Do not convert if attribute building function returned falsy value.
-		if ( modelValue === null ) {
-			return;
-		}
-
 		if ( onlyViewNameIsDefined( config.view, data.viewItem ) ) {
 			match.match.name = true;
 		} else {
@@ -889,6 +880,15 @@ function prepareToAttributeConverter( config, shallow ) {
 			return;
 		}
 
+		const modelKey = config.model.key;
+		const modelValue = typeof config.model.value == 'function' ?
+			config.model.value( data.viewItem, conversionApi ) : config.model.value;
+
+		// Do not convert if attribute building function returned falsy value.
+		if ( modelValue === null ) {
+			return;
+		}
+
 		// Since we are converting to attribute we need a range on which we will set the attribute.
 		// If the range is not created yet, let's create it by converting children of the current node first.
 		if ( !data.modelRange ) {

diff --git a/packages/ckeditor5-engine/tests/conversion/upcasthelpers.js b/packages/ckeditor5-engine/tests/conversion/upcasthelpers.js
@@ -713,6 +713,41 @@ describe( 'UpcastHelpers', () => {
 				'<div border="border"><div shade="shade"></div></div>'
 			);
 		} );
+
+		// #9536.
+		describe( 'calling the `model.value()` callback', () => {
+			it( 'should not call the `model.view()` callback if the attribute was already consumed', () => {
+				const spy = sinon.spy();
+
+				upcastHelpers.attributeToAttribute( {
+					view: {
+						name: 'span',
+						styles: {
+							'text-align': /[\s\S]+/
+						}
+					},
+					model: {
+						key: 'alignment',
+						value: spy
+					}
+				} );
+
+				upcastDispatcher.on( 'element:span', ( evt, data, conversionApi ) => {
+					conversionApi.consumable.consume( data.viewItem, {
+						styles: [ 'text-align' ]
+					} );
+				} );
+
+				const viewElement = viewParse( '<span style="text-align:center;">Foo.</span>' );
+
+				expectResult(
+					viewElement,
+					'Foo.'
+				);
+
+				expect( spy.called ).to.equal( false );
+			} );
+		} );
 	} );
 
 	describe( 'elementToMarker()', () => {

diff --git a/packages/ckeditor5-paste-from-office/tests/_data/table/tablecellproperties/model.html b/packages/ckeditor5-paste-from-office/tests/_data/table/tablecellproperties/model.html
@@ -1,4 +1,4 @@
-<table borderColor="{}" borderStyle="none" borderWidth="{}">
+<table borderColor="{}" borderWidth="{}">
 	<tableRow>
 		<tableCell backgroundColor="#CCCCCC" borderColor="windowtext" borderStyle="solid" borderWidth="1.0pt" padding="{"top":"0cm","bottom":"0cm","right":"5.4pt","left":"5.4pt"}" width="59.0%">
 			<paragraph><$text bold="true">Project Phase</$text></paragraph>

diff --git a/packages/ckeditor5-table/docs/_snippets/features/table-default-properties.html b/packages/ckeditor5-table/docs/_snippets/features/table-default-properties.html
@@ -0,0 +1,30 @@
+<style id="table-default-properties-styles">
+    .table-default-properties-container .ck-content .table {
+        float: right;
+        width: 500px;
+        height: 250px;
+    }
+
+    .table-default-properties-container .ck-content .table table {
+        border-style: dashed;
+        border-color: hsl(90, 75%, 60%);
+        border-width: 3px;
+    }
+</style>
+
+<div class="table-default-properties-container">
+    <div id="snippet-table-default-properties">
+        <figure class="table">
+            <table>
+                <tr>
+                    <td>Cell number 1</td>
+                    <td>Cell number 2</td>
+                </tr>
+                <tr>
+                    <td>Cell number 3</td>
+                    <td>Cell number 4</td>
+                </tr>
+            </table>
+        </figure>
+    </div>
+</div>
diff --git a/packages/ckeditor5-table/docs/_snippets/features/table-default-properties.js b/packages/ckeditor5-table/docs/_snippets/features/table-default-properties.js
@@ -0,0 +1,42 @@
+/**
+ * @license Copyright (c) 2003-2021, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
+ */
+
+/* globals ClassicEditor, CKEditorPlugins, console, window, document */
+
+ClassicEditor
+	.create( document.querySelector( '#snippet-table-default-properties' ), {
+		extraPlugins: [
+			CKEditorPlugins.TableProperties,
+			CKEditorPlugins.TableCellProperties
+		],
+		table: {
+			contentToolbar: [ 'tableColumn', 'tableRow', 'mergeTableCells', 'tableProperties', 'tableCellProperties' ],
+			tableProperties: {
+				defaultProperties: {
+					borderStyle: 'dashed',
+					borderColor: 'hsl(90, 75%, 60%)',
+					borderWidth: '3px',
+					alignment: 'right',
+					width: '500px',
+					height: '250px'
+				}
+			}
+		},
+		image: {
+			toolbar: [
+				'imageStyle:full',
+				'imageStyle:side',
+				'|',
+				'imageTextAlternative'
+			]
+		},
+		placeholder: 'Insert the new table with applied the default styles.'
+	} )
+	.then( editor => {
+		window.editorDefaultStyles = editor;
+	} )
+	.catch( err => {
+		console.error( err.stack );
+	} );
diff --git a/packages/ckeditor5-table/docs/features/table.md b/packages/ckeditor5-table/docs/features/table.md
@@ -221,14 +221,66 @@ ClassicEditor
 	.catch( ... );
 ```
 
+### Default table styles
+
+The table styles feature allows for configuring the default look of the tables in the editor. The configuration object should be synchronized with the {@link builds/guides/integration/content-styles editor content styles}.
+
+The **“Table properties”** button in the toolbar will show the table properties applied to the table.
+
+The stylesheet for the editor displayed below looks as follows:
+
+```css
+.ck-content .table {
+    float: right;
+    width: 500px;
+    height: 250px;
+}
+
+.ck-content .table table {
+    border-style: dashed;
+    border-color: 'hsl(90, 75%, 60%)';
+    border-width: 3px;
+}
+```
+
+The same values must be passed to the editor configuration as the {@link module:table/tableproperties~TablePropertiesOptions `table.tableProperties.defaultProperties`} object:
+
+```js
+const tableConfig = {
+    table: {
+        tableProperties: {
+            // The default styles for tables in the editor. They should be synchronized with the content styles.
+            defaultProperties: {
+	            borderStyle: 'dashed',
+	            borderColor: 'hsl(90, 75%, 60%)',
+	            borderWidth: '3px',
+	            alignment: 'right',
+	            width: '500px',
+	            height: '250px'
+            }
+        }
+    }
+};
+```
+
+The table element should be aligned to the `right` side by default. Its size should be `500x250px`. Border style should be `dashed`, `3px` of its width, and the color specified as `“Light green”`. The same will be applied for new tables if they will be inserted into the editor
+
+{@snippet features/table-default-properties}
+
+Read more about {@link module:table/tableproperties~TablePropertiesOptions all supported styles} for the table default properties feature.
+
+<info-box>
+	The default table styles **do** impact the {@link builds/guides/integration/basic-api#setting-the-editor-data data loaded into the editor}. Default properties will not be kept in the editor model.
+</info-box>
+
 ## Block vs inline content in table cells
 
-The table feature allows creating block content (like paragraphs, lists, headings, etc.) in table cells. However, if a table cell contains just one paragraph and this paragraph has no special attributes (like text alignment), the cell content is considered "inline" and the paragraph is not rendered.
+The table feature allows for creating block content (like paragraphs, lists, headings, etc.) inside table cells. However, if a table cell contains just one paragraph and this paragraph has no special attributes (like text alignment), the cell content is considered "inline" and the paragraph is not rendered.
 
-This means that a table cell can be in two states: with inline content or with block content. The reason for this differentiation is that most tables contain only inline content (e.g. in the [demo](#demos) above) and it is common for "data tables" to not contain any block content. In such scenario, printing out `<p>` elements would be semantically incorrect and also unnecessary. There are, however, scenarios where the user wants to create, for example, a list inside the table and then support for block content is necessary, too.
+This means that a table cell can have two states: with inline content or with block content. The reason for this differentiation is that most tables contain only inline content (e.g. in the [demo](#demos) above) and it is common for "data tables" to not contain any block content. In such a scenario, printing out `<p>` elements would be semantically incorrect and also unnecessary. There are, however, scenarios where the user wants to create, for example, a list inside a table cell and then the support for block content is necessary.
 
 <info-box>
-	"Rendering" here means the view layer. In the model a cell is always filled with at least a `<paragraph>`. This is because of consistency, as &mdash; since a cell always has some block content &mdash; the text is never directly inside `<tableCell>`. This also allows features like <kbd>Enter</kbd> support to work out of the box (since a `<paragraph>` exists in the model, it can be split despite the fact that it is not present in the view).
+	"Rendering" here refers to the view layer. In the model, a cell is always filled with at least a `<paragraph>`. This is because of consistency, as &mdash; since a cell always has some block content &mdash; the text is never directly inside the `<tableCell>`. This also allows features like <kbd>Enter</kbd> support to work out of the box (since a `<paragraph>` exists in the model, it can be split despite the fact that it is not present in the view).
 </info-box>
 
 ### Inline content
@@ -325,7 +377,7 @@ The above model structure will be rendered to the data and to the editing view a
 ```
 
 <info-box info>
-	At the moment it is not possible to completely disallow block content in tables. See the [discussion on GitHub](https://github.com/ckeditor/ckeditor5-table/issues/101) about adding a configuration option that would enable that. Add a 👍&nbsp; if you need this feature.
+	At the moment, it is not possible to completely disallow block content in tables. See the [discussion on GitHub](https://github.com/ckeditor/ckeditor5-table/issues/101) about adding a configuration option that would enable that. Feel free to upvote 👍&nbsp; if this feature is important to you.
 </info-box>
 
 ## Disallow nesting tables

diff --git a/packages/ckeditor5-table/src/converters/tableproperties.js b/packages/ckeditor5-table/src/converters/tableproperties.js
@@ -12,25 +12,31 @@
  *
  * @param {module:engine/conversion/conversion~Conversion} conversion
  * @param {Object} options
- * @param {String} options.modelElement
- * @param {String} options.modelAttribute
- * @param {String} options.styleName
+ * @param {String} options.modelAttribute The attribute to set.
+ * @param {String} options.styleName The style name to convert.
+ * @param {String} options.viewElement The view element name that should be converted.
+ * @param {String} options.defaultValue The default value for the specified `modelAttribute`.
  * @param {Boolean} [options.reduceBoxSides=false]
  */
-export function upcastStyleToAttribute( conversion, { modelElement, modelAttribute, styleName, reduceBoxSides = false } ) {
+export function upcastStyleToAttribute( conversion, options ) {
+	const { viewElement, defaultValue, modelAttribute, styleName, reduceBoxSides = false } = options;
+
 	conversion.for( 'upcast' ).attributeToAttribute( {
 		view: {
+			name: viewElement,
 			styles: {
 				[ styleName ]: /[\s\S]+/
 			}
 		},
 		model: {
-			name: modelElement,
 			key: modelAttribute,
 			value: viewElement => {
 				const normalized = viewElement.getNormalizedStyle( styleName );
+				const value = reduceBoxSides ? reduceBoxSidesValue( normalized ) : normalized;
 
-				return reduceBoxSides ? reduceBoxSidesValue( normalized ) : normalized;
+				if ( defaultValue !== value ) {
+					return value;
+				}
 			}
 		}
 	} );
@@ -41,8 +47,12 @@ export function upcastStyleToAttribute( conversion, { modelElement, modelAttribu
  *
  * @param {module:engine/conversion/conversion~Conversion} conversion
  * @param {String} viewElementName
+ * @param {Object} [defaultBorder={}] The default border values.
+ * @param {String} defaultBorder.color The default `borderColor` value.
+ * @param {String} defaultBorder.style The default `borderStyle` value.
+ * @param {String} defaultBorder.width The default `borderWidth` value.
  */
-export function upcastBorderStyles( conversion, viewElementName ) {
+export function upcastBorderStyles( conversion, viewElementName, defaultBorder = {} ) {
 	conversion.for( 'upcast' ).add( dispatcher => dispatcher.on( 'element:' + viewElementName, ( evt, data, conversionApi ) => {
 		// If the element was not converted by element-to-element converter,
 		// we should not try to convert the style. See #8393.
@@ -84,13 +94,29 @@ export function upcastBorderStyles( conversion, viewElementName ) {
 
 		conversionApi.consumable.consume( data.viewItem, matcherPattern );
 
-		const normalizedBorderStyle = data.viewItem.getNormalizedStyle( 'border-style' );
-		const normalizedBorderColor = data.viewItem.getNormalizedStyle( 'border-color' );
-		const normalizedBorderWidth = data.viewItem.getNormalizedStyle( 'border-width' );
+		const normalizedBorder = {
+			style: data.viewItem.getNormalizedStyle( 'border-style' ),
+			color: data.viewItem.getNormalizedStyle( 'border-color' ),
+			width: data.viewItem.getNormalizedStyle( 'border-width' )
+		};
 
-		conversionApi.writer.setAttribute( 'borderStyle', reduceBoxSidesValue( normalizedBorderStyle ), modelElement );
-		conversionApi.writer.setAttribute( 'borderColor', reduceBoxSidesValue( normalizedBorderColor ), modelElement );
-		conversionApi.writer.setAttribute( 'borderWidth', reduceBoxSidesValue( normalizedBorderWidth ), modelElement );
+		const reducedBorder = {
+			style: reduceBoxSidesValue( normalizedBorder.style ),
+			color: reduceBoxSidesValue( normalizedBorder.color ),
+			width: reduceBoxSidesValue( normalizedBorder.width )
+		};
+
+		if ( reducedBorder.style !== defaultBorder.style ) {
+			conversionApi.writer.setAttribute( 'borderStyle', reducedBorder.style, modelElement );
+		}
+
+		if ( reducedBorder.color !== defaultBorder.color ) {
+			conversionApi.writer.setAttribute( 'borderColor', reducedBorder.color, modelElement );
+		}
+
+		if ( reducedBorder.width !== defaultBorder.width ) {
+			conversionApi.writer.setAttribute( 'borderWidth', reducedBorder.width, modelElement );
+		}
 	} ) );
 }
 

diff --git a/packages/ckeditor5-table/src/tablecellproperties/tablecellpropertiesediting.js b/packages/ckeditor5-table/src/tablecellproperties/tablecellpropertiesediting.js
@@ -204,6 +204,6 @@ function enableProperty( schema, conversion, options ) {
 		allowAttributes: [ modelAttribute ]
 	} );
 
-	upcastStyleToAttribute( conversion, { modelElement: 'tableCell', ...options } );
+	upcastStyleToAttribute( conversion, { viewElement: /^(td|th)$/, ...options } );
 	downcastAttributeToStyle( conversion, { modelElement: 'tableCell', ...options } );
 }
diff --git a/packages/ckeditor5-table/src/tablecellproperties/ui/tablecellpropertiesview.js b/packages/ckeditor5-table/src/tablecellproperties/ui/tablecellpropertiesview.js
@@ -825,5 +825,10 @@ export default class TableCellPropertiesView extends View {
 }
 
 function isBorderStyleSet( value ) {
-	return !!value;
+	// TODO: Unify this with TablePropertiesView when implementing the default cell properties.
+	if ( !value || !value.length ) {
+		return false;
+	}
+
+	return value !== 'none';
 }