From 2c90735bb19e6de6d5dd1fa8def372cceb89ae38 Mon Sep 17 00:00:00 2001
From: Chenlei Hu <huchenlei@proton.me>
Date: Sat, 5 Oct 2024 18:08:48 -0400
Subject: [PATCH] Restore top/bottom menu location setting (#1123)

* Rename floating to top

* Adjust teleport target

* Fix dropdown direction for bottom menubar

* Fix z-index
---
 browser_tests/actionbar.spec.ts       |  2 +-
 browser_tests/browserTabTitle.spec.ts |  2 +-
 browser_tests/extensionAPI.spec.ts    |  2 +-
 browser_tests/groupNode.spec.ts       |  4 ++--
 browser_tests/interaction.spec.ts     |  2 +-
 browser_tests/menu.spec.ts            |  6 +++---
 browser_tests/propertiesPanel.spec.ts |  2 +-
 browser_tests/templates.spec.ts       |  2 +-
 src/assets/css/style.css              |  3 ++-
 src/components/topbar/TopMenubar.vue  | 19 +++++++++++++++++--
 src/stores/coreSettings.ts            |  7 ++++---
 src/types/apiTypes.ts                 |  2 +-
 12 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/browser_tests/actionbar.spec.ts b/browser_tests/actionbar.spec.ts
index ae0c257cf..5185ca045 100644
--- a/browser_tests/actionbar.spec.ts
+++ b/browser_tests/actionbar.spec.ts
@@ -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 }) => {
diff --git a/browser_tests/browserTabTitle.spec.ts b/browser_tests/browserTabTitle.spec.ts
index 45dc0420f..abbe90dfe 100644
--- a/browser_tests/browserTabTitle.spec.ts
+++ b/browser_tests/browserTabTitle.spec.ts
@@ -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 }) => {
diff --git a/browser_tests/extensionAPI.spec.ts b/browser_tests/extensionAPI.spec.ts
index 7bbbbac3d..9902d668e 100644
--- a/browser_tests/extensionAPI.spec.ts
+++ b/browser_tests/extensionAPI.spec.ts
@@ -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 }) => {
diff --git a/browser_tests/groupNode.spec.ts b/browser_tests/groupNode.spec.ts
index 0c86272eb..e24255ce0 100644
--- a/browser_tests/groupNode.spec.ts
+++ b/browser_tests/groupNode.spec.ts
@@ -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()
@@ -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()
 
diff --git a/browser_tests/interaction.spec.ts b/browser_tests/interaction.spec.ts
index 2cd25a51c..743884e02 100644
--- a/browser_tests/interaction.spec.ts
+++ b/browser_tests/interaction.spec.ts
@@ -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 }) => {
diff --git a/browser_tests/menu.spec.ts b/browser_tests/menu.spec.ts
index 840c97517..801ac3d66 100644
--- a/browser_tests/menu.spec.ts
+++ b/browser_tests/menu.spec.ts
@@ -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 }) => {
@@ -508,7 +508,7 @@ 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 ({
@@ -516,7 +516,7 @@ test.describe('Menu', () => {
     }) => {
       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')
     })
   })
 
diff --git a/browser_tests/propertiesPanel.spec.ts b/browser_tests/propertiesPanel.spec.ts
index b9c96b714..8e997bdd8 100644
--- a/browser_tests/propertiesPanel.spec.ts
+++ b/browser_tests/propertiesPanel.spec.ts
@@ -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 }) => {
diff --git a/browser_tests/templates.spec.ts b/browser_tests/templates.spec.ts
index 16c603e65..75968bdf8 100644
--- a/browser_tests/templates.spec.ts
+++ b/browser_tests/templates.spec.ts
@@ -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 }) => {
diff --git a/src/assets/css/style.css b/src/assets/css/style.css
index de0a12ac2..1e9596df4 100644
--- a/src/assets/css/style.css
+++ b/src/assets/css/style.css
@@ -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;
 }
diff --git a/src/components/topbar/TopMenubar.vue b/src/components/topbar/TopMenubar.vue
index 12bb63cb6..b6680c025 100644
--- a/src/components/topbar/TopMenubar.vue
+++ b/src/components/topbar/TopMenubar.vue
@@ -1,5 +1,5 @@
 <template>
-  <teleport to=".comfyui-body-top">
+  <teleport :to="teleportTarget">
     <div
       ref="topMenuRef"
       class="comfyui-menu flex items-center"
@@ -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" />
@@ -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
 
@@ -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>
diff --git a/src/stores/coreSettings.ts b/src/stores/coreSettings.ts
index c13689ada..84d044f98 100644
--- a/src/stores/coreSettings.ts
+++ b/src/stores/coreSettings.ts
@@ -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
     }
diff --git a/src/types/apiTypes.ts b/src/types/apiTypes.ts
index 4a460f452..abe72831a 100644
--- a/src/types/apiTypes.ts
+++ b/src/types/apiTypes.ts
@@ -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(),