8
0
Quellcode durchsuchen

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.

Szymon Cofalik vor 6 Jahren
Ursprung
Commit
2e3dc41d96

+ 70 - 76
packages/ckeditor5-table/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<Number, Map.<Number, Number>>}
+		 * @member {Map<Number, Map.<Number, module:engine/model/element~Element>>}
 		 * @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 ) ) {
@@ -342,24 +326,37 @@ export default class TableWalker {
 	}
 
 	/**
+	 * 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.
  */

+ 29 - 16
packages/ckeditor5-table/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 } );
+		} );
 	} );
 } );