Skip to content

Commit 14f7734

Browse files
committed
fix: harden control presentation activation
1 parent cfad4ac commit 14f7734

5 files changed

Lines changed: 257 additions & 34 deletions

File tree

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

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,22 @@ describe('ControlPresentation', () => {
1212
it('renders the control passed as children', () => {
1313
render(
1414
<ControlPresentation>
15-
<input aria-label="Subject" data-testid="subject" />
15+
<input aria-label="Subject" />
1616
</ControlPresentation>,
1717
)
18-
const control = screen.getByTestId('subject')
18+
const control = screen.getByRole('textbox', { name: 'Subject' })
1919
expect(control.tagName).toBe('INPUT')
2020
})
2121

22-
it('is polymorphic any element can be the control', () => {
22+
it('is polymorphic - any element can be the control', () => {
2323
render(
2424
<ControlPresentation>
25-
<button type="button" aria-label="Subject" data-testid="subject">
25+
<button type="button" aria-label="Subject">
2626
Choose
2727
</button>
2828
</ControlPresentation>,
2929
)
30-
const control = screen.getByTestId('subject')
30+
const control = screen.getByRole('button', { name: 'Subject' })
3131
expect(control.tagName).toBe('BUTTON')
3232
expect(control).toHaveTextContent('Choose')
3333
})
@@ -46,7 +46,7 @@ describe('ControlPresentation', () => {
4646
/>
4747
</ControlPresentation>,
4848
)
49-
const control = screen.getByTestId('subject')
49+
const control = screen.getByRole('textbox', { name: 'Subject' })
5050
expect(control).toHaveAttribute('type', 'email')
5151
expect(control).toHaveAttribute('placeholder', 'you@example.com')
5252
expect(control).toHaveAttribute('data-custom', 'yes')
@@ -57,16 +57,39 @@ describe('ControlPresentation', () => {
5757
it('focuses the control when a non-interactive startSlot is clicked', async () => {
5858
render(
5959
<ControlPresentation startSlot={<TestIcon />}>
60-
<input aria-label="Subject" data-testid="subject" />
60+
<input aria-label="Subject" />
6161
</ControlPresentation>,
6262
)
63-
const control = screen.getByTestId('subject')
63+
const control = screen.getByRole('textbox', { name: 'Subject' })
6464
expect(control).not.toHaveFocus()
6565

6666
await userEvent.click(screen.getByTestId('test-icon'))
6767
expect(control).toHaveFocus()
6868
})
6969

70+
it('does not forward interactive slot clicks to the control', async () => {
71+
const onControlClick = jest.fn()
72+
const onSlotClick = jest.fn()
73+
render(
74+
<ControlPresentation
75+
endSlot={
76+
<button type="button" onClick={onSlotClick}>
77+
Clear
78+
</button>
79+
}
80+
>
81+
<input aria-label="Subject" onClick={onControlClick} />
82+
</ControlPresentation>,
83+
)
84+
85+
const slotButton = screen.getByRole('button', { name: 'Clear' })
86+
await userEvent.click(slotButton)
87+
88+
expect(onSlotClick).toHaveBeenCalledTimes(1)
89+
expect(onControlClick).not.toHaveBeenCalled()
90+
expect(slotButton).toHaveFocus()
91+
})
92+
7093
it('merges exceptionallySetClassName onto the wrapper', () => {
7194
const { container } = render(
7295
<ControlPresentation exceptionallySetClassName="custom-class">
@@ -93,6 +116,15 @@ describe('ControlPresentation', () => {
93116
expect(screen.getByTestId('b')).toBeInTheDocument()
94117
})
95118

119+
it('renders numeric zero slot content', () => {
120+
render(
121+
<ControlPresentation startSlot={0} endSlot={0}>
122+
<input aria-label="Subject" />
123+
</ControlPresentation>,
124+
)
125+
expect(screen.getAllByText('0')).toHaveLength(2)
126+
})
127+
96128
describe('a11y', () => {
97129
it('renders with no a11y violations', async () => {
98130
const { container } = render(

src/control-presentation/control-presentation.tsx

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ export type ControlPresentationProps = {
2424
* or dropdown-trigger chevron).
2525
*/
2626
endSlot?: SlotContent
27-
} & Omit<ComponentProps<typeof OutlinedControlContainer>, 'borderRadius'>
27+
28+
/**
29+
* The single control element to wrap.
30+
*/
31+
children: React.ReactElement
32+
} & Omit<ComponentProps<typeof OutlinedControlContainer>, 'borderRadius' | 'children'>
2833

2934
/**
3035
* The visual chrome of an inline, single-row, text-field-style input: a
@@ -47,11 +52,15 @@ export const ControlPresentation = forwardRef<HTMLDivElement, ControlPresentatio
4752
onClick={onClick}
4853
exceptionallySetClassName={classNames(styles.container, exceptionallySetClassName)}
4954
>
50-
{startSlot ? (
55+
{startSlot != null ? (
5156
<Slot className={[styles.slot, styles.startSlot]}>{startSlot}</Slot>
5257
) : null}
53-
<div className={styles.control}>{children}</div>
54-
{endSlot ? <Slot className={[styles.slot, styles.endSlot]}>{endSlot}</Slot> : null}
58+
<div className={styles.control} data-reactist-control>
59+
{children}
60+
</div>
61+
{endSlot != null ? (
62+
<Slot className={[styles.slot, styles.endSlot]}>{endSlot}</Slot>
63+
) : null}
5564
</OutlinedControlContainer>
5665
)
5766
},

src/control-presentation/outlined-control-container.module.css

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,17 @@
1818
* The :not(='false') exclusion treats aria-readonly="false" /
1919
* data-readonly="false" as "not read-only" per the ARIA spec. */
2020
.container:has(
21-
input:read-only,
22-
textarea:read-only,
23-
[aria-readonly]:not([aria-readonly='false']),
24-
[data-readonly]:not([data-readonly='false'])
25-
) {
21+
[data-reactist-control] input:read-only,
22+
[data-reactist-control] textarea:read-only,
23+
[data-reactist-control] [aria-readonly]:not([aria-readonly='false']),
24+
[data-reactist-control] [data-readonly]:not([data-readonly='false'])
25+
),
26+
.container:not(:has([data-reactist-control])):has(
27+
input:read-only,
28+
textarea:read-only,
29+
[aria-readonly]:not([aria-readonly='false']),
30+
[data-readonly]:not([data-readonly='false'])
31+
) {
2632
background-color: var(--reactist-field-readonly-background);
2733
color: var(--reactist-field-readonly-content);
2834
}
@@ -32,17 +38,29 @@
3238
* keep elements focusable while announcing as disabled), and the
3339
* data-attr convention used by Ariakit/Radix. */
3440
.container:has(
35-
:disabled,
36-
[aria-disabled]:not([aria-disabled='false']),
37-
[data-disabled]:not([data-disabled='false'])
38-
) {
41+
[data-reactist-control] :disabled,
42+
[data-reactist-control] [aria-disabled]:not([aria-disabled='false']),
43+
[data-reactist-control] [data-disabled]:not([data-disabled='false'])
44+
),
45+
.container:not(:has([data-reactist-control])):has(
46+
:disabled,
47+
[aria-disabled]:not([aria-disabled='false']),
48+
[data-disabled]:not([data-disabled='false'])
49+
) {
3950
background-color: var(--reactist-field-disabled-background);
4051
color: var(--reactist-field-disabled-content);
4152
}
4253

4354
/* Interactive border (hover/focus) — suppressed when the control is
4455
* in any disabled state. */
45-
.container:not(
56+
.container:has([data-reactist-control]):not(
57+
:has(
58+
[data-reactist-control] :disabled,
59+
[data-reactist-control] [aria-disabled]:not([aria-disabled='false']),
60+
[data-reactist-control] [data-disabled]:not([data-disabled='false'])
61+
)
62+
):hover,
63+
.container:not(:has([data-reactist-control])):not(
4664
:has(
4765
:disabled,
4866
[aria-disabled]:not([aria-disabled='false']),
@@ -52,7 +70,14 @@
5270
border-color: var(--reactist-inputs-hover);
5371
}
5472

55-
.container:not(
73+
.container:has([data-reactist-control]):not(
74+
:has(
75+
[data-reactist-control] :disabled,
76+
[data-reactist-control] [aria-disabled]:not([aria-disabled='false']),
77+
[data-reactist-control] [data-disabled]:not([data-disabled='false'])
78+
)
79+
):focus-within,
80+
.container:not(:has([data-reactist-control])):not(
5681
:has(
5782
:disabled,
5883
[aria-disabled]:not([aria-disabled='false']),
@@ -63,7 +88,8 @@
6388
}
6489

6590
/* Invalid: matches the ARIA convention only. */
66-
.container:has([aria-invalid]:not([aria-invalid='false'])) {
91+
.container:has([data-reactist-control] [aria-invalid]:not([aria-invalid='false'])),
92+
.container:not(:has([data-reactist-control])):has([aria-invalid]:not([aria-invalid='false'])) {
6793
border-color: var(--reactist-inputs-alert) !important;
6894
}
6995

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

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,16 +78,46 @@ describe('OutlinedControlContainer', () => {
7878
it('focuses the inner control when the wrapper is clicked', async () => {
7979
const { container } = render(
8080
<OutlinedControlContainer>
81-
<input data-testid="control" aria-label="control" />
81+
<input aria-label="control" />
8282
</OutlinedControlContainer>,
8383
)
84-
const control = screen.getByTestId('control')
84+
const control = screen.getByRole('textbox', { name: 'control' })
8585
expect(control).not.toHaveFocus()
8686

8787
await userEvent.click(container.firstElementChild as Element)
8888
expect(control).toHaveFocus()
8989
})
9090

91+
it('does not dispatch a synthetic click for text inputs', async () => {
92+
const onControlClick = jest.fn()
93+
const { container } = render(
94+
<OutlinedControlContainer>
95+
<input aria-label="control" onClick={onControlClick} />
96+
</OutlinedControlContainer>,
97+
)
98+
const control = screen.getByRole('textbox', { name: 'control' })
99+
100+
await userEvent.click(container.firstElementChild as Element)
101+
102+
expect(control).toHaveFocus()
103+
expect(onControlClick).not.toHaveBeenCalled()
104+
})
105+
106+
it('does not dispatch a synthetic click for textareas', async () => {
107+
const onControlClick = jest.fn()
108+
const { container } = render(
109+
<OutlinedControlContainer>
110+
<textarea aria-label="control" onClick={onControlClick} />
111+
</OutlinedControlContainer>,
112+
)
113+
const control = screen.getByRole('textbox', { name: 'control' })
114+
115+
await userEvent.click(container.firstElementChild as Element)
116+
117+
expect(control).toHaveFocus()
118+
expect(onControlClick).not.toHaveBeenCalled()
119+
})
120+
91121
it('does not double-fire when the inner control is clicked directly', async () => {
92122
const onControlClick = jest.fn()
93123
render(
@@ -101,6 +131,23 @@ describe('OutlinedControlContainer', () => {
101131
expect(onControlClick).toHaveBeenCalledTimes(1)
102132
})
103133

134+
it('activates button-like controls when the wrapper is clicked', async () => {
135+
const onControlClick = jest.fn()
136+
const { container } = render(
137+
<OutlinedControlContainer>
138+
<button type="button" onClick={onControlClick}>
139+
Choose
140+
</button>
141+
</OutlinedControlContainer>,
142+
)
143+
const control = screen.getByRole('button', { name: 'Choose' })
144+
145+
await userEvent.click(container.firstElementChild as Element)
146+
147+
expect(control).toHaveFocus()
148+
expect(onControlClick).toHaveBeenCalledTimes(1)
149+
})
150+
104151
it('calls a consumer onClick passed via Box props when the wrapper is clicked', async () => {
105152
const onClick = jest.fn()
106153
const { container } = render(
@@ -133,6 +180,36 @@ describe('OutlinedControlContainer', () => {
133180
expect(control).toHaveFocus()
134181
})
135182

183+
it('does not activate aria-disabled controls from wrapper clicks', async () => {
184+
const onControlClick = jest.fn()
185+
const { container } = render(
186+
<OutlinedControlContainer>
187+
<button type="button" aria-disabled onClick={onControlClick}>
188+
Choose
189+
</button>
190+
</OutlinedControlContainer>,
191+
)
192+
193+
await userEvent.click(container.firstElementChild as Element)
194+
195+
expect(onControlClick).not.toHaveBeenCalled()
196+
})
197+
198+
it('does not activate data-disabled controls from wrapper clicks', async () => {
199+
const onControlClick = jest.fn()
200+
const { container } = render(
201+
<OutlinedControlContainer>
202+
<button type="button" data-disabled onClick={onControlClick}>
203+
Choose
204+
</button>
205+
</OutlinedControlContainer>,
206+
)
207+
208+
await userEvent.click(container.firstElementChild as Element)
209+
210+
expect(onControlClick).not.toHaveBeenCalled()
211+
})
212+
136213
it('uses showPicker for native <select>', async () => {
137214
const showPicker = jest.fn()
138215
const { container } = render(

0 commit comments

Comments
 (0)