Skip to content

feat(site): desktop panel toolbar, zoom modes, and pop-out window#25585

Open
tracyjohnsonux wants to merge 40 commits into
mainfrom
feat/desktop-panel-redesign-v2
Open

feat(site): desktop panel toolbar, zoom modes, and pop-out window#25585
tracyjohnsonux wants to merge 40 commits into
mainfrom
feat/desktop-panel-redesign-v2

Conversation

@tracyjohnsonux
Copy link
Copy Markdown
Contributor

@tracyjohnsonux tracyjohnsonux commented May 21, 2026

Redesigns the agent desktop panel with a persistent toolbar, zoom modes, and a detachable pop-out window.

Changes

Toolbar (DesktopToolbar)

  • Persistent top bar with right-aligned controls: Take/Release control, Zoom toggle, Detach
  • All buttons use consistent subtle variant with icon + label
  • h-8 height, bg-surface-primary background with bottom border

Zoom modes

  • Defaults to fit-to-window (scaleViewport = true) so the full 1920x1080 desktop is visible
  • Toggle to native 100% resolution via toolbar button or keyboard shortcuts (Ctrl+0 fit, Ctrl+1 native)
  • noVNC background color overridden from hardcoded rgb(40,40,40) to --surface-secondary so letterbox margins match the app theme in light and dark mode

Pop-out window

  • New route at /agents/:agentId/desktop for a dedicated desktop window
  • Opens via toolbar "Detach" button at 50% of screen size, centered
  • BroadcastChannel coordination: sidebar shows placeholder with "Bring back" button
  • Closing the pop-out window automatically restores the sidebar panel

Other

  • useDesktopConnection hook accepts scaleViewport option, synced to the RFB instance via a secondary effect
  • DesktopPanelContext extended with agent and workspace fields
  • Replaces the previous hover-overlay take/release control UX with toolbar buttons

Generated by Coder Agents on behalf of @tracyjohnsonux

…m, toolbar, and pop-out

- Default to 100% native resolution (scaleViewport=false) with
  scroll-wheel panning for navigating the desktop at full size
- Add DesktopToolbar overlay with workspace app icons (left),
  zoom toggle, pop-out button, and take/release control (right)
- Support pop-out to dedicated browser window at /agents/:agentId/desktop
  with resizeSession=true so remote desktop matches window size
- BroadcastChannel coordination between sidebar panel and pop-out
- Keyboard shortcuts: Ctrl+0 for fit-to-window, Ctrl+1 for 100%
- Toolbar auto-hides to thin strip when controlling desktop
- Hook accepts scaleViewport and resizeSession options to avoid
  direct RFB mutation (React Compiler compatible)
- Extended DesktopPanelContext with agent and workspace data
  for toolbar app icon rendering
…n, 50% pop-out

- Make toolbar always visible (remove auto-hide when controlling)
- Replace dock-style app icons with Mac-like 'Apps' dropdown menu
- Pop-out window defaults to 50% screen size with native (100%) zoom
- Toolbar uses compact h-8 height with border-bottom styling
Fetch chat and workspace data in DesktopPopoutPage so the toolbar
has access to workspace agent apps for the Apps dropdown menu.
Use capture phase on the wheel handler so it fires before noVNC
forwards the event to the remote desktop. This prevents XFCE
workspace switching when scrolling to pan the desktop view.
Change toolbar from absolute positioning to flex column layout so
it takes its own space above the VNC canvas instead of overlapping
the desktop content. Applies to both sidebar panel and pop-out.
… toolbar

Command-type apps open terminal tabs when clicked, which is wrong
for the desktop context. Only show web apps (those without a command
property) in the Apps dropdown.
…pdown

Add desktop apps (Chrome) to the toolbar Apps dropdown that launch
inside the VNC session via a fire-and-forget terminal connection
with DISPLAY=:1 set. Separate from web apps (code-server) which
open in browser tabs. Desktop apps list is configurable via the
DesktopApp interface.
Use the agent's reconnecting PTY websocket API to execute desktop
app commands with DISPLAY=:1 set. Sends the command as raw text
through /api/v2/workspaceagents/{id}/pty, then closes the socket
after a short delay. No more popup windows.
…iner in fit mode

When scale mode is 'fit', enable resizeSession so the remote
desktop resolution matches the panel size. Chrome maximized
fills exactly the available space. Resizing the panel adjusts
the desktop resolution accordingly.
- Sync scaleViewport/resizeSession to RFB via ref mutation,
  triggered by rfbInstance state changes and option changes
