mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 14:30:41 +00:00
fix: stop Escape key propagation in Select components (#10397)
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
import { mount } from '@vue/test-utils'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { nextTick } from 'vue'
|
||||
import { nextTick, ref } from 'vue'
|
||||
import { createI18n } from 'vue-i18n'
|
||||
|
||||
import MultiSelect from './MultiSelect.vue'
|
||||
@@ -21,26 +21,59 @@ const i18n = createI18n({
|
||||
}
|
||||
})
|
||||
|
||||
describe('MultiSelect', () => {
|
||||
function createWrapper() {
|
||||
return mount(MultiSelect, {
|
||||
attachTo: document.body,
|
||||
global: {
|
||||
plugins: [i18n]
|
||||
},
|
||||
props: {
|
||||
modelValue: [],
|
||||
label: 'Category',
|
||||
options: [
|
||||
{ name: 'One', value: 'one' },
|
||||
{ name: 'Two', value: 'two' }
|
||||
]
|
||||
const options = [
|
||||
{ name: 'Option A', value: 'a' },
|
||||
{ name: 'Option B', value: 'b' },
|
||||
{ name: 'Option C', value: 'c' }
|
||||
]
|
||||
|
||||
function mountInParent(
|
||||
multiSelectProps: Record<string, unknown> = {},
|
||||
modelValue: { name: string; value: string }[] = []
|
||||
) {
|
||||
const parentEscapeCount = { value: 0 }
|
||||
|
||||
const Parent = {
|
||||
template:
|
||||
'<div @keydown.escape="onEsc"><MultiSelect v-model="sel" :options="options" v-bind="extraProps" /></div>',
|
||||
components: { MultiSelect },
|
||||
setup() {
|
||||
return {
|
||||
sel: ref(modelValue),
|
||||
options,
|
||||
extraProps: multiSelectProps,
|
||||
onEsc: () => {
|
||||
parentEscapeCount.value++
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
const wrapper = mount(Parent, {
|
||||
attachTo: document.body,
|
||||
global: { plugins: [i18n] }
|
||||
})
|
||||
|
||||
return { wrapper, parentEscapeCount }
|
||||
}
|
||||
|
||||
function dispatchEscape(element: Element) {
|
||||
element.dispatchEvent(
|
||||
new KeyboardEvent('keydown', {
|
||||
key: 'Escape',
|
||||
code: 'Escape',
|
||||
bubbles: true
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
function findContentElement(): HTMLElement | null {
|
||||
return document.querySelector('[data-dismissable-layer]')
|
||||
}
|
||||
|
||||
describe('MultiSelect', () => {
|
||||
it('keeps open-state border styling available while the dropdown is open', async () => {
|
||||
const wrapper = createWrapper()
|
||||
const { wrapper } = mountInParent()
|
||||
|
||||
const trigger = wrapper.get('button[aria-haspopup="listbox"]')
|
||||
|
||||
@@ -57,4 +90,65 @@ describe('MultiSelect', () => {
|
||||
|
||||
wrapper.unmount()
|
||||
})
|
||||
|
||||
describe('Escape key propagation', () => {
|
||||
it('stops Escape from propagating to parent when popover is open', async () => {
|
||||
const { wrapper, parentEscapeCount } = mountInParent()
|
||||
|
||||
const trigger = wrapper.find('button[aria-haspopup="listbox"]')
|
||||
await trigger.trigger('click')
|
||||
await nextTick()
|
||||
|
||||
const content = findContentElement()
|
||||
expect(content).not.toBeNull()
|
||||
|
||||
dispatchEscape(content!)
|
||||
await nextTick()
|
||||
|
||||
expect(parentEscapeCount.value).toBe(0)
|
||||
|
||||
wrapper.unmount()
|
||||
})
|
||||
|
||||
it('closes the popover when Escape is pressed', async () => {
|
||||
const { wrapper } = mountInParent()
|
||||
|
||||
const trigger = wrapper.find('button[aria-haspopup="listbox"]')
|
||||
await trigger.trigger('click')
|
||||
await nextTick()
|
||||
expect(trigger.attributes('data-state')).toBe('open')
|
||||
|
||||
const content = findContentElement()
|
||||
dispatchEscape(content!)
|
||||
await nextTick()
|
||||
|
||||
expect(trigger.attributes('data-state')).toBe('closed')
|
||||
|
||||
wrapper.unmount()
|
||||
})
|
||||
})
|
||||
|
||||
describe('selected count badge', () => {
|
||||
it('shows selected count when items are selected', () => {
|
||||
const { wrapper } = mountInParent({}, [
|
||||
{ name: 'Option A', value: 'a' },
|
||||
{ name: 'Option B', value: 'b' }
|
||||
])
|
||||
|
||||
expect(wrapper.text()).toContain('2')
|
||||
|
||||
wrapper.unmount()
|
||||
})
|
||||
|
||||
it('does not show count badge when no items are selected', () => {
|
||||
const { wrapper } = mountInParent()
|
||||
const multiSelect = wrapper.findComponent(MultiSelect)
|
||||
const spans = multiSelect.findAll('span')
|
||||
const countBadge = spans.find((s) => /^\d+$/.test(s.text().trim()))
|
||||
|
||||
expect(countBadge).toBeUndefined()
|
||||
|
||||
wrapper.unmount()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
<template>
|
||||
<ComboboxRoot
|
||||
v-model="selectedItems"
|
||||
v-model:open="isOpen"
|
||||
multiple
|
||||
by="value"
|
||||
:disabled
|
||||
@@ -13,17 +14,10 @@
|
||||
:aria-label="label || t('g.multiSelectDropdown')"
|
||||
:class="
|
||||
cn(
|
||||
'relative inline-flex cursor-pointer items-center select-none',
|
||||
size === 'md' ? 'h-8' : 'h-10',
|
||||
'rounded-lg bg-secondary-background text-base-foreground',
|
||||
'transition-all duration-200 ease-in-out',
|
||||
'hover:bg-secondary-background-hover',
|
||||
'border-[2.5px] border-solid border-transparent',
|
||||
selectedCount > 0
|
||||
? 'border-base-foreground'
|
||||
: 'focus-visible:border-node-component-border data-[state=open]:border-node-component-border',
|
||||
disabled &&
|
||||
'cursor-default opacity-30 hover:bg-secondary-background'
|
||||
selectTriggerVariants({
|
||||
size,
|
||||
border: selectedCount > 0 ? 'active' : 'none'
|
||||
})
|
||||
)
|
||||
"
|
||||
>
|
||||
@@ -45,9 +39,7 @@
|
||||
{{ selectedCount }}
|
||||
</span>
|
||||
</div>
|
||||
<div
|
||||
class="flex shrink-0 cursor-pointer items-center justify-center px-3"
|
||||
>
|
||||
<div :class="selectDropdownClass">
|
||||
<i class="icon-[lucide--chevron-down] text-muted-foreground" />
|
||||
</div>
|
||||
</ComboboxTrigger>
|
||||
@@ -59,19 +51,8 @@
|
||||
:side-offset="8"
|
||||
align="start"
|
||||
:style="popoverStyle"
|
||||
:class="
|
||||
cn(
|
||||
'z-3000 overflow-hidden',
|
||||
'rounded-lg p-2',
|
||||
'bg-base-background text-base-foreground',
|
||||
'border border-solid border-border-default',
|
||||
'shadow-md',
|
||||
'data-[state=closed]:animate-out data-[state=open]:animate-in',
|
||||
'data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0',
|
||||
'data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95',
|
||||
'data-[side=bottom]:slide-in-from-top-2'
|
||||
)
|
||||
"
|
||||
:class="selectContentClass"
|
||||
@keydown="onContentKeydown"
|
||||
@focus-outside="preventFocusDismiss"
|
||||
>
|
||||
<div
|
||||
@@ -132,13 +113,7 @@
|
||||
v-for="opt in filteredOptions"
|
||||
:key="opt.value"
|
||||
:value="opt"
|
||||
:class="
|
||||
cn(
|
||||
'group flex h-10 shrink-0 cursor-pointer items-center gap-2 rounded-lg px-2 outline-none',
|
||||
'hover:bg-secondary-background-hover',
|
||||
'data-highlighted:bg-secondary-background-selected data-highlighted:hover:bg-secondary-background-selected'
|
||||
)
|
||||
"
|
||||
:class="cn('group', selectItemVariants({ layout: 'multi' }))"
|
||||
>
|
||||
<div
|
||||
class="flex size-4 shrink-0 items-center justify-center rounded-sm transition-all duration-200 group-data-[state=checked]:bg-primary-background group-data-[state=unchecked]:bg-secondary-background [&>span]:flex"
|
||||
@@ -151,7 +126,7 @@
|
||||
</div>
|
||||
<span>{{ opt.name }}</span>
|
||||
</ComboboxItem>
|
||||
<ComboboxEmpty class="px-3 pb-4 text-sm text-muted-foreground">
|
||||
<ComboboxEmpty :class="selectEmptyMessageClass">
|
||||
{{ $t('g.noResultsFound') }}
|
||||
</ComboboxEmpty>
|
||||
</ComboboxViewport>
|
||||
@@ -176,13 +151,21 @@ import {
|
||||
ComboboxTrigger,
|
||||
ComboboxViewport
|
||||
} from 'reka-ui'
|
||||
import { computed } from 'vue'
|
||||
import { computed, ref } from 'vue'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
|
||||
import Button from '@/components/ui/button/Button.vue'
|
||||
import { usePopoverSizing } from '@/composables/usePopoverSizing'
|
||||
import { cn } from '@/utils/tailwindUtil'
|
||||
|
||||
import {
|
||||
selectContentClass,
|
||||
selectDropdownClass,
|
||||
selectEmptyMessageClass,
|
||||
selectItemVariants,
|
||||
selectTriggerVariants,
|
||||
stopEscapeToDocument
|
||||
} from './select.variants'
|
||||
import type { SelectOption } from './types'
|
||||
|
||||
defineOptions({
|
||||
@@ -232,8 +215,16 @@ const selectedItems = defineModel<SelectOption[]>({
|
||||
const searchQuery = defineModel<string>('searchQuery', { default: '' })
|
||||
|
||||
const { t } = useI18n()
|
||||
const isOpen = ref(false)
|
||||
const selectedCount = computed(() => selectedItems.value.length)
|
||||
|
||||
function onContentKeydown(event: KeyboardEvent) {
|
||||
if (event.key === 'Escape') {
|
||||
stopEscapeToDocument(event)
|
||||
isOpen.value = false
|
||||
}
|
||||
}
|
||||
|
||||
function preventFocusDismiss(event: FocusOutsideEvent) {
|
||||
event.preventDefault()
|
||||
}
|
||||
|
||||
116
src/components/input/SingleSelect.test.ts
Normal file
116
src/components/input/SingleSelect.test.ts
Normal file
@@ -0,0 +1,116 @@
|
||||
import { mount } from '@vue/test-utils'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { nextTick, ref } from 'vue'
|
||||
import { createI18n } from 'vue-i18n'
|
||||
|
||||
import SingleSelect from './SingleSelect.vue'
|
||||
|
||||
const i18n = createI18n({
|
||||
legacy: false,
|
||||
locale: 'en',
|
||||
messages: {
|
||||
en: {
|
||||
g: {
|
||||
singleSelectDropdown: 'Single-select dropdown'
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
const options = [
|
||||
{ name: 'Option A', value: 'a' },
|
||||
{ name: 'Option B', value: 'b' },
|
||||
{ name: 'Option C', value: 'c' }
|
||||
]
|
||||
|
||||
function dispatchEscape(element: Element) {
|
||||
element.dispatchEvent(
|
||||
new KeyboardEvent('keydown', {
|
||||
key: 'Escape',
|
||||
code: 'Escape',
|
||||
bubbles: true
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
function findContentElement(): HTMLElement | null {
|
||||
return document.querySelector('[data-dismissable-layer]')
|
||||
}
|
||||
|
||||
function mountInParent(modelValue?: string) {
|
||||
const parentEscapeCount = { value: 0 }
|
||||
|
||||
const Parent = {
|
||||
template:
|
||||
'<div @keydown.escape="onEsc"><SingleSelect v-model="sel" :options="options" label="Pick" /></div>',
|
||||
components: { SingleSelect },
|
||||
setup() {
|
||||
return {
|
||||
sel: ref(modelValue),
|
||||
options,
|
||||
onEsc: () => {
|
||||
parentEscapeCount.value++
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const wrapper = mount(Parent, {
|
||||
attachTo: document.body,
|
||||
global: { plugins: [i18n] }
|
||||
})
|
||||
|
||||
return { wrapper, parentEscapeCount }
|
||||
}
|
||||
|
||||
async function openSelect(triggerEl: HTMLElement) {
|
||||
if (!triggerEl.hasPointerCapture) {
|
||||
triggerEl.hasPointerCapture = () => false
|
||||
triggerEl.releasePointerCapture = () => {}
|
||||
}
|
||||
triggerEl.dispatchEvent(
|
||||
new PointerEvent('pointerdown', {
|
||||
button: 0,
|
||||
pointerType: 'mouse',
|
||||
bubbles: true
|
||||
})
|
||||
)
|
||||
await nextTick()
|
||||
}
|
||||
|
||||
describe('SingleSelect', () => {
|
||||
describe('Escape key propagation', () => {
|
||||
it('stops Escape from propagating to parent when popover is open', async () => {
|
||||
const { wrapper, parentEscapeCount } = mountInParent()
|
||||
|
||||
const trigger = wrapper.find('button[role="combobox"]')
|
||||
await openSelect(trigger.element as HTMLElement)
|
||||
|
||||
const content = findContentElement()
|
||||
expect(content).not.toBeNull()
|
||||
|
||||
dispatchEscape(content!)
|
||||
await nextTick()
|
||||
|
||||
expect(parentEscapeCount.value).toBe(0)
|
||||
|
||||
wrapper.unmount()
|
||||
})
|
||||
|
||||
it('closes the popover when Escape is pressed', async () => {
|
||||
const { wrapper } = mountInParent()
|
||||
|
||||
const trigger = wrapper.find('button[role="combobox"]')
|
||||
await openSelect(trigger.element as HTMLElement)
|
||||
expect(trigger.attributes('data-state')).toBe('open')
|
||||
|
||||
const content = findContentElement()
|
||||
dispatchEscape(content!)
|
||||
await nextTick()
|
||||
|
||||
expect(trigger.attributes('data-state')).toBe('closed')
|
||||
|
||||
wrapper.unmount()
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -1,23 +1,15 @@
|
||||
<template>
|
||||
<SelectRoot v-model="selectedItem" :disabled>
|
||||
<SelectRoot v-model="selectedItem" v-model:open="isOpen" :disabled>
|
||||
<SelectTrigger
|
||||
v-bind="$attrs"
|
||||
:aria-label="label || t('g.singleSelectDropdown')"
|
||||
:aria-busy="loading || undefined"
|
||||
:aria-invalid="invalid || undefined"
|
||||
:class="
|
||||
cn(
|
||||
'relative inline-flex cursor-pointer items-center select-none',
|
||||
size === 'md' ? 'h-8' : 'h-10',
|
||||
'rounded-lg',
|
||||
'bg-secondary-background text-base-foreground',
|
||||
'transition-all duration-200 ease-in-out',
|
||||
'hover:bg-secondary-background-hover',
|
||||
'border-[2.5px] border-solid',
|
||||
invalid ? 'border-destructive-background' : 'border-transparent',
|
||||
'focus:border-node-component-border focus:outline-none',
|
||||
'disabled:cursor-default disabled:opacity-30 disabled:hover:bg-secondary-background'
|
||||
)
|
||||
selectTriggerVariants({
|
||||
size,
|
||||
border: invalid ? 'invalid' : 'none'
|
||||
})
|
||||
"
|
||||
>
|
||||
<div
|
||||
@@ -35,9 +27,7 @@
|
||||
<slot v-else name="icon" />
|
||||
<SelectValue :placeholder="label" class="truncate" />
|
||||
</div>
|
||||
<div
|
||||
class="flex shrink-0 cursor-pointer items-center justify-center px-3"
|
||||
>
|
||||
<div :class="selectDropdownClass">
|
||||
<i class="icon-[lucide--chevron-down] text-muted-foreground" />
|
||||
</div>
|
||||
</SelectTrigger>
|
||||
@@ -48,20 +38,8 @@
|
||||
:side-offset="8"
|
||||
align="start"
|
||||
:style="optionStyle"
|
||||
:class="
|
||||
cn(
|
||||
'z-3000 overflow-hidden',
|
||||
'rounded-lg p-2',
|
||||
'bg-base-background text-base-foreground',
|
||||
'border border-solid border-border-default',
|
||||
'shadow-md',
|
||||
'min-w-(--reka-select-trigger-width)',
|
||||
'data-[state=closed]:animate-out data-[state=open]:animate-in',
|
||||
'data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0',
|
||||
'data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95',
|
||||
'data-[side=bottom]:slide-in-from-top-2'
|
||||
)
|
||||
"
|
||||
:class="cn(selectContentClass, 'min-w-(--reka-select-trigger-width)')"
|
||||
@keydown="onContentKeydown"
|
||||
>
|
||||
<SelectViewport
|
||||
:style="{ maxHeight: `min(${listMaxHeight}, 50vh)` }"
|
||||
@@ -71,16 +49,7 @@
|
||||
v-for="opt in options"
|
||||
:key="opt.value"
|
||||
:value="opt.value"
|
||||
:class="
|
||||
cn(
|
||||
'relative flex w-full cursor-pointer items-center justify-between select-none',
|
||||
'gap-3 rounded-sm px-2 py-3 text-sm outline-none',
|
||||
'hover:bg-secondary-background-hover',
|
||||
'focus:bg-secondary-background-hover',
|
||||
'data-[state=checked]:bg-secondary-background-selected',
|
||||
'data-[state=checked]:hover:bg-secondary-background-selected'
|
||||
)
|
||||
"
|
||||
:class="selectItemVariants({ layout: 'single' })"
|
||||
>
|
||||
<SelectItemText class="truncate">
|
||||
{{ opt.name }}
|
||||
@@ -112,11 +81,19 @@ import {
|
||||
SelectValue,
|
||||
SelectViewport
|
||||
} from 'reka-ui'
|
||||
import { ref } from 'vue'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
|
||||
import { usePopoverSizing } from '@/composables/usePopoverSizing'
|
||||
import { cn } from '@/utils/tailwindUtil'
|
||||
|
||||
import {
|
||||
selectContentClass,
|
||||
selectDropdownClass,
|
||||
selectItemVariants,
|
||||
selectTriggerVariants,
|
||||
stopEscapeToDocument
|
||||
} from './select.variants'
|
||||
import type { SelectOption } from './types'
|
||||
|
||||
defineOptions({
|
||||
@@ -155,6 +132,14 @@ const {
|
||||
const selectedItem = defineModel<string | undefined>({ required: true })
|
||||
|
||||
const { t } = useI18n()
|
||||
const isOpen = ref(false)
|
||||
|
||||
function onContentKeydown(event: KeyboardEvent) {
|
||||
if (event.key === 'Escape') {
|
||||
stopEscapeToDocument(event)
|
||||
isOpen.value = false
|
||||
}
|
||||
}
|
||||
|
||||
const optionStyle = usePopoverSizing({
|
||||
minWidth: popoverMinWidth,
|
||||
|
||||
50
src/components/input/select.variants.ts
Normal file
50
src/components/input/select.variants.ts
Normal file
@@ -0,0 +1,50 @@
|
||||
import { cva } from 'cva'
|
||||
|
||||
export const selectTriggerVariants = cva({
|
||||
base: 'relative inline-flex cursor-pointer items-center select-none rounded-lg bg-secondary-background text-base-foreground outline-none transition-all duration-200 ease-in-out hover:bg-secondary-background-hover border-[2.5px] border-solid disabled:cursor-default disabled:opacity-30 disabled:hover:bg-secondary-background',
|
||||
variants: {
|
||||
size: {
|
||||
md: 'h-8',
|
||||
lg: 'h-10'
|
||||
},
|
||||
border: {
|
||||
none: 'border-transparent focus-visible:border-node-component-border data-[state=open]:border-node-component-border',
|
||||
active: 'border-base-foreground',
|
||||
invalid: 'border-destructive-background'
|
||||
}
|
||||
},
|
||||
defaultVariants: {
|
||||
size: 'lg',
|
||||
border: 'none'
|
||||
}
|
||||
})
|
||||
|
||||
export const selectItemVariants = cva({
|
||||
base: 'flex cursor-pointer items-center px-2 outline-none hover:bg-secondary-background-hover',
|
||||
variants: {
|
||||
layout: {
|
||||
multi:
|
||||
'h-10 shrink-0 gap-2 rounded-lg data-highlighted:bg-secondary-background-selected data-highlighted:hover:bg-secondary-background-selected',
|
||||
single:
|
||||
'relative w-full justify-between gap-3 rounded-sm py-3 text-sm select-none focus:bg-secondary-background-hover data-[state=checked]:bg-secondary-background-selected data-[state=checked]:hover:bg-secondary-background-selected'
|
||||
}
|
||||
},
|
||||
defaultVariants: {
|
||||
layout: 'multi'
|
||||
}
|
||||
})
|
||||
|
||||
export const selectContentClass =
|
||||
'z-3000 overflow-hidden rounded-lg p-2 bg-base-background text-base-foreground border border-solid border-border-default shadow-md data-[state=closed]:animate-out data-[state=open]:animate-in data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2'
|
||||
|
||||
export const selectDropdownClass =
|
||||
'flex shrink-0 cursor-pointer items-center justify-center px-3'
|
||||
|
||||
export const selectEmptyMessageClass = 'px-3 pb-4 text-sm text-muted-foreground'
|
||||
|
||||
export function stopEscapeToDocument(event: KeyboardEvent) {
|
||||
if (event.key === 'Escape') {
|
||||
event.stopPropagation()
|
||||
event.stopImmediatePropagation()
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user