refactor: address PR review feedback - split exitViaBreadcrumb, add breadcrumb getter, rename var

Amp-Thread-ID: https://ampcode.com/threads/T-019d36f8-0597-7259-a52f-f49e95e9b213
Co-authored-by: Amp <amp@ampcode.com>

browser_tests/fixtures/VueNodeHelpers.ts

@@ -58,7 +58,7 @@ export class VueNodeHelpers {
   async getNodeIds(): Promise<string[]> {
     const allNodes = await this.nodes.all()
     const ids = await Promise.all(
-      allNodes.map((l) => l.getAttribute('data-node-id'))
+      allNodes.map((node) => node.getAttribute('data-node-id'))
     )
     return ids.filter((id): id is string => id !== null)
   }

browser_tests/fixtures/helpers/SubgraphHelper.ts

@@ -326,21 +326,24 @@ export class SubgraphHelper {
     })
   }
 
+  get breadcrumb(): Locator {
+    return this.page.getByTestId(TestIds.breadcrumb.subgraph)
+  }
+
+  async exitSubgraph(): Promise<void> {
+    await this.page.evaluate(() => {
+      const canvas = window.app!.canvas
+      const graph = canvas.graph
+      if (!graph) return
+      canvas.setGraph(graph.rootGraph)
+    })
+    await this.comfyPage.nextFrame()
+    await expect.poll(async () => this.isInSubgraph()).toBe(false)
+  }
+
   async exitViaBreadcrumb(): Promise<void> {
-    const breadcrumb = this.page.getByTestId(TestIds.breadcrumb.subgraph)
-    const rootItem = breadcrumb.getByTestId(TestIds.breadcrumb.root)
-
-    if (await rootItem.isVisible()) {
-      await rootItem.click()
-    } else {
-      await this.page.evaluate(() => {
-        const canvas = window.app!.canvas
-        const graph = canvas.graph
-        if (!graph) return
-        canvas.setGraph(graph.rootGraph)
-      })
-    }
-
+    const rootItem = this.breadcrumb.getByTestId(TestIds.breadcrumb.root)
+    await rootItem.click()
     await this.comfyPage.nextFrame()
     await expect.poll(async () => this.isInSubgraph()).toBe(false)
   }
@@ -500,7 +503,7 @@ export class SubgraphHelper {
       canvas.graph!.convertToSubgraph(canvas.selectedItems)
     })
     await this.comfyPage.nextFrame()
-    await this.exitViaBreadcrumb()
+    await this.exitSubgraph()
     await this.comfyPage.canvas.click()
     await this.comfyPage.nextFrame()
   }

browser_tests/tests/subgraph-promoted-slot-position.spec.ts

