fix: address review findings for no-force-option PR

- Wrap modifier key press/release in try/finally to prevent leaks
- Remove YAGNI ControlOrMeta resolution from mouseClickAt
- Narrow NodeReference.click options type to match mouseClickAt
- Remove redundant waitFor and double nextFrame calls

Amp-Thread-ID: https://ampcode.com/threads/T-019d7ac2-edd8-75b0-bad7-d81d82ece077
Co-authored-by: Amp <amp@ampcode.com>

browser_tests/fixtures/components/Topbar.ts

@@ -71,9 +71,7 @@ export class Topbar {
   async closeWorkflowTab(tabName: string) {
     const tab = this.getWorkflowTab(tabName)
     await tab.hover()
-    const closeBtn = tab.locator('.close-button')
-    await closeBtn.waitFor({ state: 'visible' })
-    await closeBtn.click()
+    await tab.locator('.close-button').click()
   }
 
   getSaveDialog(): Locator {

browser_tests/fixtures/helpers/CanvasHelper.ts

@@ -94,24 +94,19 @@ export class CanvasHelper {
     position: Position,
     options?: {
       button?: 'left' | 'right' | 'middle'
-      modifiers?: ('Shift' | 'Control' | 'Alt' | 'Meta' | 'ControlOrMeta')[]
+      modifiers?: ('Shift' | 'Control' | 'Alt' | 'Meta')[]
     }
   ): Promise<void> {
     const abs = await this.toAbsolute(position)
-    const resolveModifier = (
-      mod: 'Shift' | 'Control' | 'Alt' | 'Meta' | 'ControlOrMeta'
-    ) =>
-      mod === 'ControlOrMeta'
-        ? process.platform === 'darwin'
-          ? 'Meta'
-          : 'Control'
-        : mod
-    const modifiers = (options?.modifiers ?? []).map(resolveModifier)
+    const modifiers = options?.modifiers ?? []
     for (const mod of modifiers) await this.page.keyboard.down(mod)
-    await this.page.mouse.click(abs.x, abs.y, {
-      button: options?.button
-    })
-    for (const mod of modifiers) await this.page.keyboard.up(mod)
+    try {
+      await this.page.mouse.click(abs.x, abs.y, {
+        button: options?.button
+      })
+    } finally {
+      for (const mod of modifiers) await this.page.keyboard.up(mod)
+    }
     await this.nextFrame()
   }
 

browser_tests/fixtures/utils/litegraphUtils.ts

@@ -1,5 +1,4 @@
 import { expect } from '@playwright/test'
-import type { Page } from '@playwright/test'
 
 import type { NodeId } from '@/platform/workflow/validation/schemas/workflowSchema'
 import { ManageGroupNode } from '@e2e/helpers/manageGroupNode'
@@ -356,7 +355,11 @@ export class NodeReference {
   }
   async click(
     position: 'title' | 'collapse',
-    options?: Parameters<Page['click']>[1] & { moveMouseToEmptyArea?: boolean }
+    options?: {
+      button?: 'left' | 'right' | 'middle'
+      modifiers?: ('Shift' | 'Control' | 'Alt' | 'Meta')[]
+      moveMouseToEmptyArea?: boolean
+    }
   ) {
     let clickPos: Position
     switch (position) {
@@ -378,7 +381,6 @@ export class NodeReference {
     }
 
     await this.comfyPage.canvasOps.mouseClickAt(clickPos, options)
-    await this.comfyPage.nextFrame()
     if (moveMouseToEmptyArea) {
       await this.comfyPage.canvasOps.moveMouseToEmptyArea()
     }