From ebc6a14db8fa4ba7aa6773a429a649d65547446e Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Mon, 12 Aug 2019 15:23:15 +0200 Subject: [PATCH 01/11] Feature: `SetHeaderColumnCommand#execute()` and `SetHeaderRowCommand#execute()` now can take `forceValue` parameter. --- src/commands/setheadercolumncommand.js | 10 +++++-- src/commands/setheaderrowcommand.js | 22 ++++++++++----- tests/commands/setheadercolumncommand.js | 24 ++++++++++++++++ tests/commands/setheaderrowcommand.js | 36 ++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 9 deletions(-) diff --git a/src/commands/setheadercolumncommand.js b/src/commands/setheadercolumncommand.js index 8f18e798..0c9f3353 100644 --- a/src/commands/setheadercolumncommand.js +++ b/src/commands/setheadercolumncommand.js @@ -61,21 +61,27 @@ export default class SetHeaderColumnCommand extends Command { * When the selection is already in a header column, it will set `headingColumns` so the heading section will end before that column. * * @fires execute + * @params {Boolean} [forceValue] If set, the command will set (`true`) or unset (`false`) header columns according to `forceValue` + * parameter instead of the current model state. */ - execute() { + execute( forceValue = null ) { 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.parent ); + const tableCell = findAncestor( 'tableCell', position ); const tableRow = tableCell.parent; const table = tableRow.parent; const currentHeadingColumns = parseInt( table.getAttribute( 'headingColumns' ) || 0 ); const { column: selectionColumn } = tableUtils.getCellLocation( tableCell ); + if ( forceValue && currentHeadingColumns > selectionColumn || forceValue === false && currentHeadingColumns <= selectionColumn ) { + return; + } + const headingColumnsToSet = currentHeadingColumns > selectionColumn ? selectionColumn : selectionColumn + 1; model.change( writer => { diff --git a/src/commands/setheaderrowcommand.js b/src/commands/setheaderrowcommand.js index 0c1066f7..25c25cca 100644 --- a/src/commands/setheaderrowcommand.js +++ b/src/commands/setheaderrowcommand.js @@ -60,8 +60,10 @@ export default class SetHeaderRowCommand extends Command { * When the selection is already in a header row, it will set `headingRows` so the heading section will end before that row. * * @fires execute + * @params {Boolean} [forceValue] If set, the command will set (`true`) or unset (`false`) header rows according to `forceValue` + * parameter instead of the current model state. */ - execute() { + execute( forceValue = null ) { const model = this.editor.model; const doc = model.document; const selection = doc.selection; @@ -74,6 +76,10 @@ export default class SetHeaderRowCommand extends Command { const currentHeadingRows = table.getAttribute( 'headingRows' ) || 0; const selectionRow = tableRow.index; + if ( forceValue && currentHeadingRows > selectionRow || forceValue === false && currentHeadingRows <= selectionRow ) { + return; + } + const headingRowsToSet = currentHeadingRows > selectionRow ? selectionRow : selectionRow + 1; model.change( writer => { @@ -151,19 +157,21 @@ function splitHorizontally( tableCell, headingRows, writer ) { attributes.rowspan = spanToSet; } + const colspan = parseInt( tableCell.getAttribute( 'colspan' ) || 1 ); + + if ( colspan > 1 ) { + attributes.colspan = colspan; + } + const startRow = table.getChildIndex( tableRow ); const endRow = startRow + newRowspan; const tableMap = [ ...new TableWalker( table, { startRow, endRow, includeSpanned: true } ) ]; let columnIndex; - for ( const { row, column, cell, colspan, cellIndex } of tableMap ) { - if ( cell === tableCell ) { + for ( const { row, column, cell, cellIndex } of tableMap ) { + if ( cell === tableCell && columnIndex === undefined ) { columnIndex = column; - - if ( colspan > 1 ) { - attributes.colspan = colspan; - } } if ( columnIndex !== undefined && columnIndex === column && row === endRow ) { diff --git a/tests/commands/setheadercolumncommand.js b/tests/commands/setheadercolumncommand.js index a95b98a0..d145780d 100644 --- a/tests/commands/setheadercolumncommand.js +++ b/tests/commands/setheadercolumncommand.js @@ -115,5 +115,29 @@ describe( 'SetHeaderColumnCommand', () => { [ '00', '01[]', '02', '03' ] ], { headingColumns: 2 } ) ); } ); + + it( 'should respect forceValue parameter #1', () => { + setData( model, modelTable( [ + [ '00', '01[]', '02', '03' ] + ], { headingColumns: 3 } ) ); + + command.execute( true ); + + expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ + [ '00', '01[]', '02', '03' ] + ], { headingColumns: 3 } ) ); + } ); + + it( 'should respect forceValue parameter #2', () => { + setData( model, modelTable( [ + [ '00', '01[]', '02', '03' ] + ], { headingColumns: 1 } ) ); + + command.execute( false ); + + expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ + [ '00', '01[]', '02', '03' ] + ], { headingColumns: 1 } ) ); + } ); } ); } ); diff --git a/tests/commands/setheaderrowcommand.js b/tests/commands/setheaderrowcommand.js index 2cf95af5..8f1a557b 100644 --- a/tests/commands/setheaderrowcommand.js +++ b/tests/commands/setheaderrowcommand.js @@ -163,6 +163,42 @@ describe( 'SetHeaderRowCommand', () => { ] ) ); } ); + it( 'should respect forceValue parameter #1', () => { + setData( model, modelTable( [ + [ '00' ], + [ '[]10' ], + [ '20' ], + [ '30' ] + ], { headingRows: 3 } ) ); + + command.execute( true ); + + expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ + [ '00' ], + [ '[]10' ], + [ '20' ], + [ '30' ] + ], { headingRows: 3 } ) ); + } ); + + it( 'should respect forceValue parameter #2', () => { + setData( model, modelTable( [ + [ '00' ], + [ '[]10' ], + [ '20' ], + [ '30' ] + ], { headingRows: 1 } ) ); + + command.execute( false ); + + expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [ + [ '00' ], + [ '[]10' ], + [ '20' ], + [ '30' ] + ], { headingRows: 1 } ) ); + } ); + it( 'should fix rowspaned cells on the edge of an table head section', () => { setData( model, modelTable( [ [ '00', '01', '02' ], From 4f19a75eb7f54bc81254bf6c1b5f1a5b71d26ec9 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Mon, 12 Aug 2019 15:24:07 +0200 Subject: [PATCH 02/11] Docs: Fix in docs ASCII art. --- src/tableutils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tableutils.js b/src/tableutils.js index 69f7bfba..eedbf9b2 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -180,7 +180,7 @@ export default class TableUtils extends Plugin { * | | c | | | c | * +---+---+---+ will give: +---+---+---+---+---+ * | d | e | f | | d | | | e | f | - * +---+ +---+ +---+---+---+ +---+ + * +---+ +---+ +---+---+---+ +---+ * | g | | h | | g | | | | h | * +---+---+---+ +---+---+---+---+---+ * | i | | i | From 2b0ed055af014b1daa347c4fb2c76d5a59bf0be0 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Tue, 13 Aug 2019 09:17:18 +0200 Subject: [PATCH 03/11] Other: `TableWalker` will return cells also for spanned cells. Fixed returned `cellIndex` value for some cases when `includeSpanned` was `true`. Other refactor in code and docs. --- src/tablewalker.js | 146 +++++++++++++++++++++---------------------- tests/tablewalker.js | 45 ++++++++----- 2 files changed, 99 insertions(+), 92 deletions(-) diff --git a/src/tablewalker.js b/src/tablewalker.js index 2f140829..b6ec54e1 100644 --- a/src/tablewalker.js +++ b/src/tablewalker.js @@ -73,7 +73,7 @@ export default class TableWalker { * @param {Number} [options.column] A column index for which this iterator will output cells. * @param {Number} [options.startRow=0] A row index for which this iterator should start. * @param {Number} [options.endRow] A row index for which this iterator should end. - * @param {Boolean} [options.includeSpanned] Also return values for spanned cells. + * @param {Boolean} [options.includeSpanned=false] Also return values for spanned cells. */ constructor( table, options = {} ) { /** @@ -151,16 +151,18 @@ export default class TableWalker { * @member {Number} * @private */ - this._cell = 0; + this._cellIndex = 0; /** * Holds a map of spanned cells in a table. * * @readonly - * @member {Map>} + * @member {Map>} * @private */ this._spannedCells = new Map(); + + this._nextCellAtColumn = -1; } /** @@ -180,62 +182,53 @@ export default class TableWalker { next() { const row = this.table.getChild( this._row ); - // Iterator is done when no row (table end) or the row is after #endRow. + // Iterator is done when there's no row (table ended) or the row is after `endRow` limit. if ( !row || this._isOverEndRow() ) { return { done: true }; } - // Spanned cell location handling. + let cell, skipCurrentValue, outValue; + if ( this._isSpanned( this._row, this._column ) ) { - // Current column must be kept as it will be updated before returning current value. - const currentColumn = this._column; - const outValue = this._formatOutValue( undefined, currentColumn ); + cell = this._getSpanned( this._row, this._column ); - // Advance to next column - always. - this._column++; + skipCurrentValue = !this.includeSpanned || this._shouldSkipRow() || this._shouldSkipColumn(); + outValue = this._formatOutValue( cell, this._column ); + } else { + cell = row.getChild( this._cellIndex ); - const skipCurrentValue = !this.includeSpanned || this._shouldSkipRow() || this._shouldSkipColumn( currentColumn, 1 ); + if ( !cell ) { + // If there are no more cells left in row advance to the next row. + this._row++; + this._column = 0; + this._cellIndex = 0; + this._nextCellAtColumn = -1; - // The current value will be returned only if #includedSpanned=true and also current row and column are not skipped. - return skipCurrentValue ? this.next() : outValue; - } - - // The cell location is not spanned by other cells. - const cell = row.getChild( this._cell ); + return this.next(); + } - if ( !cell ) { - // If there are no more cells left in row advance to next row. - this._row++; - // And reset column & cell indexes. - this._column = 0; - this._cell = 0; + const colspan = parseInt( cell.getAttribute( 'colspan' ) || 1 ); + const rowspan = parseInt( cell.getAttribute( 'rowspan' ) || 1 ); - // Return next value. - return this.next(); - } + // Record this cell spans if it's not 1x1 cell. + if ( colspan > 1 || rowspan > 1 ) { + this._recordSpans( this._row, this._column, rowspan, colspan, cell ); + } - // Read table cell attributes. - const colspan = parseInt( cell.getAttribute( 'colspan' ) || 1 ); - const rowspan = parseInt( cell.getAttribute( 'rowspan' ) || 1 ); + this._nextCellAtColumn = this._column + colspan; - // Record this cell spans if it's not 1x1 cell. - if ( colspan > 1 || rowspan > 1 ) { - this._recordSpans( this._row, this._column, rowspan, colspan ); + skipCurrentValue = this._shouldSkipRow() || this._shouldSkipColumn(); + outValue = this._formatOutValue( cell, this._column, rowspan, colspan ); } - // Current column must be kept as it will be updated before returning current value. - const currentColumn = this._column; - const outValue = this._formatOutValue( cell, currentColumn, rowspan, colspan ); - - // Advance to next column before returning value. + // Advance to the next column before returning value. this._column++; - // Advance to next cell in a parent row before returning value. - this._cell++; - - const skipCurrentValue = this._shouldSkipRow() || this._shouldSkipColumn( currentColumn, colspan ); + if ( this._column == this._nextCellAtColumn ) { + this._cellIndex++; + } - // The current value will be returned only if current row & column are not skipped. + // The current value will be returned only if current row and column are not skipped. return skipCurrentValue ? this.next() : outValue; } @@ -252,8 +245,8 @@ export default class TableWalker { /** * Checks if the current row is over {@link #endRow}. * - * @returns {Boolean} * @private + * @returns {Boolean} */ _isOverEndRow() { // If {@link #endRow) is defined skip all rows above it. @@ -263,13 +256,12 @@ export default class TableWalker { /** * A common method for formatting the iterator's output value. * - * @param {module:engine/model/element~Element|undefined} cell The table cell to output. It might be undefined for spanned cell - * locations. - * @param {Number} column Column index (use the cached value) + * @private + * @param {module:engine/model/element~Element} cell The table cell to output. + * @param {Number} column Column index (use the cached value). * @param {Number} rowspan Rowspan of the current cell. * @param {Number} colspan Colspan of the current cell. * @returns {{done: boolean, value: {cell: *, row: Number, column: *, rowspan: *, colspan: *, cellIndex: Number}}} - * @private */ _formatOutValue( cell, column, rowspan = 1, colspan = 1 ) { return { @@ -280,7 +272,7 @@ export default class TableWalker { column, rowspan, colspan, - cellIndex: this._cell + cellIndex: this._cellIndex } }; } @@ -288,8 +280,8 @@ export default class TableWalker { /** * Checks if the current row should be skipped. * - * @returns {Boolean} * @private + * @returns {Boolean} */ _shouldSkipRow() { const rowIsBelowStartRow = this._row < this.startRow; @@ -301,33 +293,25 @@ export default class TableWalker { /** * Checks if the current column should be skipped. * - * @param {Number} column - * @param {Number} colspan - * @returns {Boolean} * @private + * @returns {Boolean} */ - _shouldSkipColumn( column, colspan ) { + _shouldSkipColumn() { if ( this.column === undefined ) { // The {@link #column} is not defined so output all columns. return false; } - // When outputting cells from given column we skip: - // - Cells that are not on that column. - const isCurrentColumn = column === this.column; - // - CSells that are before given column and they overlaps given column. - const isPreviousThatOverlapsColumn = column < this.column && column + colspan > this.column; - - return !isCurrentColumn && !isPreviousThatOverlapsColumn; + return this.column != this._column; } /** * Checks if the current cell location (row x column) is spanned by another cell. * + * @private * @param {Number} row Row index of a cell location to check. * @param {Number} column Column index of a cell location to check. * @returns {Boolean} - * @private */ _isSpanned( row, column ) { if ( !this._spannedCells.has( row ) ) { @@ -341,25 +325,38 @@ export default class TableWalker { return rowSpans.has( column ); } + /** + * Returns the cell element that is spanned at `row` x `column` location. + * + * @private + * @param {Number} row Row index of the cell location. + * @param {Number} column Column index of the cell location. + * @returns {module:engine/model/element~Element} + */ + _getSpanned( row, column ) { + return this._spannedCells.get( row ).get( column ); + } + /** * Updates spanned cells map relative to the current cell location and its span dimensions. * + * @private * @param {Number} row Row index of a cell. * @param {Number} column Column index of a cell. * @param {Number} rowspan Cell height. * @param {Number} colspan Cell width. - * @private + * @param {module:engine/model/element~Element} cell Cell that is spanned. */ - _recordSpans( row, column, rowspan, colspan ) { + _recordSpans( row, column, rowspan, colspan, cell ) { // This will update all cell locations after current column - ie a cell has colspan set. for ( let columnToUpdate = column + 1; columnToUpdate <= column + colspan - 1; columnToUpdate++ ) { - this._markSpannedCell( row, columnToUpdate ); + this._markSpannedCell( row, columnToUpdate, cell ); } // This will update all rows below current up to row's height. for ( let rowToUpdate = row + 1; rowToUpdate < row + rowspan; rowToUpdate++ ) { for ( let columnToUpdate = column; columnToUpdate <= column + colspan - 1; columnToUpdate++ ) { - this._markSpannedCell( rowToUpdate, columnToUpdate ); + this._markSpannedCell( rowToUpdate, columnToUpdate, cell ); } } } @@ -367,18 +364,19 @@ export default class TableWalker { /** * Marks the cell location as spanned by another cell. * + * @private * @param {Number} row Row index of the cell location. * @param {Number} column Column index of the cell location. - * @private + * @param {module:engine/model/element~Element} cell Cell that is spanned. */ - _markSpannedCell( row, column ) { + _markSpannedCell( row, column, cell ) { if ( !this._spannedCells.has( row ) ) { this._spannedCells.set( row, new Map() ); } const rowSpans = this._spannedCells.get( row ); - rowSpans.set( column, true ); + rowSpans.set( column, cell ); } } @@ -386,14 +384,10 @@ export default class TableWalker { * An object returned by {@link module:table/tablewalker~TableWalker} when traversing table cells. * * @typedef {Object} module:table/tablewalker~TableWalkerValue - * @property {module:engine/model/element~Element} [cell] The current table cell. Might be empty if - * {@link module:table/tablewalker~TableWalker#includeSpanned} is set to `true`. + * @property {module:engine/model/element~Element} cell The current table cell. * @property {Number} row The row index of a cell. * @property {Number} column The column index of a cell. Column index is adjusted to widths and heights of previous cells. - * @property {Number} [colspan] The `colspan` attribute of a cell. It is always defined even if the model attribute is not present. Not - * set if {@link module:table/tablewalker~TableWalker#includeSpanned} is set to `true`. - * @property {Number} [rowspan] The `rowspan` attribute of a cell. It is always defined even if the model attribute is not present. Not - * set if {@link module:table/tablewalker~TableWalker#includeSpanned} is set to `true`. - * @property {Number} cellIndex The index of the current cell in a parent row. When using the `includeSpanned` option it will indicate the - * next child index if #cell is empty (which indicates that the cell is spanned by another cell). + * @property {Number} colspan The `colspan` attribute of a cell. It is always defined even if the model attribute is not present. + * @property {Number} rowspan The `rowspan` attribute of a cell. It is always defined even if the model attribute is not present. + * @property {Number} cellIndex The index of the current cell in a parent row. */ diff --git a/tests/tablewalker.js b/tests/tablewalker.js index e059ab20..d912a868 100644 --- a/tests/tablewalker.js +++ b/tests/tablewalker.js @@ -157,11 +157,11 @@ describe( 'TableWalker', () => { { row: 0, column: 0, index: 0, data: '00' }, { row: 0, column: 1, index: 1, data: '01' }, { row: 1, column: 0, index: 0, data: '10' }, - { row: 1, column: 1, index: 1, data: undefined } + { row: 1, column: 1, index: 1, data: '01' } ], { includeSpanned: true } ); } ); - it( 'should output spanned cells as empty cell', () => { + it( 'should output spanned cells', () => { testWalker( [ [ { colspan: 2, rowspan: 3, contents: '00' }, '02' ], [ '12' ], @@ -169,17 +169,17 @@ describe( 'TableWalker', () => { [ '30', { colspan: 2, contents: '31' } ] ], [ { row: 0, column: 0, index: 0, data: '00' }, - { row: 0, column: 1, index: 1, data: undefined }, + { row: 0, column: 1, index: 0, data: '00' }, { row: 0, column: 2, index: 1, data: '02' }, - { row: 1, column: 0, index: 0, data: undefined }, - { row: 1, column: 1, index: 0, data: undefined }, + { row: 1, column: 0, index: 0, data: '00' }, + { row: 1, column: 1, index: 0, data: '00' }, { row: 1, column: 2, index: 0, data: '12' }, - { row: 2, column: 0, index: 0, data: undefined }, - { row: 2, column: 1, index: 0, data: undefined }, + { row: 2, column: 0, index: 0, data: '00' }, + { row: 2, column: 1, index: 0, data: '00' }, { row: 2, column: 2, index: 0, data: '22' }, { row: 3, column: 0, index: 0, data: '30' }, { row: 3, column: 1, index: 1, data: '31' }, - { row: 3, column: 2, index: 2, data: undefined } + { row: 3, column: 2, index: 1, data: '31' } ], { includeSpanned: true } ); } ); @@ -191,7 +191,7 @@ describe( 'TableWalker', () => { { row: 0, column: 0, index: 0, data: '00' }, { row: 0, column: 1, index: 1, data: '01' }, { row: 1, column: 0, index: 0, data: '10' }, - { row: 1, column: 1, index: 1, data: undefined } + { row: 1, column: 1, index: 1, data: '01' } ], { includeSpanned: true } ); } ); @@ -202,16 +202,16 @@ describe( 'TableWalker', () => { [ '22' ], [ '30', '31', '32' ] ], [ - { row: 1, column: 0, index: 0, data: undefined }, - { row: 1, column: 1, index: 0, data: undefined }, + { row: 1, column: 0, index: 0, data: '00' }, + { row: 1, column: 1, index: 0, data: '00' }, { row: 1, column: 2, index: 0, data: '12' }, - { row: 2, column: 0, index: 0, data: undefined }, - { row: 2, column: 1, index: 0, data: undefined }, + { row: 2, column: 0, index: 0, data: '00' }, + { row: 2, column: 1, index: 0, data: '00' }, { row: 2, column: 2, index: 0, data: '22' } ], { includeSpanned: true, startRow: 1, endRow: 2 } ); } ); - it( 'should output rowspanned cells at the end of a table row', () => { + it( 'should output rowspanned cells at the end of a table row with startRow & endRow options', () => { testWalker( [ [ '00', { rowspan: 2, contents: '01' } ], [ '10' ], @@ -220,7 +220,7 @@ describe( 'TableWalker', () => { { row: 0, column: 0, index: 0, data: '00' }, { row: 0, column: 1, index: 1, data: '01' }, { row: 1, column: 0, index: 0, data: '10' }, - { row: 1, column: 1, index: 1, data: undefined } + { row: 1, column: 1, index: 1, data: '01' } ], { startRow: 0, endRow: 1, includeSpanned: true } ); } ); } ); @@ -233,9 +233,22 @@ describe( 'TableWalker', () => { [ '22' ], [ '30', '31', '32' ] ], [ - { row: 0, column: 0, index: 0, data: '00' }, { row: 3, column: 1, index: 1, data: '31' } ], { column: 1 } ); } ); + + it( 'should output only cells on given column, includeSpanned = true', () => { + testWalker( [ + [ { colspan: 2, rowspan: 3, contents: '00' }, '02' ], + [ '12' ], + [ '22' ], + [ '30', '31', '32' ] + ], [ + { row: 0, column: 1, index: 0, data: '00' }, + { row: 1, column: 1, index: 0, data: '00' }, + { row: 2, column: 1, index: 0, data: '00' }, + { row: 3, column: 1, index: 1, data: '31' } + ], { column: 1, includeSpanned: true } ); + } ); } ); } ); From e4e03660ec9f66bf9a19b814ee0a9c16b943c632 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Tue, 13 Aug 2019 13:12:34 +0200 Subject: [PATCH 04/11] Fix: Removed table cell view element will be unbound in the mapper. --- src/converters/downcast.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/converters/downcast.js b/src/converters/downcast.js index 725a627e..8162ca17 100644 --- a/src/converters/downcast.js +++ b/src/converters/downcast.js @@ -345,6 +345,7 @@ function renameViewTableCell( tableCell, desiredCellElementName, conversionApi, renamedCell = viewWriter.rename( desiredCellElementName, viewCell ); } + conversionApi.mapper.unbindViewElement( viewCell ); conversionApi.mapper.bindElements( tableCell, renamedCell ); } From af961ef40f829dafc543ad1e09cbeea1c4426caa Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Tue, 13 Aug 2019 13:12:59 +0200 Subject: [PATCH 05/11] Internal: Refactored `tableUtils.insertColumns` after changes in `TableWalker`. --- src/tableutils.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/tableutils.js b/src/tableutils.js index eedbf9b2..b2ad5f37 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -219,13 +219,16 @@ export default class TableUtils extends Plugin { const tableWalker = new TableWalker( table, { column: insertAt, includeSpanned: true } ); - for ( const { row, column, cell, colspan, rowspan, cellIndex } of tableWalker ) { + for ( const { row, cell, cellIndex } of tableWalker ) { // When iterating over column the table walker outputs either: // - cells at given column index (cell "e" from method docs), // - spanned columns (spanned cell from row between cells "g" and "h" - spanned by "e", only if `includeSpanned: true`), // - or a cell from the same row which spans over this column (cell "a"). - if ( column !== insertAt ) { + const rowspan = parseInt( cell.getAttribute( 'rowspan' ) || 1 ); + const colspan = parseInt( cell.getAttribute( 'colspan' ) || 1 ); + + if ( cell.index !== insertAt && colspan > 1 ) { // If column is different than `insertAt`, it is a cell that spans over an inserted column (cell "a" & "i"). // For such cells expand them by a number of columns inserted. writer.setAttribute( 'colspan', colspan + columnsToInsert, cell ); From 2531644c227a1f12247fabaa9c04858a464ef86e Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Wed, 14 Aug 2019 11:15:27 +0200 Subject: [PATCH 06/11] Internal: Refactored `SetHeaderColumnCommand#execute()` and `SetHeaderRowCommand#execute()` to use `#value` for simpler code. --- src/commands/setheadercolumncommand.js | 5 ++--- src/commands/setheaderrowcommand.js | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/commands/setheadercolumncommand.js b/src/commands/setheadercolumncommand.js index 0c9f3353..5dee56c4 100644 --- a/src/commands/setheadercolumncommand.js +++ b/src/commands/setheadercolumncommand.js @@ -75,14 +75,13 @@ export default class SetHeaderColumnCommand extends Command { const tableRow = tableCell.parent; const table = tableRow.parent; - const currentHeadingColumns = parseInt( table.getAttribute( 'headingColumns' ) || 0 ); const { column: selectionColumn } = tableUtils.getCellLocation( tableCell ); - if ( forceValue && currentHeadingColumns > selectionColumn || forceValue === false && currentHeadingColumns <= selectionColumn ) { + if ( forceValue === this.value ) { return; } - const headingColumnsToSet = currentHeadingColumns > selectionColumn ? selectionColumn : selectionColumn + 1; + const headingColumnsToSet = this.value ? selectionColumn : selectionColumn + 1; model.change( writer => { updateNumericAttribute( 'headingColumns', headingColumnsToSet, table, writer, 0 ); diff --git a/src/commands/setheaderrowcommand.js b/src/commands/setheaderrowcommand.js index 25c25cca..e29aea90 100644 --- a/src/commands/setheaderrowcommand.js +++ b/src/commands/setheaderrowcommand.js @@ -76,11 +76,11 @@ export default class SetHeaderRowCommand extends Command { const currentHeadingRows = table.getAttribute( 'headingRows' ) || 0; const selectionRow = tableRow.index; - if ( forceValue && currentHeadingRows > selectionRow || forceValue === false && currentHeadingRows <= selectionRow ) { + if ( forceValue === this.value ) { return; } - const headingRowsToSet = currentHeadingRows > selectionRow ? selectionRow : selectionRow + 1; + const headingRowsToSet = this.value ? selectionRow : selectionRow + 1; model.change( writer => { if ( headingRowsToSet ) { From 594fb72d8fa0050ce2665e53ed2e65fc7b7877ea Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Wed, 14 Aug 2019 11:32:51 +0200 Subject: [PATCH 07/11] Feature: Introduced `TableWalkerValue#isSpanned`. --- src/tablewalker.js | 9 ++- tests/tablewalker.js | 139 ++++++++++++++++++++++--------------------- 2 files changed, 76 insertions(+), 72 deletions(-) diff --git a/src/tablewalker.js b/src/tablewalker.js index b6ec54e1..f4a6b536 100644 --- a/src/tablewalker.js +++ b/src/tablewalker.js @@ -193,7 +193,7 @@ export default class TableWalker { cell = this._getSpanned( this._row, this._column ); skipCurrentValue = !this.includeSpanned || this._shouldSkipRow() || this._shouldSkipColumn(); - outValue = this._formatOutValue( cell, this._column ); + outValue = this._formatOutValue( cell, this._column, true ); } else { cell = row.getChild( this._cellIndex ); @@ -218,7 +218,7 @@ export default class TableWalker { this._nextCellAtColumn = this._column + colspan; skipCurrentValue = this._shouldSkipRow() || this._shouldSkipColumn(); - outValue = this._formatOutValue( cell, this._column, rowspan, colspan ); + outValue = this._formatOutValue( cell, this._column, false, rowspan, colspan ); } // Advance to the next column before returning value. @@ -259,17 +259,19 @@ export default class TableWalker { * @private * @param {module:engine/model/element~Element} cell The table cell to output. * @param {Number} column Column index (use the cached value). + * @param {Boolean} isSpanned Whether the value is returned for a spanned cell location or actual cell. * @param {Number} rowspan Rowspan of the current cell. * @param {Number} colspan Colspan of the current cell. * @returns {{done: boolean, value: {cell: *, row: Number, column: *, rowspan: *, colspan: *, cellIndex: Number}}} */ - _formatOutValue( cell, column, rowspan = 1, colspan = 1 ) { + _formatOutValue( cell, column, isSpanned, rowspan = 1, colspan = 1 ) { return { done: false, value: { cell, row: this._row, column, + isSpanned, rowspan, colspan, cellIndex: this._cellIndex @@ -387,6 +389,7 @@ export default class TableWalker { * @property {module:engine/model/element~Element} cell The current table cell. * @property {Number} row The row index of a cell. * @property {Number} column The column index of a cell. Column index is adjusted to widths and heights of previous cells. + * @param {Boolean} isSpanned Whether the value is returned for a spanned cell location or actual cell. * @property {Number} colspan The `colspan` attribute of a cell. It is always defined even if the model attribute is not present. * @property {Number} rowspan The `rowspan` attribute of a cell. It is always defined even if the model attribute is not present. * @property {Number} cellIndex The index of the current cell in a parent row. diff --git a/tests/tablewalker.js b/tests/tablewalker.js index d912a868..5584870d 100644 --- a/tests/tablewalker.js +++ b/tests/tablewalker.js @@ -37,9 +37,10 @@ describe( 'TableWalker', () => { result.push( tableInfo ); } - const formattedResult = result.map( ( { row, column, cell, cellIndex } ) => ( { + const formattedResult = result.map( ( { row, column, isSpanned, cell, cellIndex } ) => ( { row, column, + isSpanned, data: cell && cell.getChild( 0 ).getChild( 0 ).data, index: cellIndex } ) ); @@ -52,10 +53,10 @@ describe( 'TableWalker', () => { [ '00', '01' ], [ '10', '11' ] ], [ - { row: 0, column: 0, index: 0, data: '00' }, - { row: 0, column: 1, index: 1, data: '01' }, - { row: 1, column: 0, index: 0, data: '10' }, - { row: 1, column: 1, index: 1, data: '11' } + { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, + { row: 0, column: 1, index: 1, data: '01', isSpanned: false }, + { row: 1, column: 0, index: 0, data: '10', isSpanned: false }, + { row: 1, column: 1, index: 1, data: '11', isSpanned: false } ] ); } ); @@ -63,8 +64,8 @@ describe( 'TableWalker', () => { testWalker( [ [ { colspan: 2, contents: '00' }, '13' ] ], [ - { row: 0, column: 0, index: 0, data: '00' }, - { row: 0, column: 2, index: 1, data: '13' } + { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, + { row: 0, column: 2, index: 1, data: '13', isSpanned: false } ] ); } ); @@ -75,13 +76,13 @@ describe( 'TableWalker', () => { [ '22' ], [ '30', '31', '32' ] ], [ - { row: 0, column: 0, index: 0, data: '00' }, - { row: 0, column: 2, index: 1, data: '02' }, - { row: 1, column: 2, index: 0, data: '12' }, - { row: 2, column: 2, index: 0, data: '22' }, - { row: 3, column: 0, index: 0, data: '30' }, - { row: 3, column: 1, index: 1, data: '31' }, - { row: 3, column: 2, index: 2, data: '32' } + { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, + { row: 0, column: 2, index: 1, data: '02', isSpanned: false }, + { row: 1, column: 2, index: 0, data: '12', isSpanned: false }, + { row: 2, column: 2, index: 0, data: '22', isSpanned: false }, + { row: 3, column: 0, index: 0, data: '30', isSpanned: false }, + { row: 3, column: 1, index: 1, data: '31', isSpanned: false }, + { row: 3, column: 2, index: 2, data: '32', isSpanned: false } ] ); } ); @@ -92,15 +93,15 @@ describe( 'TableWalker', () => { [ '33' ], [ '41', '42', '43' ] ], [ - { row: 0, column: 0, index: 0, data: '11' }, - { row: 0, column: 1, index: 1, data: '12' }, - { row: 0, column: 2, index: 2, data: '13' }, - { row: 1, column: 1, index: 0, data: '22' }, - { row: 1, column: 2, index: 1, data: '23' }, - { row: 2, column: 2, index: 0, data: '33' }, - { row: 3, column: 0, index: 0, data: '41' }, - { row: 3, column: 1, index: 1, data: '42' }, - { row: 3, column: 2, index: 2, data: '43' } + { row: 0, column: 0, index: 0, data: '11', isSpanned: false }, + { row: 0, column: 1, index: 1, data: '12', isSpanned: false }, + { row: 0, column: 2, index: 2, data: '13', isSpanned: false }, + { row: 1, column: 1, index: 0, data: '22', isSpanned: false }, + { row: 1, column: 2, index: 1, data: '23', isSpanned: false }, + { row: 2, column: 2, index: 0, data: '33', isSpanned: false }, + { row: 3, column: 0, index: 0, data: '41', isSpanned: false }, + { row: 3, column: 1, index: 1, data: '42', isSpanned: false }, + { row: 3, column: 2, index: 2, data: '43', isSpanned: false } ] ); } ); @@ -112,10 +113,10 @@ describe( 'TableWalker', () => { [ '33' ], [ '41', '42', '43' ] ], [ - { row: 2, column: 2, index: 0, data: '33' }, - { row: 3, column: 0, index: 0, data: '41' }, - { row: 3, column: 1, index: 1, data: '42' }, - { row: 3, column: 2, index: 2, data: '43' } + { row: 2, column: 2, index: 0, data: '33', isSpanned: false }, + { row: 3, column: 0, index: 0, data: '41', isSpanned: false }, + { row: 3, column: 1, index: 1, data: '42', isSpanned: false }, + { row: 3, column: 2, index: 2, data: '43', isSpanned: false } ], { startRow: 2 } ); } ); } ); @@ -128,22 +129,22 @@ describe( 'TableWalker', () => { [ '33' ], [ '41', '42', '43' ] ], [ - { row: 0, column: 0, index: 0, data: '11' }, - { row: 0, column: 2, index: 1, data: '13' }, - { row: 1, column: 2, index: 0, data: '23' }, - { row: 2, column: 2, index: 0, data: '33' } + { row: 0, column: 0, index: 0, data: '11', isSpanned: false }, + { row: 0, column: 2, index: 1, data: '13', isSpanned: false }, + { row: 1, column: 2, index: 0, data: '23', isSpanned: false }, + { row: 2, column: 2, index: 0, data: '33', isSpanned: false } ], { endRow: 2 } ); } ); - it( 'should iterate over given row 0 only', () => { + it( 'should iterate over given row only', () => { testWalker( [ [ { colspan: 2, rowspan: 3, contents: '11' }, '13' ], [ '23' ], [ '33' ], [ '41', '42', '43' ] ], [ - { row: 0, column: 0, index: 0, data: '11' }, - { row: 0, column: 2, index: 1, data: '13' } + { row: 0, column: 0, index: 0, data: '11', isSpanned: false }, + { row: 0, column: 2, index: 1, data: '13', isSpanned: false } ], { endRow: 0 } ); } ); } ); @@ -154,10 +155,10 @@ describe( 'TableWalker', () => { [ '00', { rowspan: 2, contents: '01' } ], [ '10' ] ], [ - { row: 0, column: 0, index: 0, data: '00' }, - { row: 0, column: 1, index: 1, data: '01' }, - { row: 1, column: 0, index: 0, data: '10' }, - { row: 1, column: 1, index: 1, data: '01' } + { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, + { row: 0, column: 1, index: 1, data: '01', isSpanned: false }, + { row: 1, column: 0, index: 0, data: '10', isSpanned: false }, + { row: 1, column: 1, index: 1, data: '01', isSpanned: true } ], { includeSpanned: true } ); } ); @@ -168,18 +169,18 @@ describe( 'TableWalker', () => { [ '22' ], [ '30', { colspan: 2, contents: '31' } ] ], [ - { row: 0, column: 0, index: 0, data: '00' }, - { row: 0, column: 1, index: 0, data: '00' }, - { row: 0, column: 2, index: 1, data: '02' }, - { row: 1, column: 0, index: 0, data: '00' }, - { row: 1, column: 1, index: 0, data: '00' }, - { row: 1, column: 2, index: 0, data: '12' }, - { row: 2, column: 0, index: 0, data: '00' }, - { row: 2, column: 1, index: 0, data: '00' }, - { row: 2, column: 2, index: 0, data: '22' }, - { row: 3, column: 0, index: 0, data: '30' }, - { row: 3, column: 1, index: 1, data: '31' }, - { row: 3, column: 2, index: 1, data: '31' } + { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, + { row: 0, column: 1, index: 0, data: '00', isSpanned: true }, + { row: 0, column: 2, index: 1, data: '02', isSpanned: false }, + { row: 1, column: 0, index: 0, data: '00', isSpanned: true }, + { row: 1, column: 1, index: 0, data: '00', isSpanned: true }, + { row: 1, column: 2, index: 0, data: '12', isSpanned: false }, + { row: 2, column: 0, index: 0, data: '00', isSpanned: true }, + { row: 2, column: 1, index: 0, data: '00', isSpanned: true }, + { row: 2, column: 2, index: 0, data: '22', isSpanned: false }, + { row: 3, column: 0, index: 0, data: '30', isSpanned: false }, + { row: 3, column: 1, index: 1, data: '31', isSpanned: false }, + { row: 3, column: 2, index: 1, data: '31', isSpanned: true } ], { includeSpanned: true } ); } ); @@ -188,10 +189,10 @@ describe( 'TableWalker', () => { [ '00', { rowspan: 2, contents: '01' } ], [ '10' ] ], [ - { row: 0, column: 0, index: 0, data: '00' }, - { row: 0, column: 1, index: 1, data: '01' }, - { row: 1, column: 0, index: 0, data: '10' }, - { row: 1, column: 1, index: 1, data: '01' } + { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, + { row: 0, column: 1, index: 1, data: '01', isSpanned: false }, + { row: 1, column: 0, index: 0, data: '10', isSpanned: false }, + { row: 1, column: 1, index: 1, data: '01', isSpanned: true } ], { includeSpanned: true } ); } ); @@ -202,12 +203,12 @@ describe( 'TableWalker', () => { [ '22' ], [ '30', '31', '32' ] ], [ - { row: 1, column: 0, index: 0, data: '00' }, - { row: 1, column: 1, index: 0, data: '00' }, - { row: 1, column: 2, index: 0, data: '12' }, - { row: 2, column: 0, index: 0, data: '00' }, - { row: 2, column: 1, index: 0, data: '00' }, - { row: 2, column: 2, index: 0, data: '22' } + { row: 1, column: 0, index: 0, data: '00', isSpanned: true }, + { row: 1, column: 1, index: 0, data: '00', isSpanned: true }, + { row: 1, column: 2, index: 0, data: '12', isSpanned: false }, + { row: 2, column: 0, index: 0, data: '00', isSpanned: true }, + { row: 2, column: 1, index: 0, data: '00', isSpanned: true }, + { row: 2, column: 2, index: 0, data: '22', isSpanned: false } ], { includeSpanned: true, startRow: 1, endRow: 2 } ); } ); @@ -217,10 +218,10 @@ describe( 'TableWalker', () => { [ '10' ], [ '20', '21' ] ], [ - { row: 0, column: 0, index: 0, data: '00' }, - { row: 0, column: 1, index: 1, data: '01' }, - { row: 1, column: 0, index: 0, data: '10' }, - { row: 1, column: 1, index: 1, data: '01' } + { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, + { row: 0, column: 1, index: 1, data: '01', isSpanned: false }, + { row: 1, column: 0, index: 0, data: '10', isSpanned: false }, + { row: 1, column: 1, index: 1, data: '01', isSpanned: true } ], { startRow: 0, endRow: 1, includeSpanned: true } ); } ); } ); @@ -233,7 +234,7 @@ describe( 'TableWalker', () => { [ '22' ], [ '30', '31', '32' ] ], [ - { row: 3, column: 1, index: 1, data: '31' } + { row: 3, column: 1, index: 1, data: '31', isSpanned: false } ], { column: 1 } ); } ); @@ -244,10 +245,10 @@ describe( 'TableWalker', () => { [ '22' ], [ '30', '31', '32' ] ], [ - { row: 0, column: 1, index: 0, data: '00' }, - { row: 1, column: 1, index: 0, data: '00' }, - { row: 2, column: 1, index: 0, data: '00' }, - { row: 3, column: 1, index: 1, data: '31' } + { row: 0, column: 1, index: 0, data: '00', isSpanned: true }, + { row: 1, column: 1, index: 0, data: '00', isSpanned: true }, + { row: 2, column: 1, index: 0, data: '00', isSpanned: true }, + { row: 3, column: 1, index: 1, data: '31', isSpanned: false } ], { column: 1, includeSpanned: true } ); } ); } ); From 328b3fa6a67757dd46b1f53e22ca372348e24a03 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Wed, 14 Aug 2019 11:34:01 +0200 Subject: [PATCH 08/11] Docs: Small fix. --- src/tablewalker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tablewalker.js b/src/tablewalker.js index f4a6b536..0e89b443 100644 --- a/src/tablewalker.js +++ b/src/tablewalker.js @@ -328,7 +328,7 @@ export default class TableWalker { } /** - * Returns the cell element that is spanned at `row` x `column` location. + * Returns the cell element that is spanned over `row` x `column` location. * * @private * @param {Number} row Row index of the cell location. From 675d914be77e85bf3fc6f83ad124b8597ed4a7f3 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Wed, 14 Aug 2019 12:07:19 +0200 Subject: [PATCH 09/11] Tests: Made `isSpanned` a default value so it doesn't have to be passed in test expecteds. --- tests/tablewalker.js | 123 +++++++++++++++++++++++-------------------- 1 file changed, 65 insertions(+), 58 deletions(-) diff --git a/tests/tablewalker.js b/tests/tablewalker.js index 5584870d..d3c98cb1 100644 --- a/tests/tablewalker.js +++ b/tests/tablewalker.js @@ -37,13 +37,20 @@ describe( 'TableWalker', () => { result.push( tableInfo ); } - const formattedResult = result.map( ( { row, column, isSpanned, cell, cellIndex } ) => ( { - row, - column, - isSpanned, - data: cell && cell.getChild( 0 ).getChild( 0 ).data, - index: cellIndex - } ) ); + const formattedResult = result.map( ( { row, column, isSpanned, cell, cellIndex } ) => { + const result = { + row, + column, + data: cell && cell.getChild( 0 ).getChild( 0 ).data, + index: cellIndex + }; + + if ( isSpanned ) { + result.isSpanned = true; + } + + return result; + } ); expect( formattedResult ).to.deep.equal( expected ); } @@ -53,10 +60,10 @@ describe( 'TableWalker', () => { [ '00', '01' ], [ '10', '11' ] ], [ - { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, - { row: 0, column: 1, index: 1, data: '01', isSpanned: false }, - { row: 1, column: 0, index: 0, data: '10', isSpanned: false }, - { row: 1, column: 1, index: 1, data: '11', isSpanned: false } + { row: 0, column: 0, index: 0, data: '00' }, + { row: 0, column: 1, index: 1, data: '01' }, + { row: 1, column: 0, index: 0, data: '10' }, + { row: 1, column: 1, index: 1, data: '11' } ] ); } ); @@ -64,8 +71,8 @@ describe( 'TableWalker', () => { testWalker( [ [ { colspan: 2, contents: '00' }, '13' ] ], [ - { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, - { row: 0, column: 2, index: 1, data: '13', isSpanned: false } + { row: 0, column: 0, index: 0, data: '00' }, + { row: 0, column: 2, index: 1, data: '13' } ] ); } ); @@ -76,13 +83,13 @@ describe( 'TableWalker', () => { [ '22' ], [ '30', '31', '32' ] ], [ - { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, - { row: 0, column: 2, index: 1, data: '02', isSpanned: false }, - { row: 1, column: 2, index: 0, data: '12', isSpanned: false }, - { row: 2, column: 2, index: 0, data: '22', isSpanned: false }, - { row: 3, column: 0, index: 0, data: '30', isSpanned: false }, - { row: 3, column: 1, index: 1, data: '31', isSpanned: false }, - { row: 3, column: 2, index: 2, data: '32', isSpanned: false } + { row: 0, column: 0, index: 0, data: '00' }, + { row: 0, column: 2, index: 1, data: '02' }, + { row: 1, column: 2, index: 0, data: '12' }, + { row: 2, column: 2, index: 0, data: '22' }, + { row: 3, column: 0, index: 0, data: '30' }, + { row: 3, column: 1, index: 1, data: '31' }, + { row: 3, column: 2, index: 2, data: '32' } ] ); } ); @@ -93,15 +100,15 @@ describe( 'TableWalker', () => { [ '33' ], [ '41', '42', '43' ] ], [ - { row: 0, column: 0, index: 0, data: '11', isSpanned: false }, - { row: 0, column: 1, index: 1, data: '12', isSpanned: false }, - { row: 0, column: 2, index: 2, data: '13', isSpanned: false }, - { row: 1, column: 1, index: 0, data: '22', isSpanned: false }, - { row: 1, column: 2, index: 1, data: '23', isSpanned: false }, - { row: 2, column: 2, index: 0, data: '33', isSpanned: false }, - { row: 3, column: 0, index: 0, data: '41', isSpanned: false }, - { row: 3, column: 1, index: 1, data: '42', isSpanned: false }, - { row: 3, column: 2, index: 2, data: '43', isSpanned: false } + { row: 0, column: 0, index: 0, data: '11' }, + { row: 0, column: 1, index: 1, data: '12' }, + { row: 0, column: 2, index: 2, data: '13' }, + { row: 1, column: 1, index: 0, data: '22' }, + { row: 1, column: 2, index: 1, data: '23' }, + { row: 2, column: 2, index: 0, data: '33' }, + { row: 3, column: 0, index: 0, data: '41' }, + { row: 3, column: 1, index: 1, data: '42' }, + { row: 3, column: 2, index: 2, data: '43' } ] ); } ); @@ -113,10 +120,10 @@ describe( 'TableWalker', () => { [ '33' ], [ '41', '42', '43' ] ], [ - { row: 2, column: 2, index: 0, data: '33', isSpanned: false }, - { row: 3, column: 0, index: 0, data: '41', isSpanned: false }, - { row: 3, column: 1, index: 1, data: '42', isSpanned: false }, - { row: 3, column: 2, index: 2, data: '43', isSpanned: false } + { row: 2, column: 2, index: 0, data: '33' }, + { row: 3, column: 0, index: 0, data: '41' }, + { row: 3, column: 1, index: 1, data: '42' }, + { row: 3, column: 2, index: 2, data: '43' } ], { startRow: 2 } ); } ); } ); @@ -129,10 +136,10 @@ describe( 'TableWalker', () => { [ '33' ], [ '41', '42', '43' ] ], [ - { row: 0, column: 0, index: 0, data: '11', isSpanned: false }, - { row: 0, column: 2, index: 1, data: '13', isSpanned: false }, - { row: 1, column: 2, index: 0, data: '23', isSpanned: false }, - { row: 2, column: 2, index: 0, data: '33', isSpanned: false } + { row: 0, column: 0, index: 0, data: '11' }, + { row: 0, column: 2, index: 1, data: '13' }, + { row: 1, column: 2, index: 0, data: '23' }, + { row: 2, column: 2, index: 0, data: '33' } ], { endRow: 2 } ); } ); @@ -143,8 +150,8 @@ describe( 'TableWalker', () => { [ '33' ], [ '41', '42', '43' ] ], [ - { row: 0, column: 0, index: 0, data: '11', isSpanned: false }, - { row: 0, column: 2, index: 1, data: '13', isSpanned: false } + { row: 0, column: 0, index: 0, data: '11' }, + { row: 0, column: 2, index: 1, data: '13' } ], { endRow: 0 } ); } ); } ); @@ -155,9 +162,9 @@ describe( 'TableWalker', () => { [ '00', { rowspan: 2, contents: '01' } ], [ '10' ] ], [ - { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, - { row: 0, column: 1, index: 1, data: '01', isSpanned: false }, - { row: 1, column: 0, index: 0, data: '10', isSpanned: false }, + { row: 0, column: 0, index: 0, data: '00' }, + { row: 0, column: 1, index: 1, data: '01' }, + { row: 1, column: 0, index: 0, data: '10' }, { row: 1, column: 1, index: 1, data: '01', isSpanned: true } ], { includeSpanned: true } ); } ); @@ -169,17 +176,17 @@ describe( 'TableWalker', () => { [ '22' ], [ '30', { colspan: 2, contents: '31' } ] ], [ - { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, + { row: 0, column: 0, index: 0, data: '00' }, { row: 0, column: 1, index: 0, data: '00', isSpanned: true }, - { row: 0, column: 2, index: 1, data: '02', isSpanned: false }, + { row: 0, column: 2, index: 1, data: '02' }, { row: 1, column: 0, index: 0, data: '00', isSpanned: true }, { row: 1, column: 1, index: 0, data: '00', isSpanned: true }, - { row: 1, column: 2, index: 0, data: '12', isSpanned: false }, + { row: 1, column: 2, index: 0, data: '12' }, { row: 2, column: 0, index: 0, data: '00', isSpanned: true }, { row: 2, column: 1, index: 0, data: '00', isSpanned: true }, - { row: 2, column: 2, index: 0, data: '22', isSpanned: false }, - { row: 3, column: 0, index: 0, data: '30', isSpanned: false }, - { row: 3, column: 1, index: 1, data: '31', isSpanned: false }, + { row: 2, column: 2, index: 0, data: '22' }, + { row: 3, column: 0, index: 0, data: '30' }, + { row: 3, column: 1, index: 1, data: '31' }, { row: 3, column: 2, index: 1, data: '31', isSpanned: true } ], { includeSpanned: true } ); } ); @@ -189,9 +196,9 @@ describe( 'TableWalker', () => { [ '00', { rowspan: 2, contents: '01' } ], [ '10' ] ], [ - { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, - { row: 0, column: 1, index: 1, data: '01', isSpanned: false }, - { row: 1, column: 0, index: 0, data: '10', isSpanned: false }, + { row: 0, column: 0, index: 0, data: '00' }, + { row: 0, column: 1, index: 1, data: '01' }, + { row: 1, column: 0, index: 0, data: '10' }, { row: 1, column: 1, index: 1, data: '01', isSpanned: true } ], { includeSpanned: true } ); } ); @@ -205,10 +212,10 @@ describe( 'TableWalker', () => { ], [ { row: 1, column: 0, index: 0, data: '00', isSpanned: true }, { row: 1, column: 1, index: 0, data: '00', isSpanned: true }, - { row: 1, column: 2, index: 0, data: '12', isSpanned: false }, + { row: 1, column: 2, index: 0, data: '12' }, { row: 2, column: 0, index: 0, data: '00', isSpanned: true }, { row: 2, column: 1, index: 0, data: '00', isSpanned: true }, - { row: 2, column: 2, index: 0, data: '22', isSpanned: false } + { row: 2, column: 2, index: 0, data: '22' } ], { includeSpanned: true, startRow: 1, endRow: 2 } ); } ); @@ -218,9 +225,9 @@ describe( 'TableWalker', () => { [ '10' ], [ '20', '21' ] ], [ - { row: 0, column: 0, index: 0, data: '00', isSpanned: false }, - { row: 0, column: 1, index: 1, data: '01', isSpanned: false }, - { row: 1, column: 0, index: 0, data: '10', isSpanned: false }, + { row: 0, column: 0, index: 0, data: '00' }, + { row: 0, column: 1, index: 1, data: '01' }, + { row: 1, column: 0, index: 0, data: '10' }, { row: 1, column: 1, index: 1, data: '01', isSpanned: true } ], { startRow: 0, endRow: 1, includeSpanned: true } ); } ); @@ -234,7 +241,7 @@ describe( 'TableWalker', () => { [ '22' ], [ '30', '31', '32' ] ], [ - { row: 3, column: 1, index: 1, data: '31', isSpanned: false } + { row: 3, column: 1, index: 1, data: '31' } ], { column: 1 } ); } ); @@ -248,7 +255,7 @@ describe( 'TableWalker', () => { { row: 0, column: 1, index: 0, data: '00', isSpanned: true }, { row: 1, column: 1, index: 0, data: '00', isSpanned: true }, { row: 2, column: 1, index: 0, data: '00', isSpanned: true }, - { row: 3, column: 1, index: 1, data: '31', isSpanned: false } + { row: 3, column: 1, index: 1, data: '31' } ], { column: 1, includeSpanned: true } ); } ); } ); From 8de7311a27a29aab940dc53ca0981827a4a6f55d Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Wed, 14 Aug 2019 13:11:59 +0200 Subject: [PATCH 10/11] Docs: Some improvements in docs. --- src/tablewalker.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/tablewalker.js b/src/tablewalker.js index 0e89b443..10557177 100644 --- a/src/tablewalker.js +++ b/src/tablewalker.js @@ -15,7 +15,6 @@ export default class TableWalker { /** * Creates an instance of the table walker. * - * * The table walker iterates internally by traversing the table from row index = 0 and column index = 0. * It walks row by row and column by column in order to output values defined in the constructor. * By default it will output only those locations that are occupied by a cell. To include also spanned rows and columns, @@ -23,6 +22,8 @@ export default class TableWalker { * * The most important values of the iterator are column and row indexes of a cell. * + * See {@link module:table/tablewalker~TableWalkerValue} what values are returned by the table walker. + * * To iterate over a given row: * * const tableWalker = new TableWalker( table, { startRow: 1, endRow: 2 } ); @@ -54,8 +55,8 @@ export default class TableWalker { * * const tableWalker = new TableWalker( table, { startRow: 1, endRow: 1, includeSpanned: true } ); * - * for ( const cellInfo of tableWalker ) { - * console.log( 'Cell at ' + cellInfo.row + ' x ' + cellInfo.column + ' : ' + ( cellInfo.cell ? 'has data' : 'is spanned' ) ); + * for ( const value of tableWalker ) { + * console.log( 'Cell at ' + value.row + ' x ' + value.column + ' : ' + ( value.isSpanned ? 'has data' : 'is spanned' ) ); * } * * will log in the console for the table from previous example: @@ -390,7 +391,9 @@ export default class TableWalker { * @property {Number} row The row index of a cell. * @property {Number} column The column index of a cell. Column index is adjusted to widths and heights of previous cells. * @param {Boolean} isSpanned Whether the value is returned for a spanned cell location or actual cell. - * @property {Number} colspan The `colspan` attribute of a cell. It is always defined even if the model attribute is not present. - * @property {Number} rowspan The `rowspan` attribute of a cell. It is always defined even if the model attribute is not present. + * @property {Number} colspan The `colspan` attribute of a cell. It the model attribute is not present, it is set to `1`. For spanned + * table locations, it is set to `1`. + * @property {Number} rowspan The `rowspan` attribute of a cell. It the model attribute is not present, it is set to `1`. For spanned + * table locations, it is set to `1`. * @property {Number} cellIndex The index of the current cell in a parent row. */ From 773fa4c0ea9ef92274d604a4ccd8923ac54da256 Mon Sep 17 00:00:00 2001 From: Maciej Date: Wed, 14 Aug 2019 13:54:55 +0200 Subject: [PATCH 11/11] Update src/tablewalker.js example. --- src/tablewalker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tablewalker.js b/src/tablewalker.js index 10557177..5636bd44 100644 --- a/src/tablewalker.js +++ b/src/tablewalker.js @@ -56,7 +56,7 @@ export default class TableWalker { * const tableWalker = new TableWalker( table, { startRow: 1, endRow: 1, includeSpanned: true } ); * * for ( const value of tableWalker ) { - * console.log( 'Cell at ' + value.row + ' x ' + value.column + ' : ' + ( value.isSpanned ? 'has data' : 'is spanned' ) ); + * console.log( 'Cell at ' + value.row + ' x ' + value.column + ' : ' + ( value.isSpanned ? 'is spanned' : 'has data' ) ); * } * * will log in the console for the table from previous example: