Skip to content

fix(notifications): re-measure bells when the mounted set changes - #149

Merged
kipavy merged 1 commit into
devfrom
fix/notification-bell-revisibility
Aug 18, 2026
Merged

fix(notifications): re-measure bells when the mounted set changes#149
kipavy merged 1 commit into
devfrom
fix/notification-bell-revisibility

Conversation

@kipavy

@kipavy kipavy commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

The bug

notificationCenterOpen lives in uiStore so a deep link can raise the popover, but each NotificationBell measures its placement in useEffect(…, [open]) — once, on the open transition. Nothing re-measures when the set of mounted bells changes.

MobileShell unmounts the foreground tab's bell on a tab switch (hosts / snippets / more / terminal) while the SFTP tab's bell stays mounted behind invisible. So:

  1. Open the notification centre on the hosts tab — one popover, correct.
  2. Switch to the SFTP tab. The hosts bell unmounts. The SFTP bell is now the visible one, but it still holds the null position it measured while hidden, and its effect does not re-run because open never changed.
  3. Result: no popover paints, the store still says open, the bell button is still drawn in its open style, and the first tap on it only sets open = false — a dead tap.

The same hole swallows a notification deep link raised while no visible bell is mounted at all (terminal tab with zero sessions, or any pushed full-screen page — host edit, keychain, members): nothing paints until a bell happens to mount.

The fix

A module-level mount counter, read through useSyncExternalStore. Every bell bumps it on mount and unmount, and the placement effect depends on it, so all bells re-measure when the mounted set changes and the popover follows the bell that is on screen.

Tests

Three tests over a two-bell harness that mirrors MobileShell (foreground bell mounted conditionally, SFTP bell always mounted):

  • only the visible bell paints while both are mounted
  • the popover follows the bell a tab switch makes visible — red before this change (expected +0 to be 1), green after
  • a link raised with no visible bell paints once one mounts

NotificationBell.test.tsx 7/7, tsc --noEmit clean.

Note

The full suite on this branch has 3 failures — TitleBar.syncState (both tests) and AccountSection.handle (distinct copy for each claim-failure status), all 5s timeouts. They reproduce identically on clean origin/dev with this change stashed, so they are pre-existing and unrelated.

The notification centre's open state is global, but each bell measured its
placement once, on the open transition. The mobile shell unmounts the
foreground tab's bell on a tab switch while the SFTP tab's bell stays mounted
behind `invisible`, so after a switch the surviving bell still held the `null`
position it measured while hidden: the popover stopped painting even though the
store said open, and the first tap on the visible bell only closed it.

Bells now re-measure whenever any bell mounts or unmounts, so the popover
follows the bell the user can actually see.
@kipavy
kipavy merged commit 5890c1e into dev Aug 18, 2026
4 checks passed
@kipavy
kipavy deleted the fix/notification-bell-revisibility branch August 18, 2026 23:46
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