- biome-ignore for rfbInstance dep (triggers re-run on connect)
- Add overflow-hidden on pop-out VNC container to eliminate right
  padding from canvas size mismatch
In fit mode: overflow-hidden since resizeSession makes the desktop
match the container exactly. In native mode: overflow-scroll with
hidden scrollbar styling so the wheel handler can pan without
visible scrollbars.
…n container

The offscreen div that holds the noVNC canvas was missing
overflow:hidden, so the native-resolution canvas caused
scrollbars on the inner container.
Always preventDefault+stopPropagation on wheel events in both the
sidebar panel and pop-out window. In native mode, translate to
scroll panning. In fit mode, just block. Prevents XFCE workspace
switching regardless of zoom mode or which view is active.
…n div

noVNC internally sets overflow:auto on its screen container div
(rfb.js line 236), causing scrollbars. Override it to hidden
immediately after RFB creation.
Remove resizeSession option since portabledesktop hardcodes
1920x1080 and the VNC server does not honor RFB resize requests.
Remove overflow:hidden hacks that were clipping content instead
of actually resizing. Restore clean two-mode approach:
- Fit mode: scaleViewport shrinks desktop to fit, overflow hidden
- Native mode: full resolution, hidden scrollbars, wheel panning
Remove wheel event capture, scrollRef, hidden-scrollbar styling,
and VncContainer component. The scroll-wheel panning approach
does not work well with the fixed 1920x1080 VNC framebuffer.
Both modes now use simple overflow:hidden containers.
The reconnecting PTY expects JSON messages ({data: '...'}), not
raw text bytes. Send the launch command as a JSON-encoded
ReconnectingPTYRequest so the agent actually executes it.
…launcher

Remove the Apps dropdown, DesktopApp interface, PTY launcher,
agent/workspace data fetching in pop-out, and all related plumbing.
Desktop apps are better handled as native XFCE desktop shortcuts
configured in the template startup script. The toolbar now contains
only zoom toggle, pop-out, and control buttons.
…le style

Move Take/Release control to left side of toolbar. Use subtle
variant (no outline) for both states. Rename to 'Take control'
and 'Release control' with full labels.
Scale viewport to fit the container by default so the full desktop
is visible without scrolling. Users can toggle to native resolution
with the zoom button or Ctrl+1.
Replace icon-only zoom button with labeled buttons:
- Scaling icon + 'Zoom to fit' (when in native mode)
- Maximize icon + 'Zoom to 100%' (when in fit mode)
Use bg-surface-primary for the VNC container background so the
letterbox bars in fit-to-window mode match the chat message boxes
in both light and dark themes.
noVNC hardcodes rgb(40,40,40) as its screen background, which
creates a visible mismatch with the app surface colors. Read
the --surface-primary CSS variable at connect time and apply it
to the RFB background property so the letterbox margins match
the chat message boxes in both light and dark mode.
Read the CSS variable into a local before setting rfb.background.
The previous code set an invalid value first, then read back the
getter (which returns the old screen style), producing a corrupted
hsl() string.
The chat message cards use bg-surface-secondary, which sits on top
of the surface-primary page background. Match the desktop letterbox
margins to surface-secondary so they blend with the message cards.
@tracyjohnsonux tracyjohnsonux changed the title feat(site/src/pages/AgentsPage): desktop panel redesign with 100% zoom, toolbar, and pop-out feat(site/src/pages/AgentsPage): desktop panel toolbar, zoom modes, and pop-out window May 21, 2026
…lContext

These were added for a planned apps dropdown that was removed from
scope. No consumer reads them.
Copy link
Copy Markdown
Contributor Author

/coder-agents-review

Copy link
Copy Markdown

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

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

First-pass review (Netero). This is a mechanical first pass only; the full review panel has not yet reviewed this PR. The panel will review after these findings are addressed.

The PR cleanly separates toolbar, panel, and pop-out page concerns, and the BroadcastChannel coordination pattern is well-chosen for cross-window state sync.

4 P2, 1 P3, 1 Nit. The P2s cluster around duplication (three copies of ScaleMode, two copies of CHANNEL_PREFIX, two copies of the keyboard shortcut handler) and missing Storybook coverage for 222 lines of new component code.

"Export from a shared location." Netero, tapping his finger on the duplicated constants


