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.

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

Implement a block reordering animation #16065

Merged
merged 23 commits into from
Jul 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/block-editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"classnames": "^2.2.5",
"dom-scroll-into-view": "^1.2.1",
"lodash": "^4.17.10",
"react-spring": "^8.0.19",
Copy link
Member

Choose a reason for hiding this comment

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

This module is 47kb minified before gzip, and we're now duplicating it across two of our packages.

At what point should we consider extracting it as a common vendor dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know :) What do you think. I don't mind extracting it but it feels very risky in terms of BC as I expect breaking changes in this kind of modules way more often than React for instance.

Copy link
Member

Choose a reason for hiding this comment

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

I don't know :) What do you think. I don't mind extracting it but it feels very risky in terms of BC as I expect breaking changes in this kind of modules way more often than React for instance.

It could be worth to bring up in a weekly JavaScript chat. Maybe (at least in the short-term) we can have some naming convention to designate some script handles as internal.

wp_register_script( '_internal_react-spring', /* ... */ );

Copy link
Member

Choose a reason for hiding this comment

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

I'm also curious whether there's anything which prohibits that a script handle be named where we embed the specific version. Obviously, unlike an "internal" designation, this might bound us to some compatibility so far as always shipping every version we've ever used, but at least it allows to migrate to newer versions over time.

wp_register_script( '[email protected]', /* ... */ );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could work. We can define the versions for scripts in Core as well (argument of that function). Something in the PHP framework itself could work to register multiple versions and enqueue specific versions as well.

Copy link
Member

Choose a reason for hiding this comment

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

I'm also curious whether there's anything which prohibits that a script handle be named where we embed the specific version.

I remember now one of the issues we had considered with this is: When multiple versions are loaded in the same page, how do we handle globals being assigned without one version clobbering the other? I recall having some thoughts on using closure scoping and/or accessor properties to let the global vary depending on the consumer, but I hadn't ever formalized a proposal.

"redux-multi": "^0.1.12",
"refx": "^3.0.0",
"rememo": "^3.0.0",
Expand Down
9 changes: 9 additions & 0 deletions packages/block-editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
import classnames from 'classnames';
import { get, reduce, size, first, last } from 'lodash';
import { animated } from 'react-spring/web.cjs';

