-
Notifications
You must be signed in to change notification settings - Fork 153
fix: keep tray panel height stable when provider content changes #342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
5591393
c484ada
a2ebde8
2af6d0c
b224ce8
d30beff
8255c65
4410fb4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1168,6 +1168,30 @@ describe("App", () => { | |
| await waitFor(() => expect(state.setSizeMock).toHaveBeenCalled()) | ||
| }) | ||
|
|
||
| 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 }) | ||
| ) | ||
| ) | ||
| }) | ||
|
||
|
|
||
| it("resizes again via ResizeObserver callback", async () => { | ||
| state.isTauriMock.mockReturnValue(true) | ||
| const OriginalResizeObserver = globalThis.ResizeObserver | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,7 +54,7 @@ export function AppShell({ | |
| containerRef, | ||
| scrollRef, | ||
| canScrollDown, | ||
| maxPanelHeightPx, | ||
| panelHeightPx, | ||
| } = usePanel({ | ||
| activeView, | ||
| setActiveView, | ||
|
|
@@ -65,13 +65,14 @@ export function AppShell({ | |
|
|
||
| const appVersion = useAppVersion() | ||
| const { updateStatus, triggerInstall, checkForUpdates } = useAppUpdate() | ||
| 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} | ||
| > | ||
|
||
| <div className="flex flex-1 min-h-0 flex-row"> | ||
| <SideNav | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,8 @@ import { getCurrentWindow, PhysicalSize, currentMonitor } from "@tauri-apps/api/ | |
| import type { ActiveView } from "@/components/side-nav" | ||
|
|
||
| const PANEL_WIDTH = 400 | ||
| // Keep the tray shell stable across short and long provider states; overflow should scroll inside the panel. | ||
| const PANEL_PREFERRED_HEIGHT = 560 | ||
| const MAX_HEIGHT_FALLBACK_PX = 600 | ||
| const MAX_HEIGHT_FRACTION_OF_MONITOR = 0.8 | ||
|
|
||
|
|
@@ -26,8 +28,8 @@ export function usePanel({ | |
| const containerRef = useRef<HTMLDivElement>(null) | ||
| const scrollRef = useRef<HTMLDivElement>(null) | ||
| const [canScrollDown, setCanScrollDown] = useState(false) | ||
| const [maxPanelHeightPx, setMaxPanelHeightPx] = useState<number | null>(null) | ||
| const maxPanelHeightPxRef = useRef<number | null>(null) | ||
| const [panelHeightPx, setPanelHeightPx] = useState<number | null>(null) | ||
| const panelHeightPxRef = useRef<number | null>(null) | ||
|
|
||
| useEffect(() => { | ||
| if (!isTauri()) return | ||
|
|
@@ -89,7 +91,6 @@ export function usePanel({ | |
| const resizeWindow = async () => { | ||
| const factor = window.devicePixelRatio | ||
| const width = Math.ceil(PANEL_WIDTH * factor) | ||
| const desiredHeightLogical = Math.max(1, container.scrollHeight) | ||
|
|
||
| let maxHeightPhysical: number | null = null | ||
| let maxHeightLogical: number | null = null | ||
|
|
@@ -110,13 +111,15 @@ export function usePanel({ | |
| maxHeightPhysical = Math.floor(maxHeightLogical * factor) | ||
| } | ||
|
|
||
| if (maxPanelHeightPxRef.current !== maxHeightLogical) { | ||
| maxPanelHeightPxRef.current = maxHeightLogical | ||
| setMaxPanelHeightPx(maxHeightLogical) | ||
| // Keep the tray panel visually stable; scrolling should happen inside the shell. | ||
| const nextPanelHeightLogical = Math.max(1, Math.min(PANEL_PREFERRED_HEIGHT, maxHeightLogical)) | ||
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
|
Comment on lines
+123
to
+128
|
||
| 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!)) | ||
|
||
|
|
||
| try { | ||
| const currentWindow = getCurrentWindow() | ||
|
|
@@ -164,6 +167,6 @@ export function usePanel({ | |
| containerRef, | ||
| scrollRef, | ||
| canScrollDown, | ||
| maxPanelHeightPx, | ||
| panelHeightPx, | ||
| } | ||
| } | ||
There was a problem hiding this comment.
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.