镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fix(channels): keep the side pane mounted under the image viewer - #3111

Merged
matt2e merged 3 commits into
mainfrom
opening-images-closes-sidebar
Nov 9, 2026
Merged

matt2e merged 3 commits into
mainfrom
opening-images-closes-sidebar

Conversation

@matt2e

@matt2e matt2e commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Why

Clicking an image in a channel timeline cleared Channel settings and unmounted the whole side pane for the length of the review. When the viewer closed, Channel settings was gone with no announcement, and any side panel that clears state on unmount had lost it: the native Huddle chat panel releases files you had attached but not sent. The side thread only survived because its text draft is saved per keystroke.

The setSettings(undefined) in openMediaReview dates from #163, when settings was a transient panel auto-closed by any other opening. #413 made it a retained tab and added the dock guards but left that clear behind.

What

  • openMediaReview no longer clears Channel settings.
  • The side pane stays mounted under the viewer. PanelDock already renders a closed dock as hidden, inert and aria-hidden when keepMounted holds, so keepMounted now also holds during a review (only when there is a panel to keep) and the pane's child guard no longer unmounts on it. Settings, side plugin panels (Huddle) and the Solo companion keep their state.
  • Side thread tabs still leave during a review, via a new hostPanes option on useConversationTabs. This is load-bearing, not a leftover: the viewer's comment composer reads the thread draft when it mounts and the thread composer reads the viewer's edits back when it remounts, so keeping the side thread mounted would hand back a stale draft. Threads moved to the main pane keep rendering through a review, as they did before.

Before / after

Left: main (3a1eb56). Right: this branch. Same steps: open ⋯ Channel actions → View channel details, click the image, press Escape.

before-after.mp4

Final frame: on main the timeline stretches to the window edge with no pane; on the branch Channel settings is still docked beside the timeline with its tab selected

Tests

  • New browser journey in tests/browser/channel-settings.spec.mjs: Channel settings stays open with its tab selected and focus returns to the tile after Escape; the dock is hidden and inert under the modal; a side-thread draft reaches the viewer's comment composer and comes back edited; a thread moved to the main pane keeps the same composer element connected through the review. Fails on main at the first pane assertion.
  • New useConversationTabs case: hostPanes: ["main"] withholds only the side host and re-hosts it afterwards, including a tab the visit showed but no longer selects.
  • channel-settings.spec.mjs and the existing "media review hands off the thread draft" journey pass in Chromium and WebKit; tsc --noEmit, Biome and Vitest for src/bundled/channels and src/features/panels are clean.

Not verified here: the native-only Huddle case (attached-but-unsent files surviving a review) needs a manual check in the native app.

Notes

  • Merges with fix(channels): make the covered conversation inert while a narrow pane covers it #2942 as a one-line, same-meaning conflict: that PR writes open={docked}; keep that plus this branch's keepMounted.
  • The other three setSettings(undefined) sites (Reply/open thread, opening a link into a side panel, opening a bottom drawer) are unchanged and belong in a follow-up once Channel settings is treated as a fully retained tab.
  • A main-pane thread on the same root as the reviewed image still keeps its pre-viewer draft, as on main; the composer's conflict banner surfaces it on the next keystroke. Unmounting only the reviewed root's thread is a possible follow-up.

🤖 Generated with Claude Code

matt2e and others added 3 commits November 9, 2026 15:11
Opening an image from the timeline cleared Channel settings and unmounted
every side-pane child for the length of the review. When the viewer closed,
Channel settings was gone, and any side panel that clears state on unmount
had lost it: the Huddle chat panel releases its attached files.

Keep the pane mounted under the modal instead. PanelDock already renders a
closed dock as hidden, inert and aria-hidden when keepMounted holds, so
openMediaReview no longer clears settings, keepMounted also holds during a
review, and the pane's child guard no longer unmounts on it. Thread tab
content still leaves during a review: the viewer's comment composer reads
the thread draft when it mounts and the thread reads the viewer's edits
back when it returns, so only secondary.hosts keeps the review guard.

Adds a browser journey to channel-settings.spec.mjs: Channel settings stays
open with its tab selected and focus returns to the tile after Escape, the
dock is hidden and inert under the modal, and a side-thread draft reaches
the viewer's comment composer and comes back edited.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: mtoohey <mtoohey@squareup.com>
79f25bb guarded every thread host on showingMediaReview. hosts covers
both panes, so a thread moved to the main pane now unmounted during a
review and came back without its scroll place, reply target or expanded
branches. Before that commit it stayed mounted and attached: the main
PanelWorkspace was never gated on a review.

useConversationTabs takes hostPanes, the panes whose thread content
renders now, and ChannelsPage passes ["main"] during a review, so only
side thread tabs leave. The review state moves above the hook call,
which now reads it. keepMounted holds during a review only when there
is a panel to keep, instead of rendering an empty hidden dock.

The side-thread guard is not a no-op kept from main, as its comment and
the earlier message said. On main the side thread stayed React-mounted
in a detached portal subtree: TabSlot removed its node when the pane
children unmounted while hosts kept portaling into it, which is the
stale-draft state of the rejected keep-everything-mounted probe. The
guard is what makes the viewer's edits reach the thread composer, which
reads its draft only on mount; the comment now says so.

Tests: a hook case opens a side and a main thread and shows hostPanes
["main"] withholds only the side host and hosts it again afterwards.
The browser journey moves the side thread to the main pane, reviews the
image and asserts the same composer element stays connected and the
thread stays in the DOM while the viewer is up; it fails when a review
withholds both panes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: mtoohey <mtoohey@squareup.com>
The hostPanes case added in bc7c74b said the side thread was "hosted
again without being shown" after hostPanes dropped. It was not: after
openThreadTab(..., "main") the side selection is still r1 and the pane is
open, so on rerender r1 is hosted through the active branch, which never
reaches the visitTab.shown retention the comment described.

Close the pane with setPaneOpen(false) before the rerender that drops
hostPanes. A side tab is active only while the pane is open and it is
selected, so r1 can now return only if the visit still remembers showing
it from before the withholding. The case asserts paneOpen is false and
r1 is hosted alongside r2, and the comment says what it checks: a
hostPanes withholding must not forget which tabs the visit showed.

Test-only; no production code changes. vitest for the file: 45 passed.
Biome check on the file: clean.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: mtoohey <mtoohey@squareup.com>
@matt2e
matt2e requested a review from a team as a code owner November 9, 2026 16:11

@kazemek21 kazemek21 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Kyle reviewed this PR and the Nexus review-tracker notes and approved with no comments.

Posted by Codex (gpt-6.1-sol) on behalf of kylez.

@matt2e
matt2e merged commit 8bb96dd into main Nov 9, 2026
62 checks passed
@matt2e
matt2e deleted the opening-images-closes-sidebar branch November 9, 2026 21:41
wpfleger96 added a commit that referenced this pull request Nov 9, 2026
…restricted-communities

* origin/main:
  ci: give WebKit shards headroom and keep fixture servers' idle connections open (#3074)
  fix(messages): offer Retry as soon as a send fails (#3080)
  fix(agents): link Settings from key failures and quote the provider (#3081)
  fix(identity): link Builderlab from Identity sync where its page holds the next step (#3082)
  fix(startup): show a recoverable screen when Buzz fails to open or crashes (#3085)
  fix(attention): ask before deleting a watch (#3092)
  fix(agents): remove the owner-access read tools (#3123)
  fix(channels): keep the side pane mounted under the image viewer (#3111)
  fix(solo): ask before Escape or Close drops a new workspace's typed fields (#3114)

# Conflicts:
#	docs/configuration.md
#	vite.config.ts
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.

2 participants