Skip to content

Commit

Permalink
Merge pull request #16880 from ckeditor/ck/6841-new
Browse files Browse the repository at this point in the history
Fix (table): Changed default table and table cell properties to match the content styles. Fixes problem with setting `border = none` on the table. Closes #6841.
  • Loading branch information
Mati365 authored Aug 27, 2024
2 parents 2d74802 + fa4c045 commit dde1b50
Show file tree
Hide file tree
Showing 13 changed files with 181 additions and 62 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<table tableBorderColor="{}" tableBorderWidth="{}">
<table tableBorderColor="{}" tableBorderStyle="none" tableBorderWidth="{}">
<tableRow>
<tableCell tableCellBackgroundColor="#CCCCCC" tableCellBorderColor="windowtext" tableCellBorderStyle="solid" tableCellBorderWidth="1.0pt" tableCellPadding="{"top":"0cm","bottom":"0cm","right":"5.4pt","left":"5.4pt"}" tableCellWidth="59.0%">
<tableCell tableCellBackgroundColor="#CCCCCC" tableCellBorderColor="windowtext" tableCellBorderWidth="1.0pt" tableCellPadding="{"top":"0cm","bottom":"0cm","right":"5.4pt","left":"5.4pt"}" tableCellWidth="59.0%">
<paragraph><$text bold="true">Project Phase</$text></paragraph>
</tableCell>
<tableCell tableCellBackgroundColor="#CCCCCC" tableCellBorderColor="windowtext" tableCellBorderStyle="{"top":"solid","bottom":"solid","right":"solid","left":"none"}" tableCellBorderWidth="1.0pt" tableCellPadding="{"top":"0cm","bottom":"0cm","right":"5.4pt","left":"5.4pt"}" tableCellWidth="24.0%">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import TableCellHorizontalAlignmentCommand from './commands/tablecellhorizontala
import TableCellBorderStyleCommand from './commands/tablecellborderstylecommand.js';
import TableCellBorderColorCommand from './commands/tablecellbordercolorcommand.js';
import TableCellBorderWidthCommand from './commands/tablecellborderwidthcommand.js';
import { getNormalizedDefaultProperties } from '../utils/table-properties.js';
import { getNormalizedDefaultCellProperties } from '../utils/table-properties.js';
import { enableProperty } from '../utils/common.js';

