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

Block editor: rewrite moving animation for better load performance #57133

Merged
merged 4 commits into from
Dec 16, 2023
Merged

Block editor: rewrite moving animation for better load performance #57133

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4f9ae7d
wip
ellatrix Dec 16, 2023
c29fafb
Fix everything
ellatrix Dec 16, 2023
321b963
Check reduced motion and call selectors in callback
ellatrix Dec 16, 2023
4bb81c4
Comments
ellatrix Dec 16, 2023
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
23 changes: 0 additions & 23 deletions packages/block-editor/src/components/block-list/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,6 @@ import { PrivateBlockContext } from './private-block-context';

import { unlock } from '../../lock-unlock';

/**
* If the block count exceeds the threshold, we disable the reordering animation
* to avoid laginess.
*/
const BLOCK_ANIMATION_THRESHOLD = 200;

/**
* Merges wrapper props with special handling for classNames and styles.
*
Expand Down Expand Up @@ -516,9 +510,7 @@ function BlockListBlockProvider( props ) {

getBlockIndex,
isTyping,
getGlobalBlockCount,
isBlockMultiSelected,
isAncestorMultiSelected,
isBlockSubtreeDisabled,
isBlockHighlighted,
__unstableIsFullySelected,
Expand Down Expand Up @@ -550,9 +542,6 @@ function BlockListBlockProvider( props ) {
const canRemove = canRemoveBlock( clientId, rootClientId );
const canMove = canMoveBlock( clientId, rootClientId );
const { name: blockName, attributes, isValid } = block;
const isPartOfMultiSelection =
isBlockMultiSelected( clientId ) ||
isAncestorMultiSelected( clientId );
const blockType = getBlockType( blockName );
const match = getActiveBlockVariation( blockName, attributes );
const { outlineMode, supportsLayout } = getSettings();
Expand Down Expand Up @@ -600,12 +589,6 @@ function BlockListBlockProvider( props ) {
index: getBlockIndex( clientId ),
blockApiVersion: blockType?.apiVersion || 1,
blockTitle: match?.title || blockType?.title,
isPartOfSelection: _isSelected || isPartOfMultiSelection,
adjustScrolling:
_isSelected || isFirstMultiSelectedBlock( clientId ),
enableAnimation:
! typing &&
getGlobalBlockCount() <= BLOCK_ANIMATION_THRESHOLD,
isSubtreeDisabled: isBlockSubtreeDisabled( clientId ),
isOutlineEnabled: outlineMode,
hasOverlay: __unstableHasActiveBlockOverlayActive( clientId ),
Expand Down Expand Up @@ -662,9 +645,6 @@ function BlockListBlockProvider( props ) {
index,
blockApiVersion,
blockTitle,
isPartOfSelection,
adjustScrolling,
enableAnimation,
isSubtreeDisabled,
isOutlineEnabled,
hasOverlay,
Expand Down Expand Up @@ -699,9 +679,6 @@ function BlockListBlockProvider( props ) {
blockApiVersion,
blockTitle,
isSelected,
isPartOfSelection,
adjustScrolling,
enableAnimation,
isSubtreeDisabled,
isOutlineEnabled,
hasOverlay,
Expand Down
10 changes: 1 addition & 9 deletions packages/block-editor/src/components/block-list/use-block-props/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,6 @@ export function useBlockProps( props = {}, { __unstableIsHtml } = {} ) {
blockApiVersion,
blockTitle,
isSelected,
isPartOfSelection,
adjustScrolling,
enableAnimation,
isSubtreeDisabled,
isOutlineEnabled,
hasOverlay,
Expand Down Expand Up @@ -114,12 +111,7 @@ export function useBlockProps( props = {}, { __unstableIsHtml } = {} ) {
useNavModeExit( clientId ),
useIsHovered( { isEnabled: isOutlineEnabled } ),
useIntersectionObserver(),
useMovingAnimation( {
isSelected: isPartOfSelection,
adjustScrolling,
enableAnimation,
triggerAnimationOnChange: index,
} ),
useMovingAnimation( { triggerAnimationOnChange: index, clientId } ),
useDisabled( { isDisabled: ! hasOverlay } ),
] );

Expand Down
78 changes: 51 additions & 27 deletions packages/block-editor/src/components/use-moving-animation/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,19 @@ import { Controller } from '@react-spring/web';
* WordPress dependencies
*/
import { useLayoutEffect, useMemo, useRef } from '@wordpress/element';
import { useReducedMotion } from '@wordpress/compose';
import { getScrollContainer } from '@wordpress/dom';
import { useSelect } from '@wordpress/data';

/**
* Internal dependencies
*/
import { store as blockEditorStore } from '../../store';

/**
* If the block count exceeds the threshold, we disable the reordering animation
* to avoid laginess.
*/
const BLOCK_ANIMATION_THRESHOLD = 200;
Copy link
Contributor

Choose a reason for hiding this comment

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