site/src/pages/AgentsPage/components/RightPanel/DesktopPanel.stories.tsx:6

Nit [DEREM-7] defaults omits onPopOut, so no story ever renders the Detach button. Adding a Connected variant with onPopOut: fn() would cover that branch. (Netero)

🤖

🤖 This review was automatically generated with Coder Agents.

Comment thread site/src/pages/AgentsPage/DesktopPopoutPage.tsx Outdated
Comment thread site/src/pages/AgentsPage/DesktopPopoutPage.tsx Outdated
Comment thread site/src/pages/AgentsPage/DesktopPopoutPage.tsx Outdated
Comment thread site/src/pages/AgentsPage/components/RightPanel/DesktopToolbar.tsx
Comment thread site/src/pages/AgentsPage/DesktopPopoutPage.tsx
Comment thread site/src/pages/AgentsPage/hooks/useDesktopConnection.ts
- Extract ScaleMode type and CHANNEL_PREFIX to shared desktopConstants.ts (DEREM-1, DEREM-2)
- Extract zoom keyboard shortcuts to useZoomShortcuts hook (DEREM-3)
- Add DesktopToolbar Storybook stories for all states (DEREM-4)
- Add onPopOut to DesktopPanel story defaults (DEREM-7)
- Fix stale scaleViewport comment in useDesktopConnection (DEREM-6)
Copy link
Copy Markdown
Contributor Author

/coder-agents-review

Copy link
Copy Markdown

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

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

Round 2 is blocked. 5 of 7 findings from round 1 were addressed in 137f439 (shared constants file, extracted hook, new toolbar stories, comment fix). Good progress.

However, DEREM-5 (no Storybook story for DesktopPopoutPage, a 122-line page component with four status branches and BroadcastChannel lifecycle) received no response and no fix. The commit message references DEREM-1/2/3/4/6/7 but not DEREM-5.

Further review is blocked until the author either adds a story for DesktopPopoutPage or responds to explain why one is not needed. The full review panel will run once all first-pass findings are addressed.

🤖 This review was automatically generated with Coder Agents.

Remove querySelector-based overflow override from try/catch (breaks
React Compiler). Use Tailwind child selector instead. Also removes
the screenEl code that was accidentally included in the prior commit.
Extract DesktopPopoutPageView from the route component so all four
status branches (connecting, connected, error, disconnected) can be
covered by Storybook stories without needing router or WebSocket
infrastructure.
Copy link
Copy Markdown
Contributor Author

/coder-agents-review

Copy link
Copy Markdown

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

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

First-pass review, round 3 (Netero). All 7 findings from round 1 are confirmed fixed. Good work consolidating shared types, constants, and the zoom shortcut hook.

2 new P2, 1 P3. The P2s are a 3.6MB unreferenced PNG that permanently inflates repo history, and the same class of type-duplication bug that DEREM-2 addressed (this time DesktopConnectionStatus in three files).

This is a mechanical first pass only; the full review panel has not yet reviewed this PR. The panel will review after these findings are addressed.

"Same class of bug, different type alias." Netero, shaking his head


site/static/Coder_Background_Dark.png:1

P2 [DEREM-8] This 3.6MB PNG has zero references anywhere in the codebase. grep -rn 'Coder_Background' across the entire tree returns nothing. No component, story, HTML template, or config imports it. Dead on arrival, and at 3.6MB it permanently inflates repository history. Remove it from the commit. (Netero)

🤖

🤖 This review was automatically generated with Coder Agents.

