Skip to content

Commit

Permalink
Performance: Optimize adjacent block selectors
Browse files Browse the repository at this point in the history
  • Loading branch information
youknowriad committed Feb 13, 2018
1 parent 15e63bf commit 124ea5d
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 103 deletions.
14 changes: 7 additions & 7 deletions editor/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ import {
isMultiSelecting,
getBlockIndex,
getEditedPostAttribute,
getNextBlock,
getPreviousBlock,
getNextBlockUid,
getPreviousBlockUid,
isBlockHovered,
isBlockMultiSelected,
isBlockSelected,
Expand Down Expand Up @@ -610,17 +610,17 @@ export class BlockListBlock extends Component {
}

const mapStateToProps = ( state, { uid, rootUID } ) => {
const previousBlock = getPreviousBlock( state, uid );
const nextBlock = getNextBlock( state, uid );
const isSelected = isBlockSelected( state, uid );
return {
previousBlockUid: !! previousBlock && previousBlock.uid,
nextBlockUid: !! nextBlock && nextBlock.uid,
previousBlockUid: getPreviousBlockUid( state, uid ),
nextBlockUid: getNextBlockUid( state, uid ),
block: getBlock( state, uid ),
isMultiSelected: isBlockMultiSelected( state, uid ),
isFirstMultiSelected: isFirstMultiSelectedBlock( state, uid ),
isHovered: isBlockHovered( state, uid ) && ! isMultiSelecting( state ),
isTyping: isTyping( state ),
// We only care about this prop when the block is selected
// Thus to avoid unnecessary rerenders we avoid updating the prop if the block is not selected.
isTyping: isSelected && isTyping( state ),
order: getBlockIndex( state, uid, rootUID ),
meta: getEditedPostAttribute( state, 'meta' ),
mode: getBlockMode( state, uid ),
Expand Down
24 changes: 12 additions & 12 deletions editor/components/writing-flow/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import {
placeCaretAtVerticalEdge,
} from '../../utils/dom';
import {
getPreviousBlock,
getNextBlock,
getPreviousBlockUid,
getNextBlockUid,
getMultiSelectedBlocksStartUid,
getMultiSelectedBlocks,
getSelectedBlock,
Expand Down Expand Up @@ -155,20 +155,20 @@ class WritingFlow extends Component {
}

expandSelection( currentStartUid, isReverse ) {
const { previousBlock, nextBlock } = this.props;
const { previousBlockUid, nextBlockUid } = this.props;

const expandedBlock = isReverse ? previousBlock : nextBlock;
if ( expandedBlock ) {
this.props.onMultiSelect( currentStartUid, expandedBlock.uid );
const expandedBlockUid = isReverse ? previousBlockUid : nextBlockUid;
if ( expandedBlockUid ) {
this.props.onMultiSelect( currentStartUid, expandedBlockUid );
}
}

moveSelection( isReverse ) {
const { previousBlock, nextBlock } = this.props;
const { previousBlockUid, nextBlockUid } = this.props;

const focusedBlock = isReverse ? previousBlock : nextBlock;
if ( focusedBlock ) {
this.props.onSelectBlock( focusedBlock.uid );
const focusedBlockUid = isReverse ? previousBlockUid : nextBlockUid;
if ( focusedBlockUid ) {
this.props.onSelectBlock( focusedBlockUid );
}
}

Expand Down Expand Up @@ -299,8 +299,8 @@ class WritingFlow extends Component {

export default connect(
( state ) => ( {
previousBlock: getPreviousBlock( state ),
nextBlock: getNextBlock( state ),
previousBlockUid: getPreviousBlockUid( state ),
nextBlockUid: getNextBlockUid( state ),
selectionStart: getMultiSelectedBlocksStartUid( state ),
hasMultiSelection: getMultiSelectedBlocks( state ).length > 1,
selectedBlock: getSelectedBlock( state ),
Expand Down
24 changes: 12 additions & 12 deletions editor/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -538,17 +538,17 @@ export function getBlockRootUID( state, uid ) {
}

/**
* Returns the block adjacent one at the given reference startUID and modifier
* Returns the UID of the block adjacent one at the given reference startUID and modifier
* directionality. Defaults start UID to the selected block, and direction as
* next block. Returns null if there is no adjacent block.
*
* @param {Object} state Global application state.
* @param {?string} startUID Optional UID of block from which to search.
* @param {?number} modifier Directionality multiplier (1 next, -1 previous).
*
* @return {?Object} Adjacent block object, or null if none exists.
* @return {?string} Return the UID of the block, or null if none exists.
*/
export function getAdjacentBlock( state, startUID, modifier = 1 ) {
export function getAdjacentBlockUid( state, startUID, modifier = 1 ) {
// Default to selected block.
if ( startUID === undefined ) {
startUID = get( getSelectedBlock( state ), 'uid' );
Expand Down Expand Up @@ -591,33 +591,33 @@ export function getAdjacentBlock( state, startUID, modifier = 1 ) {
}

// Assume incremented index is within the set.
return getBlock( state, orderSet[ nextIndex ] );
return orderSet[ nextIndex ];
}

/**
* Returns the previous block from the given reference startUID. Defaults start
* Returns the previous block's UID from the given reference startUID. Defaults start
* UID to the selected block. Returns null if there is no previous block.
*
* @param {Object} state Global application state.
* @param {?string} startUID Optional UID of block from which to search.
*
* @return {?Object} Adjacent block object, or null if none exists.
* @return {?string} Adjacent block's UID, or null if none exists.
*/
export function getPreviousBlock( state, startUID ) {
return getAdjacentBlock( state, startUID, -1 );
export function getPreviousBlockUid( state, startUID ) {
return getAdjacentBlockUid( state, startUID, -1 );
}

/**
* Returns the next block from the given reference startUID. Defaults start UID
* Returns the next block's UID from the given reference startUID. Defaults start UID
* to the selected block. Returns null if there is no next block.
*
* @param {Object} state Global application state.
* @param {?string} startUID Optional UID of block from which to search.
*
* @return {?Object} Adjacent block object, or null if none exists.
* @return {?string} Adjacent block's UID, or null if none exists.
*/
export function getNextBlock( state, startUID ) {
return getAdjacentBlock( state, startUID, 1 );
export function getNextBlockUid( state, startUID ) {
return getAdjacentBlockUid( state, startUID, 1 );
}

/**
Expand Down
84 changes: 12 additions & 72 deletions editor/store/test/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ const {
getMultiSelectedBlocksEndUid,
getBlockOrder,
getBlockIndex,
getPreviousBlock,
getNextBlock,
getPreviousBlockUid,
getNextBlockUid,
isBlockSelected,
isBlockWithinSelection,
isBlockMultiSelected,
Expand Down Expand Up @@ -1372,40 +1372,25 @@ describe( 'selectors', () => {
} );
} );

describe( 'getPreviousBlock', () => {
describe( 'getPreviousBlockUid', () => {
it( 'should return the previous block', () => {
const state = {
editor: {
present: {
blocksByUid: {
23: { uid: 23, name: 'core/heading', attributes: {} },
123: { uid: 123, name: 'core/paragraph', attributes: {} },
},
blockOrder: {
'': [ 123, 23 ],
},
},
},
};

expect( getPreviousBlock( state, 23 ) ).toEqual( {
uid: 123,
name: 'core/paragraph',
attributes: {},
innerBlocks: [],
} );
expect( getPreviousBlockUid( state, 23 ) ).toEqual( 123 );
} );

it( 'should return the previous block (nested context)', () => {
const state = {
editor: {
present: {
blocksByUid: {
23: { uid: 23, name: 'core/heading', attributes: {} },
123: { uid: 123, name: 'core/paragraph', attributes: {} },
56: { uid: 56, name: 'core/heading', attributes: {} },
456: { uid: 456, name: 'core/paragraph', attributes: {} },
},
blockOrder: {
'': [ 123, 23 ],
123: [ 456, 56 ],
Expand All @@ -1414,42 +1399,27 @@ describe( 'selectors', () => {
},
};

expect( getPreviousBlock( state, 56, '123' ) ).toEqual( {
uid: 456,
name: 'core/paragraph',
attributes: {},
innerBlocks: [],
} );
expect( getPreviousBlockUid( state, 56, '123' ) ).toEqual( 456 );
} );

it( 'should return null for the first block', () => {
const state = {
editor: {
present: {
blocksByUid: {
23: { uid: 23, name: 'core/heading', attributes: {} },
123: { uid: 123, name: 'core/paragraph', attributes: {} },
},
blockOrder: {
'': [ 123, 23 ],
},
},
},
};

expect( getPreviousBlock( state, 123 ) ).toBeNull();
expect( getPreviousBlockUid( state, 123 ) ).toBeNull();
} );

it( 'should return null for the first block (nested context)', () => {
const state = {
editor: {
present: {
blocksByUid: {
23: { uid: 23, name: 'core/heading', attributes: {} },
123: { uid: 123, name: 'core/paragraph', attributes: {} },
56: { uid: 56, name: 'core/heading', attributes: {} },
456: { uid: 456, name: 'core/paragraph', attributes: {} },
},
blockOrder: {
'': [ 123, 23 ],
123: [ 456, 56 ],
Expand All @@ -1458,44 +1428,29 @@ describe( 'selectors', () => {
},
};

expect( getPreviousBlock( state, 456, '123' ) ).toBeNull();
expect( getPreviousBlockUid( state, 456, '123' ) ).toBeNull();
} );
} );

describe( 'getNextBlock', () => {
describe( 'getNextBlockUid', () => {
it( 'should return the following block', () => {
const state = {
editor: {
present: {
blocksByUid: {
23: { uid: 23, name: 'core/heading', attributes: {} },
123: { uid: 123, name: 'core/paragraph', attributes: {} },
},
blockOrder: {
'': [ 123, 23 ],
},
},
},
};

expect( getNextBlock( state, 123 ) ).toEqual( {
uid: 23,
name: 'core/heading',
attributes: {},
innerBlocks: [],
} );
expect( getNextBlockUid( state, 123 ) ).toEqual( 23 );
} );

it( 'should return the following block (nested context)', () => {
const state = {
editor: {
present: {
blocksByUid: {
23: { uid: 23, name: 'core/heading', attributes: {} },
123: { uid: 123, name: 'core/paragraph', attributes: {} },
56: { uid: 56, name: 'core/heading', attributes: {} },
456: { uid: 456, name: 'core/paragraph', attributes: {} },
},
blockOrder: {
'': [ 123, 23 ],
123: [ 456, 56 ],
Expand All @@ -1504,42 +1459,27 @@ describe( 'selectors', () => {
},
};

expect( getNextBlock( state, 456, '123' ) ).toEqual( {
uid: 56,
name: 'core/heading',
attributes: {},
innerBlocks: [],
} );
expect( getNextBlockUid( state, 456, '123' ) ).toEqual( 56 );
} );

it( 'should return null for the last block', () => {
const state = {
editor: {
present: {
blocksByUid: {
23: { uid: 23, name: 'core/heading', attributes: {} },
123: { uid: 123, name: 'core/paragraph', attributes: {} },
},
blockOrder: {
'': [ 123, 23 ],
},
},
},
};

expect( getNextBlock( state, 23 ) ).toBeNull();
expect( getNextBlockUid( state, 23 ) ).toBeNull();
} );

it( 'should return null for the last block (nested context)', () => {
const state = {
editor: {
present: {
blocksByUid: {
23: { uid: 23, name: 'core/heading', attributes: {} },
123: { uid: 123, name: 'core/paragraph', attributes: {} },
56: { uid: 56, name: 'core/heading', attributes: {} },
456: { uid: 456, name: 'core/paragraph', attributes: {} },
},
blockOrder: {
'': [ 123, 23 ],
123: [ 456, 56 ],
Expand All @@ -1548,7 +1488,7 @@ describe( 'selectors', () => {
},
};

expect( getNextBlock( state, 56, '123' ) ).toBeNull();
expect( getNextBlockUid( state, 56, '123' ) ).toBeNull();
} );
} );

Expand Down

0 comments on commit 124ea5d

Please sign in to comment.