/**
* WordPress dependencies
Expand Down Expand Up @@ -50,6 +51,7 @@ import InserterWithShortcuts from '../inserter-with-shortcuts';
import Inserter from '../inserter';
import useHoveredArea from './hover-area';
import { isInsideRootBlock } from '../../utils/dom';
import useMovingAnimation from './moving-animation';

/**
* Prevents default dragging behavior within a block to allow for multi-
Expand Down Expand Up @@ -97,6 +99,8 @@ function BlockListBlock( {
toggleSelection,
onShiftSelection,
onSelectionStart,
animateOnChange,
enableAnimation,
} ) {
// Random state used to rerender the component if needed, ideally we don't need this
const [ , updateRerenderState ] = useState( {} );
Expand Down Expand Up @@ -247,6 +251,9 @@ function BlockListBlock( {
}
}, [ isFirstMultiSelected ] );

// Block Reordering animation
const style = useMovingAnimation( wrapper, isSelected || isPartOfMultiSelection, enableAnimation, animateOnChange );

// Other event handlers

/**
Expand Down Expand Up @@ -458,6 +465,8 @@ function BlockListBlock( {
tabIndex="0"
aria-label={ blockLabel }
childHandledEvents={ [ 'onDragStart', 'onMouseDown' ] }
tagName={ animated.div }
style={ style }
{ ...blockWrapperProps }
>
{ shouldShowInsertionPoint && (
Expand Down
21 changes: 18 additions & 3 deletions packages/block-editor/src/components/block-list/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/
import {
findLast,
map,
invert,
mapValues,
sortBy,
Expand All @@ -29,13 +28,20 @@ import BlockListBlock from './block';
import BlockListAppender from '../block-list-appender';
import { getBlockDOMNode } from '../../utils/dom';

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

const forceSyncUpdates = ( WrappedComponent ) => ( props ) => {
return (
<AsyncModeProvider value={ false }>
<WrappedComponent { ...props } />
</AsyncModeProvider>
);
};

class BlockList extends Component {
constructor( props ) {
super( props );
Expand Down Expand Up @@ -198,11 +204,12 @@ class BlockList extends Component {
multiSelectedBlockClientIds,
hasMultiSelection,
renderAppender,
enableAnimation,
} = this.props;

return (
<div className="editor-block-list__layout block-editor-block-list__layout">
{ map( blockClientIds, ( clientId ) => {
{ blockClientIds.map( ( clientId ) => {
const isBlockInSelection = hasMultiSelection ?
multiSelectedBlockClientIds.includes( clientId ) :
selectedBlockClientId === clientId;
Expand All @@ -214,11 +221,17 @@ class BlockList extends Component {
isBlockInSelection={ isBlockInSelection }
>
<BlockListBlock
rootClientId={ rootClientId }
clientId={ clientId }
blockRef={ this.setBlockRef }
onSelectionStart={ this.onSelectionStart }
rootClientId={ rootClientId }
isDraggable={ isDraggable }

// This prop is explicitely computed and passed down
// to avoid being impacted by the async mode
// otherwise there might be a small delay to trigger the animation.
animateOnChange={ blockClientIds }
enableAnimation={ enableAnimation }
/>
</BlockAsyncModeProvider>
);
Expand Down Expand Up @@ -248,6 +261,7 @@ export default compose( [
getSelectedBlockClientId,
getMultiSelectedBlockClientIds,
hasMultiSelection,
getGlobalBlockCount,
} = select( 'core/block-editor' );

const { rootClientId } = ownProps;
Expand All @@ -261,6 +275,7 @@ export default compose( [
selectedBlockClientId: getSelectedBlockClientId(),
multiSelectedBlockClientIds: getMultiSelectedBlockClientIds(),
hasMultiSelection: hasMultiSelection(),
enableAnimation: getGlobalBlockCount() <= BLOCK_ANIMATION_THRESHOLD,
};
} ),
withDispatch( ( dispatch ) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/**
* External dependencies
*/
import { useSpring, interpolate } from 'react-spring/web.cjs';

/**
* WordPress dependencies
*/
import { useState, useLayoutEffect } from '@wordpress/element';
import { useReducedMotion } from '@wordpress/compose';

/**
* Hook used to compute the styles required to move a div into a new position.
*
* The way this animation works is the following:
* - It first renders the element as if there was no animation.
* - It takes a snapshot of the position of the block to use it
* as a destination point for the animation.
* - It restores the element to the previous position using a CSS transform
* - It uses the "resetAnimation" flag to reset the animation
* from the beginning in order to animate to the new destination point.
*
* @param {Object} ref Reference to the element to animate.
* @param {boolean} isSelected Whether it's the current block or not.
* @param {boolean} enableAnimation Enable/Disable animation.
* @param {*} triggerAnimationOnChange Variable used to trigger the animation if it changes.
*
* @return {Object} Style object.
*/
function useMovingAnimation( ref, isSelected, enableAnimation, triggerAnimationOnChange ) {
const prefersReducedMotion = useReducedMotion() || ! enableAnimation;
const [ resetAnimation, setResetAnimation ] = useState( false );
const [ transform, setTransform ] = useState( { x: 0, y: 0 } );

const previous = ref.current ? ref.current.getBoundingClientRect() : null;
useLayoutEffect( () => {
if ( prefersReducedMotion ) {
return;
}
ref.current.style.transform = 'none';
const destination = ref.current.getBoundingClientRect();
const newTransform = {
x: previous ? previous.left - destination.left : 0,
y: previous ? previous.top - destination.top : 0,
};
ref.current.style.transform = `translate3d(${ newTransform.x }px,${ newTransform.y }px,0)`;
setResetAnimation( true );
setTransform( newTransform );
}, [ triggerAnimationOnChange ] );
useLayoutEffect( () => {
if ( resetAnimation ) {
setResetAnimation( false );
}
}, [ resetAnimation ] );
const animationProps = useSpring( {
from: transform,
to: {
x: 0,
y: 0,
},
reset: resetAnimation,
config: { mass: 5, tension: 2000, friction: 200 },
Copy link
Member

Choose a reason for hiding this comment

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

I'm not completely satisfied with this set yet, but we can tweak it later.

immediate: prefersReducedMotion,
} );

return {
transformOrigin: 'center',
transform: interpolate(
[
animationProps.x,
animationProps.y,
],
( x, y ) => x === 0 && y === 0 ? undefined : `translate3d(${ x }px,${ y }px,0)`
),
zIndex: interpolate(
[
animationProps.x,
animationProps.y,
],
( x, y ) => ! isSelected || ( x === 0 && y === 0 ) ? undefined : `1`
),
};
}

