Skip to content

Commit

Permalink
Remove superflouous update of selectedCell already handled by RichTex…
Browse files Browse the repository at this point in the history
…t unstableOnFocus prop
  • Loading branch information
talldan committed Aug 9, 2019
1 parent f874a33 commit 1b17951
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 13 deletions.
7 changes: 3 additions & 4 deletions packages/block-library/src/table/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
*/
import {
createTable,
updateSelectedCell,
updateSelectedCellAttributes,
getCellAttribute,
insertRow,
deleteRow,
Expand Down Expand Up @@ -182,7 +182,7 @@ export class TableEdit extends Component {

const { attributes, setAttributes } = this.props;

setAttributes( updateSelectedCell(
setAttributes( updateSelectedCellAttributes(
attributes,
selectedCell,
( cellAttributes ) => ( { ...cellAttributes, content } ),
Expand All @@ -209,7 +209,7 @@ export class TableEdit extends Component {
};

const { attributes, setAttributes } = this.props;
const newAttributes = updateSelectedCell(
const newAttributes = updateSelectedCellAttributes(
attributes,
columnSelection,
( cellAttributes ) => ( { ...cellAttributes, align } ),
Expand Down Expand Up @@ -599,7 +599,6 @@ export class TableEdit extends Component {
className={ tableClasses }
tableState={ attributes }
selectedCell={ selectedCell }
onNavigation={ ( cellLocation ) => this.updateSelectedCell( cellLocation ) }
>
<Section name="head" rows={ head } />
<Section name="body" rows={ body } />
Expand Down
3 changes: 1 addition & 2 deletions packages/block-library/src/table/navigable-table.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function getCellContentEditableElement( tableElement, { sectionName, rowIndex, c
return rowElement.querySelectorAll( '*[contenteditable="true"]' )[ columnIndex ];
}

export default function NavigableTable( { children, className, tableState, selectedCell, onNavigation } ) {
export default function NavigableTable( { children, className, tableState, selectedCell } ) {
const tableRef = useRef();

const handleKeyDown = ( event ) => {
Expand Down Expand Up @@ -143,7 +143,6 @@ export default function NavigableTable( { children, className, tableState, selec
}

placeCaretAtHorizontalEdge( contentEditableElement, isReverse );
onNavigation( nextCellLocation );
};

// Disable reason: Wrapper itself is non-interactive, but must capture
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/table/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export function getCellAttribute( state, cellLocation, attributeName ) {
*
* @return {Object} New table state including the updated cells.
*/
export function updateSelectedCell( state, selection, updateCell ) {
export function updateSelectedCellAttributes( state, selection, updateCell ) {
if ( ! selection ) {
return state;
}
Expand Down
12 changes: 6 additions & 6 deletions packages/block-library/src/table/test/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
isEmptyTableSection,
isEmptyRow,
isCellSelected,
updateSelectedCell,
updateSelectedCellAttributes,
getCellAbove,
getCellBelow,
getCellToLeft,
Expand Down Expand Up @@ -1213,21 +1213,21 @@ describe( 'isCellSelected', () => {
} );
} );

describe( 'updateSelectedCell', () => {
describe( 'updateSelectedCellAttributes', () => {
it( 'returns an unchanged table state if there is no selection', () => {
const updated = updateSelectedCell( table, undefined, ( cell ) => ( { ...cell, content: 'test' } ) );
const updated = updateSelectedCellAttributes( table, undefined, ( cell ) => ( { ...cell, content: 'test' } ) );
expect( table ).toEqual( updated );
} );

it( 'returns an unchanged table state if the selection is outside the bounds of the table', () => {
const cellSelection = { type: 'cell', sectionName: 'body', rowIndex: 100, columnIndex: 100 };
const updated = updateSelectedCell( table, cellSelection, ( cell ) => ( { ...cell, content: 'test' } ) );
const updated = updateSelectedCellAttributes( table, cellSelection, ( cell ) => ( { ...cell, content: 'test' } ) );
expect( table ).toEqual( updated );
} );

it( 'updates only the individual cell when the selection type is `cell`', () => {
const cellSelection = { type: 'cell', sectionName: 'body', rowIndex: 0, columnIndex: 0 };
const updated = updateSelectedCell( table, cellSelection, ( cell ) => ( { ...cell, content: 'test' } ) );
const updated = updateSelectedCellAttributes( table, cellSelection, ( cell ) => ( { ...cell, content: 'test' } ) );

expect( updated ).toEqual( {
body: [
Expand All @@ -1247,7 +1247,7 @@ describe( 'updateSelectedCell', () => {

it( 'updates every cell in the column when the selection type is `column`', () => {
const cellSelection = { type: 'column', columnIndex: 1 };
const updated = updateSelectedCell( table, cellSelection, ( cell ) => ( { ...cell, content: 'test' } ) );
const updated = updateSelectedCellAttributes( table, cellSelection, ( cell ) => ( { ...cell, content: 'test' } ) );

expect( updated ).toEqual( {
body: [
Expand Down

0 comments on commit 1b17951

Please sign in to comment.