fix(notifications): re-measure bells when the mounted set changes - #149
Merged
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
notificationCenterOpenlives inuiStoreso a deep link can raise the popover, but eachNotificationBellmeasures its placement inuseEffect(…, [open])— once, on the open transition. Nothing re-measures when the set of mounted bells changes.MobileShellunmounts the foreground tab's bell on a tab switch (hosts / snippets / more / terminal) while the SFTP tab's bell stays mounted behindinvisible. So:nullposition it measured while hidden, and its effect does not re-run becauseopennever changed.open = false— a dead tap.The same hole swallows a
notificationdeep 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):expected +0 to be 1), green afterNotificationBell.test.tsx7/7,tsc --noEmitclean.Note
The full suite on this branch has 3 failures —
TitleBar.syncState(both tests) andAccountSection.handle(distinct copy for each claim-failure status), all 5s timeouts. They reproduce identically on cleanorigin/devwith this change stashed, so they are pre-existing and unrelated.