@@ -76,7 +76,7 @@ test.describe(
       await comfyPage.page.waitForSelector(dialog, { state: 'hidden' })
 
       // Navigate back to parent graph
-      await comfyPage.subgraph.exitViaBreadcrumb()
+      await comfyPage.subgraph.exitSubgraph()
 
       // Verify slot position is still at the widget row after rename
       const after = await SubgraphHelper.getTextSlotPosition(

browser_tests/tests/subgraph.spec.ts

@@ -516,7 +516,7 @@ test.describe('Subgraph Operations', { tag: ['@slow', '@subgraph'] }, () => {
       // Navigate into subgraph
       await subgraphNode.navigateIntoSubgraph()
 
-      const breadcrumb = comfyPage.page.getByTestId(TestIds.breadcrumb.subgraph)
+      const breadcrumb = comfyPage.subgraph.breadcrumb
       await breadcrumb.waitFor({ state: 'visible', timeout: 20000 })
       const initialBreadcrumbText = await breadcrumb.textContent()
 
@@ -560,9 +560,7 @@ test.describe('Subgraph Operations', { tag: ['@slow', '@subgraph'] }, () => {
       await comfyPage.nextFrame()
 
       expect(await comfyPage.subgraph.isInSubgraph()).toBe(true)
-      await expect(
-        comfyPage.page.getByTestId(TestIds.breadcrumb.subgraph)
-      ).toBeVisible()
+      await expect(comfyPage.subgraph.breadcrumb).toBeVisible()
 
       await comfyPage.workflow.loadWorkflow('default')
       await comfyPage.nextFrame()
@@ -580,9 +578,7 @@ test.describe('Subgraph Operations', { tag: ['@slow', '@subgraph'] }, () => {
       await comfyPage.workflow.loadWorkflow('subgraphs/basic-subgraph')
       await comfyPage.nextFrame()
 
-      const breadcrumb = comfyPage.page
-        .getByTestId(TestIds.breadcrumb.subgraph)
-        .locator('.p-breadcrumb')
+      const breadcrumb = comfyPage.subgraph.breadcrumb.locator('.p-breadcrumb')
 
       const subgraphNode = await comfyPage.nodeOps.getNodeRefById('2')
       await subgraphNode.navigateIntoSubgraph()
@@ -709,12 +705,15 @@ test.describe('Subgraph Operations', { tag: ['@slow', '@subgraph'] }, () => {
       await comfyPage.subgraph.removeSlot('input', 'text')
 
       // Wait for breadcrumb to be visible
-      await comfyPage.page
-        .getByTestId(TestIds.breadcrumb.subgraph)
-        .waitFor({ state: 'visible', timeout: 5000 })
+      await comfyPage.subgraph.breadcrumb.waitFor({
+        state: 'visible',
+        timeout: 5000
+      })
 
       // Click breadcrumb to navigate back to parent graph
-      const homeBreadcrumb = comfyPage.page.getByTestId(TestIds.breadcrumb.root)
+      const homeBreadcrumb = comfyPage.subgraph.breadcrumb.getByTestId(
+        TestIds.breadcrumb.root
+      )
       await homeBreadcrumb.waitFor({ state: 'visible' })
       await homeBreadcrumb.click()
       await comfyPage.nextFrame()
@@ -797,9 +796,7 @@ test.describe('Subgraph Operations', { tag: ['@slow', '@subgraph'] }, () => {
       // Navigate into subgraph
       const subgraphNode = await comfyPage.nodeOps.getNodeRefById('2')
       await subgraphNode.navigateIntoSubgraph()
-      await comfyPage.page
-        .getByTestId(TestIds.breadcrumb.subgraph)
-        .waitFor({ state: 'visible' })
+      await comfyPage.subgraph.breadcrumb.waitFor({ state: 'visible' })
 
       // Verify we're in a subgraph
       expect(await comfyPage.subgraph.isInSubgraph()).toBe(true)
@@ -825,9 +822,7 @@ test.describe('Subgraph Operations', { tag: ['@slow', '@subgraph'] }, () => {
 
       const subgraphNode = await comfyPage.nodeOps.getNodeRefById('2')
       await subgraphNode.navigateIntoSubgraph()
-      await comfyPage.page
-        .getByTestId(TestIds.breadcrumb.subgraph)
-        .waitFor({ state: 'visible' })
+      await comfyPage.subgraph.breadcrumb.waitFor({ state: 'visible' })
 
       // Verify we're in a subgraph
       if (!(await comfyPage.subgraph.isInSubgraph())) {

browser_tests/tests/subgraphInputSlotRename.spec.ts

@@ -76,7 +76,7 @@ test.describe(
       await page.waitForSelector(dialog, { state: 'hidden' })
 
       // 5. Navigate back to parent graph and re-enable Vue nodes
-      await comfyPage.subgraph.exitViaBreadcrumb()
+      await comfyPage.subgraph.exitSubgraph()
       await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', true)
       await comfyPage.vueNodes.waitForNodes()
 

browser_tests/tests/subgraphLifecycle.spec.ts

@@ -149,7 +149,7 @@ test.describe(
         const clipNode = await comfyPage.nodeOps.getNodeRefById('10')
         await clipNode.delete()
 
-        await comfyPage.subgraph.exitViaBreadcrumb()
+        await comfyPage.subgraph.exitSubgraph()
 
         await expect
           .poll(async () => {
@@ -189,7 +189,7 @@ test.describe(
         const clipNode = await comfyPage.nodeOps.getNodeRefById('10')
         await clipNode.delete()
 
-        await comfyPage.subgraph.exitViaBreadcrumb()
+        await comfyPage.subgraph.exitSubgraph()
 
         await expect(
           comfyPage.page.getByTestId(TestIds.widgets.domWidgetTextarea)

browser_tests/tests/subgraphPromotion.spec.ts

@@ -233,7 +233,7 @@ test.describe(
         await comfyPage.nextFrame()
 
         // Navigate back to parent graph
-        await comfyPage.subgraph.exitViaBreadcrumb()
+        await comfyPage.subgraph.exitSubgraph()
 
         // Promoted textarea on SubgraphNode should have the same value
         const promotedTextarea = comfyPage.page.getByTestId(
@@ -267,7 +267,7 @@ test.describe(
           )
           await expect(interiorTextarea).toHaveValue(testContent)
 
-          await comfyPage.subgraph.exitViaBreadcrumb()
+          await comfyPage.subgraph.exitSubgraph()
 
           const promotedTextarea = comfyPage.page.getByTestId(
             TestIds.widgets.domWidgetTextarea
@@ -313,7 +313,7 @@ test.describe(
         await comfyPage.nextFrame()
 
         // Navigate back to parent
-        await comfyPage.subgraph.exitViaBreadcrumb()
+        await comfyPage.subgraph.exitSubgraph()
 
         // SubgraphNode should now have the promoted widget
         const widgetCount = await getPromotedWidgetCount(comfyPage, '2')
@@ -348,7 +348,7 @@ test.describe(
         await comfyPage.nextFrame()
 
         // Navigate back and verify promotion took effect
-        await comfyPage.subgraph.exitViaBreadcrumb()
+        await comfyPage.subgraph.exitSubgraph()
         await fitToViewInstant(comfyPage)
         await comfyPage.nextFrame()
         await comfyPage.nextFrame()
@@ -380,7 +380,7 @@ test.describe(
         await comfyPage.nextFrame()
 
         // Navigate back to parent
-        await comfyPage.subgraph.exitViaBreadcrumb()
+        await comfyPage.subgraph.exitSubgraph()
 
         // SubgraphNode should have fewer widgets
         const finalWidgetCount = await getPromotedWidgetCount(comfyPage, '2')
@@ -716,7 +716,7 @@ test.describe(
 
         await comfyPage.subgraph.removeSlot('input')
 
-        await comfyPage.subgraph.exitViaBreadcrumb()
+        await comfyPage.subgraph.exitSubgraph()
 
         const finalNames = await getPromotedWidgetNames(comfyPage, '5')
         const expectedNames = [...initialNames]
@@ -746,8 +746,8 @@ test.describe(
         // Remove the text input slot
         await comfyPage.subgraph.removeSlot('input', 'text')
 
-        // Navigate back via breadcrumb
-        await comfyPage.subgraph.exitViaBreadcrumb()
+        // Navigate back
+        await comfyPage.subgraph.exitSubgraph()
 
         // Widget count should be reduced
         const finalWidgetCount = await getPromotedWidgetCount(comfyPage, '11')

browser_tests/tests/subgraphViewport.spec.ts

@@ -92,7 +92,7 @@ test.describe('Subgraph viewport restoration', { tag: '@subgraph' }, () => {
     await comfyPage.vueNodes.enterSubgraph('11')
     await comfyPage.nextFrame()
 
-    await comfyPage.subgraph.exitViaBreadcrumb()
+    await comfyPage.subgraph.exitSubgraph()
 
     await expect
       .poll(