From 54c0c4ad878dff52c6866a2ecad7484e569b7d55 Mon Sep 17 00:00:00 2001 From: cixzhang Date: Wed, 24 Jun 2026 19:04:24 +0000 Subject: [PATCH 1/3] fix(ToggleButton): run pressedChangeAction in a transition for pending state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pressedChangeAction was fired as a non-awaited promise, so the documented loading spinner never appeared and rapid clicks could fire the action multiple times. Wrap it in useTransition so the button shows its loading state (disabled + aria-busy) while the async action is pending, and guard against re-entrant clicks until it settles — matching Button's clickAction. --- ...lebutton-pressedchangeaction-transition.md | 12 +++ .../src/ToggleButton/ToggleButton.test.tsx | 74 +++++++++++++++++-- .../core/src/ToggleButton/ToggleButton.tsx | 24 ++++-- 3 files changed, 99 insertions(+), 11 deletions(-) create mode 100644 .changeset/togglebutton-pressedchangeaction-transition.md diff --git a/.changeset/togglebutton-pressedchangeaction-transition.md b/.changeset/togglebutton-pressedchangeaction-transition.md new file mode 100644 index 0000000000000..b4a8b013dc516 --- /dev/null +++ b/.changeset/togglebutton-pressedchangeaction-transition.md @@ -0,0 +1,12 @@ +--- +'@astryxdesign/core': patch +--- + +[fix] ToggleButton runs pressedChangeAction in a transition so it shows a pending state +@cixzhang + +`pressedChangeAction` was fired as a non-awaited promise, so the documented +loading spinner never appeared and rapid clicks could fire the action multiple +times. It is now wrapped in a transition: the button shows its loading state +(disabled + `aria-busy`) while the async action is in flight, and re-entrant +clicks are ignored until it settles — matching `Button`'s `clickAction`. diff --git a/packages/core/src/ToggleButton/ToggleButton.test.tsx b/packages/core/src/ToggleButton/ToggleButton.test.tsx index fa5eb3fb319f9..32ea56e80cc03 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} 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,72 @@ describe('ToggleButton', () => { ); expect(screen.getByTestId('bold-toggle')).toBeInTheDocument(); }); + + it('shows a loading spinner while pressedChangeAction is pending', 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); + + // While the action is in flight the button is disabled and aria-busy. + expect(pressedChangeAction).toHaveBeenCalledWith(true); + expect(button).toBeDisabled(); + expect(button).toHaveAttribute('aria-busy', 'true'); + + // Once the action settles the pending state clears. + await act(async () => { + resolveAction?.(); + }); + expect(button).not.toBeDisabled(); + expect(button).not.toHaveAttribute('aria-busy', 'true'); + }); + + it('ignores re-entrant clicks while pressedChangeAction is pending', 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); + await user.click(button); + + // Second click is a no-op while the first action is still pending. + expect(pressedChangeAction).toHaveBeenCalledTimes(1); + + await act(async () => { + resolveAction?.(); + }); + }); }); // ============================================================================= diff --git a/packages/core/src/ToggleButton/ToggleButton.tsx b/packages/core/src/ToggleButton/ToggleButton.tsx index d59e242bb54b5..d0f7fd05e0b5e 100644 --- a/packages/core/src/ToggleButton/ToggleButton.tsx +++ b/packages/core/src/ToggleButton/ToggleButton.tsx @@ -20,7 +20,7 @@ * - /packages/cli/templates/blocks/components/ToggleButton/ (showcase blocks) */ -import React, {useCallback, type ReactNode} from 'react'; +import React, {useCallback, useRef, useTransition, type ReactNode} from 'react'; import * as stylex from '@stylexjs/stylex'; import {colorVars, fontWeightVars} from '../theme/tokens.stylex'; @@ -231,8 +231,15 @@ export function ToggleButton({ const resolvedIcon = isPressed && pressedIcon ? pressedIcon : icon; + // Wrap pressedChangeAction in a transition so the button shows a loading + // spinner while the async action is pending and re-entrant clicks are + // ignored until it settles. Mirrors Button's clickAction handling. + const [isPending, startTransition] = useTransition(); + const actionInFlightRef = useRef(false); + const isLoadingState = isLoading || isPending; + const handleClick = useCallback(() => { - if (isDisabled || isLoading) { + if (isDisabled || isLoadingState || actionInFlightRef.current) { return; } @@ -244,12 +251,19 @@ export function ToggleButton({ const newState = !isPressed; onPressedChangeProp(newState); if (pressedChangeAction) { - void pressedChangeAction(newState); + actionInFlightRef.current = true; + startTransition(async () => { + try { + await pressedChangeAction(newState); + } finally { + actionInFlightRef.current = false; + } + }); } } }, [ isDisabled, - isLoading, + isLoadingState, group, value, onPressedChangeProp, @@ -289,7 +303,7 @@ export function ToggleButton({ variant="ghost" size={size} isDisabled={isDisabled} - isLoading={isLoading} + isLoading={isLoadingState} isIconOnly={isIconOnly} aria-pressed={isPressed} icon={resolvedIcon} From c628f63b8e96b8965b24b149dbdeffb946fefb93 Mon Sep 17 00:00:00 2001 From: cixzhang Date: Thu, 25 Jun 2026 00:17:10 +0000 Subject: [PATCH 2/3] fix(ToggleButton): make pressedChangeAction interruptible with optimistic state Address review feedback: model the toggle on Switch's useOptimistic pattern. - Track pressed state optimistically so the new state shows immediately and a click mid-flight derives its next value from the in-progress state (true -> false -> true), instead of stalling on the committed value. - Remove the re-entry guard: the action is now interruptible. Re-clicks start a new transition rather than being dropped. - Run onPressedChange + pressedChangeAction inside the transition so a sync but suspending handler (e.g. a router navigation that suspends on data) also drives the pending state. pressedChangeAction now accepts void | Promise. - Debounce the loading spinner so fast actions show the optimistic state without a spinner flash and stay interruptible. --- ...lebutton-pressedchangeaction-transition.md | 19 ++- .../src/ToggleButton/ToggleButton.doc.mjs | 4 +- .../src/ToggleButton/ToggleButton.test.tsx | 105 ++++++++++++++-- .../core/src/ToggleButton/ToggleButton.tsx | 112 +++++++++++++----- 4 files changed, 192 insertions(+), 48 deletions(-) diff --git a/.changeset/togglebutton-pressedchangeaction-transition.md b/.changeset/togglebutton-pressedchangeaction-transition.md index b4a8b013dc516..f7d4ceddafe46 100644 --- a/.changeset/togglebutton-pressedchangeaction-transition.md +++ b/.changeset/togglebutton-pressedchangeaction-transition.md @@ -2,11 +2,20 @@ '@astryxdesign/core': patch --- -[fix] ToggleButton runs pressedChangeAction in a transition so it shows a pending state +[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 rapid clicks could fire the action multiple -times. It is now wrapped in a transition: the button shows its loading state -(disabled + `aria-busy`) while the async action is in flight, and re-entrant -clicks are ignored until it settles — matching `Button`'s `clickAction`. +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 32ea56e80cc03..5aa2e6cb721f0 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, act} 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'; @@ -193,7 +193,7 @@ describe('ToggleButton', () => { expect(screen.getByTestId('bold-toggle')).toBeInTheDocument(); }); - it('shows a loading spinner while pressedChangeAction is pending', async () => { + it('shows the optimistic pressed state immediately, before any spinner', async () => { const user = userEvent.setup(); let resolveAction: (() => void) | undefined; const pressedChangeAction = vi.fn( @@ -213,22 +213,25 @@ describe('ToggleButton', () => { ); const button = screen.getByRole('button', {name: 'Favorite'}); + expect(button).toHaveAttribute('aria-pressed', 'false'); + await user.click(button); - // While the action is in flight the button is disabled and aria-busy. + // 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).toBeDisabled(); - expect(button).toHaveAttribute('aria-busy', 'true'); + expect(button).toHaveAttribute('aria-pressed', 'true'); + expect(button).not.toBeDisabled(); + expect(button).not.toHaveAttribute('aria-busy', 'true'); - // Once the action settles the pending state clears. + // Settle the action so the pending transition doesn't leak into later tests. await act(async () => { resolveAction?.(); + await Promise.resolve(); }); - expect(button).not.toBeDisabled(); - expect(button).not.toHaveAttribute('aria-busy', 'true'); }); - it('ignores re-entrant clicks while pressedChangeAction is pending', async () => { + 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( @@ -249,14 +252,92 @@ describe('ToggleButton', () => { const button = screen.getByRole('button', {name: 'Favorite'}); await user.click(button); - await user.click(button); - // Second click is a no-op while the first action is still pending. - expect(pressedChangeAction).toHaveBeenCalledTimes(1); + // 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'); + // Once the action settles the pending state clears. 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 d0f7fd05e0b5e..bd5f76d6aec38 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, useRef, useTransition, 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. @@ -221,54 +266,63 @@ export function ToggleButton({ // Read group context if inside a group const group = useToggleButtonGroup(); - // Resolve state from group or props - const isPressed = + // The committed pressed state, resolved from the group or props. + 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; - // Wrap pressedChangeAction in a transition so the button shows a loading - // spinner while the async action is pending and re-entrant clicks are - // ignored until it settles. Mirrors Button's clickAction handling. + // 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(); - const actionInFlightRef = useRef(false); - const isLoadingState = isLoading || isPending; + // 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 || isLoadingState || actionInFlightRef.current) { + 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) { - actionInFlightRef.current = true; - startTransition(async () => { - try { - await pressedChangeAction(newState); - } finally { - actionInFlightRef.current = false; - } - }); - } + return; } + + // Standalone toggle. Derive the next state from the optimistic value so a + // click while an action is in flight flips from the in-progress state. + const newState = !optimisticPressed; + startTransition(async () => { + setOptimisticPressed(newState); + onPressedChangeProp?.(newState); + await pressedChangeAction?.(newState); + }); }, [ isDisabled, - isLoadingState, group, value, + optimisticPressed, onPressedChangeProp, pressedChangeAction, - isPressed, + setOptimisticPressed, ]); // Label with font weight shift and width reservation From 5f5fe879ac541b9b3d3c74f6302eb5ee505ed728 Mon Sep 17 00:00:00 2001 From: cixzhang Date: Thu, 25 Jun 2026 16:48:52 +0000 Subject: [PATCH 3/3] chore(ToggleButton): drop comments that restate self-explanatory code --- packages/core/src/ToggleButton/ToggleButton.test.tsx | 1 - packages/core/src/ToggleButton/ToggleButton.tsx | 5 ----- 2 files changed, 6 deletions(-) diff --git a/packages/core/src/ToggleButton/ToggleButton.test.tsx b/packages/core/src/ToggleButton/ToggleButton.test.tsx index 5aa2e6cb721f0..7c683bb1fc2c7 100644 --- a/packages/core/src/ToggleButton/ToggleButton.test.tsx +++ b/packages/core/src/ToggleButton/ToggleButton.test.tsx @@ -259,7 +259,6 @@ describe('ToggleButton', () => { await waitFor(() => expect(button).toBeDisabled()); expect(button).toHaveAttribute('aria-busy', 'true'); - // Once the action settles the pending state clears. await act(async () => { resolveAction?.(); await Promise.resolve(); diff --git a/packages/core/src/ToggleButton/ToggleButton.tsx b/packages/core/src/ToggleButton/ToggleButton.tsx index bd5f76d6aec38..cfcd20697aa8a 100644 --- a/packages/core/src/ToggleButton/ToggleButton.tsx +++ b/packages/core/src/ToggleButton/ToggleButton.tsx @@ -263,10 +263,8 @@ export function ToggleButton({ style, ...props }: ToggleButtonProps): ReactNode { - // Read group context if inside a group const group = useToggleButtonGroup(); - // The committed pressed state, resolved from the group or props. const committedPressed = group && value != null ? group.selectedValues.has(value) @@ -307,8 +305,6 @@ export function ToggleButton({ return; } - // Standalone toggle. Derive the next state from the optimistic value so a - // click while an action is in flight flips from the in-progress state. const newState = !optimisticPressed; startTransition(async () => { setOptimisticPressed(newState); @@ -325,7 +321,6 @@ export function ToggleButton({ setOptimisticPressed, ]); - // Label with font weight shift and width reservation // isIconOnly prop is the source of truth for icon-only rendering. const labelContent = children != null ? (