Skip to content

Commit

Permalink
Restore top/bottom menu location setting (#1123)
Browse files Browse the repository at this point in the history
* Rename floating to top

* Adjust teleport target

* Fix dropdown direction for bottom menubar

* Fix z-index
  • Loading branch information
huchenlei authored Oct 5, 2024
1 parent 4d5fbef commit 2c90735
Show file tree
Hide file tree
Showing 12 changed files with 35 additions and 18 deletions.
2 changes: 1 addition & 1 deletion browser_tests/actionbar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const test = mergeTests(comfyPageFixture, webSocketFixture)

test.describe('Actionbar', () => {
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Floating')
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
})

test.afterEach(async ({ comfyPage }) => {
Expand Down
2 changes: 1 addition & 1 deletion browser_tests/browserTabTitle.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { comfyPageFixture as test } from './ComfyPage'
test.describe('Browser tab title', () => {
test.describe('Beta Menu', () => {
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Floating')
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
})

test.afterEach(async ({ comfyPage }) => {
Expand Down
2 changes: 1 addition & 1 deletion browser_tests/extensionAPI.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { comfyPageFixture as test } from './ComfyPage'

test.describe('Topbar commands', () => {
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Floating')
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
})

test.afterEach(async ({ comfyPage }) => {
Expand Down
4 changes: 2 additions & 2 deletions browser_tests/groupNode.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ test.describe('Group Node', () => {
let libraryTab

test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Floating')
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
libraryTab = comfyPage.menu.nodeLibraryTab
await comfyPage.convertAllNodesToGroupNode(groupNodeName)
await libraryTab.open()
Expand Down Expand Up @@ -182,7 +182,7 @@ test.describe('Group Node', () => {
}

test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Floating')
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
await comfyPage.loadWorkflow(WORKFLOW_NAME)
await comfyPage.menu.nodeLibraryTab.open()

Expand Down
2 changes: 1 addition & 1 deletion browser_tests/interaction.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ test.describe('Load workflow', () => {

test.describe('Load duplicate workflow', () => {
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Floating')
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
})

test.afterEach(async ({ comfyPage }) => {
Expand Down
6 changes: 3 additions & 3 deletions browser_tests/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { comfyPageFixture as test } from './ComfyPage'

test.describe('Menu', () => {
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Floating')
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
})

test.afterEach(async ({ comfyPage }) => {
Expand Down Expand Up @@ -508,15 +508,15 @@ test.describe('Menu', () => {
comfyPage
}) => {
await comfyPage.setSetting('Comfy.UseNewMenu', position)
expect(await comfyPage.getSetting('Comfy.UseNewMenu')).toBe('Floating')
expect(await comfyPage.getSetting('Comfy.UseNewMenu')).toBe('Top')
})

test(`Can migrate deprecated menu positions on initial load (${position})`, async ({
comfyPage
}) => {
await comfyPage.setSetting('Comfy.UseNewMenu', position)
await comfyPage.setup()
expect(await comfyPage.getSetting('Comfy.UseNewMenu')).toBe('Floating')
expect(await comfyPage.getSetting('Comfy.UseNewMenu')).toBe('Top')
})
})

Expand Down
2 changes: 1 addition & 1 deletion browser_tests/propertiesPanel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { comfyPageFixture as test } from './ComfyPage'

test.describe('Properties Panel', () => {
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Floating')
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
})

test.afterEach(async ({ comfyPage }) => {
Expand Down
2 changes: 1 addition & 1 deletion browser_tests/templates.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { comfyPageFixture as test } from './ComfyPage'

test.describe('Templates', () => {
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Floating')
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
})

test.afterEach(async ({ comfyPage }) => {
Expand Down
3 changes: 2 additions & 1 deletion src/assets/css/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ body {
/* Span across all columns */
grid-column: 1/-1;
grid-row: 3;
z-index: 10;
/* Bottom menu bar dropdown needs to be above of graph canvas splitter overlay which is z-index: 999 */
z-index: 1000;
display: flex;
flex-direction: column;
}
Expand Down
19 changes: 17 additions & 2 deletions src/components/topbar/TopMenubar.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<template>
<teleport to=".comfyui-body-top">
<teleport :to="teleportTarget">
<div
ref="topMenuRef"
class="comfyui-menu flex items-center"
Expand All @@ -11,7 +11,9 @@
:model="items"
class="top-menubar border-none p-0 bg-transparent"
:pt="{
rootList: 'gap-0 flex-nowrap w-auto'
rootList: 'gap-0 flex-nowrap w-auto',
submenu: `dropdown-direction-${dropdownDirection}`,
item: 'relative'
}"
/>
<Divider layout="vertical" class="mx-2" />
Expand Down Expand Up @@ -42,6 +44,15 @@ const workflowTabsPosition = computed(() =>
const betaMenuEnabled = computed(
() => settingStore.get('Comfy.UseNewMenu') !== 'Disabled'
)
const teleportTarget = computed(() =>
settingStore.get('Comfy.UseNewMenu') === 'Top'
? '.comfyui-body-top'
: '.comfyui-body-bottom'
)
const dropdownDirection = computed(() =>
settingStore.get('Comfy.UseNewMenu') === 'Top' ? 'down' : 'up'
)
const menuItemsStore = useMenuItemStore()
const items = menuItemsStore.menuItems
Expand Down Expand Up @@ -99,4 +110,8 @@ eventBus.on((event: string, payload: any) => {
.top-menubar .p-menubar-item-link svg {
display: none;
}
.p-menubar-submenu.dropdown-direction-up {
@apply top-auto bottom-full flex-col-reverse;
}
</style>
7 changes: 4 additions & 3 deletions src/stores/coreSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,10 +376,11 @@ export const CORE_SETTINGS: SettingParams[] = [
name: 'Use new menu and workflow management.',
experimental: true,
type: 'combo',
options: ['Disabled', 'Floating'],
options: ['Disabled', 'Top', 'Bottom'],
migrateDeprecatedValue: (value: string) => {
if (['Top', 'Bottom'].includes(value)) {
return 'Floating'
// Floating is now supported by dragging the docked actionbar off.
if (value === 'Floating') {
return 'Top'
}
return value
}
Expand Down
2 changes: 1 addition & 1 deletion src/types/apiTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ const zSettings = z.record(z.any()).and(
'Comfy.SnapToGrid.GridSize': z.number(),
'Comfy.TextareaWidget.FontSize': z.number(),
'Comfy.TextareaWidget.Spellcheck': z.boolean(),
'Comfy.UseNewMenu': z.enum(['Disabled', 'Floating']),
'Comfy.UseNewMenu': z.enum(['Disabled', 'Top', 'Bottom']),
'Comfy.TreeExplorer.ItemPadding': z.number(),
'Comfy.Validation.Workflows': z.boolean(),
'Comfy.Workflow.SortNodeIdOnSave': z.boolean(),
Expand Down

0 comments on commit 2c90735

Please sign in to comment.