Skip to content

Commit 0187de1

Browse files
authored
prevent default event action in keyup (#312)
When we are listening to a keydown event, and when a `space` event enters. If you then event.preventDefault(), then we still trigger the click event in firefox. To get around this, we have to make sure that we cancel the `space` event in the keyup event.
1 parent 8feec8c commit 0187de1

File tree

11 files changed

+139
-21
lines changed

11 files changed

+139
-21
lines changed

packages/@headlessui-react/src/components/disclosure/disclosure.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,28 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
171171
case Keys.Space:
172172
case Keys.Enter:
173173
event.preventDefault()
174+
event.stopPropagation()
174175
dispatch({ type: ActionTypes.ToggleDisclosure })
175176
break
176177
}
177178
},
178179
[dispatch]
179180
)
180181

182+
let handleKeyUp = useCallback(
183+
(event: ReactKeyboardEvent<HTMLButtonElement>) => {
184+
switch (event.key) {
185+
case Keys.Space:
186+
// Required for firefox, event.preventDefault() in handleKeyDown for
187+
// the Space key doesn't cancel the handleKeyUp, which in turn
188+
// triggers a *click*.
189+
event.preventDefault()
190+
break
191+
}
192+
},
193+
[dispatch]
194+
)
195+
181196
let handleClick = useCallback(
182197
(event: ReactMouseEvent) => {
183198
if (isDisabledReactIssue7711(event.currentTarget)) return
@@ -200,6 +215,7 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
200215
'aria-expanded': state.disclosureState === DisclosureStates.Open ? true : undefined,
201216
'aria-controls': state.linkedPanel ? state.panelId : undefined,
202217
onKeyDown: handleKeyDown,
218+
onKeyUp: handleKeyUp,
203219
onClick: handleClick,
204220
}
205221

packages/@headlessui-react/src/components/listbox/listbox.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,20 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
307307
[dispatch, state, d]
308308
)
309309

310+
let handleKeyUp = useCallback(
311+
(event: ReactKeyboardEvent<HTMLButtonElement>) => {
312+
switch (event.key) {
313+
case Keys.Space:
314+
// Required for firefox, event.preventDefault() in handleKeyDown for
315+
// the Space key doesn't cancel the handleKeyUp, which in turn
316+
// triggers a *click*.
317+
event.preventDefault()
318+
break
319+
}
320+
},
321+
[dispatch]
322+
)
323+
310324
let handleClick = useCallback(
311325
(event: ReactMouseEvent) => {
312326
if (isDisabledReactIssue7711(event.currentTarget)) return event.preventDefault()
@@ -341,6 +355,7 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
341355
'aria-labelledby': labelledby,
342356
disabled: state.disabled,
343357
onKeyDown: handleKeyDown,
358+
onKeyUp: handleKeyUp,
344359
onClick: handleClick,
345360
}
346361

packages/@headlessui-react/src/components/menu/menu.tsx

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,20 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
251251
[dispatch, d]
252252
)
253253

254+
let handleKeyUp = useCallback(
255+
(event: ReactKeyboardEvent<HTMLButtonElement>) => {
256+
switch (event.key) {
257+
case Keys.Space:
258+
// Required for firefox, event.preventDefault() in handleKeyDown for
259+
// the Space key doesn't cancel the handleKeyUp, which in turn
260+
// triggers a *click*.
261+
event.preventDefault()
262+
break
263+
}
264+
},
265+
[dispatch]
266+
)
267+
254268
let handleClick = useCallback(
255269
(event: ReactMouseEvent) => {
256270
if (isDisabledReactIssue7711(event.currentTarget)) return event.preventDefault()
@@ -279,6 +293,7 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
279293
'aria-controls': state.itemsRef.current?.id,
280294
'aria-expanded': state.menuState === MenuStates.Open ? true : undefined,
281295
onKeyDown: handleKeyDown,
296+
onKeyUp: handleKeyUp,
282297
onClick: handleClick,
283298
}
284299

@@ -415,6 +430,20 @@ let Items = forwardRefWithAs(function Items<TTag extends ElementType = typeof DE
415430
[dispatch, searchDisposables, state]
416431
)
417432

433+
let handleKeyUp = useCallback(
434+
(event: ReactKeyboardEvent<HTMLButtonElement>) => {
435+
switch (event.key) {
436+
case Keys.Space:
437+
// Required for firefox, event.preventDefault() in handleKeyDown for
438+
// the Space key doesn't cancel the handleKeyUp, which in turn
439+
// triggers a *click*.
440+
event.preventDefault()
441+
break
442+
}
443+
},
444+
[dispatch]
445+
)
446+
418447
let slot = useMemo<ItemsRenderPropArg>(() => ({ open: state.menuState === MenuStates.Open }), [
419448
state,
420449
])
@@ -424,6 +453,7 @@ let Items = forwardRefWithAs(function Items<TTag extends ElementType = typeof DE
424453
'aria-labelledby': state.buttonRef.current?.id,
425454
id,
426455
onKeyDown: handleKeyDown,
456+
onKeyUp: handleKeyUp,
427457
role: 'menu',
428458
tabIndex: 0,
429459
ref: itemsRef,

packages/@headlessui-react/src/components/popover/popover.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,12 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
357357
let handleKeyUp = useCallback(
358358
(event: ReactKeyboardEvent<HTMLButtonElement>) => {
359359
if (isWithinPanel) return
360+
if (event.key === Keys.Space) {
361+
// Required for firefox, event.preventDefault() in handleKeyDown for
362+
// the Space key doesn't cancel the handleKeyUp, which in turn
363+
// triggers a *click*.
364+
event.preventDefault()
365+
}
360366
if (state.popoverState !== PopoverStates.Open) return
361367
if (!state.panel) return
362368
if (!state.button) return

packages/@headlessui-react/src/components/radio-group/radio-group.tsx

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import React, {
22
createContext,
33
useCallback,
44
useContext,
5-
useEffect,
65
useMemo,
76
useReducer,
87
useRef,
@@ -11,6 +10,7 @@ import React, {
1110
Dispatch,
1211
ElementType,
1312
MutableRefObject,
13+
KeyboardEvent as ReactKeyboardEvent,
1414
} from 'react'
1515

1616
import { Props, Expand } from '../../types'
@@ -148,11 +148,11 @@ export function RadioGroup<
148148
}
149149
}, [radioGroupRef])
150150

151-
useEffect(() => {
152-
let container = radioGroupRef.current
153-
if (!container) return
151+
let handleKeyDown = useCallback(
152+
(event: ReactKeyboardEvent<HTMLButtonElement>) => {
153+
let container = radioGroupRef.current
154+
if (!container) return
154155

155-
function handler(event: KeyboardEvent) {
156156
switch (event.key) {
157157
case Keys.ArrowLeft:
158158
case Keys.ArrowUp:
@@ -206,18 +206,17 @@ export function RadioGroup<
206206
}
207207
break
208208
}
209-
}
210-
211-
container.addEventListener('keydown', handler)
212-
return () => container!.removeEventListener('keydown', handler)
213-
}, [radioGroupRef, options, triggerChange])
209+
},
210+
[radioGroupRef, options, triggerChange]
211+
)
214212

215213
let propsWeControl = {
216214
ref: radioGroupRef,
217215
id,
218216
role: 'radiogroup',
219217
'aria-labelledby': labelledby,
220218
'aria-describedby': describedby,
219+
onKeyDown: handleKeyDown,
221220
}
222221

223222
return (

packages/@headlessui-vue/src/components/disclosure/disclosure.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ export let DisclosureButton = defineComponent({
8787
'aria-controls': this.ariaControls,
8888
onClick: this.handleClick,
8989
onKeydown: this.handleKeyDown,
90+
onKeyup: this.handleKeyUp,
9091
}
9192

9293
return render({
@@ -116,10 +117,21 @@ export let DisclosureButton = defineComponent({
116117
case Keys.Space:
117118
case Keys.Enter:
118119
event.preventDefault()
120+
event.stopPropagation()
119121
api.toggleDisclosure()
120122
break
121123
}
122124
},
125+
handleKeyUp(event: KeyboardEvent) {
126+
switch (event.key) {
127+
case Keys.Space:
128+
// Required for firefox, event.preventDefault() in handleKeyDown for
129+
// the Space key doesn't cancel the handleKeyUp, which in turn
130+
// triggers a *click*.
131+
event.preventDefault()
132+
break
133+
}
134+
},
123135
}
124136
},
125137
})

packages/@headlessui-vue/src/components/listbox/listbox.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ export let ListboxButton = defineComponent({
258258
: undefined,
259259
disabled: api.disabled,
260260
onKeydown: this.handleKeyDown,
261+
onKeyup: this.handleKeyUp,
261262
onClick: this.handleClick,
262263
}
263264

@@ -299,6 +300,17 @@ export let ListboxButton = defineComponent({
299300
}
300301
}
301302

303+
function handleKeyUp(event: KeyboardEvent) {
304+
switch (event.key) {
305+
case Keys.Space:
306+
// Required for firefox, event.preventDefault() in handleKeyDown for
307+
// the Space key doesn't cancel the handleKeyUp, which in turn
308+
// triggers a *click*.
309+
event.preventDefault()
310+
break
311+
}
312+
}
313+
302314
function handleClick(event: MouseEvent) {
303315
if (api.disabled) return
304316
if (api.listboxState.value === ListboxStates.Open) {
@@ -311,7 +323,7 @@ export let ListboxButton = defineComponent({
311323
}
312324
}
313325

314-
return { id, el: api.buttonRef, handleKeyDown, handleClick }
326+
return { id, el: api.buttonRef, handleKeyDown, handleKeyUp, handleClick }
315327
},
316328
})
317329

packages/@headlessui-vue/src/components/menu/menu.test.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@ describe('Rendering', () => {
326326
' - aria-controls',
327327
' - aria-expanded',
328328
' - onKeydown',
329+
' - onKeyup',
329330
' - onClick',
330331
'',
331332
'You can apply a few solutions:',
@@ -427,6 +428,7 @@ describe('Rendering', () => {
427428
' - aria-labelledby',
428429
' - id',
429430
' - onKeydown',
431+
' - onKeyup',
430432
' - role',
431433
' - tabIndex',
432434
' - ref',

packages/@headlessui-vue/src/components/menu/menu.ts

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ export let MenuButton = defineComponent({
178178
'aria-controls': dom(api.itemsRef)?.id,
179179
'aria-expanded': api.menuState.value === MenuStates.Open ? true : undefined,
180180
onKeydown: this.handleKeyDown,
181+
onKeyup: this.handleKeyUp,
181182
onClick: this.handleClick,
182183
}
183184

@@ -221,6 +222,17 @@ export let MenuButton = defineComponent({
221222
}
222223
}
223224

225+
function handleKeyUp(event: KeyboardEvent) {
226+
switch (event.key) {
227+
case Keys.Space:
228+
// Required for firefox, event.preventDefault() in handleKeyDown for
229+
// the Space key doesn't cancel the handleKeyUp, which in turn
230+
// triggers a *click*.
231+
event.preventDefault()
232+
break
233+
}
234+
}
235+
224236
function handleClick(event: MouseEvent) {
225237
if (props.disabled) return
226238
if (api.menuState.value === MenuStates.Open) {
@@ -234,12 +246,7 @@ export let MenuButton = defineComponent({
234246
}
235247
}
236248

237-
return {
238-
id,
239-
el: api.buttonRef,
240-
handleKeyDown,
241-
handleClick,
242-
}
249+
return { id, el: api.buttonRef, handleKeyDown, handleKeyUp, handleClick }
243250
},
244251
})
245252

@@ -262,6 +269,7 @@ export let MenuItems = defineComponent({
262269
'aria-labelledby': dom(api.buttonRef)?.id,
263270
id: this.id,
264271
onKeydown: this.handleKeyDown,
272+
onKeyup: this.handleKeyUp,
265273
role: 'menu',
266274
tabIndex: 0,
267275
ref: 'el',
@@ -369,7 +377,18 @@ export let MenuItems = defineComponent({
369377
}
370378
}
371379

372-
return { id, el: api.itemsRef, handleKeyDown }
380+
function handleKeyUp(event: KeyboardEvent) {
381+
switch (event.key) {
382+
case Keys.Space:
383+
// Required for firefox, event.preventDefault() in handleKeyDown for
384+
// the Space key doesn't cancel the handleKeyUp, which in turn
385+
// triggers a *click*.
386+
event.preventDefault()
387+
break
388+
}
389+
}
390+
391+
return { id, el: api.itemsRef, handleKeyDown, handleKeyUp }
373392
},
374393
})
375394

packages/@headlessui-vue/src/components/popover/popover.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,12 @@ export let PopoverButton = defineComponent({
303303
},
304304
handleKeyUp(event: KeyboardEvent) {
305305
if (isWithinPanel) return
306+
if (event.key === Keys.Space) {
307+
// Required for firefox, event.preventDefault() in handleKeyDown for
308+
// the Space key doesn't cancel the handleKeyUp, which in turn
309+
// triggers a *click*.
310+
event.preventDefault()
311+
}
306312
if (api.popoverState.value !== PopoverStates.Open) return
307313
if (!api.panel) return
308314
if (!api.button) return

0 commit comments

Comments
 (0)