Skip to content

fix: keep tray panel height stable when provider content changes#342

Open
prayzey wants to merge 8 commits intorobinebers:mainfrom
prayzey:fix/stable-tray-panel-height
Open

fix: keep tray panel height stable when provider content changes#342
prayzey wants to merge 8 commits intorobinebers:mainfrom
prayzey:fix/stable-tray-panel-height

Conversation

@prayzey
Copy link
Copy Markdown

@prayzey prayzey commented Apr 4, 2026

Description

Fixes the tray panel resizing vertically when switching between providers or views with different amounts of content.

Before this change, the window height followed the current content height, so short states and longer states made the whole panel jump up and down.

This change keeps a stable preferred panel height and lets the content scroll inside the panel when needed.

Related Issue

N/A

Related to the general sizing/compactness discussion in #106, but this specifically fixes panel height jitter rather than adding a compact mode.

Type of Change

  • Bug fix
  • New feature
  • New provider plugin
  • Documentation
  • Performance improvement
  • Other (describe below)

Testing

  • I ran bun run build and it succeeded
  • I ran bun run test and all tests pass
  • I added regression tests for stable panel height and small-screen clamping

Screenshots

  • Before: panel height changes depending on which provider state is visible
image image
  • After: panel height stays stable and only the inner content scrolls
image

newosu

Checklist

  • I read CONTRIBUTING.md
  • My PR targets the main branch
  • I did not introduce new dependencies without justification

Summary by cubic

Keeps the tray panel height stable across provider/view changes so the window stops jumping. Uses a preferred 560px height with in-panel scrolling, clamps for small monitors, and refines the scrollbar and layout for better spacing.

  • Bug Fixes
    • Stable panelHeightPx with a fixed card height in AppShell; dedup window resizes; recalc on DPI/scale changes and window moves.
    • Clamp height to monitor bounds with a small minimum; content scrolls inside the panel.
    • Thin .panel-scroll on the outer edge; padding moved to an inner content wrapper; footer now has px-3 to match the gutter; updated styles and tests.

Written for commit 4410fb4. Summary will update on new commits.

@github-actions github-actions bot added the core label Apr 4, 2026
@prayzey prayzey marked this pull request as ready for review April 4, 2026 22:10
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/hooks/app/use-panel.ts">

<violation number="1" location="src/hooks/app/use-panel.ts:115">
P2: Panel height clamp is not recalculated on monitor/scale context changes, so fixed-height tray can remain too tall after moving to a smaller display.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

@validatedev
Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Collaborator

@validatedev validatedev left a comment

Choose a reason for hiding this comment

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

One issue I have seen behaviorally that users expect a scrollbar to appear while scrolling, similar to how browsers handle it, and I don't have such thing while testing your build. Not sure whether it should stay visible at all times or only show during active scrolling. Nice thinking though, thanks!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR stabilizes the Tauri tray panel height across provider/view changes to prevent vertical “jitter,” using a preferred fixed panel height and relying on inner scrolling when content overflows.

Changes:

  • Introduces a preferred panel height (560px) with monitor-based clamping in usePanel, instead of sizing the window to content height.
  • Updates AppShell to use a fixed height (and maxHeight) for the card container derived from the computed panel height.
  • Adds regression tests for preferred-height sizing and small-monitor clamping.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
src/hooks/app/use-panel.ts Computes a stable panelHeightPx (preferred + clamped) and resizes the Tauri window accordingly.
src/components/app/app-shell.tsx Applies a fixed card height based on panelHeightPx so overflow scrolls inside the panel.
src/App.test.tsx Adds assertions for preferred height (560px) and clamping behavior on smaller monitors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +114 to +122
// Keep the tray panel visually stable; scrolling should happen inside the shell.
const nextPanelHeightLogical = Math.max(1, Math.min(PANEL_PREFERRED_HEIGHT, maxHeightLogical))

if (panelHeightPxRef.current !== nextPanelHeightLogical) {
panelHeightPxRef.current = nextPanelHeightLogical
setPanelHeightPx(nextPanelHeightLogical)
}

const desiredHeightPhysical = Math.ceil(desiredHeightLogical * factor)
const height = Math.ceil(Math.min(desiredHeightPhysical, maxHeightPhysical!))
const height = Math.ceil(Math.min(nextPanelHeightLogical * factor, maxHeightPhysical!))
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

resizeWindow now sets a fixed preferred height, but the effect still reruns on activeView/displayPlugins changes. Since displayPlugins updates frequently, this can cause repeated currentMonitor() calls and setSize() calls even when the target size hasn't changed. Consider removing displayPlugins from the dependency list (or running this effect only on mount) and/or caching the last {width,height} in a ref to skip no-op setSize calls.

Copilot uses AI. Check for mistakes.
Comment on lines +114 to +116
// Keep the tray panel visually stable; scrolling should happen inside the shell.
const nextPanelHeightLogical = Math.max(1, Math.min(PANEL_PREFERRED_HEIGHT, maxHeightLogical))

Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

nextPanelHeightLogical can be clamped below the shell's non-content overhead (padding + arrow), especially on very small monitors or high devicePixelRatio. In that case the UI will be clipped because AppShell enforces a minimum card height of 1px. Consider enforcing a minimum panel height that accounts for the shell overhead (or moving the overhead constant into the sizing logic) so the total window height can always fit the chrome.

