Skip to content

Commit

Permalink
Merge pull request chakra-ui#5442 from chakra-ui/fix/lazy-animation
Browse files Browse the repository at this point in the history
fix: lazy popover and menu unmounting
  • Loading branch information
segunadebayo authored Jan 25, 2022
2 parents 6d3b6d3 + 2a17cb6 commit f4846a6
Show file tree
Hide file tree
Showing 12 changed files with 110 additions and 21 deletions.
7 changes: 7 additions & 0 deletions .changeset/happy-rings-move.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@chakra-ui/popover": patch
"@chakra-ui/menu": patch
---

Fix issue where the content of a lazy popover or menu gets unmounted before
(framer-motion) animation ends leading to a janky user experience.
6 changes: 6 additions & 0 deletions .changeset/itchy-boxes-notice.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@chakra-ui/hooks": minor
---

Add `useAnimationState` hook to help track motion component animations. Used in
popopover and menu lazy modes
18 changes: 9 additions & 9 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,21 @@

node_modules/
npm-debug.log
packages/*/lib
packages/*/.next
packages/**/lib
packages/**/.next
storybook-static
packages/*/coverage
packages/*/dist
packages/**/coverage
packages/**/dist
tooling/**/dist
packages/*/node_modules
packages/**/node_modules
npm-debug.log*
packages/*/.env
packages/**/.env
*.log
*.cache
yarn-error.log
packages/*/.rpt2_cache
packages/*/.rts2_cache*
packages/*/tsconfig.tsbuildinfo
packages/**/.rpt2_cache
packages/**/.rts2_cache*
packages/**/tsconfig.tsbuildinfo
tsconfig.tsbuildinfo
.next
.idea
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"license": "MIT",
"scripts": {
"pkg": "manypkg run",
"dev": "preconstruct dev",
"prepare": "husky install",
"fix:pkgs": "manypkg fix",
"version:pkgs": "changeset version",
Expand Down
9 changes: 8 additions & 1 deletion packages/hooks/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,15 @@
"types": "dist/chakra-ui-hooks.cjs.d.ts",
"files": [
"dist",
"src"
"src",
"use-animation-state"
],
"preconstruct": {
"entrypoints": [
"index.ts",
"use-animation-state.ts"
]
},
"publishConfig": {
"access": "public"
},
Expand Down
1 change: 1 addition & 0 deletions packages/hooks/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ export * from "./use-timeout"
export * from "./use-unmount-effect"
export * from "./use-update-effect"
export * from "./use-why-update"
export * from "./use-animation-state"
41 changes: 41 additions & 0 deletions packages/hooks/src/use-animation-state.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { getOwnerWindow } from "@chakra-ui/utils"
import { RefObject, useEffect, useState } from "react"
import { useEventListener } from "./use-event-listener"

export type UseAnimationStateProps = {
isOpen: boolean
ref: RefObject<HTMLElement>
}

export function useAnimationState(props: UseAnimationStateProps) {
const { isOpen, ref } = props

const [mounted, setMounted] = useState(isOpen)
const [once, setOnce] = useState(false)

useEffect(() => {
if (!once) {
setMounted(isOpen)
setOnce(true)
}
}, [isOpen, once, mounted])

useEventListener(
"animationend",
() => {
setMounted(isOpen)
},
() => ref.current,
)

const hidden = isOpen ? false : !mounted && once

return {
present: !hidden,
onComplete() {
const win = getOwnerWindow(ref.current)
const evt = new win.CustomEvent("animationend", { bubbles: true })
ref.current?.dispatchEvent(evt)
},
}
}
4 changes: 4 additions & 0 deletions packages/hooks/use-animation-state/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"main": "dist/chakra-ui-hooks-use-animation-state.cjs.js",
"module": "dist/chakra-ui-hooks-use-animation-state.esm.js"
}
22 changes: 15 additions & 7 deletions packages/menu/src/menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
useStyles,
useTheme,
} from "@chakra-ui/system"
import { cx, runIfFn, __DEV__ } from "@chakra-ui/utils"
import { callAll, cx, runIfFn, __DEV__ } from "@chakra-ui/utils"
import { CustomDomComponent, motion, Variants } from "framer-motion"
import * as React from "react"
import {
Expand Down Expand Up @@ -148,16 +148,20 @@ const motionVariants: Variants = {
}

// @future: only call `motion(chakra.div)` when we drop framer-motion v3 support
const MotionDiv: CustomDomComponent<PropsOf<typeof chakra.div>> =
const MenuTransition: CustomDomComponent<PropsOf<typeof chakra.div>> =
"custom" in motion
? (motion as any).custom(chakra.div)
: (motion as any)(chakra.div)

export const MenuList = forwardRef<MenuListProps, "div">((props, ref) => {
const { rootProps, ...rest } = props
const { isOpen, onTransitionEnd } = useMenuContext()
const {
isOpen,
onTransitionEnd,
unstable__animationState: animated,
} = useMenuContext()

const menulistProps = useMenuList(rest, ref) as HTMLAttributes
const ownProps = useMenuList(rest, ref) as any
const positionerProps = useMenuPositioner(rootProps)

const styles = useStyles()
Expand All @@ -167,14 +171,18 @@ export const MenuList = forwardRef<MenuListProps, "div">((props, ref) => {
{...positionerProps}
__css={{ zIndex: props.zIndex ?? styles.list?.zIndex }}
>
<MotionDiv
{...menulistProps}
<MenuTransition
{...ownProps}
/**
* We could call this on either `onAnimationComplete` or `onUpdate`.
* It seems the re-focusing works better with the `onUpdate`
*/
onUpdate={onTransitionEnd}
className={cx("chakra-menu__menu-list", menulistProps.className)}
onAnimationComplete={callAll(
animated.onComplete,
ownProps.onAnimationComplete,
)}
className={cx("chakra-menu__menu-list", ownProps.className)}
variants={motionVariants}
initial={false}
animate={isOpen ? "enter" : "exit"}
Expand Down
7 changes: 6 additions & 1 deletion packages/menu/src/use-menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
useUnmountEffect,
useUpdateEffect,
} from "@chakra-ui/hooks"
import { useAnimationState } from "@chakra-ui/hooks/use-animation-state"
import { usePopper, UsePopperProps } from "@chakra-ui/popper"
import {
createContext,
Expand Down Expand Up @@ -230,6 +231,8 @@ export function useMenu(props: UseMenuProps = {}) {
shouldFocus: true,
})

const animationState = useAnimationState({ isOpen, ref: menuRef })

/**
* Generate unique ids for menu's list and button
*/
Expand Down Expand Up @@ -275,6 +278,7 @@ export function useMenu(props: UseMenuProps = {}) {
openAndFocusFirstItem,
openAndFocusLastItem,
onTransitionEnd: refocus,
unstable__animationState: animationState,
descendants,
popper,
buttonId,
Expand Down Expand Up @@ -396,6 +400,7 @@ export function useMenuList(
menuId,
isLazy,
lazyBehavior,
unstable__animationState: animated,
} = menu

const descendants = useMenuDescendantsContext()
Expand Down Expand Up @@ -473,7 +478,7 @@ export function useMenuList(
hasBeenSelected: hasBeenOpened.current,
isLazy,
lazyBehavior,
isSelected: isOpen,
isSelected: animated.present,
})

return {
Expand Down
9 changes: 7 additions & 2 deletions packages/popover/src/popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
useStyles,
useTheme,
} from "@chakra-ui/system"
import { cx, runIfFn, __DEV__ } from "@chakra-ui/utils"
import { callAll, cx, runIfFn, __DEV__ } from "@chakra-ui/utils"
import * as React from "react"
import { PopoverProvider, usePopoverContext } from "./popover-context"
import { PopoverTransition, PopoverTransitionProps } from "./popover-transition"
Expand Down Expand Up @@ -99,7 +99,8 @@ export const PopoverContent = forwardRef<PopoverContentProps, "section">(
(props, ref) => {
const { rootProps, ...contentProps } = props

const { getPopoverProps, getPopoverPositionerProps } = usePopoverContext()
const { getPopoverProps, getPopoverPositionerProps, onAnimationComplete } =
usePopoverContext()

const styles = useStyles()
const contentStyles: SystemStyleObject = {
Expand All @@ -117,6 +118,10 @@ export const PopoverContent = forwardRef<PopoverContentProps, "section">(
>
<PopoverTransition
{...getPopoverProps(contentProps, ref)}
onAnimationComplete={callAll(
onAnimationComplete,
contentProps.onAnimationComplete,
)}
className={cx("chakra-popover__content", props.className)}
__css={contentStyles}
/>
Expand Down
6 changes: 5 additions & 1 deletion packages/popover/src/use-popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
useFocusOnShow,
useIds,
} from "@chakra-ui/hooks"
import { useAnimationState } from "@chakra-ui/hooks/use-animation-state"
import { popperCSSVars, usePopper, UsePopperProps } from "@chakra-ui/popper"
import { HTMLProps, mergeRefs, PropGetter } from "@chakra-ui/react-utils"
import {
Expand Down Expand Up @@ -176,6 +177,8 @@ export function usePopover(props: UsePopoverProps = {}) {
enabled: isOpen || !!computePositionOnMount,
})

const animated = useAnimationState({ isOpen, ref: popoverRef })

useFocusOnPointerDown({
enabled: isOpen,
ref: triggerRef,
Expand All @@ -197,7 +200,7 @@ export function usePopover(props: UsePopoverProps = {}) {
hasBeenSelected: hasBeenOpened.current,
isLazy,
lazyBehavior,
isSelected: isOpen,
isSelected: animated.present,
})

const getPopoverProps: PropGetter = useCallback(
Expand Down Expand Up @@ -422,6 +425,7 @@ export function usePopover(props: UsePopoverProps = {}) {
return {
forceUpdate,
isOpen,
onAnimationComplete: animated.onComplete,
onClose,
getAnchorProps,
getArrowProps,
Expand Down

0 comments on commit f4846a6

Please sign in to comment.