export default useMovingAnimation;
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { reduce } from 'lodash';
/**
* WordPress dependencies
*/
import { Component, forwardRef } from '@wordpress/element';
import { Component, forwardRef, createElement } from '@wordpress/element';

/**
* Component which renders a div with passed props applied except the optional
Expand Down Expand Up @@ -66,7 +66,7 @@ export class IgnoreNestedEvents extends Component {
}

render() {
const { childHandledEvents = [], forwardedRef, ...props } = this.props;
const { childHandledEvents = [], forwardedRef, tagName = 'div', ...props } = this.props;

const eventHandlers = reduce( [
...childHandledEvents,
Expand Down Expand Up @@ -96,7 +96,7 @@ export class IgnoreNestedEvents extends Component {
return result;
}, {} );

return <div ref={ forwardedRef } { ...props } { ...eventHandlers } />;
return createElement( tagName, { ref: forwardedRef, ...props, ...eventHandlers } );
}
}

Expand Down
9 changes: 8 additions & 1 deletion packages/compose/src/hooks/use-reduced-motion/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,20 @@
*/
import useMediaQuery from '../use-media-query';

/**
* Whether or not the user agent is Internet Explorer.
*
* @type {boolean}
*/
const IS_IE = window.navigator.userAgent.indexOf( 'Trident' ) >= 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a heads up that this line breaks the mobile app.
I created a PR on gutenberg-mobile side to solve it: wordpress-mobile/gutenberg-mobile#1195

IMG_2271

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry about that. Should we maybe create a custom version of useReducedMotion that always return "true" on mobile? Is there an equivalent to prefers-reduced-motion in mobile?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the equivalent landed on 0.60 which was just released: facebook/react-native#23839

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, let's add the equivalent hook once we upgrade to 0.60


/**
* Hook returning whether the user has a preference for reduced motion.
*
* @return {boolean} Reduced motion preference value.
*/
const useReducedMotion =
process.env.FORCE_REDUCED_MOTION ?
process.env.FORCE_REDUCED_MOTION || IS_IE ?
() => true :
() => useMediaQuery( '(prefers-reduced-motion: reduce)' );

Expand Down
4 changes: 2 additions & 2 deletions packages/e2e-tests/specs/block-deletion.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ describe( 'block deletion -', () => {
// Click on something that's not a block.
await page.click( '.editor-post-title' );

// Click on the third (image) block so that its wrapper is selected and backspace to delete it.
await page.click( '.block-editor-block-list__block:nth-child(3) .components-placeholder__label' );
// Click on the image block so that its wrapper is selected and backspace to delete it.
await page.click( '.wp-block[data-type="core/image"] .components-placeholder__label' );
await page.keyboard.press( 'Backspace' );

expect( await getEditedPostContent() ).toMatchSnapshot();
Expand Down