By moving this into useMovingAnimation, the moving animation is now prevented in the list view when the global block count is greater than 200. Was that intentional?

I think for the list view case, where list items are displayed conditionally based on windowing logic, we might not need to guard it behind this global block count threshold. The reason I'm thinking about this is because over in #56625 I'm exploring displacing list view items when dragging within the list view, and useMovingAnimation provides a handy way of smoothly animating a dropped list view item into its final position.

I noticed while editing the blog home template in TT4 that the getGlobalBlockCount() already starts out pretty high (165 in my test site) so it's quite easy to get to over 200 and then lose the animation.


function getAbsolutePosition( element ) {
return {
Expand All @@ -28,30 +39,20 @@ function getAbsolutePosition( element ) {
* - It uses the "resetAnimation" flag to reset the animation
* from the beginning in order to animate to the new destination point.
*
* @param {Object} $1 Options
* @param {boolean} $1.isSelected Whether it's the current block or not.
* @param {boolean} $1.adjustScrolling Adjust the scroll position to the current block.
* @param {boolean} $1.enableAnimation Enable/Disable animation.
* @param {*} $1.triggerAnimationOnChange Variable used to trigger the animation if it changes.
* @param {Object} $1 Options
* @param {*} $1.triggerAnimationOnChange Variable used to trigger the animation if it changes.
* @param {string} $1.clientId
*/
function useMovingAnimation( {
isSelected,
adjustScrolling,
enableAnimation,
triggerAnimationOnChange,
} ) {
function useMovingAnimation( { triggerAnimationOnChange, clientId } ) {
const ref = useRef();
const prefersReducedMotion = useReducedMotion() || ! enableAnimation;

// We do not want to trigger the animation when a block gets selected, so we
// use a ref to keep track of the value for use later in a callback.
const isSelectedRef = useRef( isSelected );
const adjustScrollingRef = useRef( adjustScrolling );

useLayoutEffect( () => {
isSelectedRef.current = isSelected;
adjustScrollingRef.current = adjustScrolling;
}, [ isSelected, adjustScrolling ] );
const {
isTyping,
getGlobalBlockCount,
isBlockSelected,
isFirstMultiSelectedBlock,
isBlockMultiSelected,
isAncestorMultiSelected,
} = useSelect( blockEditorStore );

// Whenever the trigger changes, we need to take a snapshot of the current
// position of the block to use it as a destination point for the animation.
Expand All @@ -70,9 +71,12 @@ function useMovingAnimation( {
}

const scrollContainer = getScrollContainer( ref.current );
const isSelected = isBlockSelected( clientId );
const adjustScrolling =
isSelected || isFirstMultiSelectedBlock( clientId );

function preserveScrollPosition() {
if ( adjustScrollingRef.current && prevRect ) {
if ( adjustScrolling && prevRect ) {
const blockRect = ref.current.getBoundingClientRect();
const diff = blockRect.top - prevRect.top;

Expand All @@ -82,13 +86,23 @@ function useMovingAnimation( {
}
}

if ( prefersReducedMotion ) {
if (
window.matchMedia( '(prefers-reduced-motion: reduce)' ).matches ||
isTyping() ||
getGlobalBlockCount() > BLOCK_ANIMATION_THRESHOLD
ellatrix marked this conversation as resolved.
Show resolved Hide resolved
) {
// If the animation is disabled and the scroll needs to be adjusted,
// just move directly to the final scroll position.
preserveScrollPosition();
return;
}

const isPartOfSelection =
isSelected ||
isBlockMultiSelected( clientId ) ||
isAncestorMultiSelected( clientId );
const zIndex = isPartOfSelection ? '1' : '';
ellatrix marked this conversation as resolved.
Show resolved Hide resolved

const controller = new Controller( {
x: 0,
y: 0,
Expand All @@ -105,7 +119,7 @@ function useMovingAnimation( {
ref.current.style.transform = finishedMoving
? null // Set to `null` to explicitly remove the transform.
: `translate3d(${ x }px,${ y }px,0)`;
ref.current.style.zIndex = isSelectedRef.current ? '1' : '';
ref.current.style.zIndex = zIndex;
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we can also set this at before animation start, then restore at the end or on cleanup.

Copy link
Member Author

Choose a reason for hiding this comment

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

(Same for transformOrigin)

Copy link
Member Author

Choose a reason for hiding this comment

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

I left the original code here. Didn't want to alter things too much and I was hoping to see a before start and stop API.

preserveScrollPosition();
},
} );
Expand All @@ -121,7 +135,17 @@ function useMovingAnimation( {
return () => {
controller.stop();
};
}, [ previous, prevRect, prefersReducedMotion ] );
}, [
previous,
prevRect,
clientId,
isTyping,
getGlobalBlockCount,
isBlockSelected,
isFirstMultiSelectedBlock,
isBlockMultiSelected,
isAncestorMultiSelected,
] );

return ref;
}
Expand Down
Loading