Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid calling redux actions constantly when moving the mouse or scrolling #44325

Merged
merged 4 commits into from
Sep 21, 2022
Merged

Avoid calling redux actions constantly when moving the mouse or scrolling #44325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ca9c830
Avoid calling redux actions constantly when moving the mouse or scrol…
youknowriad Sep 21, 2022
53006e8
Make the undo/redo stack indepedent of the blockVisibility changes
youknowriad Sep 21, 2022
c2ef6c9
Fix typo
youknowriad Sep 21, 2022
9194d1a
Fix e2e test
youknowriad Sep 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 42 additions & 6 deletions packages/block-editor/src/components/block-list/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,23 @@ import classnames from 'classnames';
/**
* WordPress dependencies
*/
import { AsyncModeProvider, useSelect, useDispatch } from '@wordpress/data';
import { useViewportMatch, useMergeRefs } from '@wordpress/compose';
import { createContext, useState, useMemo } from '@wordpress/element';
import {
AsyncModeProvider,
useSelect,
useDispatch,
useRegistry,
} from '@wordpress/data';
import {
useViewportMatch,
useMergeRefs,
useDebounce,
} from '@wordpress/compose';
import {
createContext,
useState,
useMemo,
useCallback,
} from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -30,6 +44,7 @@ import {
const elementContext = createContext();

export const IntersectionObserver = createContext();
const pendingBlockVisibilityUpdatesPerRegistry = new WeakMap();

function Root( { className, ...settings } ) {
const [ element, setElement ] = useState();
Expand All @@ -47,7 +62,24 @@ function Root( { className, ...settings } ) {
},
[]
);
const registry = useRegistry();
const { setBlockVisibility } = useDispatch( blockEditorStore );

const delayedBlockVisibilityUpdates = useDebounce(
useCallback( () => {
const updates = {};
pendingBlockVisibilityUpdatesPerRegistry
.get( registry )
.forEach( ( [ id, isIntersecting ] ) => {
updates[ id ] = isIntersecting;
} );
setBlockVisibility( updates );
}, [ registry ] ),
300,
{
trailing: true,
}
);
const intersectionObserver = useMemo( () => {
const { IntersectionObserver: Observer } = window;

Expand All @@ -56,12 +88,16 @@ function Root( { className, ...settings } ) {
}

return new Observer( ( entries ) => {
const updates = {};
if ( ! pendingBlockVisibilityUpdatesPerRegistry.get( registry ) ) {
pendingBlockVisibilityUpdatesPerRegistry.set( registry, [] );
}
for ( const entry of entries ) {
const clientId = entry.target.getAttribute( 'data-block' );
updates[ clientId ] = entry.isIntersecting;
pendingBlockVisibilityUpdatesPerRegistry
.get( registry )
.push( [ clientId, entry.isIntersecting ] );
}
setBlockVisibility( updates );
delayedBlockVisibilityUpdates();
} );
}, [] );
const innerBlocksProps = useInnerBlocksProps(
Expand Down
12 changes: 9 additions & 3 deletions packages/block-editor/src/components/block-list/use-in-between-inserter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
/**
* WordPress dependencies
*/
import { useRefEffect } from '@wordpress/compose';

import { useRefEffect, useDebounce } from '@wordpress/compose';
import { useSelect, useDispatch } from '@wordpress/data';
import { useContext } from '@wordpress/element';

Expand Down Expand Up @@ -33,6 +32,10 @@ export function useInBetweenInserter() {
const { showInsertionPoint, hideInsertionPoint } =
useDispatch( blockEditorStore );

const delayedShowInsertionPoint = useDebounce( showInsertionPoint, 500, {
trailing: true,
} );

return useRefEffect(
( node ) => {
if ( isInBetweenInserterDisabled ) {
Expand All @@ -53,6 +56,7 @@ export function useInBetweenInserter() {
'block-editor-block-list__layout'
)
) {
delayedShowInsertionPoint.cancel();
if ( isBlockInsertionPointVisible() ) {
hideInsertionPoint();
}
Expand Down Expand Up @@ -134,6 +138,7 @@ export function useInBetweenInserter() {
( event.clientX > elementRect.right ||
event.clientX < elementRect.left ) )
) {
delayedShowInsertionPoint.cancel();
if ( isBlockInsertionPointVisible() ) {
hideInsertionPoint();
}
Expand All @@ -145,13 +150,14 @@ export function useInBetweenInserter() {
// Don't show the in-between inserter before the first block in
// the list (preserves the original behaviour).
if ( index === 0 ) {
delayedShowInsertionPoint.cancel();
if ( isBlockInsertionPointVisible() ) {
hideInsertionPoint();
}
return;
}

showInsertionPoint( rootClientId, index, {
delayedShowInsertionPoint( rootClientId, index, {
__unstableWithInserter: true,
} );
}
Expand Down
6 changes: 3 additions & 3 deletions packages/block-editor/src/components/block-tools/insertion-point.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,13 @@ function InsertionPointPopover( {
...( ! isVertical ? horizontalLine.rest : verticalLine.rest ),
opacity: 1,
borderRadius: '2px',
transition: { delay: isInserterShown ? 0.5 : 0, type: 'tween' },
transition: { delay: isInserterShown ? 0.1 : 0, type: 'tween' },
},
hover: {
...( ! isVertical ? horizontalLine.hover : verticalLine.hover ),
opacity: 1,
borderRadius: '2px',
transition: { delay: 0.5, type: 'tween' },
transition: { delay: 0.1, type: 'tween' },
},
};

Expand All @@ -165,7 +165,7 @@ function InsertionPointPopover( {
},
rest: {
scale: 1,
transition: { delay: 0.4, type: 'tween' },
transition: { type: 'tween' },
},
};

Expand Down
36 changes: 22 additions & 14 deletions packages/block-editor/src/store/reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,6 @@ const withBlockReset = ( reducer ) => ( state, action ) => {
order: mapBlockOrder( action.blocks ),
parents: mapBlockParents( action.blocks ),
controlledInnerBlocks: {},
visibility: {},
};

const subTree = buildBlockTree( newState, action.blocks );
Expand Down Expand Up @@ -1162,17 +1161,6 @@ export const blocks = flow(
}
return state;
},

visibility( state = {}, action ) {
if ( action.type === 'SET_BLOCK_VISIBILITY' ) {
return {
...state,
...action.updates,
};
}

return state;
},
} );

/**
Expand Down Expand Up @@ -1215,6 +1203,25 @@ export function draggedBlocks( state = [], action ) {
return state;
}

/**
* Reducer tracking the visible blocks.
*
* @param {Record<string,boolean>} state Current state.
* @param {Object} action Dispatched action.
*
* @return {Record<string,boolean>} Block visibility.
*/
export function blockVisibility( state = {}, action ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving blockVisibility state of the "blocks" state means any change to "blockVisibility" won't have an impact on the undo/redo behavior.

before this a call to SET_BLOCK_VISIBILITY would force an undo state even if we were typing in the middle of a word.

if ( action.type === 'SET_BLOCK_VISIBILITY' ) {
return {
...state,
...action.updates,
};
}

return state;
}

/**
* Internal helper reducer for selectionStart and selectionEnd. Can hold a block
* selection, represented by an object with property clientId.
Expand Down Expand Up @@ -1660,7 +1667,7 @@ export function hasBlockMovingClientId( state = null, action ) {
*
* @return {[string,Object]} Updated state.
*/
export function lastBlockAttributesChange( state, action ) {
export function lastBlockAttributesChange( state = null, action ) {
switch ( action.type ) {
case 'UPDATE_BLOCK':
if ( ! action.updates.attributes ) {
Expand All @@ -1681,7 +1688,7 @@ export function lastBlockAttributesChange( state, action ) {
);
}

return null;
return state;
}

/**
Expand Down Expand Up @@ -1813,4 +1820,5 @@ export default combineReducers( {
highlightedBlock,
lastBlockInserted,
temporarilyEditingAsBlocks,
blockVisibility,
} );
8 changes: 4 additions & 4 deletions packages/block-editor/src/store/selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2670,7 +2670,7 @@ export function wasBlockJustInserted( state, clientId, source ) {
* @return {boolean} True if the block is visible.
*/
export function isBlockVisible( state, clientId ) {
return state.blocks.visibility?.[ clientId ] ?? true;
return state.blockVisibility?.[ clientId ] ?? true;
}

/**
Expand All @@ -2682,12 +2682,12 @@ export function isBlockVisible( state, clientId ) {
export const __unstableGetVisibleBlocks = createSelector(
( state ) => {
return new Set(
Object.keys( state.blocks.visibility ).filter(
( key ) => state.blocks.visibility[ key ]
Object.keys( state.blockVisibility ).filter(
( key ) => state.blockVisibility[ key ]
)
);
},
( state ) => [ state.blocks.visibility ]
( state ) => [ state.blockVisibility ]
);

export const __unstableGetContentLockingParent = createSelector(
Expand Down
17 changes: 0 additions & 17 deletions packages/block-editor/src/store/test/reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,6 @@ describe( 'state', () => {
chicken: '',
},
controlledInnerBlocks: {},
visibility: {},
} );
expect( state.tree.chicken ).not.toBe(
existingState.tree.chicken
Expand Down Expand Up @@ -375,7 +374,6 @@ describe( 'state', () => {
chicken: '',
},
controlledInnerBlocks: {},
visibility: {},
} );
expect( state.tree.chicken ).not.toBe(
existingState.tree.chicken
Expand Down Expand Up @@ -525,7 +523,6 @@ describe( 'state', () => {
[ newChildBlockId3 ]: 'chicken',
},
controlledInnerBlocks: {},
visibility: {},
} );

expect( state.tree[ '' ].innerBlocks[ 0 ] ).toBe(
Expand Down Expand Up @@ -635,7 +632,6 @@ describe( 'state', () => {
[ newChildBlockId ]: 'chicken',
},
controlledInnerBlocks: {},
visibility: {},
} );

// The block object of the parent should be updated.
Expand All @@ -657,7 +653,6 @@ describe( 'state', () => {
isIgnoredChange: false,
tree: {},
controlledInnerBlocks: {},
visibility: {},
} );
} );

Expand Down Expand Up @@ -3085,18 +3080,6 @@ describe( 'state', () => {
'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1': { food: 'banana' },
} );
} );

it( 'returns null on anything other than block attributes update', () => {
draganescu marked this conversation as resolved.
Show resolved Hide resolved
const original = deepFreeze( {
'afd1cb17-2c08-4e7a-91be-007ba7ddc3a1': { food: 'banana' },
} );

const state = lastBlockAttributesChange( original, {
type: '__INERT__',
} );

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

describe( 'lastBlockInserted', () => {
Expand Down
3 changes: 3 additions & 0 deletions packages/e2e-tests/specs/editor/plugins/cpt-locking.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ describe( 'cpt locking', () => {

it( 'should not allow blocks to be inserted in inner blocks', async () => {
await page.click( 'button[aria-label="Two columns; equal split"]' );
await page.evaluate(
() => new Promise( window.requestIdleCallback )
);
expect(
await page.$(
'.wp-block-column .block-editor-button-block-appender'
Expand Down