refactor: collapsible

chakra-ui · Apr 23, 2024 · ce2b2b3 · ce2b2b3
1 parent 67dcceb
commit ce2b2b3
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 43 deletions.
diff --git a/.changeset/three-beers-clap.md b/.changeset/three-beers-clap.md
@@ -0,0 +1,5 @@
+---
+"@zag-js/collapsible": patch
+---
+
+Fix issue where initial height animation can sometimes run.
diff --git a/.xstate/collapsible.js b/.xstate/collapsible.js
@@ -18,7 +18,6 @@ const fetchMachine = createMachine({
     "isOpenControlled": false,
     "isOpenControlled": false
   },
-  entry: ["computeSize"],
   on: {
     UPDATE_CONTEXT: {
       actions: "updateContext"
@@ -27,43 +26,38 @@ const fetchMachine = createMachine({
   states: {
     closed: {
       tags: ["closed"],
-      entry: ["computeSize"],
       on: {
         "CONTROLLED.OPEN": {
-          target: "open",
-          actions: ["computeSize"]
+          target: "open"
         },
         OPEN: [{
           cond: "isOpenControlled",
           actions: ["invokeOnOpen"]
         }, {
           target: "open",
-          actions: ["allowAnimation", "invokeOnOpen", "computeSize"]
+          actions: ["setInitial", "computeSize", "invokeOnOpen"]
         }]
       }
     },
     closing: {
       tags: ["open"],
       activities: ["trackAnimationEvents"],
       on: {
-        "CONTROLLED.CLOSE": {
-          target: "closed",
-          actions: ["invokeOnExitComplete"]
-        },
+        "CONTROLLED.CLOSE": "closed",
         "CONTROLLED.OPEN": "open",
         OPEN: [{
           cond: "isOpenControlled",
           actions: ["invokeOnOpen"]
         }, {
           target: "open",
-          actions: ["allowAnimation", "invokeOnOpen"]
+          actions: ["setInitial", "invokeOnOpen"]
         }],
         CLOSE: [{
           cond: "isOpenControlled",
-          actions: ["invokeOnClose"]
+          actions: ["invokeOnExitComplete"]
         }, {
           target: "closed",
-          actions: ["allowAnimation", "computeSize", "invokeOnExitComplete"]
+          actions: ["setInitial", "computeSize", "invokeOnExitComplete"]
         }],
         "ANIMATION.END": {
           target: "closed",
@@ -75,15 +69,14 @@ const fetchMachine = createMachine({
       tags: ["open"],
       on: {
         "CONTROLLED.CLOSE": {
-          target: "closing",
-          actions: ["computeSize"]
+          target: "closing"
         },
         CLOSE: [{
           cond: "isOpenControlled",
-          actions: ["invokeOnClose"]
+          actions: ["computeSize", "invokeOnClose"]
         }, {
           target: "closing",
-          actions: ["allowAnimation", "computeSize"]
+          actions: ["setInitial", "computeSize", "invokeOnClose"]
         }]
       }
     }

diff --git a/packages/machines/collapsible/src/collapsible.connect.ts b/packages/machines/collapsible/src/collapsible.connect.ts
@@ -12,7 +12,7 @@ export function connect<T extends PropTypes>(state: State, send: Send, normalize
   const width = state.context.width
   const disabled = !!state.context.disabled
 
-  const skipMountAnimation = state.context.isMountAnimationPrevented && open
+  const skip = !state.context.initial && open
 
   return {
     disabled,
@@ -32,7 +32,7 @@ export function connect<T extends PropTypes>(state: State, send: Send, normalize
 
     contentProps: normalize.element({
       ...parts.content.attrs,
-      "data-state": skipMountAnimation ? undefined : open ? "open" : "closed",
+      "data-state": skip ? undefined : open ? "open" : "closed",
       id: dom.getContentId(state.context),
       "data-disabled": dataAttr(disabled),
       hidden: !visible,
@@ -51,7 +51,8 @@ export function connect<T extends PropTypes>(state: State, send: Send, normalize
       "data-disabled": dataAttr(disabled),
       "aria-controls": dom.getContentId(state.context),
       "aria-expanded": visible || false,
-      onClick() {
+      onClick(event) {
+        if (event.defaultPrevented) return
         if (disabled) return
         send({ type: open ? "CLOSE" : "OPEN", src: "trigger.click" })
       },

diff --git a/packages/machines/collapsible/src/collapsible.machine.ts b/packages/machines/collapsible/src/collapsible.machine.ts
@@ -15,24 +15,20 @@ export function machine(userContext: UserDefinedContext) {
         ...ctx,
         height: 0,
         width: 0,
-        isMountAnimationPrevented: !!ctx.open,
+        initial: false,
         stylesRef: null,
       },
 
       watch: {
-        open: ["allowAnimation", "toggleVisibility"],
+        open: ["setInitial", "computeSize", "toggleVisibility"],
       },
 
-      entry: ["computeSize"],
-
       states: {
         closed: {
           tags: ["closed"],
-          entry: ["computeSize"],
           on: {
             "CONTROLLED.OPEN": {
               target: "open",
-              actions: ["computeSize"],
             },
             OPEN: [
               {
@@ -41,7 +37,7 @@ export function machine(userContext: UserDefinedContext) {
               },
               {
                 target: "open",
-                actions: ["allowAnimation", "invokeOnOpen", "computeSize"],
+                actions: ["setInitial", "computeSize", "invokeOnOpen"],
               },
             ],
           },
@@ -51,10 +47,7 @@ export function machine(userContext: UserDefinedContext) {
           tags: ["open"],
           activities: ["trackAnimationEvents"],
           on: {
-            "CONTROLLED.CLOSE": {
-              target: "closed",
-              actions: ["invokeOnExitComplete"],
-            },
+            "CONTROLLED.CLOSE": "closed",
             "CONTROLLED.OPEN": "open",
             OPEN: [
               {
@@ -63,17 +56,17 @@ export function machine(userContext: UserDefinedContext) {
               },
               {
                 target: "open",
-                actions: ["allowAnimation", "invokeOnOpen"],
+                actions: ["setInitial", "invokeOnOpen"],
               },
             ],
             CLOSE: [
               {
                 guard: "isOpenControlled",
-                actions: ["invokeOnClose"],
+                actions: ["invokeOnExitComplete"],
               },
               {
                 target: "closed",
-                actions: ["allowAnimation", "computeSize", "invokeOnExitComplete"],
+                actions: ["setInitial", "computeSize", "invokeOnExitComplete"],
               },
             ],
             "ANIMATION.END": {
@@ -82,21 +75,21 @@ export function machine(userContext: UserDefinedContext) {
             },
           },
         },
+
         open: {
           tags: ["open"],
           on: {
             "CONTROLLED.CLOSE": {
               target: "closing",
-              actions: ["computeSize"],
             },
             CLOSE: [
               {
                 guard: "isOpenControlled",
-                actions: ["invokeOnClose"],
+                actions: ["computeSize", "invokeOnClose"],
               },
               {
                 target: "closing",
-                actions: ["allowAnimation", "computeSize"],
+                actions: ["setInitial", "computeSize", "invokeOnClose"],
               },
             ],
           },
@@ -144,11 +137,13 @@ export function machine(userContext: UserDefinedContext) {
         },
       },
       actions: {
-        allowAnimation(ctx) {
-          ctx.isMountAnimationPrevented = false
+        setInitial(ctx) {
+          ctx.initial = true
         },
         computeSize: (ctx) => {
-          raf(() => {
+          ctx._rafCleanup?.()
+
+          ctx._rafCleanup = raf(() => {
             const contentEl = dom.getContentEl(ctx)
             if (!contentEl) return
 
@@ -169,7 +164,7 @@ export function machine(userContext: UserDefinedContext) {
             ctx.width = rect.width
 
             // kick off any animations/transitions that were originally set up if it isn't the initial mount
-            if (!ctx.isMountAnimationPrevented) {
+            if (ctx.initial) {
               contentEl.style.animationName = ctx.stylesRef.animationName
               contentEl.style.animationDuration = ctx.stylesRef.animationDuration
             }

diff --git a/packages/machines/collapsible/src/collapsible.types.ts b/packages/machines/collapsible/src/collapsible.types.ts
@@ -66,9 +66,14 @@ interface PrivateContext {
   stylesRef: Record<string, any> | null
   /**
    * @internal
-   * Whether the mount animation is prevented
+   * Whether the initial animation is allowed
    */
-  isMountAnimationPrevented: boolean
+  initial: boolean
+  /**
+   * @internal
+   * The requestAnimationFrame id
+   */
+  _rafCleanup?: VoidFunction
 }
 
 export type UserDefinedContext = RequiredBy<PublicContext, "id">

diff --git a/starters/react/components/accordion.tsx b/starters/react/components/accordion.tsx
@@ -30,7 +30,7 @@ export const Accordion = (props: Props) => {
               {...itemContentProps}
               dir={context.dir}
               ids={{ content: itemContentProps.id }}
-              open={itemState.open}
+              open={itemState.expanded}
             >
               {item.description}
             </Collapsible>