const VALIGN_VALUES_REG_EXP = /^(top|middle|bottom)$/;
Expand Down Expand Up @@ -76,9 +76,9 @@ export default class TableCellPropertiesEditing extends Plugin {
const schema = editor.model.schema;
const conversion = editor.conversion;

editor.config.define( 'table.tableCellProperties.defaultProperties', {} );
editor.config.define( 'table.tableCellProperties.defaultProperties', { } );

const defaultTableCellProperties = getNormalizedDefaultProperties(
const defaultTableCellProperties = getNormalizedDefaultCellProperties(
editor.config.get( 'table.tableCellProperties.defaultProperties' )!,
{
includeVerticalAlignmentProperty: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { getTableWidgetAncestor } from '../utils/ui/widget.js';
import { getBalloonCellPositionData, repositionContextualBalloon } from '../utils/ui/contextualballoon.js';

import tableCellProperties from './../../theme/icons/table-cell-properties.svg';
import { getNormalizedDefaultProperties, type NormalizedDefaultProperties } from '../utils/table-properties.js';
import { getNormalizedDefaultCellProperties, type NormalizedDefaultProperties } from '../utils/table-properties.js';
import type { GetCallback, ObservableChangeEvent } from 'ckeditor5/src/utils.js';

import type TableCellBorderStyleCommand from './commands/tablecellborderstylecommand.js';
Expand Down Expand Up @@ -119,7 +119,7 @@ export default class TableCellPropertiesUI extends Plugin {
const editor = this.editor;
const t = editor.t;

this._defaultTableCellProperties = getNormalizedDefaultProperties(
this._defaultTableCellProperties = getNormalizedDefaultCellProperties(
editor.config.get( 'table.tableCellProperties.defaultProperties' )!,
{
includeVerticalAlignmentProperty: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { Plugin } from 'ckeditor5/src/core.js';

import TableEditing from './../tableediting.js';
import TableCellWidthCommand from './commands/tablecellwidthcommand.js';
import { getNormalizedDefaultProperties } from '../utils/table-properties.js';
import { getNormalizedDefaultCellProperties } from '../utils/table-properties.js';
import { enableProperty } from '../utils/common.js';

/**
Expand Down Expand Up @@ -41,7 +41,7 @@ export default class TableCellWidthEditing extends Plugin {
public init(): void {
const editor = this.editor;

const defaultTableCellProperties = getNormalizedDefaultProperties(
const defaultTableCellProperties = getNormalizedDefaultCellProperties(
editor.config.get( 'table.tableCellProperties.defaultProperties' )!
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import TableBorderWidthCommand from './commands/tableborderwidthcommand.js';
import TableWidthCommand from './commands/tablewidthcommand.js';
import TableHeightCommand from './commands/tableheightcommand.js';
import TableAlignmentCommand from './commands/tablealignmentcommand.js';
import { getNormalizedDefaultProperties } from '../utils/table-properties.js';
import { getNormalizedDefaultTableProperties } from '../utils/table-properties.js';

const ALIGN_VALUES_REG_EXP = /^(left|center|right)$/;
const FLOAT_VALUES_REG_EXP = /^(left|none|right)$/;
Expand Down Expand Up @@ -71,9 +71,12 @@ export default class TablePropertiesEditing extends Plugin {

editor.config.define( 'table.tableProperties.defaultProperties', {} );

const defaultTableProperties = getNormalizedDefaultProperties( editor.config.get( 'table.tableProperties.defaultProperties' )!, {
includeAlignmentProperty: true
} );
const defaultTableProperties = getNormalizedDefaultTableProperties(
editor.config.get( 'table.tableProperties.defaultProperties' )!,
{
includeAlignmentProperty: true
}
);

editor.data.addStyleProcessorRules( addBorderRules );
enableBorderProperties( schema, conversion, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
} from '../utils/ui/table-properties.js';
import { getSelectionAffectedTableWidget } from '../utils/ui/widget.js';
import { getBalloonTablePositionData, repositionContextualBalloon } from '../utils/ui/contextualballoon.js';
import { getNormalizedDefaultProperties, type NormalizedDefaultProperties } from '../utils/table-properties.js';
import { getNormalizedDefaultTableProperties, type NormalizedDefaultProperties } from '../utils/table-properties.js';
import type { Batch } from 'ckeditor5/src/engine.js';
import type { EventInfo, ObservableChangeEvent } from 'ckeditor5/src/utils.js';

Expand Down Expand Up @@ -117,9 +117,13 @@ export default class TablePropertiesUI extends Plugin {
const editor = this.editor;
const t = editor.t;

this._defaultTableProperties = getNormalizedDefaultProperties( editor.config.get( 'table.tableProperties.defaultProperties' )!, {
includeAlignmentProperty: true
} );
this._defaultTableProperties = getNormalizedDefaultTableProperties(
editor.config.get( 'table.tableProperties.defaultProperties' )!,
{
includeAlignmentProperty: true
}
);

this._balloon = editor.plugins.get( ContextualBalloon );

editor.ui.componentFactory.add( 'tableProperties', locale => {
Expand Down
86 changes: 74 additions & 12 deletions packages/ckeditor5-table/src/utils/table-properties.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,24 +78,46 @@ export interface NormalizedDefaultProperties {
horizontalAlignment?: string;
}

/**
* Options used to determine which properties should be added to the normalized configuration.
*/
export type NormalizeTableDefaultPropertiesOptions = {

/**
* Whether the "alignment" property should be added.
*/
includeAlignmentProperty?: boolean;

/**
* Whether the "padding" property should be added.
*/
includePaddingProperty?: boolean;

/**
* Whether the "verticalAlignment" property should be added.
*/
includeVerticalAlignmentProperty?: boolean;

/**
* Whether the "horizontalAlignment" property should be added.
*/
includeHorizontalAlignmentProperty?: boolean;

/**
* Whether the content is right-to-left.
*/
isRightToLeftContent?: boolean;
};

/**
* Returns the normalized configuration.
*
* @param options.includeAlignmentProperty Whether the "alignment" property should be added.
* @param options.includePaddingProperty Whether the "padding" property should be added.
* @param options.includeVerticalAlignmentProperty Whether the "verticalAlignment" property should be added.
* @param options.includeHorizontalAlignmentProperty Whether the "horizontalAlignment" property should be added.
* @param options.isRightToLeftContent Whether the content is right-to-left.
* @param config The configuration to normalize.
* @param options Options used to determine which properties should be added.
*/
export function getNormalizedDefaultProperties(
config: Partial<NormalizedDefaultProperties> | undefined,
options: {
includeAlignmentProperty?: boolean;
includePaddingProperty?: boolean;
includeVerticalAlignmentProperty?: boolean;
includeHorizontalAlignmentProperty?: boolean;
isRightToLeftContent?: boolean;
} = {}
options: NormalizeTableDefaultPropertiesOptions = {}
): NormalizedDefaultProperties {
const normalizedConfig: NormalizedDefaultProperties = {
borderStyle: 'none',
Expand Down Expand Up @@ -125,3 +147,43 @@ export function getNormalizedDefaultProperties(

return normalizedConfig;
}

/**
* Returns the normalized default table properties.
*
* @param config The configuration to normalize.
* @param options Options used to determine which properties should be added.
*/
export function getNormalizedDefaultTableProperties(
config: Partial<NormalizedDefaultProperties> | undefined,
options?: NormalizeTableDefaultPropertiesOptions
): NormalizedDefaultProperties {
return getNormalizedDefaultProperties( {
// It adds support for border none in the table element, keep it in sync with the content styles
// See more: https://github.com/ckeditor/ckeditor5/issues/6841#issuecomment-1959195608
borderStyle: 'double',
borderColor: 'hsl(0, 0%, 70%)',
borderWidth: '1px',
...config
}, options );
}

/**
* Returns the normalized default cell properties.
*
* @param config The configuration to normalize.
* @param options Options used to determine which properties should be added.
*/
export function getNormalizedDefaultCellProperties(
config: Partial<NormalizedDefaultProperties> | undefined,
options?: NormalizeTableDefaultPropertiesOptions
): NormalizedDefaultProperties {
return getNormalizedDefaultProperties( {
// It adds support for border none in the table element, keep it in sync with the content styles
// See more: https://github.com/ckeditor/ckeditor5/issues/6841#issuecomment-1959195608
borderStyle: 'solid',
borderColor: 'hsl(0, 0%, 75%)',
borderWidth: '1px',
...config
}, options );
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,24 @@ describe( 'table cell properties', () => {
} );

describe( 'upcast conversion', () => {
it( 'should upcast border shorthand', () => {
it( 'should not upcast border values which are same as default', () => {
editor.setData( '<table><tr><td style="border:1px solid #f00">foo</td></tr></table>' );

const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] );

expect( tableCell.getAttribute( 'tableCellBorderColor' ) ).to.equal( '#f00' );
expect( tableCell.getAttribute( 'tableCellBorderStyle' ) ).to.equal( 'solid' );
expect( tableCell.getAttribute( 'tableCellBorderWidth' ) ).to.equal( '1px' );
expect( tableCell.getAttribute( 'tableCellBorderStyle' ) ).to.be.undefined;
expect( tableCell.getAttribute( 'tableCellBorderWidth' ) ).to.be.undefined;
} );

it( 'should upcast border shorthand', () => {
editor.setData( '<table><tr><td style="border:2px dashed #f00">foo</td></tr></table>' );

const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] );

expect( tableCell.getAttribute( 'tableCellBorderColor' ) ).to.equal( '#f00' );
expect( tableCell.getAttribute( 'tableCellBorderStyle' ) ).to.equal( 'dashed' );
expect( tableCell.getAttribute( 'tableCellBorderWidth' ) ).to.equal( '2px' );
} );

it( 'should upcast border-color shorthand', () => {
Expand All @@ -116,21 +126,21 @@ describe( 'table cell properties', () => {
} );

it( 'should upcast border-width shorthand', () => {
editor.setData( '<table><tr><td style="border-width:1px">foo</td></tr></table>' );
editor.setData( '<table><tr><td style="border-width:3px">foo</td></tr></table>' );

const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] );

expect( tableCell.getAttribute( 'tableCellBorderWidth' ) ).to.equal( '1px' );
expect( tableCell.getAttribute( 'tableCellBorderWidth' ) ).to.equal( '3px' );
} );

it( 'should upcast border-top shorthand', () => {
editor.setData( '<table><tr><td style="border-top:1px solid #f00">foo</td></tr></table>' );
editor.setData( '<table><tr><td style="border-top:2px double #f00">foo</td></tr></table>' );

const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] );

assertTRBLAttribute( tableCell, 'tableCellBorderColor', '#f00', null, null, null );
assertTRBLAttribute( tableCell, 'tableCellBorderStyle', 'solid', null, null, null );
assertTRBLAttribute( tableCell, 'tableCellBorderWidth', '1px', null, null, null );
assertTRBLAttribute( tableCell, 'tableCellBorderStyle', 'double', null, null, null );
assertTRBLAttribute( tableCell, 'tableCellBorderWidth', '2px', null, null, null );
} );

it( 'should upcast border-right shorthand', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -693,9 +693,9 @@ describe( 'table cell properties', () => {

expect( contextualBalloon.visibleView ).to.equal( tableCellPropertiesView );
expect( tableCellPropertiesView ).to.include( {
borderStyle: 'none',
borderColor: '',
borderWidth: '',
borderStyle: 'solid',
borderColor: 'hsl(0, 0%, 75%)',
borderWidth: '1px',
height: '',
padding: '',
backgroundColor: '',
Expand Down
8 changes: 4 additions & 4 deletions packages/ckeditor5-table/tests/tableclipboard-paste.js
Original file line number Diff line number Diff line change
Expand Up @@ -3935,15 +3935,15 @@ describe( 'table clipboard', () => {
);

pasteTable( [
[ { contents: 'aa', style: 'border:1px solid #f00;background:#ba7' }, 'ab' ],
[ { contents: 'aa', style: 'border:3px dashed #f00;background:#ba7' }, 'ab' ],
[ 'ba', 'bb' ]
] );

const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] );

expect( tableCell.getAttribute( 'tableCellBorderColor' ) ).to.equal( '#f00' );
expect( tableCell.getAttribute( 'tableCellBorderStyle' ) ).to.equal( 'solid' );
expect( tableCell.getAttribute( 'tableCellBorderWidth' ) ).to.equal( '1px' );
expect( tableCell.getAttribute( 'tableCellBorderStyle' ) ).to.equal( 'dashed' );
expect( tableCell.getAttribute( 'tableCellBorderWidth' ) ).to.equal( '3px' );
expect( tableCell.getAttribute( 'tableCellBackgroundColor' ) ).to.equal( '#ba7' );
} );

Expand Down Expand Up @@ -4032,7 +4032,7 @@ describe( 'table clipboard', () => {
await createEditor( [ TableCellPropertiesEditing ] );

const color = 'rgb(242, 242, 242)';
const style = 'solid';
const style = 'double';
const width = '2px';

pasteHtml( editor,
Expand Down
Loading

0 comments on commit dde1b50

Please sign in to comment.