From 8dee207b8cccf61dc4ea12e0d0c3547588b5985c Mon Sep 17 00:00:00 2001
From: Benjamin Lu <benceruleanlu@proton.me>
Date: Fri, 17 Oct 2025 15:12:03 -0700
Subject: [PATCH] WorkflowTabs: cleanup scroll/overflow handling and watcher
 disposal (#6080)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Summary

Refactor and cleanup `WorkflowTabs` scroll/overflow handling to improve
stability, ensure proper watcher disposal, and keep the active tab in
view more reliably.

note: honestly a nit, can drop if review is too annoying

## Changes

- **What**:
- Replace `ScrollPanel` ref with `containerRef` and query
`.p-scrollpanel-content` within the container.
- Add computed `scrollContent` to centralize access to the scrollable
element.
- Add `ensureActiveTabVisible({ waitForDom })` option to skip `nextTick`
when not needed.
- Rework watchers with explicit `WatchStopHandle`s and `onCleanup` to
stop previous watchers and dispose `overflowObserver` correctly when
`scrollContent` changes.
- Update arrow enable logic by watching `arrivedState.left/right`
together and setting `leftArrowEnabled`/`rightArrowEnabled` immediately.
- Only measure and re-ensure visibility when overflowing; call
`scrollState.measure()` and `ensureActiveTabVisible({ waitForDom: false
})` after arrows update.
- **Breaking**: None
- **Dependencies**: None

## Review Focus

- Correctness of watcher lifecycle and cleanup when `scrollContent`
changes
- Arrow enablement behavior at scroll boundaries
- Reliability of `ensureActiveTabVisible` across tab selection and
overflow changes
- Regressions in scroll performance and tab visibility

## Screenshots (if applicable)

N/A

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6080-WorkflowTabs-cleanup-scroll-overflow-handling-and-watcher-disposal-28d6d73d3650819cba6de70fb10354ad)
by [Unito](https://www.unito.io)
---
 src/components/topbar/WorkflowTabs.vue | 113 +++++++++++++++----------
 1 file changed, 70 insertions(+), 43 deletions(-)

diff --git a/src/components/topbar/WorkflowTabs.vue b/src/components/topbar/WorkflowTabs.vue
index 66b1f3573..da3b89876 100644
--- a/src/components/topbar/WorkflowTabs.vue
+++ b/src/components/topbar/WorkflowTabs.vue
@@ -1,5 +1,6 @@
 <template>
   <div
+    ref="containerRef"
     class="workflow-tabs-container flex h-full max-w-full flex-auto flex-row overflow-hidden"
     :class="{ 'workflow-tabs-container-desktop': isDesktop }"
   >
@@ -13,7 +14,6 @@
       @mousedown="whileMouseDown($event, () => scroll(-1))"
     />
     <ScrollPanel
-      ref="scrollPanelRef"
       class="no-drag overflow-hidden"
       :pt:content="{
         class: 'p-0 w-full flex',
@@ -74,6 +74,7 @@ import ContextMenu from 'primevue/contextmenu'
 import ScrollPanel from 'primevue/scrollpanel'
 import SelectButton from 'primevue/selectbutton'
 import { computed, nextTick, onUpdated, ref, watch } from 'vue'
+import type { WatchStopHandle } from 'vue'
 import { useI18n } from 'vue-i18n'
 
 import WorkflowTab from '@/components/topbar/WorkflowTab.vue'
@@ -108,7 +109,7 @@ const workflowService = useWorkflowService()
 
 const rightClickedTab = ref<WorkflowOption | undefined>()
 const menu = ref()
-const scrollPanelRef = ref()
+const containerRef = ref<HTMLElement | null>(null)
 const showOverflowArrows = ref(false)
 const leftArrowEnabled = ref(false)
 const rightArrowEnabled = ref(false)
@@ -221,26 +222,34 @@ const handleWheel = (event: WheelEvent) => {
   })
 }
 
