refactor(tooltip): fix controlled open state

chakra-ui · Jan 10, 2025 · 7891d15 · 7891d15
1 parent b450499
commit 7891d15
Show file tree

Hide file tree

Showing 9 changed files with 131 additions and 36 deletions.
diff --git a/.changeset/quick-lizards-ring.md b/.changeset/quick-lizards-ring.md
@@ -0,0 +1,7 @@
+---
+"@zag-js/hover-card": patch
+"@zag-js/tooltip": patch
+---
+
+Fix issue where controlled open state could get into an inconsistent machine state during the `opening` or `closing`
+state.
diff --git a/.xstate/hover-card.js b/.xstate/hover-card.js
@@ -55,24 +55,28 @@ const fetchMachine = createMachine({
       },
       on: {
         "CONTROLLED.OPEN": "open",
+        "CONTROLLED.CLOSE": "closed",
         POINTER_LEAVE: [{
           cond: "isOpenControlled",
-          actions: ["invokeOnClose"]
+          // We trigger toggleVisibility manually since the `ctx.open` has not changed yet (at this point)
+          actions: ["invokeOnClose", "toggleVisibility"]
         }, {
           target: "closed",
           actions: ["invokeOnClose"]
         }],
         TRIGGER_BLUR: [{
           cond: "isOpenControlled && !isPointer",
-          actions: ["invokeOnClose"]
+          // We trigger toggleVisibility manually since the `ctx.open` has not changed yet (at this point)
+          actions: ["invokeOnClose", "toggleVisibility"]
         }, {
           cond: "!isPointer",
           target: "closed",
           actions: ["invokeOnClose"]
         }],
         CLOSE: [{
           cond: "isOpenControlled",
-          actions: ["invokeOnClose"]
+          // We trigger toggleVisibility manually since the `ctx.open` has not changed yet (at this point)
+          actions: ["invokeOnClose", "toggleVisibility"]
         }, {
           target: "closed",
           actions: ["invokeOnClose"]
@@ -122,6 +126,7 @@ const fetchMachine = createMachine({
       },
       on: {
         "CONTROLLED.CLOSE": "closed",
+        "CONTROLLED.OPEN": "open",
         POINTER_ENTER: {
           target: "open",
           // no need to invokeOnOpen here because it's still open (but about to close)

diff --git a/.xstate/tooltip.js b/.xstate/tooltip.js
@@ -14,11 +14,14 @@ const fetchMachine = createMachine({
   initial: ctx.open ? "open" : "closed",
   activities: ["trackFocusVisible"],
   context: {
+    "isOpenControlled": false,
     "noVisibleTooltip && !hasPointerMoveOpened": false,
     "!hasPointerMoveOpened": false,
     "isOpenControlled": false,
     "isOpenControlled": false,
     "isOpenControlled": false,
+    "isOpenControlled": false,
+    "isOpenControlled": false,
     "isVisible": false,
     "isOpenControlled": false,
     "isInteractive": false,
@@ -38,10 +41,13 @@ const fetchMachine = createMachine({
       entry: ["clearGlobalId"],
       on: {
         "CONTROLLED.OPEN": "open",
-        OPEN: {
+        OPEN: [{
+          cond: "isOpenControlled",
+          actions: ["invokeOnOpen"]
+        }, {
           target: "open",
           actions: ["invokeOnOpen"]
-        },
+        }],
         POINTER_LEAVE: {
           actions: ["clearPointerMoveOpened"]
         },
@@ -79,15 +85,20 @@ const fetchMachine = createMachine({
         }],
         POINTER_LEAVE: [{
           cond: "isOpenControlled",
-          actions: ["clearPointerMoveOpened", "invokeOnClose"]
+          // We trigger toggleVisibility manually since the `ctx.open` has not changed yet (at this point)
+          actions: ["clearPointerMoveOpened", "invokeOnClose", "toggleVisibility"]
         }, {
           target: "closed",
           actions: ["clearPointerMoveOpened", "invokeOnClose"]
         }],
-        CLOSE: {
+        CLOSE: [{
+          cond: "isOpenControlled",
+          // We trigger toggleVisibility manually since the `ctx.open` has not changed yet (at this point)
+          actions: ["invokeOnClose", "toggleVisibility"]
+        }, {
           target: "closed",
           actions: ["invokeOnClose"]
-        }
+        }]
       }
     },
     open: {
@@ -96,10 +107,13 @@ const fetchMachine = createMachine({
       entry: ["setGlobalId"],
       on: {
         "CONTROLLED.CLOSE": "closed",
-        CLOSE: {
+        CLOSE: [{
+          cond: "isOpenControlled",
+          actions: ["invokeOnClose"]
+        }, {
           target: "closed",
           actions: ["invokeOnClose"]
-        },
+        }],
         POINTER_LEAVE: [{
           cond: "isVisible",
           target: "closing",
@@ -146,7 +160,8 @@ const fetchMachine = createMachine({
         }],
         POINTER_MOVE: [{
           cond: "isOpenControlled",
-          actions: ["setPointerMoveOpened", "invokeOnOpen"]
+          // We trigger toggleVisibility manually since the `ctx.open` has not changed yet (at this point)
+          actions: ["setPointerMoveOpened", "invokeOnOpen", "toggleVisibility"]
         }, {
           target: "open",
           actions: ["setPointerMoveOpened", "invokeOnOpen"]
@@ -174,9 +189,9 @@ const fetchMachine = createMachine({
     CLOSE_DELAY: 500
   },
   guards: {
+    "isOpenControlled": ctx => ctx["isOpenControlled"],
     "noVisibleTooltip && !hasPointerMoveOpened": ctx => ctx["noVisibleTooltip && !hasPointerMoveOpened"],
     "!hasPointerMoveOpened": ctx => ctx["!hasPointerMoveOpened"],
-    "isOpenControlled": ctx => ctx["isOpenControlled"],
     "isVisible": ctx => ctx["isVisible"],
     "isInteractive": ctx => ctx["isInteractive"]
   }

diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts
@@ -387,6 +387,7 @@ export declare namespace StateMachine {
   }
 
   export type HookOptions<TContext extends Dict, TState extends StateSchema, TEvent extends EventObject> = {
+    sync?: boolean
     actions?: ActionMap<TContext, TState, TEvent> | undefined
     state?: StateInit<TContext, TState> | undefined
     context?: UserContext<TContext> | undefined

diff --git a/packages/frameworks/react/src/use-snapshot.ts b/packages/frameworks/react/src/use-snapshot.ts
@@ -15,7 +15,7 @@ export function useSnapshot<
   TEvent extends S.EventObject = S.AnyEventObject,
 >(
   service: Machine<TContext, TState, TEvent>,
-  options?: S.HookOptions<TContext, TState, TEvent> & { sync?: boolean },
+  options?: S.HookOptions<TContext, TState, TEvent>,
 ): S.State<TContext, TState, TEvent> {
   //
   type State = S.State<TContext, TState, TEvent>

diff --git a/packages/machines/hover-card/src/hover-card.machine.ts b/packages/machines/hover-card/src/hover-card.machine.ts
@@ -59,10 +59,12 @@ export function machine(userContext: UserDefinedContext) {
           },
           on: {
             "CONTROLLED.OPEN": "open",
+            "CONTROLLED.CLOSE": "closed",
             POINTER_LEAVE: [
               {
                 guard: "isOpenControlled",
-                actions: ["invokeOnClose"],
+                // We trigger toggleVisibility manually since the `ctx.open` has not changed yet (at this point)
+                actions: ["invokeOnClose", "toggleVisibility"],
               },
               {
                 target: "closed",
@@ -72,7 +74,8 @@ export function machine(userContext: UserDefinedContext) {
             TRIGGER_BLUR: [
               {
                 guard: and("isOpenControlled", not("isPointer")),
-                actions: ["invokeOnClose"],
+                // We trigger toggleVisibility manually since the `ctx.open` has not changed yet (at this point)
+                actions: ["invokeOnClose", "toggleVisibility"],
               },
               {
                 guard: not("isPointer"),
@@ -83,7 +86,8 @@ export function machine(userContext: UserDefinedContext) {
             CLOSE: [
               {
                 guard: "isOpenControlled",
-                actions: ["invokeOnClose"],
+                // We trigger toggleVisibility manually since the `ctx.open` has not changed yet (at this point)
+                actions: ["invokeOnClose", "toggleVisibility"],
               },
               {
                 target: "closed",
@@ -146,6 +150,7 @@ export function machine(userContext: UserDefinedContext) {
           },
           on: {
             "CONTROLLED.CLOSE": "closed",
+            "CONTROLLED.OPEN": "open",
             POINTER_ENTER: {
               target: "open",
               // no need to invokeOnOpen here because it's still open (but about to close)
@@ -156,10 +161,16 @@ export function machine(userContext: UserDefinedContext) {
       },
     },
     {
+      delays: {
+        OPEN_DELAY: (ctx) => ctx.openDelay,
+        CLOSE_DELAY: (ctx) => ctx.closeDelay,
+      },
+
       guards: {
         isPointer: (ctx) => !!ctx.isPointer,
         isOpenControlled: (ctx) => !!ctx["open.controlled"],
       },
+
       activities: {
         trackPositioning(ctx) {
           ctx.currentPlacement = ctx.positioning.placement
@@ -172,6 +183,7 @@ export function machine(userContext: UserDefinedContext) {
             },
           })
         },
+
         trackDismissableElement(ctx, _evt, { send }) {
           const getContentEl = () => dom.getContentEl(ctx)
           return trackDismissableElement(getContentEl, {
@@ -186,6 +198,7 @@ export function machine(userContext: UserDefinedContext) {
           })
         },
       },
+
       actions: {
         invokeOnClose(ctx) {
           ctx.onOpenChange?.({ open: false })
@@ -212,13 +225,11 @@ export function machine(userContext: UserDefinedContext) {
           })
         },
         toggleVisibility(ctx, evt, { send }) {
-          send({ type: ctx.open ? "CONTROLLED.OPEN" : "CONTROLLED.CLOSE", previousEvent: evt })
+          queueMicrotask(() => {
+            send({ type: ctx.open ? "CONTROLLED.OPEN" : "CONTROLLED.CLOSE", previousEvent: evt })
+          })
         },
       },
-      delays: {
-        OPEN_DELAY: (ctx) => ctx.openDelay,
-        CLOSE_DELAY: (ctx) => ctx.closeDelay,
-      },
     },
   )
 }
diff --git a/packages/machines/tooltip/src/tooltip.machine.ts b/packages/machines/tooltip/src/tooltip.machine.ts
@@ -26,6 +26,7 @@ export function machine(userContext: UserDefinedContext) {
         interactive: false,
         closeOnScroll: true,
         closeOnClick: true,
+        disabled: false,
         ...ctx,
         currentPlacement: undefined,
         hasPointerMoveOpened: false,
@@ -50,10 +51,16 @@ export function machine(userContext: UserDefinedContext) {
           entry: ["clearGlobalId"],
           on: {
             "CONTROLLED.OPEN": "open",
-            OPEN: {
-              target: "open",
-              actions: ["invokeOnOpen"],
-            },
+            OPEN: [
+              {
+                guard: "isOpenControlled",
+                actions: ["invokeOnOpen"],
+              },
+              {
+                target: "open",
+                actions: ["invokeOnOpen"],
+              },
+            ],
             POINTER_LEAVE: {
               actions: ["clearPointerMoveOpened"],
             },
@@ -102,17 +109,25 @@ export function machine(userContext: UserDefinedContext) {
             POINTER_LEAVE: [
               {
                 guard: "isOpenControlled",
-                actions: ["clearPointerMoveOpened", "invokeOnClose"],
+                // We trigger toggleVisibility manually since the `ctx.open` has not changed yet (at this point)
+                actions: ["clearPointerMoveOpened", "invokeOnClose", "toggleVisibility"],
               },
               {
                 target: "closed",
                 actions: ["clearPointerMoveOpened", "invokeOnClose"],
               },
             ],
-            CLOSE: {
-              target: "closed",
-              actions: ["invokeOnClose"],
-            },
+            CLOSE: [
+              {
+                guard: "isOpenControlled",
+                // We trigger toggleVisibility manually since the `ctx.open` has not changed yet (at this point)
+                actions: ["invokeOnClose", "toggleVisibility"],
+              },
+              {
+                target: "closed",
+                actions: ["invokeOnClose"],
+              },
+            ],
           },
         },
 
@@ -122,10 +137,16 @@ export function machine(userContext: UserDefinedContext) {
           entry: ["setGlobalId"],
           on: {
             "CONTROLLED.CLOSE": "closed",
-            CLOSE: {
-              target: "closed",
-              actions: ["invokeOnClose"],
-            },
+            CLOSE: [
+              {
+                guard: "isOpenControlled",
+                actions: ["invokeOnClose"],
+              },
+              {
+                target: "closed",
+                actions: ["invokeOnClose"],
+              },
+            ],
             POINTER_LEAVE: [
               {
                 guard: "isVisible",
@@ -183,7 +204,8 @@ export function machine(userContext: UserDefinedContext) {
             POINTER_MOVE: [
               {
                 guard: "isOpenControlled",
-                actions: ["setPointerMoveOpened", "invokeOnOpen"],
+                // We trigger toggleVisibility manually since the `ctx.open` has not changed yet (at this point)
+                actions: ["setPointerMoveOpened", "invokeOnOpen", "toggleVisibility"],
               },
               {
                 target: "open",
@@ -292,7 +314,9 @@ export function machine(userContext: UserDefinedContext) {
           })
         },
         toggleVisibility(ctx, evt, { send }) {
-          send({ type: ctx.open ? "CONTROLLED.OPEN" : "CONTROLLED.CLOSE", previousEvent: evt })
+          queueMicrotask(() => {
+            send({ type: ctx.open ? "CONTROLLED.OPEN" : "CONTROLLED.CLOSE", previousEvent: evt })
+          })
         },
         setPointerMoveOpened(ctx) {
           ctx.hasPointerMoveOpened = true

diff --git a/starters/react/app/page.tsx b/starters/react/app/page.tsx
@@ -11,6 +11,7 @@ const routes = [
   { path: "/date-picker/controlled-range-picker", name: "DateRangePicker - Controlled" },
   { path: "/hover-card/controlled", name: "HoverCard - Controlled" },
   { path: "/tooltip/controlled", name: "Tooltip - Controlled" },
+  { path: "/tooltip/controlled-multiple", name: "Tooltip - Controlled Multiple" },
   { path: "/menu/controlled", name: "Menu - Controlled" },
   { path: "/collapsible/controlled", name: "Collapsible - Controlled" },
   { path: "/collapsible/uncontrolled", name: "Collapsible - Uncontrolled" },

diff --git a/starters/react/app/tooltip/controlled-multiple/page.tsx b/starters/react/app/tooltip/controlled-multiple/page.tsx
@@ -0,0 +1,31 @@
+"use client"
+
+import { Tooltip } from "@/components/tooltip"
+import { useState } from "react"
+
+const TooltipDemo = () => {
+  const [open, setOpen] = useState(false)
+  return (
+    <Tooltip
+      label="Tooltip Content"
+      positioning={{ placement: "right" }}
+      open={open}
+      onOpenChange={(e) => setOpen(e.open)}
+    >
+      <button>Some tooltip</button>
+    </Tooltip>
+  )
+}
+
+export default function Page() {
+  return (
+    <div style={{ padding: "40px", height: "200vh" }}>
+      <h1>Tooltip Controlled (Multiple)</h1>
+      <div style={{ display: "flex", flexDirection: "column", gap: "4px" }}>
+        {[...Array.from({ length: 10 })].map((_, index) => (
+          <TooltipDemo key={index} />
+        ))}
+      </div>
+    </div>
+  )
+}