diff --git a/.changeset/quick-lizards-ring.md b/.changeset/quick-lizards-ring.md new file mode 100644 index 0000000000..76cf844fb6 --- /dev/null +++ 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 index a18ddec4de..2a146f03dc 100644 --- a/.xstate/hover-card.js +++ b/.xstate/hover-card.js @@ -55,16 +55,19 @@ 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", @@ -72,7 +75,8 @@ const fetchMachine = createMachine({ }], 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 index 530c646121..51e58eefc3 100644 --- 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 index fc35ec48fd..db5cac0953 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -387,6 +387,7 @@ export declare namespace StateMachine { } export type HookOptions = { + sync?: boolean actions?: ActionMap | undefined state?: StateInit | undefined context?: UserContext | undefined diff --git a/packages/frameworks/react/src/use-snapshot.ts b/packages/frameworks/react/src/use-snapshot.ts index 820b86ba01..6f072dca22 100644 --- 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, - options?: S.HookOptions & { sync?: boolean }, + options?: S.HookOptions, ): S.State { // type State = S.State diff --git a/packages/machines/hover-card/src/hover-card.machine.ts b/packages/machines/hover-card/src/hover-card.machine.ts index 28444605a2..d4138774cb 100644 --- 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 index bd42477ba5..7e89e05a84 100644 --- 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 index 8edef74a42..2905958514 100644 --- 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 new file mode 100644 index 0000000000..ccec8c2632 --- /dev/null +++ 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 ( + setOpen(e.open)} + > + + + ) +} + +export default function Page() { + return ( +
+

Tooltip Controlled (Multiple)

+
+ {[...Array.from({ length: 10 })].map((_, index) => ( + + ))} +
+
+ ) +}