Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Select): fix touch devices bugs #1472

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions packages/radix-vue/src/Select/SelectItem.vue
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const isSelected = computed(() => rootContext.modelValue?.value === props.value)
const isFocused = ref(false)
const textValue = ref(props.textValue ?? '')
const textId = useId(undefined, 'radix-vue-select-item-text')
const pointerTypeRef = ref<PointerEvent['pointerType']>('touch')

async function handleSelect(ev?: PointerEvent) {
await nextTick()
sadeghbarati marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -70,6 +71,9 @@ async function handlePointerMove(event: PointerEvent) {
await nextTick()
if (event.defaultPrevented)
return

pointerTypeRef.value = event.pointerType

if (disabled.value) {
contentContext.onItemLeave?.()
}
Expand All @@ -84,8 +88,9 @@ async function handlePointerLeave(event: PointerEvent) {
await nextTick()
if (event.defaultPrevented)
return
if (event.currentTarget === document.activeElement)
if (event.currentTarget === document.activeElement) {
contentContext.onItemLeave?.()
}
}

async function handleKeyDown(event: KeyboardEvent) {
Expand Down Expand Up @@ -145,11 +150,19 @@ provideSelectItemContext({
:as-child="asChild"
@focus="isFocused = true"
@blur="isFocused = false"
@pointerup="handleSelect"
@click="() => {
if (pointerTypeRef !== 'mouse') {
handleSelect()
}
}"
@pointerup="() => {
if (pointerTypeRef === 'mouse') {
handleSelect()
}
}"
@pointerdown="(event) => {
(event.currentTarget as HTMLElement).focus({ preventScroll: true })
pointerTypeRef = event.pointerType;
}"
@touchend.prevent.stop
@pointermove="handlePointerMove"
@pointerleave="handlePointerLeave"
@keydown="handleKeyDown"
Expand Down
29 changes: 11 additions & 18 deletions packages/radix-vue/src/Select/SelectTrigger.vue
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export interface SelectTriggerProps extends PrimitiveProps {
</script>

<script setup lang="ts">
import { computed, onMounted } from 'vue'
import { computed, onMounted, ref } from 'vue'
import {
injectSelectRootContext,
} from './SelectRoot.vue'
Expand All @@ -21,6 +21,7 @@ const props = withDefaults(defineProps<SelectTriggerProps>(), {
})
const rootContext = injectSelectRootContext()

const pointerTypeRef = ref<PointerEvent['pointerType']>('touch')
const isDisabled = computed(() => rootContext.disabled?.value || props.disabled)

const { forwardRef, currentElement: triggerElement } = useForwardExpose()
Expand Down Expand Up @@ -72,21 +73,22 @@ function handlePointerOpen(event: PointerEvent) {
:as-child="asChild"
:as="as"
@click="
(event: MouseEvent) => {
(event: PointerEvent) => {
// Whilst browsers generally have no issue focusing the trigger when clicking
// on a label, Safari seems to struggle with the fact that there's no `onClick`.
// We force `focus` in this case. Note: this doesn't create any other side-effect
// because we are preventing default in `onPointerDown` so effectively
// this only runs for a label 'click'
(event?.currentTarget as HTMLElement)?.focus();

if (pointerTypeRef !== 'mouse') {
handlePointerOpen(event);
}
}
"
@pointerdown="
(event: PointerEvent) => {
// Prevent opening on touch down.
// https://github.com/unovue/radix-vue/issues/804
if (event.pointerType === 'touch')
return event.preventDefault();
pointerTypeRef = event.pointerType;

// prevent implicit pointer capture
// https://www.w3.org/TR/pointerevents3/#implicit-pointer-capture
Expand All @@ -97,28 +99,19 @@ function handlePointerOpen(event: PointerEvent) {

// 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) {
if (event.button === 0 && event.ctrlKey === false && event.pointerType === 'mouse') {
handlePointerOpen(event)
// prevent trigger from stealing focus from the active item after opening.
event.preventDefault();
}
}
"
@pointerup.prevent="
(event: PointerEvent) => {
// Only open on pointer up when using touch devices
// https://github.com/unovue/radix-vue/issues/804
if (event.pointerType === 'touch')
handlePointerOpen(event)
}
"
@keydown="
(event) => {
const isTypingAhead = search !== '';
const isModifierKey = event.ctrlKey || event.altKey || event.metaKey;
if (!isModifierKey && event.key.length === 1)
if (isTypingAhead && event.key === ' ') return;
handleTypeaheadSearch(event.key);
if (!isModifierKey && event.key.length === 1) handleTypeaheadSearch(event.key);
if (isTypingAhead && event.key === ' ') return;
if (OPEN_KEYS.includes(event.key)) {
handleOpen();
event.preventDefault();
Expand Down
Loading