Bartosz Dziewoński has uploaded a new change for review.
https://gerrit.wikimedia.org/r/172193
Change subject: [WIP] Don't un-merge spanned cells when spanned cell is removed
......................................................................
[WIP] Don't un-merge spanned cells when spanned cell is removed
Previously we inserted regular cells in place of every placeholder in
the matrix, which meant that deleting a row which happened to contain
a rowspanned cell would split the rest of the cell apart. (Same for
columns.)
Now we only replace one of the placeholders with a new spanned cell,
appropriately smaller. The problem is, it should inherit all of the
properties and content of the removed cell, but it currently doesn't.
I need to figure that out.
Bug: 73213
Change-Id: I62e56657c18bc67be859ece882770ce008bc3838
---
M src/dm/nodes/ve.dm.TableCellNode.js
M src/ui/actions/ve.ui.TableAction.js
2 files changed, 27 insertions(+), 15 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/93/172193/1
diff --git a/src/dm/nodes/ve.dm.TableCellNode.js
b/src/dm/nodes/ve.dm.TableCellNode.js
index ba267d1..e1ead94 100644
--- a/src/dm/nodes/ve.dm.TableCellNode.js
+++ b/src/dm/nodes/ve.dm.TableCellNode.js
@@ -88,7 +88,10 @@
/**
* Creates data that can be inserted into the model to create a new table cell.
*
- * @param {Object} [options] An object with property 'style' which can be
either 'header' or 'data'.
+ * @param {Object} [options]
+ * @param {string} [options.style='data'] Either 'header' or 'data'
+ * @param {number} [options.rowspan=1]
+ * @param {number} [options.colspan=1]
* @return {Array} Model data for a new table cell
*/
ve.dm.TableCellNode.static.createData = function ( options ) {
@@ -97,7 +100,9 @@
{
type: 'tableCell',
attributes: {
- style: options.style || 'data'
+ style: options.style || 'data',
+ rowspan: options.rowspan || 1,
+ colspan: options.colspan || 1
}
},
{ type: 'paragraph' },
diff --git a/src/ui/actions/ve.ui.TableAction.js
b/src/ui/actions/ve.ui.TableAction.js
index 8126098..8199f2c 100644
--- a/src/ui/actions/ve.ui.TableAction.js
+++ b/src/ui/actions/ve.ui.TableAction.js
@@ -446,10 +446,10 @@
surfaceModel = this.surface.getModel();
// Deleting cells can have two additional consequences:
- // 1. The cell is a Placeholder. The owner's span might be decreased.
+ // 1. The cell is a Placeholder. The owner's span must be decreased.
// 2. The cell is owner of placeholders which get orphaned by the
deletion.
- // New, empty cells must be inserted to replace the placeholders and
keep the
- // table in proper shape.
+ // The first of the placeholders now becomes the real cell, with the
span adjusted.
+ // It should inherit all of its properties and content, but
currently doesn't.
// Insertions and deletions of cells must be done in an appropriate
order, so that the transactions
// do not interfere with each other. To achieve that, we record
insertions and deletions and
// sort them by the position of the cell (row, column) in the table
matrix.
@@ -492,13 +492,14 @@
}
endRow = cell.row + cell.node.getRowspan() - 1;
endCol = cell.col + cell.node.getColspan() - 1;
-
+
// Record the insertion to apply it later
- for ( row = startRow; row <= endRow; row++ ) {
- for ( col = startCol; col <= endCol; col++ ) {
- actions.push( { action: 'insert', cell:
matrix.getCell( row, col ) } );
- }
- }
+ actions.push( {
+ action: 'insert',
+ cell: matrix.getCell( startRow, startCol ),
+ colspan: 1 + endCol - startCol,
+ rowspan: 1 + endRow - startRow
+ } );
}
// Cell nodes only get deleted when deleting columns (otherwise
row nodes)
@@ -518,7 +519,9 @@
// First replace orphaned placeholders which are below the last
deleted row,
// thus, this works with regard to transaction offsets
for ( i = 0; i < actions.length; i++ ) {
- txs.push( this.replacePlaceholder( matrix,
actions[i].cell ) );
+ txs.push( this.replacePlaceholder(
+ matrix, actions[i].cell, actions[i].colspan,
actions[i].rowspan
+ ) );
}
// Remove rows in reverse order to have valid transaction
offsets
for ( row = maxIndex; row >= minIndex; row-- ) {
@@ -528,7 +531,9 @@
} else {
for ( i = 0; i < actions.length; i++ ) {
if ( actions[i].action === 'insert' ) {
- txs.push( this.replacePlaceholder( matrix,
actions[i].cell ) );
+ txs.push( this.replacePlaceholder(
+ matrix, actions[i].cell,
actions[i].colspan, actions[i].rowspan
+ ) );
} else {
txs.push( ve.dm.Transaction.newFromRemoval(
surfaceModel.getDocument(), actions[i].cell.node.getOuterRange() ) );
}
@@ -542,9 +547,11 @@
*
* @param {ve.dm.TableMatrix} matrix Table matrix
* @param {ve.dm.TableMatrixCell} placeholder Placeholder cell to replace
+ * @param {number} [colspan] Column span to set
+ * @param {number} [rowspan] Row span to set
* @return {ve.dm.Transaction} Transaction
*/
-ve.ui.TableAction.prototype.replacePlaceholder = function ( matrix,
placeholder ) {
+ve.ui.TableAction.prototype.replacePlaceholder = function ( matrix,
placeholder, colspan, rowspan ) {
var range, offset, data, style,
// For inserting the new cell a reference cell node
// which is used to get an insertion offset.
@@ -561,7 +568,7 @@
offset = range.start;
style = placeholder.node.getStyle();
}
- data = ve.dm.TableCellNode.static.createData( { style: style } );
+ data = ve.dm.TableCellNode.static.createData( { style: style, colspan:
colspan, rowspan: rowspan } );
return ve.dm.Transaction.newFromInsertion( surfaceModel.getDocument(),
offset, data );
};
--
To view, visit https://gerrit.wikimedia.org/r/172193
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I62e56657c18bc67be859ece882770ce008bc3838
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <matma....@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
