diff --git a/.changeset/togglebutton-pressedchangeaction-transition.md b/.changeset/togglebutton-pressedchangeaction-transition.md new file mode 100644 index 0000000000000..f7d4ceddafe46 --- /dev/null +++ b/.changeset/togglebutton-pressedchangeaction-transition.md @@ -0,0 +1,21 @@ +--- +'@astryxdesign/core': patch +--- + +[fix] ToggleButton runs pressedChangeAction in an interruptible transition with optimistic state +@cixzhang + +`pressedChangeAction` was fired as a non-awaited promise, so the documented +loading spinner never appeared and the toggle ignored the action's lifecycle. +It now runs inside a transition with an optimistic pressed state, matching +`Switch`: + +- The optimistic pressed state flips immediately on click; the spinner is + debounced so a fast action shows the new state without a spinner flash. +- The action is interruptible — clicking again while it is pending starts a + new transition with the next optimistic state (e.g. true -> false -> true), + instead of being dropped or guarded out. +- Synchronous handlers are supported too: a `pressedChangeAction` (or + `onPressedChange`) that synchronously triggers a suspending update, such as + a router navigation that suspends on data, also drives the pending state. + `pressedChangeAction` now accepts `void | Promise`. diff --git a/packages/core/src/ToggleButton/ToggleButton.doc.mjs b/packages/core/src/ToggleButton/ToggleButton.doc.mjs index 71d8802d35a7c..42307ad95688e 100644 --- a/packages/core/src/ToggleButton/ToggleButton.doc.mjs +++ b/packages/core/src/ToggleButton/ToggleButton.doc.mjs @@ -39,8 +39,8 @@ export const docs = { }, { name: 'pressedChangeAction', - type: '(isPressed: boolean) => Promise', - description: 'Async action handler for API-backed toggles. Shows loading spinner while pending.', + type: '(isPressed: boolean) => void | Promise', + description: 'Action handler for API- or navigation-backed toggles, run in a transition. Shows an optimistic pressed state immediately and a (debounced) spinner while pending; interruptible by re-clicks.', }, { name: 'size', diff --git a/packages/core/src/ToggleButton/ToggleButton.test.tsx b/packages/core/src/ToggleButton/ToggleButton.test.tsx index fa5eb3fb319f9..7c683bb1fc2c7 100644 --- a/packages/core/src/ToggleButton/ToggleButton.test.tsx +++ b/packages/core/src/ToggleButton/ToggleButton.test.tsx @@ -9,7 +9,7 @@ */ import {describe, it, expect, vi} from 'vitest'; -import {render, screen} from '@testing-library/react'; +import {render, screen, act, fireEvent, waitFor} from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import {useState} from 'react'; import {ToggleButton} from './ToggleButton'; @@ -71,11 +71,7 @@ describe('ToggleButton', () => { it('sets aria-pressed=true when pressed', () => { render( - {}} - />, + {}} />, ); expect(screen.getByRole('button')).toHaveAttribute('aria-pressed', 'true'); }); @@ -196,6 +192,152 @@ describe('ToggleButton', () => { ); expect(screen.getByTestId('bold-toggle')).toBeInTheDocument(); }); + + it('shows the optimistic pressed state immediately, before any spinner', async () => { + const user = userEvent.setup(); + let resolveAction: (() => void) | undefined; + const pressedChangeAction = vi.fn( + async () => + new Promise(resolve => { + resolveAction = resolve; + }), + ); + + render( + {}} + pressedChangeAction={pressedChangeAction} + />, + ); + + const button = screen.getByRole('button', {name: 'Favorite'}); + expect(button).toHaveAttribute('aria-pressed', 'false'); + + await user.click(button); + + // The optimistic state flips immediately. The spinner is debounced, so the + // button is not disabled or aria-busy yet — it stays interruptible. + expect(pressedChangeAction).toHaveBeenCalledWith(true); + expect(button).toHaveAttribute('aria-pressed', 'true'); + expect(button).not.toBeDisabled(); + expect(button).not.toHaveAttribute('aria-busy', 'true'); + + // Settle the action so the pending transition doesn't leak into later tests. + await act(async () => { + resolveAction?.(); + await Promise.resolve(); + }); + }); + + it('shows a loading spinner once the action stays pending past the delay', async () => { + const user = userEvent.setup(); + let resolveAction: (() => void) | undefined; + const pressedChangeAction = vi.fn( + async () => + new Promise(resolve => { + resolveAction = resolve; + }), + ); + + render( + {}} + pressedChangeAction={pressedChangeAction} + />, + ); + + const button = screen.getByRole('button', {name: 'Favorite'}); + await user.click(button); + + // The optimistic state shows immediately; the spinner is debounced and + // appears only after the action stays pending past the delay window. + expect(button).toHaveAttribute('aria-pressed', 'true'); + await waitFor(() => expect(button).toBeDisabled()); + expect(button).toHaveAttribute('aria-busy', 'true'); + + await act(async () => { + resolveAction?.(); + await Promise.resolve(); + }); + expect(button).not.toBeDisabled(); + expect(button).not.toHaveAttribute('aria-busy', 'true'); + }); + + it('interrupts an in-flight action on re-click (true -> false -> true)', async () => { + // Each click interrupts the previous transition. The actions are resolved + // at the end so the pending transition doesn't leak into later tests. + const resolvers: (() => void)[] = []; + const pressedChangeAction = vi.fn( + async () => + new Promise(resolve => { + resolvers.push(resolve); + }), + ); + + render( + {}} + pressedChangeAction={pressedChangeAction} + />, + ); + + const button = screen.getByRole('button', {name: 'Favorite'}); + + // Each click derives the next state from the optimistic (in-progress) + // value, so rapid clicks toggle rather than being dropped. fireEvent keeps + // the clicks within the spinner debounce window so the button stays + // interruptible. + await act(async () => { + fireEvent.click(button); + }); + expect(button).toHaveAttribute('aria-pressed', 'true'); + await act(async () => { + fireEvent.click(button); + }); + expect(button).toHaveAttribute('aria-pressed', 'false'); + await act(async () => { + fireEvent.click(button); + }); + expect(button).toHaveAttribute('aria-pressed', 'true'); + + expect(pressedChangeAction).toHaveBeenCalledTimes(3); + expect(pressedChangeAction).toHaveBeenNthCalledWith(1, true); + expect(pressedChangeAction).toHaveBeenNthCalledWith(2, false); + expect(pressedChangeAction).toHaveBeenNthCalledWith(3, true); + + await act(async () => { + resolvers.forEach(resolve => resolve()); + await Promise.resolve(); + }); + }); + + it('supports a synchronous pressedChangeAction', async () => { + const user = userEvent.setup(); + // A sync handler (e.g. a router navigation) with no returned promise. + const pressedChangeAction = vi.fn((_next: boolean) => {}); + const onPressedChange = vi.fn(); + + render( + , + ); + + const button = screen.getByRole('button', {name: 'Favorite'}); + await user.click(button); + + expect(onPressedChange).toHaveBeenCalledWith(true); + expect(pressedChangeAction).toHaveBeenCalledWith(true); + }); }); // ============================================================================= diff --git a/packages/core/src/ToggleButton/ToggleButton.tsx b/packages/core/src/ToggleButton/ToggleButton.tsx index d59e242bb54b5..cfcd20697aa8a 100644 --- a/packages/core/src/ToggleButton/ToggleButton.tsx +++ b/packages/core/src/ToggleButton/ToggleButton.tsx @@ -20,7 +20,14 @@ * - /packages/cli/templates/blocks/components/ToggleButton/ (showcase blocks) */ -import React, {useCallback, type ReactNode} from 'react'; +import React, { + useCallback, + useEffect, + useOptimistic, + useState, + useTransition, + type ReactNode, +} from 'react'; import * as stylex from '@stylexjs/stylex'; import {colorVars, fontWeightVars} from '../theme/tokens.stylex'; @@ -29,6 +36,37 @@ import {useToggleButtonGroup} from './ToggleButtonGroup'; import type {BaseProps} from '../BaseProps'; import {xdsThemeProps} from '../utils/xdsThemeProps'; +// ============================================================================= +// Constants & helpers +// ============================================================================= + +/** + * The spinner only appears once the action has been pending for this long. + * A fast action shows the optimistic pressed state immediately with no spinner + * flash, and rapid re-clicks can interrupt the in-flight action before the + * button locks behind the spinner. + */ +const PENDING_SPINNER_DELAY_MS = 150; + +/** + * Returns `true` only once `active` has stayed `true` for `delayMs`. + * Used to debounce the loading spinner so the optimistic state shows first. + */ +function useDelayed(active: boolean, delayMs: number): boolean { + const [delayed, setDelayed] = useState(false); + useEffect(() => { + if (!active) { + return undefined; + } + const timer = setTimeout(() => setDelayed(true), delayMs); + return () => { + clearTimeout(timer); + setDelayed(false); + }; + }, [active, delayMs]); + return active && delayed; +} + // ============================================================================= // Styles // ============================================================================= @@ -91,8 +129,15 @@ export interface ToggleButtonProps extends BaseProps { onPressedChange?: (isPressed: boolean) => void; /** - * Async action handler for API-backed toggles. - * The button shows a loading spinner while the promise is pending. + * Action handler for API- or navigation-backed toggles, run inside a + * transition. The button shows a loading spinner while the action is + * pending — whether it returns a promise or synchronously triggers a + * suspending update (e.g. a router navigation that suspends on data). + * + * Because it runs in a transition, the toggle is *interruptible*: clicking + * again while an action is pending starts a new transition with the next + * optimistic state, so the action reflects the latest intent rather than + * being dropped. * * @example * ``` @@ -106,7 +151,7 @@ export interface ToggleButtonProps extends BaseProps { * /> * ``` */ - pressedChangeAction?: (isPressed: boolean) => Promise; + pressedChangeAction?: (isPressed: boolean) => void | Promise; /** * The size of the toggle button. @@ -218,46 +263,64 @@ export function ToggleButton({ style, ...props }: ToggleButtonProps): ReactNode { - // Read group context if inside a group const group = useToggleButtonGroup(); - // Resolve state from group or props - const isPressed = + const committedPressed = group && value != null ? group.selectedValues.has(value) : (isPressedProp ?? false); const size = sizeProp ?? group?.size ?? 'md'; const isDisabled = group?.isDisabled ?? isDisabledProp; + // Track the pressed state optimistically. While an action is pending, the + // button reflects the intended (optimistic) state immediately, and a click + // mid-flight derives its next state from this value — so rapid toggles read + // true -> false -> true rather than stalling on the last committed value. + const [optimisticPressed, setOptimisticPressed] = + useOptimistic(committedPressed); + const isPressed = optimisticPressed; + const resolvedIcon = isPressed && pressedIcon ? pressedIcon : icon; + // Run the toggle inside a transition. The action is interruptible: clicking + // again while it is pending starts a fresh transition with the next + // optimistic state instead of being dropped, so there is no re-entry guard. + // Both onPressedChange and pressedChangeAction run inside the transition, + // which means a synchronous-but-suspending handler (e.g. a router navigation + // that suspends on data) also drives the pending state — not just promises. + const [isPending, startTransition] = useTransition(); + // Debounce the spinner so a fast action shows the optimistic state without a + // spinner flash, and rapid re-clicks can interrupt before the button locks. + const showSpinner = useDelayed(isPending, PENDING_SPINNER_DELAY_MS); + const isLoadingState = isLoading || showSpinner; + const handleClick = useCallback(() => { - if (isDisabled || isLoading) { + if (isDisabled) { return; } if (group && value != null) { - // Delegate to group context + // Group mode delegates selection to the group; no async-action path. group.toggle(value); - } else if (onPressedChangeProp) { - // Standalone toggle - const newState = !isPressed; - onPressedChangeProp(newState); - if (pressedChangeAction) { - void pressedChangeAction(newState); - } + return; } + + const newState = !optimisticPressed; + startTransition(async () => { + setOptimisticPressed(newState); + onPressedChangeProp?.(newState); + await pressedChangeAction?.(newState); + }); }, [ isDisabled, - isLoading, group, value, + optimisticPressed, onPressedChangeProp, pressedChangeAction, - isPressed, + setOptimisticPressed, ]); - // Label with font weight shift and width reservation // isIconOnly prop is the source of truth for icon-only rendering. const labelContent = children != null ? ( @@ -289,7 +352,7 @@ export function ToggleButton({ variant="ghost" size={size} isDisabled={isDisabled} - isLoading={isLoading} + isLoading={isLoadingState} isIconOnly={isIconOnly} aria-pressed={isPressed} icon={resolvedIcon}