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

Navigator: add support for exit animation #64777

Merged
merged 32 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
ee11150
Refactor screen logic for better clarity and hook deps
ciampo Aug 24, 2024
2832ef0
Add exit animation, rewrite screen animations / DOM rendering logic
ciampo Aug 24, 2024
e6ab671
Only add inset CSS rule when animating out
ciampo Aug 24, 2024
0a869dc
Parametrise animation
ciampo Aug 24, 2024
85b5214
CHANGELOG
ciampo Aug 25, 2024
7b2df6a
Add fallback timeout
ciampo Aug 26, 2024
9792e28
Mention wrapper height in README
ciampo Aug 28, 2024
12a185f
Use `useReducedMotion()` hook instead of custom logic
ciampo Aug 29, 2024
668a222
Extract useScreenAnimatePresence hook, tidy up
ciampo Aug 29, 2024
98c7b20
Forward animationEnd
ciampo Aug 29, 2024
808780e
Add `setWrapperHeight` functionality via context
ciampo Aug 29, 2024
1e0ae6b
Use `clip` instead of `hidden` for overflow-x
ciampo Aug 29, 2024
4a19bb1
Less aggressive clipping for screens that are animating out
ciampo Aug 29, 2024
5dde128
Better sizing styles for screen, to keep it more stable while transit…
ciampo Aug 29, 2024
ea5f1ed
Refine internal animation logic for less jumpy animations
ciampo Aug 29, 2024
c9e9998
Remove unnecessary Storybook styles
ciampo Aug 29, 2024
205b9aa
Change wording
ciampo Aug 29, 2024
9931a50
Improve logic:
ciampo Aug 31, 2024
c98ba81
Add timeout fallback for in animations too
ciampo Sep 2, 2024
d572679
Fix animation delay for forwards.out animation
ciampo Sep 2, 2024
80bddac
Use display: grid instead of absolute positioning to set provider min…
ciampo Sep 5, 2024
2900c46
Simplify navigatorScreenAnimation
ciampo Sep 6, 2024
56a1cb9
Remove unnecessary state element ref
ciampo Sep 12, 2024
396e3f3
Use "start" and "end" instead of "backwards" and "forwards"
ciampo Sep 12, 2024
ec1d6a7
Do not rely on `usePrevious`
ciampo Sep 12, 2024
effbe13
Use CSS transitions
ciampo Sep 13, 2024
ce54ad6
Fix Storybook example
ciampo Sep 13, 2024
759e55f
Revert "Use CSS transitions"
ciampo Sep 20, 2024
3988e43
Switch to data-attributes for less runtime emotion calculations
ciampo Sep 20, 2024
b15f9af
Add back fallback animation timeout
ciampo Sep 20, 2024
d67f6f2
Move CHANGELOG entry to unreleased section
ciampo Sep 20, 2024
283dc1e
Clean up code, reduce diff for easier reviewing
ciampo Sep 20, 2024
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 packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

