Skip to content

fix(tabs): the tab context menu bulk closes and pin - #41

Merged
compgeniuses merged 2 commits into
consolidationfrom
prd/tab-menu-fix
Oct 23, 2026
Merged

compgeniuses merged 2 commits into
consolidationfrom
prd/tab-menu-fix

Conversation

@compgeniuses

Copy link
Copy Markdown
Contributor

The tab context menu's bulk closes did nothing, and the pin item was labelled with an icon's name. Both are fixed here, each with the test that proves it.

Why the closes did nothing

useTabs refreshed its tab list only when the router's location changed. But every one of these actions writes the list through URLTabSyncEngine.updateURL, which changes the URL with history.replaceState — something React Router never sees. So:

  • Close Other Tabs / Close Tabs to Right / Close Tabs to Left wrote the new list to the URL, left the hook holding the list it started with, and the tabs stayed on screen. The action looked like it did nothing.
  • Only the two actions that also navigated appeared to work — Close All Except Pinned, and closing the active tab — because the navigation is what refreshed the list.

The hook now follows the engine's own notifications, which fixes the whole family at once. Pin and unpin were stale for exactly the same reason, which is why they are in this PR.

Close Other Tabs also navigates to the tab it kept, so the page follows the tab that became active instead of showing a tab that is gone — which is what you were doing by hand ("i cant close other tabs without navigating inside them").

The pin item

It carried the icon's name as its label — MapPinIcon Tab — the same class of leak the tab titles were already fixed for, and unlike every other item it left the menu open over the tab it had just changed. It now reads Pin Tab / Unpin Tab and closes the menu.

Verified

  • src/hooks/tabs/__tests__/useTabsBulkClose.test.tsx (new) drives the real hook over the real engine for the five closes, and mounts the three tabs the URL describes. Written first and watched fail: 4 of the 6 failed before the fix — the tab list simply did not change.
  • The pin item's label and its menu closing are covered in TabLabelRendering.test.tsx, which already guarded against icon names leaking into labels.
  • 264 tests pass across src/hooks/tabs, src/services/tabs and src/components/tabbedWorkspace.

Not in this PR

Two things I noticed while working and deliberately did not chase here:

  • Duplicate Tab and Open in New Window leave the menu open too. They act correctly; only the menu's dismissal is inconsistent. Same one-line shape as the pin fix if you want it.
  • The browser console reports Index creation warning for app_settings: no such column: "[object Object]" on first run — a real defect in index creation, unrelated to tabs.

…ork [tab-menu]

Two faults, both in what the menu's actions wrote rather than in the menu itself.

The hook refreshed its tab list only from a router location change, but every one of these actions
writes the list through URLTabSyncEngine.updateURL, which changes the URL with
history.replaceState - something React Router never sees. So closing the other tabs, or the tabs to
one side, changed the URL and left the tabs on screen: the action looked like it did nothing. Only
the actions that also navigated (close all but pinned, closing the active tab) appeared to work. The
hook now follows the engine's own notifications, which fixes the whole family at once - pin and
unpin were stale for the same reason. Close Other Tabs also navigates to the tab it kept, so the
page follows the tab that became active instead of showing one that is gone.

The pin item carried the icon's name as its label ('MapPinIcon Tab' - the same class of leak the tab
titles were fixed for) and, unlike every other item, left the menu open over the tab it had just
changed.

Tests: useTabsBulkClose.test.tsx drives the real hook over the real engine for the five closes; the
pin item's label and its menu closing are covered in TabLabelRendering. 264 tests pass across the
tab subsystem.
@compgeniuses
compgeniuses merged commit 4348d68 into consolidation Oct 23, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant