浏览代码

Other: Refactor exports fo downcast converters helper module.

Maciej Gołaszewski 7 年之前
父节点
当前提交
d3d2700f1b

+ 24 - 26
packages/ckeditor5-table/src/converters/downcasttable.js

@@ -4,7 +4,7 @@
  */
 
 /**
- * @module table/converters/downcasttable
+ * @module table/converters/downcast
  */
 
 import ViewPosition from '@ckeditor/ckeditor5-engine/src/view/position';
@@ -18,7 +18,7 @@ import TableWalker from './../tablewalker';
  *
  * @returns {Function} Conversion helper.
  */
-export default function downcastTable() {
+export function downcastInsertTable() {
 	return dispatcher => dispatcher.on( 'insert:table', ( evt, data, conversionApi ) => {
 		const table = data.item;
 
@@ -30,7 +30,7 @@ export default function downcastTable() {
 		conversionApi.consumable.consume( table, 'attribute:headingRows:table' );
 		conversionApi.consumable.consume( table, 'attribute:headingColumns:table' );
 
-		// The <thead> and <tbody> elements are created on the fly when needed & cached by `getTableSection()` function.
+		// The <thead> and <tbody> elements are created on the fly when needed & cached by `getOrCreateTableSection()` function.
 		const tableSections = {};
 
 		const tableElement = conversionApi.writer.createContainerElement( 'table' );
@@ -45,6 +45,7 @@ export default function downcastTable() {
 
 			// Check if row was converted
 			const trElement = getOrCreateTr( tableRow, row, tableSection, conversionApi );
+
 			createViewTableCellElement( tableWalkerValue, ViewPosition.createAt( trElement, 'end' ), conversionApi );
 		}
 
@@ -55,13 +56,6 @@ export default function downcastTable() {
 	}, { priority: 'normal' } );
 }
 
-function getSectionName( treeWalkerValue ) {
-	const { row, table: { headingRows } } = treeWalkerValue;
-	const isHead = row < headingRows;
-
-	return isHead ? 'thead' : 'tbody';
-}
-
 /**
  * Model row element to view <tr> element conversion helper.
  *
@@ -181,14 +175,14 @@ export function downcastAttributeChange( attribute ) {
 			}
 
 			// Check whether current columnIndex is overlapped by table cells from previous rows.
-			const cellElementName = getCellElementName( tableWalkerValue );
+			const desiredCellElementName = getCellElementName( tableWalkerValue );
 
 			const viewCell = conversionApi.mapper.toViewElement( cell );
 
 			// If in single change we're converting attribute changes and inserting cell the table cell might not be inserted into view
 			// because of child conversion is done after parent.
-			if ( viewCell && viewCell.name !== cellElementName ) {
-				conversionApi.writer.rename( viewCell, cellElementName );
+			if ( viewCell && viewCell.name !== desiredCellElementName ) {
+				conversionApi.writer.rename( viewCell, desiredCellElementName );
 			}
 		}
 
@@ -237,31 +231,35 @@ function getOrCreateTr( tableRow, rowIndex, tableSection, conversionApi ) {
 	return trElement;
 }
 
-// Returns `th` for heading cells and `td` for other cells.
-// It is based on tableCell location (rowIndex x columnIndex) and the sizes of column & row headings sizes.
+// Returns `th` for heading cells and `td` for other cells for current table walker value.
 //
-// @param {Number} rowIndex
-// @param {Number} columnIndex
-// @param {Number} headingRows
-// @param {Number} headingColumns
+// @param {module:table/tablewalker~TableWalkerValue} tableWalkerValue
 // @returns {String}
 function getCellElementName( tableWalkerValue ) {
-	const headingRows = tableWalkerValue.table.headingRows;
+	const { row, column, table: { headingRows, headingColumns } } = tableWalkerValue;
 
 	// Column heading are all tableCells in the first `columnHeading` rows.
-	const isHeadingForAColumn = headingRows && headingRows > tableWalkerValue.row;
+	const isColumnHeading = headingRows && headingRows > row;
 
 	// So a whole row gets <th> element.
-	if ( isHeadingForAColumn ) {
+	if ( isColumnHeading ) {
 		return 'th';
 	}
 
-	const headingColumns = tableWalkerValue.table.headingColumns;
-
 	// Row heading are tableCells which columnIndex is lower then headingColumns.
-	const isHeadingForARow = headingColumns && headingColumns > tableWalkerValue.column;
+	const isRowHeading = headingColumns && headingColumns > column;
+
+	return isRowHeading ? 'th' : 'td';
+}
+
+// Returns table section name for current table walker value.
+//
+// @param {module:table/tablewalker~TableWalkerValue} tableWalkerValue
+// @returns {String}
+function getSectionName( tableWalkerValue ) {
+	const { row, table: { headingRows } } = tableWalkerValue;
 
-	return isHeadingForARow ? 'th' : 'td';
+	return row < headingRows ? 'thead' : 'tbody';
 }
 
 // Creates or returns an existing <tbody> or <thead> element witch caching.

+ 2 - 2
packages/ckeditor5-table/src/tableediting.js

@@ -10,7 +10,7 @@
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
 import upcastTable from './converters/upcasttable';
-import downcastTable, { downcastInsertCell, downcastInsertRow } from './converters/downcasttable';
+import { downcastInsertCell, downcastInsertRow, downcastInsertTable } from './converters/downcast';
 import InsertTableCommand from './inserttablecommand';
 import InsertRowCommand from './insertrowcommand';
 import InsertColumnCommand from './insertcolumncommand';
@@ -53,7 +53,7 @@ export default class TablesEditing extends Plugin {
 
 		// Table conversion.
 		conversion.for( 'upcast' ).add( upcastTable() );
-		conversion.for( 'downcast' ).add( downcastTable() );
+		conversion.for( 'downcast' ).add( downcastInsertTable() );
 
 		// Insert conversion
 		conversion.for( 'downcast' ).add( downcastInsertRow() );

+ 155 - 152
packages/ckeditor5-table/tests/converters/downcasttable.js

@@ -7,14 +7,15 @@ import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtest
 import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
 import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
-import downcastTable, {
+import {
 	downcastAttributeChange,
 	downcastInsertCell,
-	downcastInsertRow
-} from '../../src/converters/downcasttable';
+	downcastInsertRow,
+	downcastInsertTable
+} from '../../src/converters/downcast';
 import { formatModelTable, formattedViewTable, modelTable, viewTable } from '../_utils/utils';
 
-describe( 'downcastTable()', () => {
+describe( 'downcast converters', () => {
 	let editor, model, doc, root, viewDocument;
 
 	beforeEach( () => {
@@ -51,7 +52,7 @@ describe( 'downcastTable()', () => {
 					isLimit: true
 				} );
 
-				conversion.for( 'downcast' ).add( downcastTable() );
+				conversion.for( 'downcast' ).add( downcastInsertTable() );
 				conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
 				conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
 
@@ -64,191 +65,193 @@ describe( 'downcastTable()', () => {
 			} );
 	} );
 
-	it( 'should create table with tbody', () => {
-		setModelData( model,
-			'<table>' +
-			'<tableRow><tableCell></tableCell></tableRow>' +
-			'</table>'
-		);
-
-		expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
-			'<table>' +
-			'<tbody>' +
-			'<tr><td></td></tr>' +
-			'</tbody>' +
-			'</table>'
-		);
-	} );
-
-	it( 'should create table with tbody and thead', () => {
-		setModelData( model,
-			'<table headingRows="1">' +
-			'<tableRow><tableCell>1</tableCell></tableRow>' +
-			'<tableRow><tableCell>2</tableCell></tableRow>' +
-			'</table>'
-		);
-
-		expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
-			'<table>' +
-			'<thead>' +
-			'<tr><th>1</th></tr>' +
-			'</thead>' +
-			'<tbody>' +
-			'<tr><td>2</td></tr>' +
-			'</tbody>' +
-			'</table>'
-		);
-	} );
-
-	it( 'should create table with thead', () => {
-		setModelData( model,
-			'<table headingRows="2">' +
-			'<tableRow><tableCell>1</tableCell></tableRow>' +
-			'<tableRow><tableCell>2</tableCell></tableRow>' +
-			'</table>'
-		);
-
-		expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
-			'<table>' +
-			'<thead>' +
-			'<tr><th>1</th></tr>' +
-			'<tr><th>2</th></tr>' +
-			'</thead>' +
-			'</table>'
-		);
-	} );
-
-	it( 'should create table with heading columns and rows', () => {
-		setModelData( model,
-			'<table headingColumns="3" headingRows="1">' +
-			'<tableRow>' +
-			'<tableCell>11</tableCell><tableCell>12</tableCell><tableCell>13</tableCell><tableCell>14</tableCell>' +
-			'</tableRow>' +
-			'<tableRow>' +
-			'<tableCell>21</tableCell><tableCell>22</tableCell><tableCell>23</tableCell><tableCell>24</tableCell>' +
-			'</tableRow>' +
-			'</table>'
-		);
-
-		expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
-			'<table>' +
-			'<thead>' +
-			'<tr><th>11</th><th>12</th><th>13</th><th>14</th></tr>' +
-			'</thead>' +
-			'<tbody>' +
-			'<tr><th>21</th><th>22</th><th>23</th><td>24</td></tr>' +
-			'</tbody>' +
-			'</table>'
-		);
-	} );
-
-	it( 'should be possible to overwrite', () => {
-		editor.conversion.elementToElement( { model: 'tableRow', view: 'tr', priority: 'high' } );
-		editor.conversion.elementToElement( { model: 'tableCell', view: 'td', priority: 'high' } );
-		editor.conversion.for( 'downcast' ).add( dispatcher => {
-			dispatcher.on( 'insert:table', ( evt, data, conversionApi ) => {
-				conversionApi.consumable.consume( data.item, 'insert' );
-
-				const tableElement = conversionApi.writer.createContainerElement( 'table', { foo: 'bar' } );
-				const viewPosition = conversionApi.mapper.toViewPosition( data.range.start );
+	describe( 'downcastInsertTable()', () => {
+		it( 'should create table with tbody', () => {
+			setModelData( model,
+				'<table>' +
+				'<tableRow><tableCell></tableCell></tableRow>' +
+				'</table>'
+			);
 
-				conversionApi.mapper.bindElements( data.item, tableElement );
-				conversionApi.writer.insert( viewPosition, tableElement );
-			}, { priority: 'high' } );
+			expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
+				'<table>' +
+				'<tbody>' +
+				'<tr><td></td></tr>' +
+				'</tbody>' +
+				'</table>'
+			);
 		} );
 
-		setModelData( model,
-			'<table>' +
-			'<tableRow><tableCell></tableCell></tableRow>' +
-			'</table>'
-		);
-
-		expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
-			'<table foo="bar">' +
-			'<tr><td></td></tr>' +
-			'</table>'
-		);
-	} );
-
-	describe( 'headingColumns attribute', () => {
-		it( 'should mark heading columns table cells', () => {
+		it( 'should create table with tbody and thead', () => {
 			setModelData( model,
-				'<table headingColumns="2">' +
-				'<tableRow><tableCell>11</tableCell><tableCell>12</tableCell><tableCell>13</tableCell></tableRow>' +
-				'<tableRow><tableCell>21</tableCell><tableCell>22</tableCell><tableCell>23</tableCell></tableRow>' +
+				'<table headingRows="1">' +
+				'<tableRow><tableCell>1</tableCell></tableRow>' +
+				'<tableRow><tableCell>2</tableCell></tableRow>' +
 				'</table>'
 			);
 
 			expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
 				'<table>' +
+				'<thead>' +
+				'<tr><th>1</th></tr>' +
+				'</thead>' +
 				'<tbody>' +
-				'<tr><th>11</th><th>12</th><td>13</td></tr>' +
-				'<tr><th>21</th><th>22</th><td>23</td></tr>' +
+				'<tr><td>2</td></tr>' +
 				'</tbody>' +
 				'</table>'
 			);
 		} );
 
-		it( 'should mark heading columns table cells when one has colspan attribute', () => {
+		it( 'should create table with thead', () => {
+			setModelData( model,
+				'<table headingRows="2">' +
+				'<tableRow><tableCell>1</tableCell></tableRow>' +
+				'<tableRow><tableCell>2</tableCell></tableRow>' +
+				'</table>'
+			);
+
+			expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
+				'<table>' +
+				'<thead>' +
+				'<tr><th>1</th></tr>' +
+				'<tr><th>2</th></tr>' +
+				'</thead>' +
+				'</table>'
+			);
+		} );
+
+		it( 'should create table with heading columns and rows', () => {
 			setModelData( model,
-				'<table headingColumns="3">' +
+				'<table headingColumns="3" headingRows="1">' +
 				'<tableRow>' +
 				'<tableCell>11</tableCell><tableCell>12</tableCell><tableCell>13</tableCell><tableCell>14</tableCell>' +
 				'</tableRow>' +
-				'<tableRow><tableCell colspan="2">21</tableCell><tableCell>23</tableCell><tableCell>24</tableCell></tableRow>' +
+				'<tableRow>' +
+				'<tableCell>21</tableCell><tableCell>22</tableCell><tableCell>23</tableCell><tableCell>24</tableCell>' +
+				'</tableRow>' +
 				'</table>'
 			);
 
 			expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
 				'<table>' +
+				'<thead>' +
+				'<tr><th>11</th><th>12</th><th>13</th><th>14</th></tr>' +
+				'</thead>' +
 				'<tbody>' +
-				'<tr><th>11</th><th>12</th><th>13</th><td>14</td></tr>' +
-				'<tr><th colspan="2">21</th><th>23</th><td>24</td></tr>' +
+				'<tr><th>21</th><th>22</th><th>23</th><td>24</td></tr>' +
 				'</tbody>' +
 				'</table>'
 			);
 		} );
 
-		it( 'should work with colspan and rowspan attributes on table cells', () => {
-			// The table in this test looks like a table below:
-			//
-			//   Row headings | Normal cells
-			//                |
-			// +----+----+----+----+
-			// | 11 | 12 | 13 | 14 |
-			// |    +----+    +----+
-			// |    | 22 |    | 24 |
-			// |----+----+    +----+
-			// | 31      |    | 34 |
-			// |         +----+----+
-			// |         | 43 | 44 |
-			// +----+----+----+----+
+		it( 'should be possible to overwrite', () => {
+			editor.conversion.elementToElement( { model: 'tableRow', view: 'tr', priority: 'high' } );
+			editor.conversion.elementToElement( { model: 'tableCell', view: 'td', priority: 'high' } );
+			editor.conversion.for( 'downcast' ).add( dispatcher => {
+				dispatcher.on( 'insert:table', ( evt, data, conversionApi ) => {
+					conversionApi.consumable.consume( data.item, 'insert' );
+
+					const tableElement = conversionApi.writer.createContainerElement( 'table', { foo: 'bar' } );
+					const viewPosition = conversionApi.mapper.toViewPosition( data.range.start );
+
+					conversionApi.mapper.bindElements( data.item, tableElement );
+					conversionApi.writer.insert( viewPosition, tableElement );
+				}, { priority: 'high' } );
+			} );
 
 			setModelData( model,
-				'<table headingColumns="3">' +
-				'<tableRow>' +
-				'<tableCell rowspan="2">11</tableCell>' +
-				'<tableCell>12</tableCell>' +
-				'<tableCell rowspan="3">13</tableCell>' +
-				'<tableCell>14</tableCell>' +
-				'</tableRow>' +
-				'<tableRow><tableCell>22</tableCell><tableCell>24</tableCell></tableRow>' +
-				'<tableRow><tableCell colspan="2" rowspan="2">31</tableCell><tableCell>34</tableCell></tableRow>' +
-				'<tableRow><tableCell>43</tableCell><tableCell>44</tableCell></tableRow>' +
+				'<table>' +
+				'<tableRow><tableCell></tableCell></tableRow>' +
 				'</table>'
 			);
 
 			expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
-				'<table>' +
-				'<tbody>' +
-				'<tr><th rowspan="2">11</th><th>12</th><th rowspan="3">13</th><td>14</td></tr>' +
-				'<tr><th>22</th><td>24</td></tr>' +
-				'<tr><th colspan="2" rowspan="2">31</th><td>34</td></tr>' +
-				'<tr><th>43</th><td>44</td></tr>' +
-				'</tbody>' +
+				'<table foo="bar">' +
+				'<tr><td></td></tr>' +
 				'</table>'
 			);
 		} );
+
+		describe( 'headingColumns attribute', () => {
+			it( 'should mark heading columns table cells', () => {
+				setModelData( model,
+					'<table headingColumns="2">' +
+					'<tableRow><tableCell>11</tableCell><tableCell>12</tableCell><tableCell>13</tableCell></tableRow>' +
+					'<tableRow><tableCell>21</tableCell><tableCell>22</tableCell><tableCell>23</tableCell></tableRow>' +
+					'</table>'
+				);
+
+				expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
+					'<table>' +
+					'<tbody>' +
+					'<tr><th>11</th><th>12</th><td>13</td></tr>' +
+					'<tr><th>21</th><th>22</th><td>23</td></tr>' +
+					'</tbody>' +
+					'</table>'
+				);
+			} );
+
+			it( 'should mark heading columns table cells when one has colspan attribute', () => {
+				setModelData( model,
+					'<table headingColumns="3">' +
+					'<tableRow>' +
+					'<tableCell>11</tableCell><tableCell>12</tableCell><tableCell>13</tableCell><tableCell>14</tableCell>' +
+					'</tableRow>' +
+					'<tableRow><tableCell colspan="2">21</tableCell><tableCell>23</tableCell><tableCell>24</tableCell></tableRow>' +
+					'</table>'
+				);
+
+				expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
+					'<table>' +
+					'<tbody>' +
+					'<tr><th>11</th><th>12</th><th>13</th><td>14</td></tr>' +
+					'<tr><th colspan="2">21</th><th>23</th><td>24</td></tr>' +
+					'</tbody>' +
+					'</table>'
+				);
+			} );
+
+			it( 'should work with colspan and rowspan attributes on table cells', () => {
+				// The table in this test looks like a table below:
+				//
+				//   Row headings | Normal cells
+				//                |
+				// +----+----+----+----+
+				// | 11 | 12 | 13 | 14 |
+				// |    +----+    +----+
+				// |    | 22 |    | 24 |
+				// |----+----+    +----+
+				// | 31      |    | 34 |
+				// |         +----+----+
+				// |         | 43 | 44 |
+				// +----+----+----+----+
+
+				setModelData( model,
+					'<table headingColumns="3">' +
+					'<tableRow>' +
+					'<tableCell rowspan="2">11</tableCell>' +
+					'<tableCell>12</tableCell>' +
+					'<tableCell rowspan="3">13</tableCell>' +
+					'<tableCell>14</tableCell>' +
+					'</tableRow>' +
+					'<tableRow><tableCell>22</tableCell><tableCell>24</tableCell></tableRow>' +
+					'<tableRow><tableCell colspan="2" rowspan="2">31</tableCell><tableCell>34</tableCell></tableRow>' +
+					'<tableRow><tableCell>43</tableCell><tableCell>44</tableCell></tableRow>' +
+					'</table>'
+				);
+
+				expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
+					'<table>' +
+					'<tbody>' +
+					'<tr><th rowspan="2">11</th><th>12</th><th rowspan="3">13</th><td>14</td></tr>' +
+					'<tr><th>22</th><td>24</td></tr>' +
+					'<tr><th colspan="2" rowspan="2">31</th><td>34</td></tr>' +
+					'<tr><th>43</th><td>44</td></tr>' +
+					'</tbody>' +
+					'</table>'
+				);
+			} );
+		} );
 	} );
 
 	describe( 'downcastInsertRow()', () => {
@@ -504,7 +507,7 @@ describe( 'downcastTable()', () => {
 		} );
 	} );
 
-	describe( 'table attribute change', () => {
+	describe( 'downcastAttributeChange()', () => {
 		it( 'should work for adding heading rows', () => {
 			setModelData( model, modelTable( [
 				[ '11', '12' ],

+ 2 - 2
packages/ckeditor5-table/tests/insertcolumncommand.js

@@ -8,7 +8,7 @@ import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model
 import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
 
 import InsertColumnCommand from '../src/insertcolumncommand';
-import downcastTable from '../src/converters/downcasttable';
+import { downcastInsertTable } from '../src/converters/downcast';
 import upcastTable from '../src/converters/upcasttable';
 import { formatModelTable, formattedModelTable, modelTable } from './_utils/utils';
 
@@ -51,7 +51,7 @@ describe( 'InsertColumnCommand', () => {
 
 				// Table conversion.
 				conversion.for( 'upcast' ).add( upcastTable() );
-				conversion.for( 'downcast' ).add( downcastTable() );
+				conversion.for( 'downcast' ).add( downcastInsertTable() );
 
 				// Table row upcast only since downcast conversion is done in `downcastTable()`.
 				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );

+ 2 - 2
packages/ckeditor5-table/tests/insertrowcommand.js

@@ -8,7 +8,7 @@ import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model
 import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
 
 import InsertRowCommand from '../src/insertrowcommand';
-import downcastTable from '../src/converters/downcasttable';
+import { downcastInsertTable } from '../src/converters/downcast';
 import upcastTable from '../src/converters/upcasttable';
 import { formatModelTable, formattedModelTable, modelTable } from './_utils/utils';
 
@@ -51,7 +51,7 @@ describe( 'InsertRowCommand', () => {
 
 				// Table conversion.
 				conversion.for( 'upcast' ).add( upcastTable() );
-				conversion.for( 'downcast' ).add( downcastTable() );
+				conversion.for( 'downcast' ).add( downcastInsertTable() );
 
 				// Table row upcast only since downcast conversion is done in `downcastTable()`.
 				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );

+ 2 - 2
packages/ckeditor5-table/tests/inserttablecommand.js

@@ -8,7 +8,7 @@ import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model
 import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
 
 import InsertTableCommand from '../src/inserttablecommand';
-import downcastTable from '../src/converters/downcasttable';
+import { downcastInsertTable } from '../src/converters/downcast';
 import upcastTable from '../src/converters/upcasttable';
 
 describe( 'InsertTableCommand', () => {
@@ -50,7 +50,7 @@ describe( 'InsertTableCommand', () => {
 
 				// Table conversion.
 				conversion.for( 'upcast' ).add( upcastTable() );
-				conversion.for( 'downcast' ).add( downcastTable() );
+				conversion.for( 'downcast' ).add( downcastInsertTable() );
 
 				// Table row upcast only since downcast conversion is done in `downcastTable()`.
 				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );