Skip to content

Commit

Permalink
fix(Select): Touch scroll triggers the select content (#807)
Browse files Browse the repository at this point in the history
* fix(select): touch scroll triggers the select content

* chore(select): remove test comments

* revert(select): revert and use simple pointer prevent on touch

* fix: item focus

---------

Co-authored-by: zernonia <zernonia@gmail.com>
  • Loading branch information
ChrisGV04 and zernonia committed Apr 5, 2024
1 parent 3d8dcb4 commit ad914c5
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 9 deletions.
6 changes: 3 additions & 3 deletions packages/radix-vue/src/Select/Select.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('given default Select', () => {
const selection = wrapper.findAll('[role=option]')[1];
(selection.element as HTMLElement).focus()
await selection.trigger('pointerup')
// not sure why need 2 pointUp to trigger the selection correctly
// Needs 2 pointerup because SelectContentImpl prevents accidental pointerup's
await fireEvent.pointerUp(selection.element)
})

Expand Down Expand Up @@ -119,7 +119,7 @@ describe('given select in a form', async () => {
const selection = wrapper.findAll('[role=option]')[1];
(selection.element as HTMLElement).focus()
await selection.trigger('pointerup')
// not sure why need 2 pointUp to trigger the selection correctly
// Needs 2 pointerup because SelectContentImpl prevents accidental pointerup's
await fireEvent.pointerUp(selection.element)
await wrapper.find('form').trigger('submit')
})
Expand All @@ -140,7 +140,7 @@ describe('given select in a form', async () => {
const selection = wrapper.findAll('[role=option]')[4];
(selection.element as HTMLElement).focus()
await selection.trigger('pointerup')
// not sure why need 2 pointUp to trigger the selection correctly
// Needs 2 pointerup because SelectContentImpl prevents accidental pointerup's
await fireEvent.pointerUp(selection.element)
await wrapper.find('form').trigger('submit')
})
Expand Down
5 changes: 5 additions & 0 deletions packages/radix-vue/src/Select/SelectContentImpl.vue
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ watchEffect((cleanupFn) => {
}
}
const handlePointerUp = (event: PointerEvent) => {
// Prevent options from being untappable on touch devices
// https://github.com/radix-vue/radix-vue/issues/804
if (event.pointerType === 'touch')
return
// If the pointer hasn't moved by a certain threshold then we prevent selecting item on `pointerup`.
if (pointerMoveDelta.x <= 10 && pointerMoveDelta.y <= 10) {
event.preventDefault()
Expand Down
3 changes: 3 additions & 0 deletions packages/radix-vue/src/Select/SelectItem.vue
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,9 @@ provideSelectItemContext({
@focus="isFocused = true"
@blur="isFocused = false"
@pointerup="handleSelect"
@pointerdown="(event) => {
(event.currentTarget as HTMLElement).focus({ preventScroll: true })
}"
@touchend.prevent.stop
@pointermove="handlePointerMove"
@pointerleave="handlePointerLeave"
Expand Down
28 changes: 22 additions & 6 deletions packages/radix-vue/src/Select/SelectTrigger.vue
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ function handleOpen() {
resetTypeahead()
}
}
function handlePointerOpen(event: PointerEvent) {
handleOpen()
rootContext.triggerPointerDownPosRef.value = {
x: Math.round(event.pageX),
y: Math.round(event.pageY),
}
}
</script>

<template>
Expand Down Expand Up @@ -75,6 +83,11 @@ function handleOpen() {
"
@pointerdown="
(event: PointerEvent) => {
// Prevent opening on touch down.
// https://github.com/radix-vue/radix-vue/issues/804
if (event.pointerType === 'touch')
return event.preventDefault();
// prevent implicit pointer capture
// https://www.w3.org/TR/pointerevents3/#implicit-pointer-capture
const target = event.target as HTMLElement;
Expand All @@ -85,17 +98,20 @@ function handleOpen() {
// only call handler if it's the left button (mousedown gets triggered by all mouse buttons)
// but not when the control key is pressed (avoiding MacOS right click)
if (event.button === 0 && event.ctrlKey === false) {
handleOpen();
rootContext.triggerPointerDownPosRef.value = {
x: Math.round(event.pageX),
y: Math.round(event.pageY),
};
handlePointerOpen(event)
// prevent trigger from stealing focus from the active item after opening.
event.preventDefault();
}
}
"
@pointerup.prevent
@pointerup.prevent="
(event: PointerEvent) => {
// Only open on pointer up when using touch devices
// https://github.com/radix-vue/radix-vue/issues/804
if (event.pointerType === 'touch')
handlePointerOpen(event)
}
"
@keydown="
(event) => {
const isTypingAhead = search !== '';
Expand Down

0 comments on commit ad914c5

Please sign in to comment.