From 3adcbfe416c0adcee1d5784390ac08e10460619b Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Mon, 9 Mar 2020 23:40:17 +0100 Subject: [PATCH 1/9] Tests: Added basic unit tests for header column command's value and isEnabled properties. --- tests/commands/setheadercolumncommand.js | 78 +++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/tests/commands/setheadercolumncommand.js b/tests/commands/setheadercolumncommand.js index f8729258..2022bf2f 100644 --- a/tests/commands/setheadercolumncommand.js +++ b/tests/commands/setheadercolumncommand.js @@ -8,6 +8,7 @@ import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model import SetHeaderColumnCommand from '../../src/commands/setheadercolumncommand'; import { defaultConversion, defaultSchema, modelTable } from '../_utils/utils'; +import TableSelection from '../../src/tableselection'; import TableUtils from '../../src/tableutils'; import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; @@ -17,7 +18,7 @@ describe( 'SetHeaderColumnCommand', () => { beforeEach( () => { return ModelTestEditor .create( { - plugins: [ TableUtils ] + plugins: [ TableUtils, TableSelection ] } ) .then( newEditor => { editor = newEditor; @@ -43,6 +44,36 @@ describe( 'SetHeaderColumnCommand', () => { setData( model, 'foo[]
' ); expect( command.isEnabled ).to.be.true; } ); + + it( 'should be true if multiple columns are selected', () => { + setData( model, modelTable( [ + [ '01', '02', '03' ] + ] ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 0 ] ), + modelRoot.getNodeByPath( [ 0, 0, 1 ] ) + ); + + expect( command.isEnabled ).to.be.true; + } ); + + it( 'should be true if multiple header columns are selected', () => { + setData( model, modelTable( [ + [ '01', '02', '03' ] + ], { headingColumns: 2 } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 0 ] ), + modelRoot.getNodeByPath( [ 0, 0, 1 ] ) + ); + + expect( command.isEnabled ).to.be.true; + } ); } ); describe( 'value', () => { @@ -64,6 +95,36 @@ describe( 'SetHeaderColumnCommand', () => { expect( command.value ).to.be.true; } ); + it( 'should be true if multiple header columns are selected', () => { + setData( model, modelTable( [ + [ '01', '02', '03' ] + ], { headingColumns: 2 } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 0 ] ), + modelRoot.getNodeByPath( [ 0, 0, 1 ] ) + ); + + expect( command.value ).to.be.true; + } ); + + it( 'should be true if multiple header columns are selected in reversed order', () => { + setData( model, modelTable( [ + [ '01', '02', '03' ] + ], { headingColumns: 2 } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 1 ] ), + modelRoot.getNodeByPath( [ 0, 0, 0 ] ) + ); + + expect( command.value ).to.be.true; + } ); + it( 'should be false if selection is in a heading row', () => { setData( model, modelTable( [ [ '01', '02[]' ], @@ -72,6 +133,21 @@ describe( 'SetHeaderColumnCommand', () => { expect( command.value ).to.be.false; } ); + + it( 'should be false if only part of selected columns are headers', () => { + setData( model, modelTable( [ + [ '01', '02', '03', '04' ] + ], { headingColumns: 2 } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 1 ] ), + modelRoot.getNodeByPath( [ 0, 0, 2 ] ) + ); + + expect( command.value ).to.be.false; + } ); } ); describe( 'execute()', () => { From ce06875d9cb9b6205db063944f89aced8bb1fa9a Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Mon, 9 Mar 2020 23:40:57 +0100 Subject: [PATCH 2/9] Tests: Implemented handling for header column command's value and isEnabled properties in case of multi-cell selection. --- src/commands/setheadercolumncommand.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/commands/setheadercolumncommand.js b/src/commands/setheadercolumncommand.js index 9e440b98..e478af08 100644 --- a/src/commands/setheadercolumncommand.js +++ b/src/commands/setheadercolumncommand.js @@ -10,6 +10,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; import { findAncestor, updateNumericAttribute } from './utils'; +import { getTableCellsInSelection } from '../tableselection/utils'; /** * The header column command. @@ -32,13 +33,8 @@ export default class SetHeaderColumnCommand extends Command { */ refresh() { const model = this.editor.model; - const doc = model.document; - const selection = doc.selection; - - const position = selection.getFirstPosition(); - const tableCell = findAncestor( 'tableCell', position ); - - const isInTable = !!tableCell; + const selectedCells = getTableCellsInSelection( model.document.selection, true ); + const isInTable = selectedCells.length > 0; this.isEnabled = isInTable; @@ -50,7 +46,7 @@ export default class SetHeaderColumnCommand extends Command { * @readonly * @member {Boolean} #value */ - this.value = isInTable && this._isInHeading( tableCell, tableCell.parent.parent ); + this.value = isInTable && selectedCells.every( cell => this._isInHeading( cell, cell.parent.parent ) ); } /** From 1e886146de1aa992acb2e8ba8c0ad1755579957f Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Tue, 10 Mar 2020 00:08:47 +0100 Subject: [PATCH 3/9] Internal: Implemented adding column headers. Toggling is not yet supported. --- src/commands/setheadercolumncommand.js | 13 ++-- tests/commands/setheadercolumncommand.js | 84 +++++++++++++++++++++++- 2 files changed, 88 insertions(+), 9 deletions(-) diff --git a/src/commands/setheadercolumncommand.js b/src/commands/setheadercolumncommand.js index e478af08..b5b97272 100644 --- a/src/commands/setheadercolumncommand.js +++ b/src/commands/setheadercolumncommand.js @@ -9,7 +9,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; -import { findAncestor, updateNumericAttribute } from './utils'; +import { updateNumericAttribute } from './utils'; import { getTableCellsInSelection } from '../tableselection/utils'; /** @@ -63,16 +63,15 @@ export default class SetHeaderColumnCommand extends Command { */ execute( options = {} ) { 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 = findAncestor( 'tableCell', position ); - const tableRow = tableCell.parent; + const selectedCells = getTableCellsInSelection( model.document.selection, true ); + const firstCell = selectedCells[ 0 ]; + const lastCell = selectedCells[ selectedCells.length - 1 ]; + const tableRow = firstCell.parent; const table = tableRow.parent; - const { column: selectionColumn } = tableUtils.getCellLocation( tableCell ); + const selectionColumn = Math.max( tableUtils.getCellLocation( firstCell ).column, tableUtils.getCellLocation( lastCell ).column ); if ( options.forceValue === this.value ) { return; diff --git a/tests/commands/setheadercolumncommand.js b/tests/commands/setheadercolumncommand.js index 2022bf2f..0b061695 100644 --- a/tests/commands/setheadercolumncommand.js +++ b/tests/commands/setheadercolumncommand.js @@ -7,7 +7,7 @@ import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltestedit import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import SetHeaderColumnCommand from '../../src/commands/setheadercolumncommand'; -import { defaultConversion, defaultSchema, modelTable } from '../_utils/utils'; +import { assertSelectedCells, defaultConversion, defaultSchema, modelTable } from '../_utils/utils'; import TableSelection from '../../src/tableselection'; import TableUtils from '../../src/tableutils'; import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; @@ -151,7 +151,7 @@ describe( 'SetHeaderColumnCommand', () => { } ); describe( 'execute()', () => { - it( 'should set heading columns attribute that cover column in which is selection', () => { + it( 'should set heading columns attribute that cover column with collapsed selection', () => { setData( model, modelTable( [ [ '00', '01[]', '02', '03' ] ] ) ); @@ -163,6 +163,29 @@ describe( 'SetHeaderColumnCommand', () => { ], { headingColumns: 2 } ) ); } ); + it( 'should set heading columns attribute that cover column with entire cell selected', () => { + setData( model, modelTable( [ + [ '00', '01', '02', '03' ] + ] ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 1 ] ), + modelRoot.getNodeByPath( [ 0, 0, 1 ] ) + ); + + command.execute(); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '01', '02', '03' ] + ], { headingColumns: 2 } ) ); + + assertSelectedCells( model, [ + [ 0, 1, 0, 0 ] + ] ); + } ); + it( 'should set heading columns attribute below current selection column', () => { setData( model, modelTable( [ [ '00', '01[]', '02', '03' ] @@ -175,6 +198,63 @@ describe( 'SetHeaderColumnCommand', () => { ], { headingColumns: 1 } ) ); } ); + describe( 'multi-cell selection', () => { + describe( 'setting header', () => { + it( 'should set it correctly in a middle of single-row, multiple cell selection ', () => { + setData( model, modelTable( [ + [ '00', '01', '02', '03' ] + ] ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 1 ] ), + modelRoot.getNodeByPath( [ 0, 0, 2 ] ) + ); + + command.execute(); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '01', '02', '03' ] + ], { + headingColumns: 3 + } ) ); + + assertSelectedCells( model, [ + [ 0, 1, 1, 0 ] + ] ); + } ); + + it( 'should set it correctly in a middle of multi-row, multiple cell selection ', () => { + setData( model, modelTable( [ + [ '00', '01', '02', '03' ], + [ '10', '11', '12', '13' ] + ] ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 1 ] ), + modelRoot.getNodeByPath( [ 0, 1, 1 ] ) + ); + + command.execute(); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '01', '02', '03' ], + [ '10', '11', '12', '13' ] + ], { + headingColumns: 2 + } ) ); + + assertSelectedCells( model, [ + [ 0, 1, 0, 0 ], + [ 0, 1, 0, 0 ] + ] ); + } ); + } ); + } ); + it( 'should toggle of selected column', () => { setData( model, modelTable( [ [ '00', '01[]', '02', '03' ] From 45e3ca30eb020d44d990b8cd33a7a33c2b3283dd Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Tue, 10 Mar 2020 00:27:48 +0100 Subject: [PATCH 4/9] Tests: Finished integrating set header column command with multi cell selection. --- src/commands/setheadercolumncommand.js | 6 +- tests/commands/setheadercolumncommand.js | 116 ++++++++++++++++++++++- 2 files changed, 118 insertions(+), 4 deletions(-) diff --git a/src/commands/setheadercolumncommand.js b/src/commands/setheadercolumncommand.js index b5b97272..8dd064d7 100644 --- a/src/commands/setheadercolumncommand.js +++ b/src/commands/setheadercolumncommand.js @@ -71,13 +71,15 @@ export default class SetHeaderColumnCommand extends Command { const tableRow = firstCell.parent; const table = tableRow.parent; - const selectionColumn = Math.max( tableUtils.getCellLocation( firstCell ).column, tableUtils.getCellLocation( lastCell ).column ); + const [ selectedColumnMin, selectedColumnMax ] = + // Returned cells might not necessary be in order, so make sure to sort it. + [ tableUtils.getCellLocation( firstCell ).column, tableUtils.getCellLocation( lastCell ).column ].sort(); if ( options.forceValue === this.value ) { return; } - const headingColumnsToSet = this.value ? selectionColumn : selectionColumn + 1; + const headingColumnsToSet = this.value ? selectedColumnMin : selectedColumnMax + 1; model.change( writer => { updateNumericAttribute( 'headingColumns', headingColumnsToSet, table, writer, 0 ); diff --git a/tests/commands/setheadercolumncommand.js b/tests/commands/setheadercolumncommand.js index 0b061695..1e17d39c 100644 --- a/tests/commands/setheadercolumncommand.js +++ b/tests/commands/setheadercolumncommand.js @@ -200,7 +200,7 @@ describe( 'SetHeaderColumnCommand', () => { describe( 'multi-cell selection', () => { describe( 'setting header', () => { - it( 'should set it correctly in a middle of single-row, multiple cell selection ', () => { + it( 'should set it correctly in a middle of single-row, multiple cell selection', () => { setData( model, modelTable( [ [ '00', '01', '02', '03' ] ] ) ); @@ -225,7 +225,7 @@ describe( 'SetHeaderColumnCommand', () => { ] ); } ); - it( 'should set it correctly in a middle of multi-row, multiple cell selection ', () => { + it( 'should set it correctly in a middle of multi-row, multiple cell selection', () => { setData( model, modelTable( [ [ '00', '01', '02', '03' ], [ '10', '11', '12', '13' ] @@ -252,6 +252,118 @@ describe( 'SetHeaderColumnCommand', () => { [ 0, 1, 0, 0 ] ] ); } ); + + it( 'should remove header columns in case of multiple cell selection', () => { + setData( model, modelTable( [ + [ '00', '01', '02', '03' ] + ], { headingColumns: 4 } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 1 ] ), + modelRoot.getNodeByPath( [ 0, 0, 2 ] ) + ); + + command.execute(); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '01', '02', '03' ] + ], { + headingColumns: 1 + } ) ); + + assertSelectedCells( model, [ + [ 0, 1, 1, 0 ] + ] ); + } ); + + it( 'should remove header columns in case of multiple cell selection - reversed order', () => { + setData( model, modelTable( [ + [ '00', '01', '02', '03' ] + ], { + headingColumns: 4 + } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 2 ] ), + modelRoot.getNodeByPath( [ 0, 0, 1 ] ) + ); + + command.execute(); + + assertEqualMarkup( getData( model, { + withoutSelection: true + } ), modelTable( [ + [ '00', '01', '02', '03' ] + ], { + headingColumns: 1 + } ) ); + + assertSelectedCells( model, [ + [ 0, 1, 1, 0 ] + ] ); + } ); + + it( 'should respect forceValue=true in case of multiple cell selection', () => { + setData( model, modelTable( [ + [ '00', '01', '02', '03' ] + ], { + headingColumns: 3 + } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 1 ] ), + modelRoot.getNodeByPath( [ 0, 0, 2 ] ) + ); + + command.execute( { forceValue: true } ); + + assertEqualMarkup( getData( model, { + withoutSelection: true + } ), modelTable( [ + [ '00', '01', '02', '03' ] + ], { + headingColumns: 3 + } ) ); + + assertSelectedCells( model, [ + [ 0, 1, 1, 0 ] + ] ); + } ); + + it( 'should respect forceValue=false in case of multiple cell selection', () => { + setData( model, modelTable( [ + [ '00', '01', '02', '03' ] + ], { + headingColumns: 1 + } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 1 ] ), + modelRoot.getNodeByPath( [ 0, 0, 2 ] ) + ); + + command.execute( { forceValue: false } ); + + assertEqualMarkup( getData( model, { + withoutSelection: true + } ), modelTable( [ + [ '00', '01', '02', '03' ] + ], { + headingColumns: 1 + } ) ); + + assertSelectedCells( model, [ + [ 0, 1, 1, 0 ] + ] ); + } ); } ); } ); From 0e5019d6e147513108be152e0a416e4fd3eee435 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Tue, 10 Mar 2020 10:35:51 +0100 Subject: [PATCH 5/9] Tests: Initial tests for heading row setter with multiple cell selection. --- tests/commands/setheaderrowcommand.js | 81 ++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/tests/commands/setheaderrowcommand.js b/tests/commands/setheaderrowcommand.js index 16805124..ca55ed04 100644 --- a/tests/commands/setheaderrowcommand.js +++ b/tests/commands/setheaderrowcommand.js @@ -7,6 +7,7 @@ import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltestedit import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import SetHeaderRowCommand from '../../src/commands/setheaderrowcommand'; import { defaultConversion, defaultSchema, modelTable } from '../_utils/utils'; +import TableSelection from '../../src/tableselection'; import TableUtils from '../../src/tableutils'; import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; @@ -16,7 +17,7 @@ describe( 'SetHeaderRowCommand', () => { beforeEach( () => { return ModelTestEditor .create( { - plugins: [ TableUtils ] + plugins: [ TableUtils, TableSelection ] } ) .then( newEditor => { editor = newEditor; @@ -42,6 +43,36 @@ describe( 'SetHeaderRowCommand', () => { setData( model, 'foo[]
' ); expect( command.isEnabled ).to.be.true; } ); + + it( 'should be true if multiple cells are selected', () => { + setData( model, modelTable( [ + [ '01', '02', '03' ] + ] ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 0 ] ), + modelRoot.getNodeByPath( [ 0, 0, 1 ] ) + ); + + expect( command.isEnabled ).to.be.true; + } ); + + it( 'should be true if multiple cells in a header row are selected', () => { + setData( model, modelTable( [ + [ '01', '02', '03' ] + ], { headingRows: 1 } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 0 ] ), + modelRoot.getNodeByPath( [ 0, 0, 1 ] ) + ); + + expect( command.isEnabled ).to.be.true; + } ); } ); describe( 'value', () => { @@ -80,6 +111,54 @@ describe( 'SetHeaderRowCommand', () => { expect( command.value ).to.be.false; } ); + + it( 'should be true if multiple header rows are selected', () => { + setData( model, modelTable( [ + [ '01', '02' ], + [ '11', '12' ] + ], { headingRows: 2 } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 1 ] ), + modelRoot.getNodeByPath( [ 0, 1, 1 ] ) + ); + + expect( command.value ).to.be.true; + } ); + + it( 'should be true if multiple header columns are selected in reversed order', () => { + setData( model, modelTable( [ + [ '01', '02' ], + [ '11', '12' ] + ], { headingRows: 2 } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 1, 1 ] ), + modelRoot.getNodeByPath( [ 0, 0, 1 ] ) + ); + + expect( command.value ).to.be.true; + } ); + + it( 'should be false if only part of selected columns are headers', () => { + setData( model, modelTable( [ + [ '01', '02' ], + [ '11', '12' ] + ], { headingRows: 1 } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 0 ] ), + modelRoot.getNodeByPath( [ 0, 1, 0 ] ) + ); + + expect( command.value ).to.be.false; + } ); } ); describe( 'execute()', () => { From 35fe09fa8ddd277903eb72ba2acb7ee61a9e1daa Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Tue, 10 Mar 2020 10:36:48 +0100 Subject: [PATCH 6/9] Internal: Integrated header row command's isEnabled and value properties with multiple cell selection. --- src/commands/setheaderrowcommand.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/commands/setheaderrowcommand.js b/src/commands/setheaderrowcommand.js index 6c6d2aac..dade2e3f 100644 --- a/src/commands/setheaderrowcommand.js +++ b/src/commands/setheaderrowcommand.js @@ -10,6 +10,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; import { createEmptyTableCell, findAncestor, updateNumericAttribute } from './utils'; +import { getTableCellsInSelection } from '../tableselection/utils'; import TableWalker from '../tablewalker'; /** @@ -32,12 +33,8 @@ export default class SetHeaderRowCommand extends Command { */ refresh() { const model = this.editor.model; - const doc = model.document; - const selection = doc.selection; - - const position = selection.getFirstPosition(); - const tableCell = findAncestor( 'tableCell', position ); - const isInTable = !!tableCell; + const selectedCells = getTableCellsInSelection( model.document.selection, true ); + const isInTable = selectedCells.length > 0; this.isEnabled = isInTable; @@ -49,7 +46,7 @@ export default class SetHeaderRowCommand extends Command { * @readonly * @member {Boolean} #value */ - this.value = isInTable && this._isInHeading( tableCell, tableCell.parent.parent ); + this.value = isInTable && selectedCells.every( cell => this._isInHeading( cell, cell.parent.parent ) ); } /** From 6729b4b73c32ca1787073d7823f7ba17a7dbd1d6 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Tue, 10 Mar 2020 11:01:12 +0100 Subject: [PATCH 7/9] Tests: Added unit tests for set header row command execute() with multiple selection. --- tests/commands/setheaderrowcommand.js | 216 +++++++++++++++++++++++++- 1 file changed, 215 insertions(+), 1 deletion(-) diff --git a/tests/commands/setheaderrowcommand.js b/tests/commands/setheaderrowcommand.js index ca55ed04..87be0adf 100644 --- a/tests/commands/setheaderrowcommand.js +++ b/tests/commands/setheaderrowcommand.js @@ -6,7 +6,7 @@ import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor'; import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import SetHeaderRowCommand from '../../src/commands/setheaderrowcommand'; -import { defaultConversion, defaultSchema, modelTable } from '../_utils/utils'; +import { assertSelectedCells, defaultConversion, defaultSchema, modelTable } from '../_utils/utils'; import TableSelection from '../../src/tableselection'; import TableUtils from '../../src/tableutils'; import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; @@ -243,6 +243,220 @@ describe( 'SetHeaderRowCommand', () => { ] ) ); } ); + describe( 'multi-cell selection', () => { + it( 'should set it correctly in a middle of multi-row table', () => { + setData( model, modelTable( [ + [ '00' ], + [ '10' ], + [ '20' ], + [ '30' ] + ] ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 1, 0 ] ), + modelRoot.getNodeByPath( [ 0, 2, 0 ] ) + ); + + command.execute(); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00' ], + [ '10' ], + [ '20' ], + [ '30' ] + ], { + headingRows: 3 + } ) ); + + assertSelectedCells( model, [ + [ 0 ], + [ 1 ], + [ 1 ], + [ 0 ] + ] ); + } ); + + it( 'should set it correctly in a middle of multi-row table - reversed selection', () => { + setData( model, modelTable( [ + [ '00' ], + [ '10' ], + [ '20' ], + [ '30' ] + ] ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 2, 0 ] ), + modelRoot.getNodeByPath( [ 0, 1, 0 ] ) + ); + + command.execute(); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00' ], + [ '10' ], + [ '20' ], + [ '30' ] + ], { + headingRows: 3 + } ) ); + + assertSelectedCells( model, [ + [ 0 ], + [ 1 ], + [ 1 ], + [ 0 ] + ] ); + } ); + + it( 'should set it correctly in a middle of multi-row, multiple cell selection', () => { + setData( model, modelTable( [ + [ '00', '01', '02', '03' ], + [ '10', '11', '12', '13' ], + [ '20', '21', '22', '23' ], + [ '30', '31', '32', '33' ] + ] ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 1, 1 ] ), + modelRoot.getNodeByPath( [ 0, 2, 2 ] ) + ); + + command.execute(); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '01', '02', '03' ], + [ '10', '11', '12', '13' ], + [ '20', '21', '22', '23' ], + [ '30', '31', '32', '33' ] + ], { + headingRows: 3 + } ) ); + + assertSelectedCells( model, [ + [ 0, 0, 0, 0 ], + [ 0, 1, 1, 0 ], + [ 0, 1, 1, 0 ], + [ 0, 0, 0, 0 ] + ] ); + } ); + + it( 'should remove header rows in case of multiple cell selection', () => { + setData( model, modelTable( [ + [ '00' ], + [ '10' ], + [ '20' ], + [ '30' ] + ], { headingRows: 4 } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 1, 0 ] ), + modelRoot.getNodeByPath( [ 0, 2, 0 ] ) + ); + + command.execute(); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00' ], + [ '10' ], + [ '20' ], + [ '30' ] + ], { + headingRows: 1 + } ) ); + + assertSelectedCells( model, [ + [ 0 ], + [ 1 ], + [ 1 ], + [ 0 ] + ] ); + } ); + + it( 'should respect forceValue=true in case of multiple row selection', () => { + setData( model, modelTable( [ + [ '00' ], + [ '10' ], + [ '20' ], + [ '30' ] + ], { + headingRows: 3 + } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 1, 0 ] ), + modelRoot.getNodeByPath( [ 0, 2, 0 ] ) + ); + + command.execute( { forceValue: true } ); + + assertEqualMarkup( getData( model, { + withoutSelection: true + } ), modelTable( [ + [ '00' ], + [ '10' ], + [ '20' ], + [ '30' ] + ], { + headingRows: 3 + } ) ); + + assertSelectedCells( model, [ + [ 0 ], + [ 1 ], + [ 1 ], + [ 0 ] + ] ); + } ); + + it( 'should respect forceValue=false in case of multiple cell selection', () => { + setData( model, modelTable( [ + [ '00' ], + [ '10' ], + [ '20' ], + [ '30' ] + ], { + headingRows: 1 + } ) ); + + const tableSelection = editor.plugins.get( TableSelection ); + const modelRoot = model.document.getRoot(); + tableSelection._setCellSelection( + modelRoot.getNodeByPath( [ 0, 1, 0 ] ), + modelRoot.getNodeByPath( [ 0, 2, 0 ] ) + ); + + command.execute( { forceValue: false } ); + + assertEqualMarkup( getData( model, { + withoutSelection: true + } ), modelTable( [ + [ '00' ], + [ '10' ], + [ '20' ], + [ '30' ] + ], { + headingRows: 1 + } ) ); + + assertSelectedCells( model, [ + [ 0 ], + [ 1 ], + [ 1 ], + [ 0 ] + ] ); + } ); + } ); + it( 'should respect forceValue parameter #1', () => { setData( model, modelTable( [ [ '00' ], From f952d629666575859acbb51e22a4800233b11321 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Tue, 10 Mar 2020 11:03:38 +0100 Subject: [PATCH 8/9] Internal: Integrated multi cell selection with set row header command. --- src/commands/setheaderrowcommand.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/commands/setheaderrowcommand.js b/src/commands/setheaderrowcommand.js index dade2e3f..4db3859f 100644 --- a/src/commands/setheaderrowcommand.js +++ b/src/commands/setheaderrowcommand.js @@ -9,7 +9,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; -import { createEmptyTableCell, findAncestor, updateNumericAttribute } from './utils'; +import { createEmptyTableCell, updateNumericAttribute } from './utils'; import { getTableCellsInSelection } from '../tableselection/utils'; import TableWalker from '../tablewalker'; @@ -63,22 +63,23 @@ export default class SetHeaderRowCommand extends Command { */ execute( options = {} ) { const model = this.editor.model; - const doc = model.document; - const selection = doc.selection; - const position = selection.getFirstPosition(); - const tableCell = findAncestor( 'tableCell', position ); - const tableRow = tableCell.parent; - const table = tableRow.parent; + const selectedCells = getTableCellsInSelection( model.document.selection, true ); + const firstCell = selectedCells[ 0 ]; + const lastCell = selectedCells[ selectedCells.length - 1 ]; + const table = firstCell.parent.parent; const currentHeadingRows = table.getAttribute( 'headingRows' ) || 0; - const selectionRow = tableRow.index; + + const [ selectedRowMin, selectedRowMax ] = + // Returned cells might not necessary be in order, so make sure to sort it. + [ firstCell.parent.index, lastCell.parent.index ].sort(); if ( options.forceValue === this.value ) { return; } - const headingRowsToSet = this.value ? selectionRow : selectionRow + 1; + const headingRowsToSet = this.value ? selectedRowMin : selectedRowMax + 1; model.change( writer => { if ( headingRowsToSet ) { From 92545d0034b17821a360ec7f097cd3d6ebfdf260 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Tue, 10 Mar 2020 14:48:54 +0100 Subject: [PATCH 9/9] Internal: Adjusted to use the new API. --- src/commands/setheadercolumncommand.js | 6 +++--- src/commands/setheaderrowcommand.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/commands/setheadercolumncommand.js b/src/commands/setheadercolumncommand.js index 8dd064d7..7064e97a 100644 --- a/src/commands/setheadercolumncommand.js +++ b/src/commands/setheadercolumncommand.js @@ -10,7 +10,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; import { updateNumericAttribute } from './utils'; -import { getTableCellsInSelection } from '../tableselection/utils'; +import { getSelectionAffectedTableCells } from '../utils'; /** * The header column command. @@ -33,7 +33,7 @@ export default class SetHeaderColumnCommand extends Command { */ refresh() { const model = this.editor.model; - const selectedCells = getTableCellsInSelection( model.document.selection, true ); + const selectedCells = getSelectionAffectedTableCells( model.document.selection ); const isInTable = selectedCells.length > 0; this.isEnabled = isInTable; @@ -65,7 +65,7 @@ export default class SetHeaderColumnCommand extends Command { const model = this.editor.model; const tableUtils = this.editor.plugins.get( 'TableUtils' ); - const selectedCells = getTableCellsInSelection( model.document.selection, true ); + const selectedCells = getSelectionAffectedTableCells( model.document.selection ); const firstCell = selectedCells[ 0 ]; const lastCell = selectedCells[ selectedCells.length - 1 ]; const tableRow = firstCell.parent; diff --git a/src/commands/setheaderrowcommand.js b/src/commands/setheaderrowcommand.js index 4db3859f..c318c887 100644 --- a/src/commands/setheaderrowcommand.js +++ b/src/commands/setheaderrowcommand.js @@ -10,7 +10,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; import { createEmptyTableCell, updateNumericAttribute } from './utils'; -import { getTableCellsInSelection } from '../tableselection/utils'; +import { getSelectionAffectedTableCells } from '../utils'; import TableWalker from '../tablewalker'; /** @@ -33,7 +33,7 @@ export default class SetHeaderRowCommand extends Command { */ refresh() { const model = this.editor.model; - const selectedCells = getTableCellsInSelection( model.document.selection, true ); + const selectedCells = getSelectionAffectedTableCells( model.document.selection ); const isInTable = selectedCells.length > 0; this.isEnabled = isInTable; @@ -64,7 +64,7 @@ export default class SetHeaderRowCommand extends Command { execute( options = {} ) { const model = this.editor.model; - const selectedCells = getTableCellsInSelection( model.document.selection, true ); + const selectedCells = getSelectionAffectedTableCells( model.document.selection ); const firstCell = selectedCells[ 0 ]; const lastCell = selectedCells[ selectedCells.length - 1 ]; const table = firstCell.parent.parent;