Comment thread site/src/pages/AgentsPage/DesktopPopoutPage.tsx Outdated
Comment thread site/src/pages/AgentsPage/DesktopPopoutPage.stories.tsx
- Change role='toolbar' to role='group' (toolbar requires arrow-key
  navigation we don't implement)
- Add aria-pressed to Take/Release control toggle button
- Replace pointer-events-none with inert attribute on VNC container
  when not controlling, with role='application' and descriptive
  aria-label for screen readers
- Add role='status' to popped-out placeholder for screen reader
  announcement
- Remove unused cn import from DesktopPopoutPage
@tracyjohnsonux tracyjohnsonux changed the title feat(site/src/pages/AgentsPage): desktop panel toolbar, zoom modes, and pop-out window feat(site): desktop panel toolbar, zoom modes, and pop-out window May 22, 2026
@tracyjohnsonux tracyjohnsonux marked this pull request as ready for review May 22, 2026 04:23
…fault

Tests assumed scaleViewport defaulted to true at RFB creation.
The hook now inits to false and syncs via a secondary effect, so
tests that verify scaleViewport=true must pass it explicitly.
- Export DesktopConnectionStatus from desktopConstants.ts, import
  in all consumers instead of defining locally (DEREM-9)
- Add play functions to DesktopToolbar and DesktopPopoutPage stories
  to verify button callbacks and status-conditional rendering (DEREM-10)
Copy link
Copy Markdown
Contributor Author

/coder-agents-review

The ResizeObserver callback re-assigns the current scaleViewport
value to trigger a noVNC rescale. It does not force a specific
value. Test now verifies the setter was called rather than checking
for a specific boolean.
Copy link
Copy Markdown

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

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

Panel review, round 4 (Bisky, Hisoka, Mafuuu, Mafu-san, Pariston, Nami, Meruem, Gon, Leorio). All 10 prior Netero findings confirmed fixed. The shared constants extraction, zoom shortcut hook, and story coverage are all clean.

The toolbar redesign, zoom modes, and pop-out architecture are well-structured. Pariston systematically challenged four framings of the problem and found no misalignment; the BroadcastChannel pattern is the simplest viable coordination mechanism, and the toolbar solves real discoverability issues.

1 P2, 6 P3, 2 Nit. The P2 is a convergent finding from 4 reviewers: the BroadcastChannel protocol has no discovery handshake, so parent navigation drops coordination state and creates dual VNC sessions.

Process observations (Mafu-san): The commit history shows a speculative development pattern (CSS override committed untested, reverted 117s later; an entire desktop launcher feature built across 8 commits then fully removed in 1). The unreferenced 3.6MB PNG (DEREM-8, now fixed) was introduced during a review-fix commit. These are workflow signals, not blocking code defects.

"The pop-out sends popout-opened exactly once. The parent's listener is registered in a useEffect. If the user navigates away, DesktopPanel unmounts, the channel closes. When the user navigates back, a fresh DesktopPanel mounts with isPoppedOut = false. The protocol has no discovery mechanism." Mafuuu, tracing the lifecycle

🤖 This review was automatically generated with Coder Agents.

Comment thread site/src/pages/AgentsPage/components/RightPanel/DesktopPanel.tsx
Comment thread site/src/router.tsx Outdated
Comment thread site/src/pages/AgentsPage/hooks/useZoomShortcuts.ts
Comment thread site/src/pages/AgentsPage/hooks/useDesktopConnection.test.ts
Comment thread site/src/pages/AgentsPage/DesktopPopoutPage.tsx
Comment thread site/src/pages/AgentsPage/DesktopPopoutPage.tsx Outdated
Comment thread site/src/pages/AgentsPage/DesktopPopoutPage.tsx
Comment thread site/src/pages/AgentsPage/hooks/useDesktopConnection.ts Outdated
Comment thread site/src/pages/AgentsPage/hooks/useDesktopConnection.ts Outdated
- Move misplaced DesktopConnectionStatus import to top (DEREM-11)
- Add BroadcastChannel retry for pop-out race condition (DEREM-13)
- Move desktop route inside RequireAuth (DEREM-14)
- Add scaleViewport toggle test (DEREM-16)
- Match error text between panel and popout views (DEREM-18)
- Remove redundant sync comment (DEREM-19)
Copy link
Copy Markdown
Contributor Author

/coder-agents-review

Copy link
Copy Markdown

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

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

Round 5 is blocked. Strong progress: 6 of 9 open findings addressed in 6d0fb13 (BroadcastChannel handshake retry, route moved inside RequireAuth, scaleViewport toggle test, improved error message, import fix, comment removal). DEREM-12 and DEREM-17 acknowledged with reasonable author explanations.

However, DEREM-15 (zoom shortcuts fire simultaneously with noVNC key forwarding when controlling the remote desktop) was deferred with "Noted for follow-up" but no linked ticket or issue. The author acknowledged the concern is valid and suggested suppressing shortcuts while isControlling is true.

This needs a human decision: fix in this PR, file a GitHub issue to track the follow-up, or explicitly accept that Ctrl+0/Ctrl+1 will fire both locally and on the remote desktop when controlling. The review panel cannot accept a known gap as permanent.

🤖 This review was automatically generated with Coder Agents.

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