Skip to content

Commit adb9bbb

Browse files
committed
Merge remote-tracking branch 'upstream/main' into research/viewport-virtualization-4916efd7f
2 parents 007e71b + 9bd1d37 commit adb9bbb

11 files changed

Lines changed: 323 additions & 16 deletions

src/components/load3d/Load3DMenuBar.vue

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
class="pointer-events-auto flex h-10 items-center gap-1 bg-interface-menu-surface px-2"
66
@wheel.stop
77
>
8-
<Popover v-model:open="catMenuOpen">
8+
<Popover v-model:open="categoryMenuOpen">
99
<PopoverTrigger as-child>
1010
<button
1111
:class="chipClass"
@@ -186,6 +186,7 @@ import {
186186
import ModelMenuGroup from '@/components/load3d/menubar/ModelMenuGroup.vue'
187187
import RecordMenuControl from '@/components/load3d/menubar/RecordMenuControl.vue'
188188
import SceneMenuGroup from '@/components/load3d/menubar/SceneMenuGroup.vue'
189+
import { usePopoverExclusivity } from '@/components/load3d/menubar/usePopoverExclusivity'
189190
import ViewerControls from '@/components/load3d/controls/ViewerControls.vue'
190191
import Popover from '@/components/ui/popover/Popover.vue'
191192
import PopoverContent from '@/components/ui/popover/PopoverContent.vue'
@@ -294,8 +295,9 @@ watch(categoryDefs, (defs) => {
294295
}
295296
})
296297
297-
const catMenuOpen = ref(false)
298-
const exportOpen = ref(false)
298+
const exclusivePopover = usePopoverExclusivity()
299+
const categoryMenuOpen = exclusivePopover('category-menu')
300+
const exportOpen = exclusivePopover('export')
299301
300302
const sceneHasImage = computed(
301303
() =>
@@ -327,7 +329,7 @@ const compact = computed(
327329
328330
function selectCategory(key: string) {
329331
activeCategory.value = key
330-
catMenuOpen.value = false
332+
categoryMenuOpen.value = false
331333
}
332334
333335
function onExport(format: string) {

src/components/load3d/menubar/CameraMenuGroup.vue

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
<span v-if="!compact">{{ cameraTypeLabel }}</span>
1111
</button>
1212

13-
<Popover v-if="isPerspective">
13+
<Popover v-if="isPerspective" v-model:open="fovOpen">
1414
<PopoverTrigger as-child>
1515
<button
1616
v-tooltip.bottom="tip(t('load3d.menuBar.fov'))"
@@ -52,6 +52,7 @@ import {
5252
panelClass,
5353
tip
5454
} from '@/components/load3d/menubar/menuBarStyles'
55+
import { usePopoverExclusivity } from '@/components/load3d/menubar/usePopoverExclusivity'
5556
import Popover from '@/components/ui/popover/Popover.vue'
5657
import PopoverContent from '@/components/ui/popover/PopoverContent.vue'
5758
import Slider from '@/components/ui/slider/Slider.vue'
@@ -67,6 +68,8 @@ const config = defineModel<CameraConfig>('config')
6768
6869
const { t } = useI18n()
6970
71+
const fovOpen = usePopoverExclusivity()('camera-fov')
72+
7073
const cameraType = computed(() => config.value?.cameraType)
7174
const isPerspective = computed(() => cameraType.value === 'perspective')
7275
const cameraTypeLabel = computed(() =>

src/components/load3d/menubar/LightMenuGroup.vue

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<template>
2-
<Popover v-if="isOriginalMaterial">
2+
<Popover v-if="isOriginalMaterial" v-model:open="intensityOpen">
33
<PopoverTrigger as-child>
44
<button
55
v-tooltip.bottom="tip(t('load3d.menuBar.intensity'))"
@@ -46,6 +46,7 @@ import {
4646
panelClass,
4747
tip
4848
} from '@/components/load3d/menubar/menuBarStyles'
49+
import { usePopoverExclusivity } from '@/components/load3d/menubar/usePopoverExclusivity'
4950
import Popover from '@/components/ui/popover/Popover.vue'
5051
import PopoverContent from '@/components/ui/popover/PopoverContent.vue'
5152
import Slider from '@/components/ui/slider/Slider.vue'
@@ -63,6 +64,8 @@ const config = defineModel<LightConfig>('config')
6364
6465
const { t } = useI18n()
6566
67+
const intensityOpen = usePopoverExclusivity()('light-intensity')
68+
6669
const settingStore = useSettingStore()
6770
const lightIntensityMinimum = settingStore.get(
6871
'Comfy.Load3D.LightIntensityMinimum'

src/components/load3d/menubar/ModelMenuGroup.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,38 @@ describe('ModelMenuGroup', () => {
5353
expect(config.materialMode).toBe('wireframe')
5454
})
5555

56+
it('only shows one popover at a time', async () => {
57+
const { user } = renderGroup()
58+
59+
await user.click(screen.getByRole('button', { name: 'Up Direction' }))
60+
expect(screen.getByRole('button', { name: '+Y' })).toBeInTheDocument()
61+
62+
await user.click(screen.getByRole('button', { name: 'Material' }))
63+
expect(screen.queryByRole('button', { name: '+Y' })).not.toBeInTheDocument()
64+
expect(
65+
screen.getByRole('button', { name: 'Wireframe' })
66+
).toBeInTheDocument()
67+
68+
await user.click(screen.getByRole('button', { name: 'Up Direction' }))
69+
expect(
70+
screen.queryByRole('button', { name: 'Wireframe' })
71+
).not.toBeInTheDocument()
72+
expect(screen.getByRole('button', { name: '+Y' })).toBeInTheDocument()
73+
74+
await user.click(screen.getByRole('button', { name: 'Up Direction' }))
75+
expect(screen.queryByRole('button', { name: '+Y' })).not.toBeInTheDocument()
76+
})
77+
78+
it('closes the open popover on Escape', async () => {
79+
const { user } = renderGroup()
80+
81+
await user.click(screen.getByRole('button', { name: 'Up Direction' }))
82+
expect(screen.getByRole('button', { name: '+Y' })).toBeInTheDocument()
83+
84+
await user.keyboard('{Escape}')
85+
expect(screen.queryByRole('button', { name: '+Y' })).not.toBeInTheDocument()
86+
})
87+
5688
it('toggles the skeleton only when supported', async () => {
5789
const config = makeConfig({ showSkeleton: false })
5890
const { user, rerender } = renderGroup({ config, hasSkeleton: false })

src/components/load3d/menubar/ModelMenuGroup.vue

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<template>
2-
<Popover>
2+
<Popover v-model:open="upDirectionOpen">
33
<PopoverTrigger as-child>
44
<button
55
v-tooltip.bottom="tip(t('load3d.menuBar.upDirection'))"
@@ -29,7 +29,7 @@
2929
</PopoverContent>
3030
</Popover>
3131

32-
<Popover v-if="materialModes.length">
32+
<Popover v-if="materialModes.length" v-model:open="materialOpen">
3333
<PopoverTrigger as-child>
3434
<button
3535
v-tooltip.bottom="tip(t('load3d.menuBar.material'))"
@@ -83,6 +83,7 @@ import {
8383
rowClass,
8484
tip
8585
} from '@/components/load3d/menubar/menuBarStyles'
86+
import { usePopoverExclusivity } from '@/components/load3d/menubar/usePopoverExclusivity'
8687
import Popover from '@/components/ui/popover/Popover.vue'
8788
import PopoverContent from '@/components/ui/popover/PopoverContent.vue'
8889
import type {
@@ -111,6 +112,10 @@ const upDirection = computed(() => config.value?.upDirection)
111112
const materialMode = computed(() => config.value?.materialMode)
112113
const showSkeleton = computed(() => config.value?.showSkeleton ?? false)
113114
115+
const exclusivePopover = usePopoverExclusivity()
116+
const upDirectionOpen = exclusivePopover('model-up-direction')
117+
const materialOpen = exclusivePopover('model-material')
118+
114119
const upDirections: UpDirection[] = [
115120
'original',
116121
'-x',

src/components/load3d/menubar/RecordMenuControl.vue

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@
8686

8787
<script setup lang="ts">
8888
import { PopoverTrigger } from 'reka-ui'
89-
import { ref } from 'vue'
9089
import { useI18n } from 'vue-i18n'
9190
9291
import {
@@ -95,6 +94,7 @@ import {
9594
rowClass,
9695
tip
9796
} from '@/components/load3d/menubar/menuBarStyles'
97+
import { usePopoverExclusivity } from '@/components/load3d/menubar/usePopoverExclusivity'
9898
import Popover from '@/components/ui/popover/Popover.vue'
9999
import PopoverContent from '@/components/ui/popover/PopoverContent.vue'
100100
import { cn } from '@comfyorg/tailwind-utils'
@@ -116,7 +116,7 @@ const emit = defineEmits<{
116116
117117
const { t } = useI18n()
118118
119-
const menuOpen = ref(false)
119+
const menuOpen = usePopoverExclusivity()('recording-menu')
120120
121121
function downloadRecording() {
122122
menuOpen.value = false

src/components/load3d/menubar/SceneMenuGroup.vue

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@
6363
<i class="icon-[lucide--globe] size-4" />
6464
<span v-if="!compact">{{ t('load3d.menuBar.panorama') }}</span>
6565
</button>
66-
<Popover v-if="isPanorama">
66+
<Popover v-if="isPanorama" v-model:open="fovOpen">
6767
<PopoverTrigger as-child>
6868
<button
6969
v-tooltip.bottom="tip(t('load3d.menuBar.fov'))"
@@ -118,6 +118,7 @@ import {
118118
panelClass,
119119
tip
120120
} from '@/components/load3d/menubar/menuBarStyles'
121+
import { usePopoverExclusivity } from '@/components/load3d/menubar/usePopoverExclusivity'
121122
import Popover from '@/components/ui/popover/Popover.vue'
122123
import PopoverContent from '@/components/ui/popover/PopoverContent.vue'
123124
import Slider from '@/components/ui/slider/Slider.vue'
@@ -157,6 +158,8 @@ const fovValue = computed(() => fov.value ?? 10)
157158
const colorRef = ref<HTMLInputElement | null>(null)
158159
const bgImageRef = ref<HTMLInputElement | null>(null)
159160
161+
const fovOpen = usePopoverExclusivity()('scene-fov')
162+
160163
function toggleGrid() {
161164
if (config.value) config.value.showGrid = !config.value.showGrid
162165
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import userEvent from '@testing-library/user-event'
2+
import { render, screen } from '@testing-library/vue'
3+
import { describe, expect, it } from 'vitest'
4+
import { defineComponent, h } from 'vue'
5+
6+
import { usePopoverExclusivity } from '@/components/load3d/menubar/usePopoverExclusivity'
7+
8+
const Toggle = defineComponent({
9+
props: { popoverId: { type: String, required: true } },
10+
setup(props) {
11+
const open = usePopoverExclusivity()(props.popoverId)
12+
return () =>
13+
h('div', [
14+
h(
15+
'span',
16+
{ 'data-testid': `${props.popoverId}-state` },
17+
open.value ? 'open' : 'closed'
18+
),
19+
h(
20+
'button',
21+
{ onClick: () => (open.value = true) },
22+
`open ${props.popoverId}`
23+
),
24+
h(
25+
'button',
26+
{ onClick: () => (open.value = false) },
27+
`close ${props.popoverId}`
28+
)
29+
])
30+
}
31+
})
32+
33+
const Menubar = defineComponent({
34+
setup() {
35+
usePopoverExclusivity()
36+
return () => [h(Toggle, { popoverId: 'a' }), h(Toggle, { popoverId: 'b' })]
37+
}
38+
})
39+
40+
function renderMenubar() {
41+
render(Menubar)
42+
return userEvent.setup()
43+
}
44+
45+
describe('usePopoverExclusivity', () => {
46+
it('keeps popovers across components mutually exclusive', async () => {
47+
const user = renderMenubar()
48+
49+
await user.click(screen.getByRole('button', { name: 'open a' }))
50+
expect(screen.getByTestId('a-state')).toHaveTextContent('open')
51+
52+
await user.click(screen.getByRole('button', { name: 'open b' }))
53+
expect(screen.getByTestId('b-state')).toHaveTextContent('open')
54+
expect(screen.getByTestId('a-state')).toHaveTextContent('closed')
55+
})
56+
57+
it('ignores a close from a popover that is not the open one', async () => {
58+
const user = renderMenubar()
59+
60+
await user.click(screen.getByRole('button', { name: 'open a' }))
61+
await user.click(screen.getByRole('button', { name: 'close b' }))
62+
63+
expect(screen.getByTestId('a-state')).toHaveTextContent('open')
64+
})
65+
})
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import type { InjectionKey, Ref, WritableComputedRef } from 'vue'
2+
import { computed, inject, provide, ref } from 'vue'
3+
4+
const openPopoverKey: InjectionKey<Ref<string | null>> =
5+
Symbol('load3dOpenPopover')
6+
7+
/**
8+
* Keeps Load3D menubar popovers mutually exclusive. `@pointerdown.stop` on the
9+
* Load3D container blocks reka-ui's document-level outside-pointerdown
10+
* dismissal, so sibling popovers cannot close each other on their own — on
11+
* Safari not even via the focus-outside fallback, because clicking a button
12+
* there does not focus it.
13+
*
14+
* Call once per component; the returned factory yields a writable `open`
15+
* binding per popover id. The open-popover id is shared with ancestor scopes
16+
* via provide/inject, so exclusivity spans the whole menubar.
17+
*/
18+
export function usePopoverExclusivity() {
19+
const openPopover = inject(openPopoverKey, null) ?? ref<string | null>(null)
20+
provide(openPopoverKey, openPopover)
21+
return function exclusivePopover(id: string): WritableComputedRef<boolean> {
22+
return computed({
23+
get: () => openPopover.value === id,
24+
set: (open) => {
25+
if (open) openPopover.value = id
26+
else if (openPopover.value === id) openPopover.value = null
27+
}
28+
})
29+
}
30+
}

0 commit comments

Comments
 (0)