Copilot uses AI. Check for mistakes.
Comment on lines 68 to 76
const cardHeightPx = panelHeightPx ? Math.max(1, panelHeightPx - ARROW_OVERHEAD_PX) : null

return (
<div ref={containerRef} className="flex flex-col items-center p-6 pt-1.5 bg-transparent">
<div className="tray-arrow" />
<div
className="relative bg-card rounded-xl overflow-hidden select-none w-full border shadow-lg flex flex-col"
style={maxPanelHeightPx ? { maxHeight: `${maxPanelHeightPx - ARROW_OVERHEAD_PX}px` } : undefined}
style={cardHeightPx ? { height: `${cardHeightPx}px`, maxHeight: `${cardHeightPx}px` } : undefined}
>
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

cardHeightPx is derived from panelHeightPx using a fixed ARROW_OVERHEAD_PX, but cardHeightPx is clamped to at least 1px. If panelHeightPx is ever < ARROW_OVERHEAD_PX (possible with small monitors/high DPI), the card + overhead will no longer fit in the window and the top/bottom chrome may be clipped. Consider clamping panelHeightPx to at least ARROW_OVERHEAD_PX + 1 (or computing these values in one place) to keep the layout internally consistent.

Copilot uses AI. Check for mistakes.
src/App.test.tsx Outdated
Comment on lines +1171 to +1193
it("keeps a stable preferred panel height when content is shorter", async () => {
state.isTauriMock.mockReturnValue(true)
state.currentMonitorMock.mockResolvedValueOnce({ size: { height: 1000 } })
render(<App />)

await waitFor(() =>
expect(state.setSizeMock).toHaveBeenCalledWith(
expect.objectContaining({ width: 400, height: 560 })
)
)
})

it("clamps the stable panel height to small monitors", async () => {
state.isTauriMock.mockReturnValue(true)
state.currentMonitorMock.mockResolvedValueOnce({ size: { height: 500 } })
render(<App />)

await waitFor(() =>
expect(state.setSizeMock).toHaveBeenCalledWith(
expect.objectContaining({ width: 400, height: 400 })
)
)
})
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

These tests use currentMonitorMock.mockResolvedValueOnce(...), but usePanel can call currentMonitor() multiple times (initial call + subsequent ResizeObserver callbacks). Using mockResolvedValueOnce makes the tests fragile and can mask regressions in later resize calls. Prefer mockResolvedValue(...) (or assert on the last setSize call / no subsequent call exceeding the clamp).

Copilot uses AI. Check for mistakes.
src/App.test.tsx Outdated
Comment on lines +1171 to +1181
it("keeps a stable preferred panel height when content is shorter", async () => {
state.isTauriMock.mockReturnValue(true)
state.currentMonitorMock.mockResolvedValueOnce({ size: { height: 1000 } })
render(<App />)

await waitFor(() =>
expect(state.setSizeMock).toHaveBeenCalledWith(
expect.objectContaining({ width: 400, height: 560 })
)
)
})
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

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

Test name/intent mismatch: "keeps a stable preferred panel height when content is shorter" doesn't currently set up a short-content scenario or verify stability across a provider/view change; it only asserts the initial resize target. Either adjust the name to match what’s asserted or extend the test to simulate a content/view change and confirm the height remains unchanged.

Copilot uses AI. Check for mistakes.
@validatedev
Copy link
Copy Markdown
Collaborator

CleanShot 2026-04-05 at 01 51 21@2x

Thanks, but the scrollbar is not in the correct place.

@prayzey
Copy link
Copy Markdown
Author

prayzey commented Apr 4, 2026

Screenshot 2026-04-05 at 12 00 16 AM

@validatedev How is this ? moved it further away from the content. Unless you want it in a different area. Thanks

@validatedev
Copy link
Copy Markdown
Collaborator

CleanShot 2026-04-05 at 01 59 35@2x Thanks again for the quick fix and the first iteration of the implementation, but I still feel like it shouldn't look like that. As you can see, there's no distance between the scroll bar and the content, and it does not appeal to me. Don't think that I'm being unnecessarily picky, I am trying to make sure the app looks right. The scroll bar on other apps tends to be in the rightmost area. I feel like we should have something like that instead: CleanShot 2026-04-05 at 02 12 31@2x

@prayzey
Copy link
Copy Markdown
Author

prayzey commented Apr 4, 2026

Thanks for the feedback! How does this feel?

image

Newscroll

@validatedev
Copy link
Copy Markdown
Collaborator

Hmm, now we have a gap on the right even without any scrollbar, which doesn’t feel ideal. But thanks, I appreciate your work. Let me work on it as well.

@validatedev
Copy link
Copy Markdown
Collaborator

CleanShot 2026-04-05 at 14 34 51@2x

I'm still working on it but I have eliminated all of the padding problems.

@davidarny
Copy link
Copy Markdown
Collaborator

davidarny commented Apr 5, 2026

@prayzey @validatedev guys make sure to support different macos scrollbar modes. maybe that's a root of your issues?

CleanShot 2026-04-05 at 23 52 16@2x

However not sure if it works the same way in tauri app as it does in browser. So basically in first two options scrollbar is overlayed, in last option a space for scrollbar will be added.

CleanShot.2026-04-05.at.23.54.29.mp4

@prayzey
Copy link
Copy Markdown
Author

prayzey commented Apr 6, 2026

Thanks for pointing that out.
Yes it does actually change the behaviour a bit.

How would you suggest we move forward so that the design can be consistent? Thank you.
image

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants