perf(sidebar): stop rerendering every row's menu on channel switch

[loganj]

🤖

Summary

• Switching between channels you have already opened ("warm switching") got slower. The browser test that measures it fails when a switch takes more than 200 ms, and it was failing often in CI.
• One cause: every sidebar row has a right-click menu (Mute, Mark read, New session, and so on). Since mute and read actions were added in feat(channels): mute conversations and mark them read or unread #91, every row has that menu. The code for each row's menu sat outside the part of the row that React skips when nothing about the row changed. So on every channel switch, all rows rendered their menus again, even though only two rows changed (the old and new selected channel).
• The menu now lives inside the row. Only the row whose menu is open gets menu content. Rows that did not change skip rendering on a switch.
• Nothing changes on screen, including the menus.

Details

• With the CPU slowed 4x to match CI speed, the median warm switch in tests/browser/channel-opening.spec.mjs fell from 257 ms to 164 ms (24 samples each, main vs this branch).
• The 200 ms limit is unchanged.
• This fixes the slowdown from feat(channels): mute conversations and mark them read or unread #91. A second slowdown of about 60 ms came with the persistent sidebar (Keep pages and channels in a persistent sidebar #234). That one is still there and will be a separate PR.
• Changed files: ChannelSidebarItem.tsx now owns the row's ContextMenuRoot and MenuPopup. ChannelSidebar.tsx passes open state, anchor and menu content for the open row only, plus stable close and focus-return callbacks.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

No blocking defects found at b0b802805a7ab61303e1dbd86011a1e19f9d20ea. Reviewed the two-file feature diff against merge-base bebcd66e0adb27efa2c320f2bf72b8f3e63a8a45 (target snapshot 0d703a60a52ada4d8b60324766b83d2ee1380de1).

The memo boundary now includes the menu provider/popup and closed rows receive stable props. The open row still receives fresh action/status content; selection, eligibility, permission/read subscriptions, placement invalidation and focus-return ownership are preserved. One non-blocking closing-animation detail is noted inline.

• Existing CI is green at run 36157040493, attempt 2. Measurements came from attempt 1 and were carried forward; the rerun addressed a WebKit user-status.spec.mjs failure outside this diff. The tested merge was 8a42d723c076a6aa9d53c1112b3527df9ec57282, combining this head with 0d703a60, not a head-only run. Chromium/WebKit menu journeys passed.
• Performance evidence is directional, not a reproduced effect size. Hosted Chromium warm samples were 97.8/104.6/97.7/111.7 ms (median 101.2), versus median 159.6 ms in the separate main run at 0d703a60. Four samples on separate runners do not independently establish the author’s local 4x-throttled 257→164 ms claim. All PR samples pass the unchanged 200 ms hard ceiling; two miss the 100 ms target. Linux WebKit’s cold/warm-opening case remains excluded, and packaged-native performance was not verified.
• Non-blocking test follow-up: the existing ChannelSidebar.test.tsx already checks unchanged-row renders. Extend that observation to the menu provider/popup to directly protect this optimization, and remove the now-redundant outer ContextMenuRoot in ChannelSidebarItem.test.tsx. No new browser matrix is needed.

This is a COMMENT review, not approval. No local builds, tests or browser execution were performed, and no CI was rerun.

[loganj]

🤖 Addressed the review in 5e8c2a6, rebased on main 80511fa:

• Closing fade (inline P3): fixed and resolved. See the thread.
• Test follow-up: ChannelSidebar.test.tsx now watches the real ContextMenuRoot and MenuPopup. It checks that an unchanged row renders neither one on a channel switch, and that menu roots render only for rows that rendered. The check fails on main's sidebar code. I removed the redundant outer ContextMenuRoot from ChannelSidebarItem.test.tsx.
• Performance evidence: agreed that four hosted samples are only directional. The local numbers used 4x CPU throttling, with 24 samples per commit, and I measured each comparison in a single run on the same machine. At the new head, the median fell from 215 ms to 161 ms against main 80511fa in the same run. I did not measure packaged native builds.