Skip to content

Commit fd3b6ea

Browse files
pawelgrimmclaude
andcommitted
refactor: drop onClick and forwardClickToControl from ControlPresentation props
Click handlers belong on the inner control, not on the wrapper. The wrapper's click-to-focus behavior is internal — there's no real need for consumers to disable it (forwardClickToControl) or hook into it (onClick) for the wrap pattern. The implementation still extracts onClick out of rest (via a type cast) to compose with the focus-forwarding handler — this path exists only for render-prop use, where Ariakit (or similar) forwards its own onClick to the wrapper-as-trigger. Consumers of ControlPresentation directly won't see onClick in autocomplete; TypeScript rejects passing it at call sites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent fc5050d commit fd3b6ea

2 files changed

Lines changed: 15 additions & 29 deletions

File tree

src/control-presentation/control-presentation.test.tsx

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -127,20 +127,7 @@ describe('ControlPresentation', () => {
127127
expect(screen.getByTestId('b')).toBeInTheDocument()
128128
})
129129

130-
it('does not forward the click to the control when forwardClickToControl is false', () => {
131-
const { container } = render(
132-
<ControlPresentation forwardClickToControl={false}>
133-
<input aria-label="Subject" data-testid="subject" />
134-
</ControlPresentation>,
135-
)
136-
const control = screen.getByTestId('subject')
137-
expect(control).not.toHaveFocus()
138-
139-
userEvent.click(container.firstElementChild as Element)
140-
expect(control).not.toHaveFocus()
141-
})
142-
143-
it('calls the consumer onClick when the wrapper is clicked', () => {
130+
it('calls a consumer onClick passed via Box props when the wrapper is clicked', () => {
144131
const onClick = jest.fn()
145132
const { container } = render(
146133
<ControlPresentation onClick={onClick}>

src/control-presentation/control-presentation.tsx

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,14 @@ export type ControlPresentationProps = {
2323
*/
2424
endSlot?: SlotContent
2525

26-
forwardClickToControl?: boolean
27-
onClick?: React.MouseEventHandler<HTMLDivElement>
28-
2926
/**
3027
* The control element (an `<input>`, an Ariakit Select trigger,
3128
* `<select>`, etc.). Attributes belonging to the control — `type`,
3229
* `value`, `onChange`, `readOnly`, `disabled`, `aria-invalid`, and so on —
3330
* are set on this element directly. The wrapper chrome (read-only
3431
* background, disabled state, error border) derives from those attributes.
32+
*
33+
* Click handlers belong on the control itself, not on this wrapper.
3534
*/
3635
children: React.ReactNode
3736
} & ObfuscatedClassName &
@@ -49,31 +48,31 @@ export type ControlPresentationProps = {
4948
*/
5049
export const ControlPresentation = forwardRef<HTMLDivElement, ControlPresentationProps>(
5150
function ControlPresentation(
52-
{
53-
startSlot,
54-
endSlot,
55-
forwardClickToControl = true,
56-
onClick,
57-
exceptionallySetClassName,
58-
children,
59-
...rest
60-
},
51+
{ startSlot, endSlot, exceptionallySetClassName, children, ...rest },
6152
ref,
6253
) {
6354
const controlWrapperRef = React.useRef<HTMLDivElement>(null)
6455
// The synthetic .click() we dispatch below bubbles back up to this
6556
// handler. Track that we're inside that re-entry so we can skip
66-
// re-invoking onClick + activation for it.
57+
// re-invoking the consumer onClick + activation for it.
6758
const isDispatchedReentryRef = React.useRef(false)
6859

60+
// `onClick` isn't part of `ControlPresentationProps` — consumers
61+
// should put click handlers on the inner control. This extraction
62+
// exists only to compose with onClick that arrives via render-prop
63+
// wrapping (e.g. Ariakit's Select trigger uses `render={<ControlPresentation/>}`
64+
// and forwards its own onClick), where the wrapper IS the trigger.
65+
const { onClick, ...restWithoutClick } = rest as typeof rest & {
66+
onClick?: React.MouseEventHandler<HTMLDivElement>
67+
}
68+
6969
function handleWrapperClick(event: React.MouseEvent<HTMLDivElement>) {
7070
if (isDispatchedReentryRef.current) {
7171
isDispatchedReentryRef.current = false
7272
return
7373
}
7474

7575
onClick?.(event)
76-
if (!forwardClickToControl) return
7776

7877
const control = controlWrapperRef.current?.firstElementChild as HTMLElement | null
7978
if (!control) return
@@ -107,7 +106,7 @@ export const ControlPresentation = forwardRef<HTMLDivElement, ControlPresentatio
107106
return (
108107
<Box
109108
ref={ref}
110-
{...rest}
109+
{...restWithoutClick}
111110
className={[styles.container, exceptionallySetClassName]}
112111
display="flex"
113112
alignItems="center"

0 commit comments

Comments
 (0)