+const scrollContent = computed(
+  () =>
+    (containerRef.value?.querySelector(
+      '.p-scrollpanel-content'
+    ) as HTMLElement | null) ?? null
+)
+
 const scroll = (direction: number) => {
-  const scrollElement = scrollPanelRef.value.$el.querySelector(
-    '.p-scrollpanel-content'
-  ) as HTMLElement
-  scrollElement.scrollBy({ left: direction * 20 })
+  const el = scrollContent.value
+  if (!el) return
+  el.scrollBy({ left: direction * 20 })
 }
 
-const ensureActiveTabVisible = async () => {
+const ensureActiveTabVisible = async (
+  options: { waitForDom?: boolean } = {}
+) => {
   if (!selectedWorkflow.value) return
 
-  await nextTick()
+  if (options.waitForDom !== false) {
+    await nextTick()
+  }
 
-  const scrollPanelElement = scrollPanelRef.value?.$el as
-    | HTMLElement
-    | undefined
-  if (!scrollPanelElement) return
+  const containerElement = containerRef.value
+  if (!containerElement) return
 
-  const activeTabElement = scrollPanelElement.querySelector(
+  const activeTabElement = containerElement.querySelector(
     '.p-togglebutton-checked'
-  ) as HTMLElement | null
+  )
   if (!activeTabElement) return
 
   activeTabElement.scrollIntoView({ block: 'nearest', inline: 'nearest' })
@@ -255,38 +264,56 @@ watch(
   { immediate: true }
 )
 
-const scrollContent = computed(
-  () =>
-    scrollPanelRef.value?.$el.querySelector(
-      '.p-scrollpanel-content'
-    ) as HTMLElement
-)
 let overflowObserver: ReturnType<typeof useOverflowObserver> | null = null
-let overflowWatch: ReturnType<typeof watch> | null = null
-watch(scrollContent, (value) => {
-  const scrollState = useScroll(value)
+let stopArrivedWatch: WatchStopHandle | null = null
+let stopOverflowWatch: WatchStopHandle | null = null
 
-  watch(scrollState.arrivedState, () => {
-    leftArrowEnabled.value = !scrollState.arrivedState.left
-    rightArrowEnabled.value = !scrollState.arrivedState.right
-  })
+watch(
+  scrollContent,
+  (el, _prev, onCleanup) => {
+    stopArrivedWatch?.()
+    stopOverflowWatch?.()
+    overflowObserver?.dispose()
 
-  overflowObserver?.dispose()
-  overflowWatch?.stop()
-  overflowObserver = useOverflowObserver(value)
-  overflowWatch = watch(
-    overflowObserver.isOverflowing,
-    (value) => {
-      showOverflowArrows.value = value
-      void nextTick(() => {
-        // Force a new check after arrows are updated
-        scrollState.measure()
-        void ensureActiveTabVisible()
-      })
-    },
-    { immediate: true }
-  )
-})
+    if (!el) return
+
+    const scrollState = useScroll(el)
+
+    stopArrivedWatch = watch(
+      [
+        () => scrollState.arrivedState.left,
+        () => scrollState.arrivedState.right
+      ],
+      ([atLeft, atRight]) => {
+        leftArrowEnabled.value = !atLeft
+        rightArrowEnabled.value = !atRight
+      },
+      { immediate: true }
+    )
+
+    overflowObserver = useOverflowObserver(el)
+    stopOverflowWatch = watch(
+      overflowObserver.isOverflowing,
+      (isOverflow) => {
+        showOverflowArrows.value = isOverflow
+        if (!isOverflow) return
+        void nextTick(() => {
+          // Force a new check after arrows are updated
+          scrollState.measure()
+          void ensureActiveTabVisible({ waitForDom: false })
+        })
+      },
+      { immediate: true }
+    )
+
+    onCleanup(() => {
+      stopArrivedWatch?.()
+      stopOverflowWatch?.()
+      overflowObserver?.dispose()
+    })
+  },
+  { immediate: true }
+)
 
 onUpdated(() => {
   if (!overflowObserver?.disposed.value) {