Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#7245: Added normalization of pasted table. #7451

Merged
merged 4 commits into from
Jun 18, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 24 additions & 12 deletions packages/ckeditor5-table/src/converters/downcast.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,8 @@ export function downcastInsertTable( options = {} ) {
for ( const tableSlot of tableWalker ) {
const { row, cell } = tableSlot;

const tableSection = getOrCreateTableSection( getSectionName( row, tableAttributes ), tableElement, conversionApi );
const tableRow = table.getChild( row );

const trElement = viewRows.get( row ) || createTr( tableRow, row, tableSection, conversionApi );
const trElement = viewRows.get( row ) || createTr( tableElement, tableRow, row, tableAttributes, conversionApi );
viewRows.set( row, trElement );

// Consume table cell - it will be always consumed as we convert whole table at once.
Expand All @@ -70,6 +68,16 @@ export function downcastInsertTable( options = {} ) {
createViewTableCellElement( tableSlot, tableAttributes, insertPosition, conversionApi, options );
}

// Insert empty TR elements if there are any rows without anchored cells. Since the model is always normalized
// this can happen only in the document fragment that only part of the table is down-casted.
for ( const tableRow of table.getChildren() ) {
const rowIndex = tableRow.index;

if ( !viewRows.has( rowIndex ) ) {
viewRows.set( rowIndex, createTr( tableElement, tableRow, rowIndex, tableAttributes, conversionApi ) );
}
}

const viewPosition = conversionApi.mapper.toViewPosition( data.range.start );

conversionApi.mapper.bindElements( table, asWidget ? tableWidget : figureElement );
Expand Down Expand Up @@ -110,9 +118,7 @@ export function downcastInsertRow() {
const viewRows = new Map();

for ( const tableSlot of tableWalker ) {
const tableSection = getOrCreateTableSection( getSectionName( row, tableAttributes ), tableElement, conversionApi );

const trElement = viewRows.get( row ) || createTr( tableRow, row, tableSection, conversionApi );
const trElement = viewRows.get( row ) || createTr( tableElement, tableRow, row, tableAttributes, conversionApi );
viewRows.set( row, trElement );

// Consume table cell - it will be always consumed as we convert whole row at once.
Expand Down Expand Up @@ -425,22 +431,28 @@ function createViewTableCellElement( tableSlot, tableAttributes, insertPosition,

// Creates a `<tr>` view element.
//
// @param {module:engine/view/element~Element} tableRow
// @param {module:engine/view/element~Element} tableElement
// @param {module:engine/model/element~Element} tableRow
// @param {Number} rowIndex
// @param {module:engine/view/element~Element} tableSection
// @param {{headingColumns, headingRows}} tableAttributes
// @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi
// @returns {module:engine/view/element~Element}
function createTr( tableRow, rowIndex, tableSection, conversionApi ) {
function createTr( tableElement, tableRow, rowIndex, tableAttributes, conversionApi ) {
// Will always consume since we're converting <tableRow> element from a parent <table>.
conversionApi.consumable.consume( tableRow, 'insert' );

const trElement = conversionApi.writer.createContainerElement( 'tr' );
const trElement = tableRow.isEmpty ?
conversionApi.writer.createEmptyElement( 'tr' ) :
conversionApi.writer.createContainerElement( 'tr' );

conversionApi.mapper.bindElements( tableRow, trElement );

const headingRows = tableRow.parent.getAttribute( 'headingRows' ) || 0;
const offset = headingRows > 0 && rowIndex >= headingRows ? rowIndex - headingRows : rowIndex;
const headingRows = tableAttributes.headingRows;
const tableSection = getOrCreateTableSection( getSectionName( rowIndex, tableAttributes ), tableElement, conversionApi );

const offset = headingRows > 0 && rowIndex >= headingRows ? rowIndex - headingRows : rowIndex;
const position = conversionApi.writer.createPositionAt( tableSection, offset );

conversionApi.writer.insert( position, trElement );

return trElement;
Expand Down
6 changes: 3 additions & 3 deletions packages/ckeditor5-table/src/converters/upcasttable.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,16 @@ export default function upcastTable() {
}

/**
* Conversion helper that skips empty <tr> from upcasting.
* Conversion helper that skips empty <tr> from upcasting at the beginning of the table.
*
* Empty row is considered a table model error.
* Empty row is considered a table model error if there are no cells spanned over that row.
*
* @returns {Function} Conversion helper.
*/
export function skipEmptyTableRow() {
return dispatcher => {
dispatcher.on( 'element:tr', ( evt, data ) => {
if ( data.viewItem.isEmpty ) {
if ( data.viewItem.isEmpty && data.modelCursor.index == 0 ) {
evt.stop();
}
}, { priority: 'high' } );
Expand Down
86 changes: 10 additions & 76 deletions packages/ckeditor5-table/src/tableclipboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ import { getColumnIndexes, getRowIndexes, getSelectionAffectedTableCells, isSele
import {
cropTableToDimensions,
getHorizontallyOverlappingCells,
getVerticallyOverlappingCells,
getVerticallyOverlappingCells, removeEmptyRowsColumns,
splitHorizontally,
splitVertically,
trimTableCellIfNeeded
trimTableCellIfNeeded,
adjustLastRowIndex, adjustLastColumnIndex
} from './utils/structure';

/**
Expand Down Expand Up @@ -113,16 +114,18 @@ export default class TableClipboard extends Plugin {
const model = this.editor.model;
const tableUtils = this.editor.plugins.get( TableUtils );

const selectedTableCells = getSelectionAffectedTableCells( model.document.selection );
// We might need to crop table before inserting so reference might change.
let pastedTable = getTableIfOnlyTableInContent( content, model );

if ( !selectedTableCells.length ) {
if ( !pastedTable ) {
return;
}

// We might need to crop table before inserting so reference might change.
let pastedTable = getTableIfOnlyTableInContent( content, model );
const selectedTableCells = getSelectionAffectedTableCells( model.document.selection );

if ( !selectedTableCells.length ) {
removeEmptyRowsColumns( pastedTable, tableUtils );

if ( !pastedTable ) {
return;
}

Expand Down Expand Up @@ -513,72 +516,3 @@ function isAffectedBySelection( index, span, limit ) {

return isInsideSelection || overlapsSelectionFromOutside;
}

// Returns adjusted last row index if selection covers part of a row with empty slots (spanned by other cells).
// The rowIndexes.last is equal to last row index but selection might be bigger.
//
// This happens *only* on rectangular selection so we analyze a case like this:
//
// +---+---+---+---+
// 0 | a | b | c | d |
// + + +---+---+
// 1 | | e | f | g |
// + +---+ +---+
// 2 | | h | | i | <- last row, each cell has rowspan = 2,
// + + + + + so we need to return 3, not 2
// 3 | | | | |
// +---+---+---+---+
function adjustLastRowIndex( table, dimensions ) {
const lastRowMap = Array.from( new TableWalker( table, {
startColumn: dimensions.firstColumn,
endColumn: dimensions.lastColumn,
row: dimensions.lastRow
} ) );

const everyCellHasSingleRowspan = lastRowMap.every( ( { cellHeight } ) => cellHeight === 1 );

// It is a "flat" row, so the last row index is OK.
if ( everyCellHasSingleRowspan ) {
return dimensions.lastRow;
}

// Otherwise get any cell's rowspan and adjust the last row index.
const rowspanAdjustment = lastRowMap[ 0 ].cellHeight - 1;
return dimensions.lastRow + rowspanAdjustment;
}

// Returns adjusted last column index if selection covers part of a column with empty slots (spanned by other cells).
// The columnIndexes.last is equal to last column index but selection might be bigger.
//
// This happens *only* on rectangular selection so we analyze a case like this:
//
// 0 1 2 3
// +---+---+---+---+
// | a |
// +---+---+---+---+
// | b | c | d |
// +---+---+---+---+
// | e | f |
// +---+---+---+---+
// | g | h |
// +---+---+---+---+
// ^
// last column, each cell has colspan = 2, so we need to return 3, not 2
function adjustLastColumnIndex( table, dimensions ) {
const lastColumnMap = Array.from( new TableWalker( table, {
startRow: dimensions.firstRow,
endRow: dimensions.lastRow,
column: dimensions.lastColumn
} ) );

const everyCellHasSingleColspan = lastColumnMap.every( ( { cellWidth } ) => cellWidth === 1 );

// It is a "flat" column, so the last column index is OK.
if ( everyCellHasSingleColspan ) {
return dimensions.lastColumn;
}

// Otherwise get any cell's colspan and adjust the last column index.
const colspanAdjustment = lastColumnMap[ 0 ].cellWidth - 1;
return dimensions.lastColumn + colspanAdjustment;
}
34 changes: 26 additions & 8 deletions packages/ckeditor5-table/src/tableselection.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import TableWalker from './tablewalker';
import TableUtils from './tableutils';

import { findAncestor } from './utils/common';
import { cropTableToDimensions } from './utils/structure';
import { getColumnIndexes, getRowIndexes, getSelectedTableCells } from './utils/selection';
import { cropTableToDimensions, adjustLastRowIndex, adjustLastColumnIndex } from './utils/structure';
import { getColumnIndexes, getRowIndexes, getSelectedTableCells, isSelectionRectangular } from './utils/selection';

import '../theme/tableselection.css';

Expand Down Expand Up @@ -90,17 +90,35 @@ export default class TableSelection extends Plugin {

return this.editor.model.change( writer => {
const documentFragment = writer.createDocumentFragment();
const tableUtils = this.editor.plugins.get( 'TableUtils' );

const { first: startColumn, last: endColumn } = getColumnIndexes( selectedCells );
const { first: startRow, last: endRow } = getRowIndexes( selectedCells );
const { first: firstColumn, last: lastColumn } = getColumnIndexes( selectedCells );
const { first: firstRow, last: lastRow } = getRowIndexes( selectedCells );

const sourceTable = findAncestor( 'table', selectedCells[ 0 ] );

let adjustedLastRow = lastRow;
let adjustedLastColumn = lastColumn;

// If the selection is rectangular there could be a case of all cells in the last row/column spanned over
// next row/column so the real lastRow/lastColumn should be updated.
if ( isSelectionRectangular( selectedCells, tableUtils ) ) {
const dimensions = {
firstColumn,
lastColumn,
firstRow,
lastRow
};

adjustedLastRow = adjustLastRowIndex( sourceTable, dimensions );
adjustedLastColumn = adjustLastColumnIndex( sourceTable, dimensions );
}

const cropDimensions = {
startRow,
startColumn,
endRow,
endColumn
startRow: firstRow,
startColumn: firstColumn,
endRow: adjustedLastRow,
endColumn: adjustedLastColumn
};

const table = cropTableToDimensions( sourceTable, cropDimensions, writer );
Expand Down
93 changes: 91 additions & 2 deletions packages/ckeditor5-table/src/utils/structure.js
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ function addHeadingsToCroppedTable( croppedTable, sourceTable, startRow, startCo
* @protected
* @param {module:engine/model/element~Element} table
* @param {module:table/tableutils~TableUtils} tableUtils
* @return {Boolean} True if removed some columns.
* @returns {Boolean} True if removed some columns.
*/
export function removeEmptyColumns( table, tableUtils ) {
const width = tableUtils.getColumns( table );
Expand Down Expand Up @@ -381,7 +381,7 @@ export function removeEmptyColumns( table, tableUtils ) {
* @param {module:engine/model/element~Element} table
* @param {module:table/tableutils~TableUtils} tableUtils
* @param {module:engine/model/batch~Batch|null} [batch] Batch that should be used for removing empty rows.
* @return {Boolean} True if removed some rows.
* @returns {Boolean} True if removed some rows.
*/
export function removeEmptyRows( table, tableUtils, batch ) {
const emptyRows = [];
Expand Down Expand Up @@ -438,3 +438,92 @@ export function removeEmptyRowsColumns( table, tableUtils, batch ) {
removeEmptyRows( table, tableUtils, batch );
}
}

/**
* Returns adjusted last row index if selection covers part of a row with empty slots (spanned by other cells).
* The `dimensions.lastRow` is equal to last row index but selection might be bigger.
*
* This happens *only* on rectangular selection so we analyze a case like this:
*
* +---+---+---+---+
* 0 | a | b | c | d |
* + + +---+---+
* 1 | | e | f | g |
* + +---+ +---+
* 2 | | h | | i | <- last row, each cell has rowspan = 2,
* + + + + + so we need to return 3, not 2
* 3 | | | | |
* +---+---+---+---+
*
* @param {module:engine/model/element~Element} table
* @param {Object} dimensions
* @param {Number} dimensions.firstRow
* @param {Number} dimensions.firstColumn
* @param {Number} dimensions.lastRow
* @param {Number} dimensions.lastColumn
* @returns {Number} Adjusted last row index.
*/
export function adjustLastRowIndex( table, dimensions ) {
const lastRowMap = Array.from( new TableWalker( table, {
startColumn: dimensions.firstColumn,
endColumn: dimensions.lastColumn,
row: dimensions.lastRow
} ) );

const everyCellHasSingleRowspan = lastRowMap.every( ( { cellHeight } ) => cellHeight === 1 );

// It is a "flat" row, so the last row index is OK.
if ( everyCellHasSingleRowspan ) {
return dimensions.lastRow;
}

// Otherwise get any cell's rowspan and adjust the last row index.
const rowspanAdjustment = lastRowMap[ 0 ].cellHeight - 1;
return dimensions.lastRow + rowspanAdjustment;
}

/**
* Returns adjusted last column index if selection covers part of a column with empty slots (spanned by other cells).
* The `dimensions.lastColumn` is equal to last column index but selection might be bigger.
*
* This happens *only* on rectangular selection so we analyze a case like this:
*
* 0 1 2 3
* +---+---+---+---+
* | a |
* +---+---+---+---+
* | b | c | d |
* +---+---+---+---+
* | e | f |
* +---+---+---+---+
* | g | h |
* +---+---+---+---+
* ^
* last column, each cell has colspan = 2, so we need to return 3, not 2
*
* @param {module:engine/model/element~Element} table
* @param {Object} dimensions
* @param {Number} dimensions.firstRow
* @param {Number} dimensions.firstColumn
* @param {Number} dimensions.lastRow
* @param {Number} dimensions.lastColumn
* @returns {Number} Adjusted last column index.
*/
export function adjustLastColumnIndex( table, dimensions ) {
const lastColumnMap = Array.from( new TableWalker( table, {
startRow: dimensions.firstRow,
endRow: dimensions.lastRow,
column: dimensions.lastColumn
} ) );

const everyCellHasSingleColspan = lastColumnMap.every( ( { cellWidth } ) => cellWidth === 1 );

// It is a "flat" column, so the last column index is OK.
if ( everyCellHasSingleColspan ) {
return dimensions.lastColumn;
}

// Otherwise get any cell's colspan and adjust the last column index.
const colspanAdjustment = lastColumnMap[ 0 ].cellWidth - 1;
return dimensions.lastColumn + colspanAdjustment;
}
Loading