WebUI: unregister scaffold Tab on unmount and stable Tab order (#6842)

Tabs register themselves into the Scaffold's tabs list on mount but never remove their entry on unmount. Since param-only navigation (e.g. the superseded-by link, repo-to-repo links) reuses the wrapper and its Scaffold, a conditionally rendered tab like the pipeline *Errors* tab or the repo *Pull requests* tab stayed visible after its condition turned false, linking to an empty page.

Fix: remove the entry in `onBeforeUnmount`. The mount-time dedup guarantees at most one entry per route, so removal by route is safe. Includes lifecycle tests (register, dedup, unregister, re-register).

Co-authored-by: Claude <claude@anthropic.com>

web/src/components/layout/scaffold/Tab.test.ts

@@ -0,0 +1,168 @@
+import { mount } from '@vue/test-utils';
+import { describe, expect, it } from 'vitest';
+import { defineComponent, h, nextTick, ref } from 'vue';
+
+import type { Tab as TabType } from '~/compositions/useTabs';
+
+import Tab from './Tab.vue';
+
+async function mountConditionalTab() {
+  const visible = ref(true);
+  const tabs = ref<TabType[]>([]);
+
+  const host = defineComponent({
+    setup() {
+      return () =>
+        h('div', [
+          visible.value
+            ? h(Tab, {
+                to: { name: 'repo-pipeline-errors' },
+                title: 'Errors',
+                icon: 'alert',
+              })
+            : null,
+        ]);
+    },
+  });
+
+  mount(host, {
+    global: {
+      provide: { tabs },
+    },
+  });
+  await nextTick();
+
+  return { visible, tabs };
+}
+
+describe('tab', () => {
+  it('registers itself on mount', async () => {
+    const { tabs } = await mountConditionalTab();
+
+    expect(tabs.value).toHaveLength(1);
+    expect(tabs.value[0].title).toBe('Errors');
+  });
+
+  it('does not register the same route twice', async () => {
+    const duplicateVisible = ref(true);
+    const tabs = ref<TabType[]>([]);
+
+    const host = defineComponent({
+      setup() {
+        return () =>
+          h('div', [
+            h(Tab, { to: { name: 'repo-pipeline-errors' }, title: 'Errors' }),
+            duplicateVisible.value ? h(Tab, { to: { name: 'repo-pipeline-errors' }, title: 'Errors' }) : null,
+          ]);
+      },
+    });
+
+    mount(host, {
+      global: {
+        provide: { tabs },
+      },
+    });
+    await nextTick();
+
+    expect(tabs.value).toHaveLength(1);
+
+    // the second instance was skipped by the dedup, so its unmount must not
+    // remove the entry registered by the first, still-mounted instance
+    duplicateVisible.value = false;
+    await nextTick();
+
+    expect(tabs.value).toHaveLength(1);
+  });
+
+  it('keeps the tab when the registering duplicate unmounts before the skipped one', async () => {
+    const firstVisible = ref(true);
+    const secondVisible = ref(true);
+    const tabs = ref<TabType[]>([]);
+
+    const host = defineComponent({
+      setup() {
+        return () =>
+          h('div', [
+            firstVisible.value ? h(Tab, { to: { name: 'repo-pipeline-errors' }, title: 'Errors' }) : null,
+            secondVisible.value ? h(Tab, { to: { name: 'repo-pipeline-errors' }, title: 'Errors' }) : null,
+          ]);
+      },
+    });
+
+    mount(host, {
+      global: {
+        provide: { tabs },
+      },
+    });
+    await nextTick();
+
+    expect(tabs.value).toHaveLength(1);
+
+    // the registering instance goes away, but a matching instance is still
+    // mounted, so the shared tab must survive
+    firstVisible.value = false;
+    await nextTick();
+
+    expect(tabs.value).toHaveLength(1);
+    expect(tabs.value[0].title).toBe('Errors');
+
+    // once the last matching instance unmounts, the tab must disappear
+    secondVisible.value = false;
+    await nextTick();
+
+    expect(tabs.value).toHaveLength(0);
+  });
+
+  it('unregisters itself on unmount', async () => {
+    const { visible, tabs } = await mountConditionalTab();
+
+    expect(tabs.value).toHaveLength(1);
+
+    visible.value = false;
+    await nextTick();
+
+    expect(tabs.value).toHaveLength(0);
+  });
+
+  it('registers again after being re-rendered', async () => {
+    const { visible, tabs } = await mountConditionalTab();
+
+    visible.value = false;
+    await nextTick();
+    visible.value = true;
+    await nextTick();
+
+    expect(tabs.value).toHaveLength(1);
+    expect(tabs.value[0].title).toBe('Errors');
+  });
+
+  it('keeps template order when a tab mounts later than its siblings', async () => {
+    const middleVisible = ref(false);
+    const tabs = ref<TabType[]>([]);
+
+    const host = defineComponent({
+      setup() {
+        return () =>
+          h('div', [
+            h(Tab, { to: { name: 'repo-pipeline' }, title: 'Tasks' }),
+            middleVisible.value ? h(Tab, { to: { name: 'repo-pipeline-errors' }, title: 'Errors' }) : null,
+            h(Tab, { to: { name: 'repo-pipeline-config' }, title: 'Config' }),
+          ]);
+      },
+    });
+
+    mount(host, {
+      global: {
+        provide: { tabs },
+      },
+    });
+    await nextTick();
+
+    expect(tabs.value.map(({ title }) => title)).toStrictEqual(['Tasks', 'Config']);
+
+    middleVisible.value = true;
+    await nextTick();
+
+    expect(tabs.value.map(({ title }) => title)).toStrictEqual(['Tasks', 'Errors', 'Config']);
+  });
+});

web/src/components/layout/scaffold/Tab.vue

@@ -1,12 +1,19 @@
-<template><span /></template>
+<template><span ref="anchor" /></template>
 
-<script setup lang="ts">
-import { onMounted } from 'vue';
+<script lang="ts">
+import { markRaw, onBeforeUnmount, onMounted, toRaw, useTemplateRef } from 'vue';
 import type { RouteLocationRaw } from 'vue-router';
 
 import type { IconNames } from '~/components/atomic/Icon.vue';
+import type { Tab } from '~/compositions/useTabs';
 import { useTabsClient } from '~/compositions/useTabs';
 
+// owners per registered tab entry; module-level so all Tab instances share
+// it, WeakMap keeps the bookkeeping private
+const ownersByTab = new WeakMap<Tab, Map<symbol, HTMLElement>>();
+</script>
+
+<script setup lang="ts">
 const props = defineProps<{
   to: RouteLocationRaw;
   title: string;
@@ -16,8 +23,14 @@ const props = defineProps<{
   matchChildren?: boolean;
 }>();
 
+const anchor = useTemplateRef('anchor');
+
 const { tabs } = useTabsClient();
 
+const ownerId = Symbol('scaffold-tab-owner');
+let registeredTab: Tab | undefined;
+let ownerAnchor: HTMLElement | undefined;
+
 // TODO: find a better way to compare routes like
 // https://github.com/vuejs/router/blob/0eaaeb9697acd40ad524d913d0348748e9797acb/packages/router/src/utils/index.ts#L17
 function isSameRoute(a: RouteLocationRaw, b: RouteLocationRaw): boolean {
@@ -25,18 +38,61 @@ function isSameRoute(a: RouteLocationRaw, b: RouteLocationRaw): boolean {
 }
 
 onMounted(() => {
-  // don't add tab if tab id is already present
-  if (tabs.value.some(({ to }) => isSameRoute(to, props.to))) {
+  ownerAnchor = markRaw(anchor.value!);
+
+  // join an existing entry for the same route as co-owner instead of
+  // registering a duplicate
+  const existing = tabs.value.find(({ to }) => isSameRoute(to, props.to));
+  if (existing) {
+    registeredTab = toRaw(existing);
+    ownersByTab.get(registeredTab)?.set(ownerId, ownerAnchor);
     return;
   }
 
-  tabs.value.push({
+  const tab = {
     to: props.to,
     title: props.title,
     count: props.count,
     icon: props.icon,
     iconClass: props.iconClass,
     matchChildren: props.matchChildren,
-  });
+    anchor: ownerAnchor,
+  };
+  registeredTab = tab;
+  ownersByTab.set(tab, new Map([[ownerId, ownerAnchor]]));
+
+  // insert before the first tab whose anchor element comes after ours, so a
+  // tab mounting later than its siblings still ends up in template order
+  const index = tabs.value.findIndex(
+    ({ anchor: other }) =>
+      other !== undefined && (anchor.value!.compareDocumentPosition(other) & Node.DOCUMENT_POSITION_FOLLOWING) !== 0,
+  );
+  if (index === -1) {
+    tabs.value.push(tab);
+  } else {
+    tabs.value.splice(index, 0, tab);
+  }
+});
+
+onBeforeUnmount(() => {
+  if (registeredTab === undefined) {
+    return;
+  }
+
+  const owners = ownersByTab.get(registeredTab);
+  owners?.delete(ownerId);
+
+  if (owners !== undefined && owners.size > 0) {
+    // another instance of the same route is still mounted, keep the entry;
+    // transfer the anchor if it was ours so ordered insertion stays correct
+    if (registeredTab.anchor === ownerAnchor) {
+      registeredTab.anchor = owners.values().next().value;
+    }
+    return;
+  }
+
+  // compare raw objects because the tabs ref wraps its entries in reactive proxies
+  tabs.value = tabs.value.filter((tab) => toRaw(tab) !== registeredTab);
+  ownersByTab.delete(registeredTab);
 });
 </script>

web/src/compositions/useTabs.ts

@@ -12,6 +12,7 @@ export interface Tab {
   icon?: IconNames;
   iconClass?: string;
   matchChildren?: boolean;
+  anchor?: HTMLElement;
 }
 
 export function useTabsProvider() {