Fix adding/deleting table columns across sections (#16410)

* Handle adding columns across table sections * Handle deleting table columns across table sections * Add e2e tests * Update changelog
WordPress · Jul 4, 2019 · 9de5431 · 9de5431
1 parent 72697ce
commit 9de5431
Show file tree

Hide file tree

Showing 6 changed files with 594 additions and 30 deletions.
diff --git a/packages/block-library/CHANGELOG.md b/packages/block-library/CHANGELOG.md
@@ -1,4 +1,10 @@
-## ## 2.6.0 (2019-06-12)
+## Master (unreleased)
+
+### Bug Fixes
+
+- Fixed insertion of columns in the table block, which now inserts columns for all table sections ([#16410](https://github.com/WordPress/gutenberg/pull/16410))
+
+## 2.6.0 (2019-06-12)
 
 - Fixed an issue with creating upgraded embed blocks that are not registered ([#15883](https://github.com/WordPress/gutenberg/issues/15883)).
 

diff --git a/packages/block-library/src/table/edit.js b/packages/block-library/src/table/edit.js
@@ -37,6 +37,7 @@ import {
 	insertColumn,
 	deleteColumn,
 	toggleSection,
+	isEmptyTableSection,
 } from './state';
 import icon from './icon';
 
@@ -241,11 +242,10 @@ export class TableEdit extends Component {
 		}
 
 		const { attributes, setAttributes } = this.props;
-		const { section, columnIndex } = selectedCell;
+		const { columnIndex } = selectedCell;
 
 		this.setState( { selectedCell: null } );
 		setAttributes( insertColumn( attributes, {
-			section,
 			columnIndex: columnIndex + delta,
 		} ) );
 	}
@@ -352,7 +352,7 @@ export class TableEdit extends Component {
 	 * @return {Object} React element for the section.
 	 */
 	renderSection( { type, rows } ) {
-		if ( ! rows.length ) {
+		if ( isEmptyTableSection( rows ) ) {
 			return null;
 		}
 
@@ -416,7 +416,7 @@ export class TableEdit extends Component {
 		} = this.props;
 		const { initialRowCount, initialColumnCount } = this.state;
 		const { hasFixedLayout, head, body, foot } = attributes;
-		const isEmpty = ! head.length && ! body.length && ! foot.length;
+		const isEmpty = isEmptyTableSection( head ) && isEmptyTableSection( body ) && isEmptyTableSection( foot );
 		const Section = this.renderSection;
 
 		if ( isEmpty ) {

diff --git a/packages/block-library/src/table/state.js b/packages/block-library/src/table/state.js
@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { times, get } from 'lodash';
+import { times, get, mapValues, every } from 'lodash';
 
 /**
  * Creates a table state.
@@ -122,21 +122,33 @@ export function deleteRow( state, {
  * @return {Object} New table state.
  */
 export function insertColumn( state, {
-	section,
 	columnIndex,
 } ) {
-	return {
-		[ section ]: state[ section ].map( ( row ) => ( {
-			cells: [
-				...row.cells.slice( 0, columnIndex ),
-				{
-					content: '',
-					tag: section === 'head' ? 'th' : 'td',
-				},
-				...row.cells.slice( columnIndex ),
-			],
-		} ) ),
-	};
+	return mapValues( state, ( section, sectionName ) => {
+		// Bail early if the table section is empty.
+		if ( isEmptyTableSection( section ) ) {
+			return section;
+		}
+
+		return section.map( ( row ) => {
+			// Bail early if the row is empty or it's an attempt to insert past
+			// the last possible index of the array.
+			if ( isEmptyRow( row ) || row.cells.length < columnIndex ) {
+				return row;
+			}
+
+			return {
+				cells: [
+					...row.cells.slice( 0, columnIndex ),
+					{
+						content: '',
+						tag: sectionName === 'head' ? 'th' : 'td',
+					},
+					...row.cells.slice( columnIndex ),
+				],
+			};
+		} );
+	} );
 }
 
 /**
@@ -149,14 +161,18 @@ export function insertColumn( state, {
  * @return {Object} New table state.
  */
 export function deleteColumn( state, {
-	section,
 	columnIndex,
 } ) {
-	return {
-		[ section ]: state[ section ].map( ( row ) => ( {
-			cells: row.cells.filter( ( cell, index ) => index !== columnIndex ),
-		} ) ).filter( ( row ) => row.cells.length ),
-	};
+	return mapValues( state, ( section ) => {
+		// Bail early if the table section is empty.
+		if ( isEmptyTableSection( section ) ) {
+			return section;
+		}
+
+		return section.map( ( row ) => ( {
+			cells: row.cells.length >= columnIndex ? row.cells.filter( ( cell, index ) => index !== columnIndex ) : row.cells,
+		} ) ).filter( ( row ) => row.cells.length );
+	} );
 }
 
 /**
@@ -169,7 +185,7 @@ export function deleteColumn( state, {
  */
 export function toggleSection( state, section ) {
 	// Section exists, replace it with an empty row to remove it.
-	if ( state[ section ] && state[ section ].length ) {
+	if ( ! isEmptyTableSection( state[ section ] ) ) {
 		return { [ section ]: [] };
 	}
 
@@ -179,3 +195,25 @@ export function toggleSection( state, section ) {
 	// Section doesn't exist, insert an empty row to create the section.
 	return insertRow( state, { section, rowIndex: 0, columnCount } );
 }
+
+/**
+ * Determines whether a table section is empty.
+ *
+ * @param {Object} sectionRows Table section state.
+ *
+ * @return {boolean} True if the table section is empty, false otherwise.
+ */
+export function isEmptyTableSection( sectionRows ) {
+	return ! sectionRows || ! sectionRows.length || every( sectionRows, isEmptyRow );
+}
+
+/**
+ * Determines whether a table row is empty.
+ *
+ * @param {Object} row Table row state.
+ *
+ * @return {boolean} True if the table section is empty, false otherwise.
+ */
+export function isEmptyRow( row ) {
+	return ! ( row.cells && row.cells.length );
+}