Skip to content

Commit

Permalink
Address review feedback
Browse files Browse the repository at this point in the history
Update params of getCellAttribute function

Do nothing if insertRows is unable to determine the correct cellCount for the insertion
  • Loading branch information
talldan committed Jul 31, 2019
1 parent 37a4e06 commit f00dd4e
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 32 deletions.
2 changes: 1 addition & 1 deletion packages/block-library/src/table/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ export class TableEdit extends Component {

const { attributes } = this.props;

return getCellAttribute( attributes, { ...selectedCell, attributeName: 'align' } );
return getCellAttribute( attributes, selectedCell, 'align' );
}

/**
Expand Down
16 changes: 3 additions & 13 deletions packages/block-library/src/table/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,9 @@

td.is-selected,
th.is-selected {
position: relative;

&::before {
content: "";
display: block;
position: absolute;
pointer-events: none;
top: -1px;
left: -1px;
right: -1px;
bottom: -1px;
border: 2px solid $blue-medium-500;
}
border-color: $blue-medium-500;
box-shadow: inset 0 0 0 1px $blue-medium-500;
border-style: double;
}

&__cell-content {
Expand Down
33 changes: 18 additions & 15 deletions packages/block-library/src/table/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,18 @@ export function getFirstRow( state ) {
/**
* Gets an attribute for a cell.
*
* @param {Object} state Current table state.
* @param {string} options.sectionName Section of the cell to update.
* @param {number} options.rowIndex Row index of the cell to update.
* @param {number} options.columnIndex Column index of the cell to update.
* @param {number} options.attributeName The name of the attribute to get the value of.
* @param {Object} state Current table state.
* @param {Object} cellLocation The location of the cell
* @param {string} attributeName The name of the attribute to get the value of.
*
* @return {*} The attribute value.
*/
export function getCellAttribute( state, {
sectionName,
rowIndex,
columnIndex,
attributeName,
} ) {
export function getCellAttribute( state, cellLocation, attributeName ) {
const {
sectionName,
rowIndex,
columnIndex,
} = cellLocation;
return get( state, [ sectionName, rowIndex, 'cells', columnIndex, attributeName ] );
}

Expand Down Expand Up @@ -142,9 +140,9 @@ export function isCellSelected( cellLocation, selection ) {
/**
* Inserts a row in the table state.
*
* @param {Object} state Current table state.
* @param {string} options.sectionName Section in which to insert the row.
* @param {number} options.rowIndex Row index at which to insert the row.
* @param {Object} state Current table state.
* @param {string} options.sectionName Section in which to insert the row.
* @param {number} options.rowIndex Row index at which to insert the row.
*
* @return {Object} New table state.
*/
Expand All @@ -154,7 +152,12 @@ export function insertRow( state, {
columnCount,
} ) {
const firstRow = getFirstRow( state );
const cellCount = columnCount || get( firstRow, [ 'cells', 'length' ], 2 );
const cellCount = columnCount === undefined ? get( firstRow, [ 'cells', 'length' ] ) : columnCount;

// Bail early if the function cannot determine how many cells to add.
if ( ! cellCount ) {
return state;
}

return {
[ sectionName ]: [
Expand Down
26 changes: 23 additions & 3 deletions packages/block-library/src/table/test/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,12 @@ describe( 'getFirstRow', () => {

describe( 'getCellAttribute', () => {
it( 'should get the cell attribute', () => {
const state = getCellAttribute( tableWithAttribute, {
const cellLocation = {
sectionName: 'body',
rowIndex: 1,
columnIndex: 1,
attributeName: 'testAttr',
} );
};
const state = getCellAttribute( tableWithAttribute, cellLocation, 'testAttr' );

expect( state ).toBe( 'testVal' );
} );
Expand Down Expand Up @@ -369,6 +369,26 @@ describe( 'insertRow', () => {

expect( state ).toEqual( expected );
} );

it( 'should have no effect if `columnCount` is not provided and the table has no existing rows', () => {
const existingState = { body: {} };
const newState = insertRow( existingState, {
sectionName: 'body',
rowIndex: 0,
} );

expect( newState ).toBe( existingState );
} );

it( 'should have no effect if `columnCount` is `0`', () => {
const state = insertRow( tableWithHead, {
sectionName: 'head',
rowIndex: 1,
columnCount: 0,
} );

expect( state ).toBe( tableWithHead );
} );
} );

describe( 'insertColumn', () => {
Expand Down

0 comments on commit f00dd4e

Please sign in to comment.