- `MenuGroup`: Simplify the MenuGroup styles within dropdown menus. ([#65561](https://github.com/WordPress/gutenberg/pull/65561)).
- `DatePicker`: Use compact button size. ([#65653](https://github.com/WordPress/gutenberg/pull/65653)).
- `Navigator`: add support for exit animation ([#64777](https://github.com/WordPress/gutenberg/pull/64777)).

## 28.8.0 (2024-09-19)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const MyNavigation = () => (
);
```

**Important note**
### Hierarchical `path`s

`Navigator` assumes that screens are organized hierarchically according to their `path`, which should follow a URL-like scheme where each path segment starts with and is separated by the `/` character.

Expand All @@ -47,6 +47,10 @@ For example:
- `/parent/:param` is a child of `/parent` as well.
- if the current screen has a `path` with value `/parent/child/grand-child`, when going "back" `Navigator` will try to recursively navigate the path hierarchy until a matching screen (or the root `/`) is found.

### Height and animations

Due to how `NavigatorScreen` animations work, it is recommended that the `NavigatorProvider` component is assigned a `height` to prevent some potential UI jumps while moving across screens.

## Props

The component accepts the following props:
Expand Down
84 changes: 46 additions & 38 deletions packages/components/src/navigator/navigator-screen/component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
useId,
} from '@wordpress/element';
import { useMergeRefs } from '@wordpress/compose';
import { isRTL as isRTLFn } from '@wordpress/i18n';
import { escapeAttribute } from '@wordpress/escape-html';
import warning from '@wordpress/warning';

Expand All @@ -29,6 +28,7 @@ import { View } from '../../view';
import { NavigatorContext } from '../context';
import * as styles from '../styles';
import type { NavigatorScreenProps } from '../types';
import { useScreenAnimatePresence } from './use-screen-animate-presence';

function UnconnectedNavigatorScreen(
props: WordPressComponentProps< NavigatorScreenProps, 'div', false >,
Expand All @@ -41,16 +41,24 @@ function UnconnectedNavigatorScreen(
}

const screenId = useId();
const { children, className, path, ...otherProps } = useContextSystem(
props,
'NavigatorScreen'
);

const {
children,
className,
path,
onAnimationEnd: onAnimationEndProp,
...otherProps
} = useContextSystem( props, 'NavigatorScreen' );

const { location, match, addScreen, removeScreen } =
useContext( NavigatorContext );
const { isInitial, isBack, focusTargetSelector, skipFocus } = location;

const isMatch = match === screenId;
const wrapperRef = useRef< HTMLDivElement >( null );
const skipAnimationAndFocusRestoration = !! isInitial && ! isBack;

// Register / unregister screen with the navigator context.
useEffect( () => {
const screen = {
id: screenId,
Expand All @@ -60,86 +68,86 @@ function UnconnectedNavigatorScreen(
return () => removeScreen( screen );
}, [ screenId, path, addScreen, removeScreen ] );

const isRTL = isRTLFn();
const { isInitial, isBack } = location;
// Animation.
const { animationStyles, shouldRenderScreen, screenProps } =
useScreenAnimatePresence( {
isMatch,
isBack,
onAnimationEnd: onAnimationEndProp,
skipAnimation: skipAnimationAndFocusRestoration,
} );

const cx = useCx();
const classes = useMemo(
() =>
cx(
styles.navigatorScreen( {
isInitial,
isBack,
isRTL,
} ),
className
),
[ className, cx, isInitial, isBack, isRTL ]
() => cx( styles.navigatorScreen, animationStyles, className ),
[ className, cx, animationStyles ]
);

// Focus restoration
ciampo marked this conversation as resolved.
Show resolved Hide resolved
const locationRef = useRef( location );

useEffect( () => {
locationRef.current = location;
}, [ location ] );

// Focus restoration
const isInitialLocation = location.isInitial && ! location.isBack;
useEffect( () => {
const wrapperEl = wrapperRef.current;
// Only attempt to restore focus:
// - if the current location is not the initial one (to avoid moving focus on page load)
// - when the screen becomes visible
// - if the wrapper ref has been assigned
// - if focus hasn't already been restored for the current location
// - if the `skipFocus` option is not set to `true`. This is useful when we trigger the navigation outside of NavigatorScreen.
if (
isInitialLocation ||
skipAnimationAndFocusRestoration ||
! isMatch ||
! wrapperRef.current ||
! wrapperEl ||
locationRef.current.hasRestoredFocus ||
location.skipFocus
skipFocus
) {
return;
}

const activeElement = wrapperRef.current.ownerDocument.activeElement;
const activeElement = wrapperEl.ownerDocument.activeElement;

// If an element is already focused within the wrapper do not focus the
// element. This prevents inputs or buttons from losing focus unnecessarily.
if ( wrapperRef.current.contains( activeElement ) ) {
if ( wrapperEl.contains( activeElement ) ) {
return;
}

let elementToFocus: HTMLElement | null = null;

// When navigating back, if a selector is provided, use it to look for the
// target element (assumed to be a node inside the current NavigatorScreen)
if ( location.isBack && location.focusTargetSelector ) {
elementToFocus = wrapperRef.current.querySelector(
location.focusTargetSelector
);
if ( isBack && focusTargetSelector ) {
elementToFocus = wrapperEl.querySelector( focusTargetSelector );
}

// If the previous query didn't run or find any element to focus, fallback
// to the first tabbable element in the screen (or the screen itself).
if ( ! elementToFocus ) {
const [ firstTabbable ] = focus.tabbable.find( wrapperRef.current );
elementToFocus = firstTabbable ?? wrapperRef.current;
const [ firstTabbable ] = focus.tabbable.find( wrapperEl );
elementToFocus = firstTabbable ?? wrapperEl;
}

locationRef.current.hasRestoredFocus = true;
elementToFocus.focus();
}, [
isInitialLocation,
skipAnimationAndFocusRestoration,
isMatch,
location.isBack,
location.focusTargetSelector,
location.skipFocus,
isBack,
focusTargetSelector,
skipFocus,
] );

const mergedWrapperRef = useMergeRefs( [ forwardedRef, wrapperRef ] );

return isMatch ? (
<View ref={ mergedWrapperRef } className={ classes } { ...otherProps }>
return shouldRenderScreen ? (
<View
ref={ mergedWrapperRef }
className={ classes }
{ ...screenProps }
{ ...otherProps }
>
{ children }
</View>
) : null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
/**
* WordPress dependencies
*/
import {
useState,
useEffect,
useLayoutEffect,
useCallback,
} from '@wordpress/element';
import { useReducedMotion } from '@wordpress/compose';
import { isRTL as isRTLFn } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import * as styles from '../styles';

// Possible values:
// - 'INITIAL': the initial state
// - 'ANIMATING_IN': start enter animation
// - 'IN': enter animation has ended
// - 'ANIMATING_OUT': start exit animation
// - 'OUT': the exit animation has ended
type AnimationStatus =
| 'INITIAL'
| 'ANIMATING_IN'
| 'IN'
| 'ANIMATING_OUT'
| 'OUT';

// Allow an extra 20% of the total animation duration to account for potential
// event loop delays.
const ANIMATION_TIMEOUT_MARGIN = 1.2;

const isEnterAnimation = (
animationDirection: 'end' | 'start',
animationStatus: AnimationStatus,
animationName: string
) =>
animationStatus === 'ANIMATING_IN' &&
animationName === styles.ANIMATION_END_NAMES[ animationDirection ].in;

const isExitAnimation = (
animationDirection: 'end' | 'start',
animationStatus: AnimationStatus,
animationName: string
) =>
animationStatus === 'ANIMATING_OUT' &&
animationName === styles.ANIMATION_END_NAMES[ animationDirection ].out;

export function useScreenAnimatePresence( {
Copy link
Member

Choose a reason for hiding this comment

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

Lots of good work here, but I'm wondering if we're trying too hard not to use framer-motion. When it's about a simple animation, using CSS makes lots of sense. But when we need to reinvent the wheel in cases like animating presence, I feel like the best bet would be to use framer-motion. Bundle size is important, but DX and UX are more important, so we might have to make compromises. Especially given that we're looking to build more complex motion and animations for many of the components and flows.

Copy link
Member

Choose a reason for hiding this comment

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

I definitely feel better about it after the refactor into a designated hook, but also it’s still unknown how bug-free the current code is, and how many other components will need to animate presence.

My instinct is that it could still make sense to migrate from Framer Motion to a lighter library like React Transition Group. But then again, if it's a substantial effort to migrate from FM to RTG, maybe even that isn't worth it.

I’d love to hear from @ciampo and @DaniGuardiola about how they currently assess the overall cost/benefit around these rewrites. Let's make sure the benefits are still big enough to justify our rewrite efforts and potential increase in maintenance cost.

Copy link
Member

Choose a reason for hiding this comment

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

As for this particular PR, I'm personally comfortable enough with the logic isolation now to merge it as an incremental improvement over trunk. Is it scalable/sustainable? Maybe not. But it seems like we could swap it out without too much difficulty if we ultimately decide to use a library.

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 have particularly strong feelings about blocking this particular PR in favor of framer-motion, but I share the sentiment on scalability, the extra maintenance cost, and moving in that direction in the future based on the amount of custom work and wheel rediscovery we need to do. It's worth considering doing more specific measurements to ensure that a refactor away from framer-motion actually yields noticeable results. We should also consider the opportunity cost - what we could have built/shipped if we reused framer-motion instead of rediscovering it and maintaining the extra layer of complexity.

isMatch,
skipAnimation,
isBack,
onAnimationEnd,
}: {
isMatch: boolean;
skipAnimation: boolean;
isBack?: boolean;
onAnimationEnd?: React.AnimationEventHandler< Element >;
} ) {
const isRTL = isRTLFn();
const prefersReducedMotion = useReducedMotion();

const [ animationStatus, setAnimationStatus ] =
useState< AnimationStatus >( 'INITIAL' );

// Start enter and exit animations when the screen is selected or deselected.
// The animation status is set to `IN` or `OUT` immediately if the animation
// should be skipped.
const becameSelected =
animationStatus !== 'ANIMATING_IN' &&
animationStatus !== 'IN' &&
isMatch;
const becameUnselected =
animationStatus !== 'ANIMATING_OUT' &&
animationStatus !== 'OUT' &&
! isMatch;
useLayoutEffect( () => {
if ( becameSelected ) {
setAnimationStatus(
skipAnimation || prefersReducedMotion ? 'IN' : 'ANIMATING_IN'
);
} else if ( becameUnselected ) {
setAnimationStatus(
skipAnimation || prefersReducedMotion ? 'OUT' : 'ANIMATING_OUT'
);
}
}, [
becameSelected,
becameUnselected,
skipAnimation,
prefersReducedMotion,
] );

// Animation attributes (derived state).
const animationDirection =
( isRTL && isBack ) || ( ! isRTL && ! isBack ) ? 'end' : 'start';
const isAnimatingIn = animationStatus === 'ANIMATING_IN';
const isAnimatingOut = animationStatus === 'ANIMATING_OUT';
let animationType: 'in' | 'out' | undefined;
if ( isAnimatingIn ) {
animationType = 'in';
} else if ( isAnimatingOut ) {
animationType = 'out';
}

const onScreenAnimationEnd = useCallback(
( e: React.AnimationEvent< HTMLElement > ) => {
onAnimationEnd?.( e );

if (
isExitAnimation(
animationDirection,
animationStatus,
e.animationName
)
) {
// When the exit animation ends on an unselected screen, set the
// status to 'OUT' to remove the screen contents from the DOM.
setAnimationStatus( 'OUT' );
} else if (
isEnterAnimation(
animationDirection,
animationStatus,
e.animationName
)
) {
// When the enter animation ends on a selected screen, set the
// status to 'IN' to ensure the screen is rendered in the DOM.
setAnimationStatus( 'IN' );
}
},
[ onAnimationEnd, animationStatus, animationDirection ]
);

// Fallback timeout to ensure that the logic is applied even if the
// `animationend` event is not triggered.
useEffect( () => {
let animationTimeout: number | undefined;

if ( isAnimatingOut ) {
animationTimeout = window.setTimeout( () => {
setAnimationStatus( 'OUT' );
animationTimeout = undefined;
}, styles.TOTAL_ANIMATION_DURATION.OUT * ANIMATION_TIMEOUT_MARGIN );
} else if ( isAnimatingIn ) {
animationTimeout = window.setTimeout( () => {
setAnimationStatus( 'IN' );
animationTimeout = undefined;
}, styles.TOTAL_ANIMATION_DURATION.IN * ANIMATION_TIMEOUT_MARGIN );
}

return () => {
if ( animationTimeout ) {
window.clearTimeout( animationTimeout );
animationTimeout = undefined;
}
};
}, [ isAnimatingOut, isAnimatingIn ] );

return {
animationStyles: styles.navigatorScreenAnimation,
// Render the screen's contents in the DOM not only when the screen is
// selected, but also while it is animating out.
shouldRenderScreen:
isMatch ||
animationStatus === 'IN' ||
animationStatus === 'ANIMATING_OUT',
screenProps: {
onAnimationEnd: onScreenAnimationEnd,
'data-animation-direction': animationDirection,
'data-animation-type': animationType,
'data-skip-animation': skipAnimation || undefined,
},
} as const;
}
11 changes: 6 additions & 5 deletions packages/components/src/navigator/stories/index.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ const meta: Meta< typeof NavigatorProvider > = {
return (
<>
<style>{ `
/* These attributes are a private implementation detail of the
Navigator component. Do not use outside of its source code. */
/* The data-wp-component attribute is a private implementation
* detail of the Navigator component. Do not use outside of
* its source code.
*/
[data-wp-component="NavigatorProvider"] {
height: calc(100vh - 2rem);
max-height: 250px;

height: 250px;
}
[data-wp-component="NavigatorScreen"]:not([data-sticky]) {
padding: 8px;
Expand Down Expand Up @@ -167,6 +167,7 @@ export const SkipFocus: StoryObj< typeof NavigatorProvider > = {
outline: '1px solid black',
outlineOffset: '-1px',
marginBlockEnd: '1rem',
display: 'contents',
} }
>
<NavigatorScreen path="/">
Expand Down
Loading
Loading