-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for default table (and cells) properties #9393
Changes from all commits
31041d1
445f60b
24b747a
801dd24
9aa0ad2
1ddab6b
efa3e19
149858d
3607d42
072100d
eddf6af
fa5f455
0166366
912cb06
5ad7484
0a1201d
e245caf
c7b23f2
88046d8
bab6ca2
a1bf418
f3292f0
b00eba2
7def432
6ae94d8
977b69e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
<div id="snippet-default-properties"></div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
/** | ||
* @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-default-properties' ), { | ||
extraPlugins: [ | ||
CKEditorPlugins.TableProperties, | ||
CKEditorPlugins.TableCellProperties | ||
], | ||
table: { | ||
contentToolbar: [ 'tableColumn', 'tableRow', 'mergeTableCells', 'tableProperties', 'tableCellProperties' ], | ||
tableProperties: { | ||
defaultProperties: { | ||
borderStyle: 'dashed', | ||
borderColor: 'hsl(0, 0%, 60%)', | ||
borderWidth: '3px', | ||
alignment: 'left' | ||
} | ||
}, | ||
tableCellProperties: { | ||
defaultProperties: { | ||
borderStyle: 'dotted', | ||
borderColor: 'hsl(120, 75%, 60%)', | ||
borderWidth: '2px', | ||
horizontalAlignment: 'right', | ||
verticalAlignment: 'bottom' | ||
} | ||
} | ||
}, | ||
image: { | ||
toolbar: [ | ||
'imageStyle:full', | ||
'imageStyle:side', | ||
'|', | ||
'imageTextAlternative' | ||
] | ||
}, | ||
placeholder: 'Insert the new table with applied the default styles.' | ||
} ) | ||
.then( editor => { | ||
window.editorDefaultStyles = editor; | ||
|
||
window.attachTourBalloon( { | ||
target: window.findToolbarItem( editor.ui.view.toolbar, 0 ), | ||
text: 'Click to insert the new table.', | ||
editor | ||
} ); | ||
} ) | ||
.catch( err => { | ||
console.error( err.stack ); | ||
} ); |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
// we need to wrap the code in the `editor.model.change()` block. | ||||||
editor.model.change( () => { | ||||||
tableUtils.insertRows( table, { at: insertAbove ? row : row + 1, copyStructureFromAbove: !insertAbove } ); | ||||||
} ); | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,96 @@ | ||||||||
/** | ||||||||
* @license Copyright (c) 2003-2021, CKSource - Frederico Knabben. All rights reserved. | ||||||||
* For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license | ||||||||
*/ | ||||||||
|
||||||||
/** | ||||||||
* @module table/converters/table-layout-post-fixer | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
*/ | ||||||||
|
||||||||
const TABLE_CELL_PROPERTIES = [ | ||||||||
'borderStyle', | ||||||||
'borderColor', | ||||||||
'borderWidth', | ||||||||
'backgroundColor', | ||||||||
'padding', | ||||||||
'horizontalAlignment', | ||||||||
'verticalAlignment', | ||||||||
'width', | ||||||||
'height' | ||||||||
]; | ||||||||
|
||||||||
/** | ||||||||
* Injects a table cell default properties post-fixer into the model. | ||||||||
* | ||||||||
* A table cell should have specified the default properties when a new cell was added into the table. | ||||||||
* | ||||||||
* @param {module:core/editor/editor~Editor} editor | ||||||||
*/ | ||||||||
export default function injectTableCellDefaultPropertiesPostFixer( editor ) { | ||||||||
editor.model.document.registerPostFixer( writer => tableCellDefaultPropertiesPostFixer( writer, editor ) ); | ||||||||
} | ||||||||
|
||||||||
// The table cell default properties post-fixer. | ||||||||
// | ||||||||
// @param {module:engine/model/writer~Writer} writer | ||||||||
// @param {module:core/editor/editor~Editor} editor | ||||||||
function tableCellDefaultPropertiesPostFixer( writer, editor ) { | ||||||||
const model = editor.model; | ||||||||
const changes = model.document.differ.getChanges(); | ||||||||
const cellProperties = editor.config.get( 'table.tableCellProperties.defaultProperties' ); | ||||||||
|
||||||||
// Do not check anything if the default cell properties are not specified. | ||||||||
if ( Object.keys( cellProperties ).length === 0 ) { | ||||||||
return false; | ||||||||
} | ||||||||
Comment on lines
+42
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be checked before registering the post-fixer. |
||||||||
|
||||||||
let wasFixed = false; | ||||||||
|
||||||||
for ( const entry of changes ) { | ||||||||
if ( entry.type != 'insert' ) { | ||||||||
continue; | ||||||||
} | ||||||||
|
||||||||
// Fix table on adding/removing table cells and rows. | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
if ( entry.name == 'tableRow' ) { | ||||||||
const tableRow = entry.position.nodeAfter; | ||||||||
|
||||||||
// For each cell in the table row... | ||||||||
for ( const tableCell of tableRow.getChildren() ) { | ||||||||
// ...check its cell properties... | ||||||||
if ( shouldApplyDefaultCellProperties( tableCell ) ) { | ||||||||
// ...and if the cell has no properties, apply the default. | ||||||||
writer.setAttributes( cellProperties, tableCell ); | ||||||||
|
||||||||
wasFixed = true; | ||||||||
} | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
// Fix table cell on adding/removing table cells and rows. | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
if ( entry.name == 'tableCell' ) { | ||||||||
const tableCell = entry.position.nodeAfter; | ||||||||
|
||||||||
if ( shouldApplyDefaultCellProperties( tableCell ) ) { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
// ...and if the cell has no properties, apply the default. | ||||||||
writer.setAttributes( cellProperties, tableCell ); | ||||||||
|
||||||||
wasFixed = true; | ||||||||
} | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
return wasFixed; | ||||||||
} | ||||||||
|
||||||||
// Checks whether the default properties should be applied for the specified cell. | ||||||||
// | ||||||||
// The default properties will be applied only if the cell does not contain any "visual" properties. | ||||||||
// | ||||||||
// @param {module:engine/model/element~Element} tableCell | ||||||||
// @returns {Boolean} | ||||||||
function shouldApplyDefaultCellProperties( tableCell ) { | ||||||||
const attrs = [ ...tableCell.getAttributeKeys() ]; | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
To be more explicit. |
||||||||
|
||||||||
return attrs.some( attributeName => TABLE_CELL_PROPERTIES.includes( attributeName ) ) === false; | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this line would be easier to read if we flip this helper logic to |
||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,9 @@ import TableCellHorizontalAlignmentCommand from './commands/tablecellhorizontala | |
import TableCellBorderStyleCommand from './commands/tablecellborderstylecommand'; | ||
import TableCellBorderColorCommand from './commands/tablecellbordercolorcommand'; | ||
import TableCellBorderWidthCommand from './commands/tablecellborderwidthcommand'; | ||
import TableWalker from '../tablewalker'; | ||
import TableUtils from '../tableutils'; | ||
import injectTableCellDefaultPropertiesPostFixer from '../converters/table-cell-default-properties-post-fixer'; | ||
|
||
const VALIGN_VALUES_REG_EXP = /^(top|bottom)$/; | ||
|
||
|
@@ -57,7 +60,7 @@ export default class TableCellPropertiesEditing extends Plugin { | |
* @inheritDoc | ||
*/ | ||
static get requires() { | ||
return [ TableEditing ]; | ||
return [ TableEditing, TableUtils ]; | ||
} | ||
|
||
/** | ||
|
@@ -69,6 +72,8 @@ export default class TableCellPropertiesEditing extends Plugin { | |
const conversion = editor.conversion; | ||
const locale = editor.locale; | ||
|
||
editor.config.define( 'table.tableCellProperties.defaultProperties', {} ); | ||
|
||
editor.data.addStyleProcessorRules( addBorderRules ); | ||
enableBorderProperties( schema, conversion ); | ||
editor.commands.add( 'tableCellBorderStyle', new TableCellBorderStyleCommand( editor ) ); | ||
|
@@ -94,6 +99,36 @@ export default class TableCellPropertiesEditing extends Plugin { | |
|
||
enableVerticalAlignmentProperty( schema, conversion ); | ||
editor.commands.add( 'tableCellVerticalAlignment', new TableCellVerticalAlignmentCommand( editor ) ); | ||
|
||
this._enableDefaultCellProperties(); | ||
|
||
injectTableCellDefaultPropertiesPostFixer( editor ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
} | ||
|
||
/** | ||
* Enables applying the default cell properties. | ||
* | ||
* @private | ||
*/ | ||
_enableDefaultCellProperties() { | ||
const editor = this.editor; | ||
const tableCellProperties = editor.config.get( 'table.tableCellProperties.defaultProperties' ); | ||
const tableUtils = editor.plugins.get( TableUtils ); | ||
|
||
// Apply default cell properties while creating a new table. | ||
this.listenTo( tableUtils, 'createTable', ( evt, [ writer ] ) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's not use the writer from the event's data. We should have here a change block. |
||
const tableElement = evt.return; | ||
|
||
for ( const item of new TableWalker( tableElement ) ) { | ||
writer.setAttributes( tableCellProperties, item.cell ); | ||
} | ||
}, { priority: 'low' } ); | ||
|
||
// Apply default cell properties while inserting new rows into the table. | ||
this.listenTo( tableUtils, 'insertRows', applyDefaultCellProperties( editor, tableCellProperties ), { priority: 'low' } ); | ||
|
||
// Apply default cell properties while inserting new rows into the table. | ||
this.listenTo( tableUtils, 'insertColumns', applyDefaultCellProperties( editor, tableCellProperties ), { priority: 'low' } ); | ||
} | ||
} | ||
|
||
|
@@ -202,3 +237,20 @@ function enableProperty( schema, conversion, modelAttribute, styleName ) { | |
upcastStyleToAttribute( conversion, 'tableCell', modelAttribute, styleName ); | ||
downcastAttributeToStyle( conversion, 'tableCell', modelAttribute, styleName ); | ||
} | ||
|
||
// A factory function that returns a handler which applies default cell properties for created cells. | ||
// | ||
// @param {module:core/editor/editor~Editor} editor | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should not pass the whole editor instance if not needed, please replace it with the model. |
||
// @param {module:table/table~TableConfig#tableCellProperties} tableCellProperties | ||
// @return {Function} | ||
function applyDefaultCellProperties( editor, tableCellProperties ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please rename this helper to indicate that it is a factory (it's not applying anything itself). |
||
return evt => { | ||
const createdCells = evt.return; | ||
|
||
editor.model.change( writer => { | ||
for ( const tableCell of createdCells ) { | ||
writer.setAttributes( tableCellProperties, tableCell ); | ||
} | ||
} ); | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.