From 1af4b0ac5d02e5d25050c646893f5bce7cbad318 Mon Sep 17 00:00:00 2001 From: Segun Adebayo Date: Thu, 6 Aug 2026 17:55:03 +0200 Subject: [PATCH 1/2] fix(react): reset all composed refs on detach when one returns a React 19 cleanup composeRefs only invoked cleanups collected from refs that returned one, so plain callback refs and object refs merged alongside a React 19 cleanup-ref never got called with null or reset to null on detach. Closes #3961 --- .changeset/compose-refs-cleanup.md | 5 + .../react/src/utils/compose-refs.test.tsx | 142 ++++++++++++++++++ packages/react/src/utils/compose-refs.ts | 9 +- 3 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 .changeset/compose-refs-cleanup.md diff --git a/.changeset/compose-refs-cleanup.md b/.changeset/compose-refs-cleanup.md new file mode 100644 index 0000000000..0cf8a2329d --- /dev/null +++ b/.changeset/compose-refs-cleanup.md @@ -0,0 +1,5 @@ +--- +"@ark-ui/react": patch +--- + +Fix `composeRefs`/`useComposedRefs` not resetting plain callback refs and object refs to `null` on detach when composed alongside a React 19 ref that returns a cleanup function. diff --git a/packages/react/src/utils/compose-refs.test.tsx b/packages/react/src/utils/compose-refs.test.tsx index 0ecc9a0a67..5981a69426 100644 --- a/packages/react/src/utils/compose-refs.test.tsx +++ b/packages/react/src/utils/compose-refs.test.tsx @@ -1,6 +1,7 @@ import { render, screen } from '@testing-library/react' import user from '@testing-library/user-event' import { useRef, useState } from 'react' +import type { RefObject } from 'react' import { composeRefs, useComposedRefs } from './compose-refs.ts' describe('Util: composeRefs', () => { @@ -25,6 +26,99 @@ describe('Util: composeRefs', () => { expect(cleanup).toHaveBeenCalledTimes(1) }) + + it('should not return a cleanup function when no ref provides one', () => { + const node = document.createElement('div') + const callbackRef = vi.fn() + const objectRef = { current: null as HTMLDivElement | null } + + const dispose = composeRefs(callbackRef, objectRef)(node) + + expect(dispose).toBeUndefined() + }) + + it('should reset a plain callback ref to null when a sibling ref returns a cleanup', () => { + const node = document.createElement('div') + const cleanup = vi.fn() + const refWithCleanup = vi.fn(() => cleanup) + const plainCallbackRef = vi.fn() + + const dispose = composeRefs(refWithCleanup, plainCallbackRef)(node) as VoidFunction | undefined + expect(plainCallbackRef).toHaveBeenCalledWith(node) + expect(plainCallbackRef).not.toHaveBeenCalledWith(null) + + dispose?.() + + expect(cleanup).toHaveBeenCalledTimes(1) + expect(plainCallbackRef).toHaveBeenCalledWith(null) + }) + + it('should reset an object ref to null when a sibling ref returns a cleanup', () => { + const node = document.createElement('div') + const cleanup = vi.fn() + const refWithCleanup = vi.fn(() => cleanup) + const objectRef = { current: null as HTMLDivElement | null } + + const dispose = composeRefs(refWithCleanup, objectRef)(node) as VoidFunction | undefined + expect(objectRef.current).toBe(node) + + dispose?.() + + expect(cleanup).toHaveBeenCalledTimes(1) + expect(objectRef.current).toBeNull() + }) + + it('should clean up every ref type when mixed together', () => { + const node = document.createElement('div') + const cleanupA = vi.fn() + const cleanupB = vi.fn() + const refWithCleanupA = vi.fn(() => cleanupA) + const refWithCleanupB = vi.fn(() => cleanupB) + const plainCallbackRef = vi.fn() + const objectRefA = { current: null as HTMLDivElement | null } + const objectRefB = { current: null as HTMLDivElement | null } + + const dispose = composeRefs(refWithCleanupA, plainCallbackRef, objectRefA, refWithCleanupB, objectRefB)(node) as + VoidFunction | undefined + + dispose?.() + + expect(cleanupA).toHaveBeenCalledTimes(1) + expect(cleanupB).toHaveBeenCalledTimes(1) + expect(plainCallbackRef).toHaveBeenCalledWith(null) + expect(objectRefA.current).toBeNull() + expect(objectRefB.current).toBeNull() + }) + + it('should ignore undefined and null refs', () => { + const node = document.createElement('div') + const cleanup = vi.fn() + const refWithCleanup = vi.fn(() => cleanup) + + expect(() => composeRefs(refWithCleanup, undefined, null as any)(node)).not.toThrow() + }) + + it('should do nothing and return no cleanup when every ref is nullish', () => { + const node = document.createElement('div') + + const dispose = composeRefs(undefined, null as any, undefined)(node) + + expect(dispose).toBeUndefined() + }) + + it('should not call a plain callback ref again on cleanup beyond the initial null call', () => { + const node = document.createElement('div') + const cleanup = vi.fn() + const refWithCleanup = vi.fn(() => cleanup) + const plainCallbackRef = vi.fn() + + const dispose = composeRefs(refWithCleanup, plainCallbackRef)(node) as VoidFunction | undefined + dispose?.() + + expect(plainCallbackRef).toHaveBeenCalledTimes(2) + expect(plainCallbackRef).toHaveBeenNthCalledWith(1, node) + expect(plainCallbackRef).toHaveBeenNthCalledWith(2, null) + }) }) describe('Util: useComposedRefs', () => { @@ -73,4 +167,52 @@ describe('Util: useComposedRefs', () => { expect(firstRef).toHaveBeenCalledWith(null) expect(secondRef).toHaveBeenCalledWith(node) }) + + it('should tear down the old cleanup-ref bundle and set up the new one when the ref set changes', () => { + const cleanup = vi.fn() + const refWithCleanup = vi.fn(() => cleanup) + const plainCallbackRef = vi.fn() + + const ComponentUnderTest = (props: { includeCleanupRef: boolean }) => { + const composedRefs = useComposedRefs(props.includeCleanupRef ? refWithCleanup : undefined, plainCallbackRef) + return
+ } + + const { rerender } = render() + const node = screen.getByTestId('node') + + expect(refWithCleanup).toHaveBeenCalledWith(node) + expect(plainCallbackRef).toHaveBeenCalledWith(node) + expect(plainCallbackRef).not.toHaveBeenCalledWith(null) + + rerender() + + expect(cleanup).toHaveBeenCalledTimes(1) + expect(plainCallbackRef).toHaveBeenCalledWith(null) + expect(plainCallbackRef).toHaveBeenLastCalledWith(node) + }) + + it('should detach plain callback and object refs on unmount when a sibling ref uses cleanup', () => { + const cleanup = vi.fn() + const refWithCleanup = vi.fn(() => cleanup) + const plainCallbackRef = vi.fn() + + const ComponentUnderTest = (props: { objectRef: RefObject }) => { + const composedRefs = useComposedRefs(refWithCleanup, plainCallbackRef, props.objectRef) + return
+ } + + const objectRef = { current: null as HTMLDivElement | null } + const { unmount } = render() + const node = screen.getByTestId('node') + + expect(objectRef.current).toBe(node) + expect(plainCallbackRef).toHaveBeenCalledWith(node) + + unmount() + + expect(cleanup).toHaveBeenCalledTimes(1) + expect(plainCallbackRef).toHaveBeenCalledWith(null) + expect(objectRef.current).toBeNull() + }) }) diff --git a/packages/react/src/utils/compose-refs.ts b/packages/react/src/utils/compose-refs.ts index 169b8a802e..beed610661 100644 --- a/packages/react/src/utils/compose-refs.ts +++ b/packages/react/src/utils/compose-refs.ts @@ -6,19 +6,26 @@ type PossibleRef = Ref | undefined export function composeRefs(...refs: PossibleRef[]): RefCallback { return (node) => { const cleanUps: VoidFunction[] = [] + let hasCustomCleanUp = false for (const ref of refs) { if (typeof ref === 'function') { const cb = ref(node) if (typeof cb === 'function') { + hasCustomCleanUp = true cleanUps.push(cb) + } else { + cleanUps.push(() => ref(null)) } } else if (ref) { ref.current = node + cleanUps.push(() => { + ref.current = null + }) } } - if (cleanUps.length) { + if (hasCustomCleanUp) { return () => { for (const cleanUp of cleanUps) { cleanUp() From 7b9da8654e391eb0e8988fab3f34dd4e00d8f017 Mon Sep 17 00:00:00 2001 From: Segun Adebayo Date: Thu, 6 Aug 2026 18:40:04 +0200 Subject: [PATCH 2/2] docs(toggle-group): add with-tooltip example composing Item and Tooltip.Trigger Compose ToggleGroup.Item with Tooltip.Trigger via asChild, giving both a matching custom id through ids.item/ids.trigger so they share one element and tab stop while keeping roving focus and tooltip positioning working. Addresses chakra-ui/ark#3934. --- .../toggle-group/examples/with-tooltip.tsx | 37 +++++++++++++++ .../toggle-group/toggle-group.stories.tsx | 1 + .../toggle-group/examples/with-tooltip.tsx | 47 +++++++++++++++++++ .../toggle-group/toggle-group.stories.tsx | 1 + .../toggle-group/examples/with-tooltip.svelte | 42 +++++++++++++++++ .../toggle-group/toggle-group.stories.ts | 7 +++ .../toggle-group/examples/with-tooltip.vue | 43 +++++++++++++++++ .../toggle-group/toggle-group.stories.ts | 8 ++++ .../content/pages/components/toggle-group.mdx | 7 +++ website/src/lib/example-registry.ts | 2 + 10 files changed, 195 insertions(+) create mode 100644 packages/react/src/components/toggle-group/examples/with-tooltip.tsx create mode 100644 packages/solid/src/components/toggle-group/examples/with-tooltip.tsx create mode 100644 packages/svelte/src/lib/components/toggle-group/examples/with-tooltip.svelte create mode 100644 packages/vue/src/components/toggle-group/examples/with-tooltip.vue diff --git a/packages/react/src/components/toggle-group/examples/with-tooltip.tsx b/packages/react/src/components/toggle-group/examples/with-tooltip.tsx new file mode 100644 index 0000000000..ccb7b7a5aa --- /dev/null +++ b/packages/react/src/components/toggle-group/examples/with-tooltip.tsx @@ -0,0 +1,37 @@ +import { Portal } from '@ark-ui/react/portal' +import { ToggleGroup } from '@ark-ui/react/toggle-group' +import { Tooltip, useTooltip } from '@ark-ui/react/tooltip' +import { BoldIcon, ItalicIcon, UnderlineIcon } from 'lucide-react' +import styles from 'styles/toggle-group.module.css' +import tooltipStyles from 'styles/tooltip.module.css' + +const items = [ + { value: 'bold', label: 'Bold', icon: }, + { value: 'italic', label: 'Italic', icon: }, + { value: 'underline', label: 'Underline', icon: }, +] + +const getTriggerId = (value?: string) => `toggle-item:${value}` + +export const WithTooltip = () => { + const tooltip = useTooltip({ ids: { trigger: getTriggerId } }) + + return ( + + + {items.map((item) => ( + + {item.icon} + + ))} + + + + + {items.find((item) => item.value === tooltip.triggerValue)?.label} + + + + + ) +} diff --git a/packages/react/src/components/toggle-group/toggle-group.stories.tsx b/packages/react/src/components/toggle-group/toggle-group.stories.tsx index 8aeb66ad56..d80e75c742 100644 --- a/packages/react/src/components/toggle-group/toggle-group.stories.tsx +++ b/packages/react/src/components/toggle-group/toggle-group.stories.tsx @@ -10,3 +10,4 @@ export { Basic } from './examples/basic.tsx' export { Controlled } from './examples/controlled.tsx' export { Multiple } from './examples/multiple.tsx' export { RootProvider } from './examples/root-provider.tsx' +export { WithTooltip } from './examples/with-tooltip.tsx' diff --git a/packages/solid/src/components/toggle-group/examples/with-tooltip.tsx b/packages/solid/src/components/toggle-group/examples/with-tooltip.tsx new file mode 100644 index 0000000000..8503d01e08 --- /dev/null +++ b/packages/solid/src/components/toggle-group/examples/with-tooltip.tsx @@ -0,0 +1,47 @@ +import { ToggleGroup } from '@ark-ui/solid/toggle-group' +import { Tooltip, useTooltip } from '@ark-ui/solid/tooltip' +import { BoldIcon, ItalicIcon, UnderlineIcon } from 'lucide-solid' +import { For } from 'solid-js' +import { Portal } from 'solid-js/web' +import styles from 'styles/toggle-group.module.css' +import tooltipStyles from 'styles/tooltip.module.css' + +const items = [ + { value: 'bold', label: 'Bold', icon: BoldIcon }, + { value: 'italic', label: 'Italic', icon: ItalicIcon }, + { value: 'underline', label: 'Underline', icon: UnderlineIcon }, +] + +const getTriggerId = (value?: string) => `toggle-item:${value}` + +export const WithTooltip = () => { + const tooltip = useTooltip({ ids: { trigger: getTriggerId } }) + + return ( + + + + {(item) => ( + ( + + + + )} + /> + )} + + + + + + {items.find((item) => item.value === tooltip().triggerValue)?.label} + + + + + ) +} diff --git a/packages/solid/src/components/toggle-group/toggle-group.stories.tsx b/packages/solid/src/components/toggle-group/toggle-group.stories.tsx index 15e97dc8e7..735a0038de 100644 --- a/packages/solid/src/components/toggle-group/toggle-group.stories.tsx +++ b/packages/solid/src/components/toggle-group/toggle-group.stories.tsx @@ -10,3 +10,4 @@ export { Basic } from './examples/basic.tsx' export { Controlled } from './examples/controlled.tsx' export { Multiple } from './examples/multiple.tsx' export { RootProvider } from './examples/root-provider.tsx' +export { WithTooltip } from './examples/with-tooltip.tsx' diff --git a/packages/svelte/src/lib/components/toggle-group/examples/with-tooltip.svelte b/packages/svelte/src/lib/components/toggle-group/examples/with-tooltip.svelte new file mode 100644 index 0000000000..3227d37eab --- /dev/null +++ b/packages/svelte/src/lib/components/toggle-group/examples/with-tooltip.svelte @@ -0,0 +1,42 @@ + + + + + {#each items as item (item.value)} + + {#snippet asChild(itemProps)} + + + + {/snippet} + + {/each} + + + + + {items.find((item) => item.value === tooltip().triggerValue)?.label} + + + + diff --git a/packages/svelte/src/lib/components/toggle-group/toggle-group.stories.ts b/packages/svelte/src/lib/components/toggle-group/toggle-group.stories.ts index 7ee11a379f..ea73cf9833 100644 --- a/packages/svelte/src/lib/components/toggle-group/toggle-group.stories.ts +++ b/packages/svelte/src/lib/components/toggle-group/toggle-group.stories.ts @@ -3,6 +3,7 @@ import BasicExample from './examples/basic.svelte' import ControlledExample from './examples/controlled.svelte' import MultipleExample from './examples/multiple.svelte' import RootProviderExample from './examples/root-provider.svelte' +import WithTooltipExample from './examples/with-tooltip.svelte' const meta: Meta = { title: 'Components / Toggle Group', @@ -33,3 +34,9 @@ export const RootProvider = { Component: RootProviderExample, }), } + +export const WithTooltip = { + render: () => ({ + Component: WithTooltipExample, + }), +} diff --git a/packages/vue/src/components/toggle-group/examples/with-tooltip.vue b/packages/vue/src/components/toggle-group/examples/with-tooltip.vue new file mode 100644 index 0000000000..9177121f45 --- /dev/null +++ b/packages/vue/src/components/toggle-group/examples/with-tooltip.vue @@ -0,0 +1,43 @@ + + + diff --git a/packages/vue/src/components/toggle-group/toggle-group.stories.ts b/packages/vue/src/components/toggle-group/toggle-group.stories.ts index 6782ebb278..db8750b71a 100644 --- a/packages/vue/src/components/toggle-group/toggle-group.stories.ts +++ b/packages/vue/src/components/toggle-group/toggle-group.stories.ts @@ -4,6 +4,7 @@ import BasicExample from './examples/basic.vue' import ControlledExample from './examples/controlled.vue' import MultipleExample from './examples/multiple.vue' import RootProviderExample from './examples/root-provider.vue' +import WithTooltipExample from './examples/with-tooltip.vue' const meta: Meta = { title: 'Components / Toggle Group', @@ -38,3 +39,10 @@ export const RootProvider = { template: '', }), } + +export const WithTooltip = { + render: () => ({ + components: { Component: WithTooltipExample }, + template: '', + }), +} diff --git a/website/src/content/pages/components/toggle-group.mdx b/website/src/content/pages/components/toggle-group.mdx index cab5e76c02..7f56060fe3 100644 --- a/website/src/content/pages/components/toggle-group.mdx +++ b/website/src/content/pages/components/toggle-group.mdx @@ -39,6 +39,13 @@ Demonstrates how to enable `multiple` selection within the group. +### With Tooltip + +Pair an item with a `Tooltip.Trigger` via `asChild`, matching their ids with `ids.item` and `ids.trigger` so they share +one element and tab stop. + + + ## API Reference ### Props diff --git a/website/src/lib/example-registry.ts b/website/src/lib/example-registry.ts index 9075747b31..7026d62c2d 100644 --- a/website/src/lib/example-registry.ts +++ b/website/src/lib/example-registry.ts @@ -548,6 +548,7 @@ import * as ToggleGroup_Basic from '@examples/toggle-group/examples/basic' import * as ToggleGroup_Controlled from '@examples/toggle-group/examples/controlled' import * as ToggleGroup_Multiple from '@examples/toggle-group/examples/multiple' import * as ToggleGroup_RootProvider from '@examples/toggle-group/examples/root-provider' +import * as ToggleGroup_WithTooltip from '@examples/toggle-group/examples/with-tooltip' import * as Toggle_Basic from '@examples/toggle/examples/basic' import * as Toggle_Context from '@examples/toggle/examples/context' import * as Toggle_Controlled from '@examples/toggle/examples/controlled' @@ -1147,6 +1148,7 @@ const exampleModules: Record = { 'toggle-group/controlled': ToggleGroup_Controlled, 'toggle-group/multiple': ToggleGroup_Multiple, 'toggle-group/root-provider': ToggleGroup_RootProvider, + 'toggle-group/with-tooltip': ToggleGroup_WithTooltip, 'toggle/basic': Toggle_Basic, 'toggle/context': Toggle_Context, 'toggle/controlled': Toggle_Controlled,