From 79f9263389b0ab14ca3df6789ed0e12469dd7023 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Tue, 8 Sep 2026 21:31:19 +0530 Subject: [PATCH 1/4] fix(app): adapt the player pane to the window it is in --- .../adapt-the-player-pane-to-the-window.md | 11 ++ .../app/src/components/browser/snapshot.ts | 19 +- packages/app/src/components/workbench.ts | 33 ++-- packages/app/src/controller/constants.ts | 12 +- packages/app/src/utils/DragController.ts | 41 +++- .../test-ui/workbench/player/snapshot.test.ts | 25 +++ .../app/tests/drag-controller-resize.test.ts | 187 ++++++++++++++++++ 7 files changed, 303 insertions(+), 25 deletions(-) create mode 100644 .changeset/adapt-the-player-pane-to-the-window.md create mode 100644 packages/app/tests/drag-controller-resize.test.ts diff --git a/.changeset/adapt-the-player-pane-to-the-window.md b/.changeset/adapt-the-player-pane-to-the-window.md new file mode 100644 index 00000000..7143f433 --- /dev/null +++ b/.changeset/adapt-the-player-pane-to-the-window.md @@ -0,0 +1,11 @@ +--- +"@wdio/devtools-app": patch +--- + +Adapt the player pane to the window it is actually in. The pane's height came from a pixel number resolved once, at construction, from whatever window happened to be open then, and nothing recomputed it: measured at 124px in a 1280x720 window and still 124px at 2560x1440, so a trace rendered into a 13px-wide box on a 2560px screen. Not mobile-specific — wrong for every trace, just least visible on a desktop one. + +Three separate things froze it, and all three had to go. `MIN_WORKBENCH_HEIGHT` was `Math.min(300, window.innerHeight * 0.3)` evaluated at module import, so it took the window open at page load and — being the pane's own `minPosition` — pinned the pane there for the life of the page; loaded in a 413px-tall window it is exactly the 124px measured. `DragController.initialPosition` took a number rather than the getter its bounds already accepted, so a window-derived default could never follow the window. And each controller registered its resize handling by assigning `window.onresize`, which is a single slot: with five controllers on the page only the last one constructed ever adjusted, and it clobbered anything else on that slot. + +A height the user dragged still wins. It is stored, and a resize only re-clamps it — so it survives a window that still has room for it and is pulled back inside one that no longer does, rather than leaving the drag handle off-screen. + +The player component also re-fitted only on `resize` and `window-drag`, which meant it depended on whoever changed the layout remembering to announce it — and the dock divider, the sidebar collapsing and browser zoom announce nothing. It now watches its own box with a `ResizeObserver`, which covers all of them. diff --git a/packages/app/src/components/browser/snapshot.ts b/packages/app/src/components/browser/snapshot.ts index 93d5a399..b436d84d 100644 --- a/packages/app/src/components/browser/snapshot.ts +++ b/packages/app/src/components/browser/snapshot.ts @@ -135,15 +135,21 @@ export class DevtoolsBrowser extends Element { @query('section') section?: HTMLElement + /** + * Watches the player's OWN box rather than the window. Re-fitting used to + * hang off `resize` and `window-drag`, so it depended on whoever changed the + * layout announcing it — and the dock divider, the sidebar collapsing and + * browser zoom announce nothing. Every one of those moves this box. + */ + #boxObserver?: ResizeObserver + /** The window events the player handles while connected, as one table so its * registration and its teardown cannot drift. Every handler is a per-instance * arrow field, so the reference removeEventListener gets is the one that was * added — a bound method would produce a new function per call and never - * detach. */ + * detach. Sizing is not among them: that is the ResizeObserver's job. */ #windowListeners(): ReadonlyArray { return [ - ['resize', this.#handleResize], - ['window-drag', this.#handleResize], ['app-mutation-highlight', this.#highlightMutation], ['app-mutation-select', this.#handleMutationSelect], ['a11y-highlight', this.#highlightBySelector], @@ -157,6 +163,11 @@ export class DevtoolsBrowser extends Element { for (const [type, handler] of this.#windowListeners()) { window.addEventListener(type, handler) } + // Safe against the observer loop: this watches the host, and the sizing it + // triggers writes to a descendant. The host is laid out by its parent + // (width/height 100%), so nothing it writes can feed back into this box. + this.#boxObserver = new ResizeObserver(() => this.#handleResize()) + this.#boxObserver.observe(this) await this.updateComplete } @@ -169,6 +180,8 @@ export class DevtoolsBrowser extends Element { for (const [type, handler] of this.#windowListeners()) { window.removeEventListener(type, handler) } + this.#boxObserver?.disconnect() + this.#boxObserver = undefined } #captureShape?: { screenshot: string; size: ImageSize | null } diff --git a/packages/app/src/components/workbench.ts b/packages/app/src/components/workbench.ts index 2046322f..60842713 100644 --- a/packages/app/src/components/workbench.ts +++ b/packages/app/src/components/workbench.ts @@ -45,7 +45,7 @@ import './browser/trace-player-controls.js' import { BROWSER_BACKDROP_GRADIENT, HEADER_HEIGHT, - MIN_WORKBENCH_HEIGHT, + minWorkbenchHeight, MIN_METATAB_WIDTH, ACTIONS_DEFAULT_WIDTH, BROWSER_HEIGHT_RATIO, @@ -124,9 +124,9 @@ export class DevtoolsWorkbench extends Element { #dragVertical = new DragController(this, { localStorageKey: 'toolbarHeight', - minPosition: MIN_WORKBENCH_HEIGHT, - maxPosition: window.innerHeight * 0.7, - initialPosition: window.innerHeight * BROWSER_HEIGHT_RATIO, + minPosition: minWorkbenchHeight, + maxPosition: () => window.innerHeight * 0.7, + initialPosition: () => window.innerHeight * BROWSER_HEIGHT_RATIO, getContainerEl: () => this.#getVerticalWindow(), direction: Direction.vertical }) @@ -157,16 +157,17 @@ export class DevtoolsWorkbench extends Element { // The live max bound keeps the handle (and pane) inside the current budget. #dragVerticalPlayer = new DragController(this, { localStorageKey: 'playerPaneHeight', - minPosition: MIN_WORKBENCH_HEIGHT, + minPosition: minWorkbenchHeight, maxPosition: () => this.#playerPaneBudget(), - initialPosition: Math.max( - MIN_WORKBENCH_HEIGHT, - window.innerHeight - - HEADER_HEIGHT - - PLAYER_CONTROLS_HEIGHT - - TRACE_TIMELINE_DEFAULT_HEIGHT - - PLAYER_DOCK_DEFAULT_HEIGHT - ), + initialPosition: () => + Math.max( + minWorkbenchHeight(), + window.innerHeight - + HEADER_HEIGHT - + PLAYER_CONTROLS_HEIGHT - + TRACE_TIMELINE_DEFAULT_HEIGHT - + PLAYER_DOCK_DEFAULT_HEIGHT + ), getContainerEl: () => this.#getVerticalWindow(), direction: Direction.vertical }) @@ -183,7 +184,7 @@ export class DevtoolsWorkbench extends Element { // Space left for the snapshot pane once the fixed rows and dock minimum eat theirs. #playerPaneBudget(): number { return Math.max( - MIN_WORKBENCH_HEIGHT, + minWorkbenchHeight(), window.innerHeight - HEADER_HEIGHT - PLAYER_CONTROLS_HEIGHT - @@ -243,13 +244,13 @@ export class DevtoolsWorkbench extends Element { const maxHeight = `calc(100vh - ${ HEADER_HEIGHT + PLAYER_CONTROLS_HEIGHT + PLAYER_DOCK_MIN_HEIGHT }px - ${this.#timelinePaneHeight()}px)` - return `flex-grow:0; flex-shrink:0; ${this.#dragVerticalPlayer.getPosition()}; max-height:${maxHeight}; min-height:${MIN_WORKBENCH_HEIGHT}px;` + return `flex-grow:0; flex-shrink:0; ${this.#dragVerticalPlayer.getPosition()}; max-height:${maxHeight}; min-height:${minWorkbenchHeight()}px;` } const raw = basisPx(this.#dragVertical.getPosition()) ?? window.innerHeight * BROWSER_HEIGHT_RATIO const capped = Math.min(raw, window.innerHeight * 0.7) - const paneHeight = Math.max(MIN_WORKBENCH_HEIGHT, capped) + const paneHeight = Math.max(minWorkbenchHeight(), capped) return `flex:0 0 ${paneHeight}px; height:${paneHeight}px; max-height:70vh; min-height:0;` } diff --git a/packages/app/src/controller/constants.ts b/packages/app/src/controller/constants.ts index d43e22d0..f8a5ef1b 100644 --- a/packages/app/src/controller/constants.ts +++ b/packages/app/src/controller/constants.ts @@ -3,7 +3,17 @@ import type { LogSource } from '@wdio/devtools-shared' export const CACHE_ID = 'wdio-trace-cache' export const SIDEBAR_MIN_WIDTH = 250 export const DARK_MODE_KEY = 'darkMode' -export const MIN_WORKBENCH_HEIGHT = Math.min(300, window.innerHeight * 0.3) +/** + * Smallest useful workbench pane, for the window AS IT IS NOW. A function + * rather than a constant: evaluated at module import it froze at whatever + * window happened to be open then, and — because it is also the pane's + * `minPosition` — pinned the pane there for the life of the page. Loaded in a + * 413px-tall window it is 124px, which is what a trace then rendered into on a + * 2560px screen. + */ +export function minWorkbenchHeight(): number { + return Math.min(300, window.innerHeight * 0.3) +} export const MIN_METATAB_WIDTH = 260 export const RERENDER_TIMEOUT = 10 export const SIDEBAR_DEFAULT_WIDTH = 350 diff --git a/packages/app/src/utils/DragController.ts b/packages/app/src/utils/DragController.ts index a9f058c4..3fada4e6 100644 --- a/packages/app/src/utils/DragController.ts +++ b/packages/app/src/utils/DragController.ts @@ -18,7 +18,9 @@ type AsyncGetElFn = () => Element | Promise type Bound = number | (() => number) interface DragControllerOptions { - initialPosition: number + /** Accepts a getter, like the bounds: a window-derived default resolved once + * at construction never follows the window it was derived from. */ + initialPosition: Bound direction: Direction localStorageKey?: string minPosition?: Bound @@ -53,6 +55,9 @@ export class DragController implements ReactiveController { #state: State = 'idle' #pointerTracker: PointerTracker | null = null + /** Whether the current position is the user's own — restored from storage or + * dragged — rather than derived from the window. */ + #userChosen = false constructor(host: DragControllerHost, options: DragControllerOptions) { this.#host = host @@ -80,8 +85,6 @@ export class DragController implements ReactiveController { return } - window.onresize = () => this.#adjustPosition() - // TODO Add typeguard to check if HTMLElement this.#draggableEl = draggableEl as HTMLElement this.#containerEl = containerEl as HTMLElement @@ -94,8 +97,34 @@ export class DragController implements ReactiveController { ? parseInt(localStorage.getItem(this.#localStorageKey)!, 10) : undefined : undefined - const initialPosition = storageValue || this.#options.initialPosition + // A stored height is the user's own choice and keeps winning; only a + // derived default follows the window. + this.#userChosen = + storageValue !== undefined && Number.isFinite(storageValue) + const initialPosition = this.#userChosen + ? storageValue! + : (resolveBound(this.#options.initialPosition) ?? 0) this.#setPosition(initialPosition, initialPosition) + // Own listener, not `window.onresize`: that is a single slot, so with five + // controllers on the page only the last one constructed ever ran — which is + // why nothing re-fitted on resize. + window.addEventListener('resize', this.#onWindowResize) + } + + /** + * Follow the window. A derived default is recomputed outright; a height the + * user dragged is only re-clamped, so it survives a resize that still has + * room for it and is pulled back inside a window that no longer does. + */ + #onWindowResize = () => { + if (this.#userChosen) { + this.#setPosition(this.#x, this.#y) + } else { + const derived = resolveBound(this.#options.initialPosition) ?? 0 + this.#setPosition(derived, derived) + } + this.#host.requestUpdate() + void this.#adjustPosition() } async #getDraggableEl() { @@ -174,6 +203,7 @@ export class DragController implements ReactiveController { if (this.#pointerTracker) { this.#pointerTracker.stop() } + window.removeEventListener('resize', this.#onWindowResize) } #handleWindowMove(pointer: Pointer) { @@ -198,6 +228,8 @@ export class DragController implements ReactiveController { const yDelta = cursorPositionY - this.#cursorPositionY this.#setPosition(oldX + xDelta, oldY + yDelta) + // From here on this pane's height is the user's, not the window's. + this.#userChosen = true if (this.#localStorageKey) { localStorage.setItem( @@ -256,7 +288,6 @@ export class DragController implements ReactiveController { const containerEl = await this.#options.getContainerEl() if (containerEl) { this.#containerEl = containerEl as HTMLElement - window.onresize = () => this.#adjustPosition() } } this.#init() diff --git a/packages/app/test-ui/workbench/player/snapshot.test.ts b/packages/app/test-ui/workbench/player/snapshot.test.ts index c49f2380..8078ae14 100644 --- a/packages/app/test-ui/workbench/player/snapshot.test.ts +++ b/packages/app/test-ui/workbench/player/snapshot.test.ts @@ -1607,6 +1607,31 @@ describe('wdio-devtools-browser', () => { expect(section.style.height).toBe('100%') }) + it('re-fits when its box changes with no event announcing it', async () => { + // The dock divider, the sidebar collapsing and browser zoom all move + // this box and fire neither `resize` nor `window-drag`. Nothing is + // dispatched here on purpose: a ResizeObserver on its own box is what + // makes those cases work. + const el = await framed() + await settle(el) + const section = shadow(el, 'section')! + await resizeScreenshotPane(el, ...paneFor(el, { w: 400, h: 300 })) + await waitUntil( + () => section.style.height !== '', + 'the frame to be sized to the capture' + ) + const before = section.getBoundingClientRect().height + + const host = el.parentElement! + host.style.height = `${parseFloat(host.style.height) + 200}px` + await waitUntil( + () => section.getBoundingClientRect().height !== before, + 'the frame to follow its own box' + ) + + expect(section.getBoundingClientRect().height).toBeGreaterThan(before) + }) + it('keeps the browser frame for a device session that has a url', async () => { // A mobile browser — Appium driving Chrome on Android — reports a // device too, and its url arrives before any DOM batch. diff --git a/packages/app/tests/drag-controller-resize.test.ts b/packages/app/tests/drag-controller-resize.test.ts new file mode 100644 index 00000000..8ee9f489 --- /dev/null +++ b/packages/app/tests/drag-controller-resize.test.ts @@ -0,0 +1,187 @@ +// @vitest-environment happy-dom +// +// The player pane took its height from a pixel number resolved once, from +// whatever window was open at construction, and nothing recomputed it: measured +// at 124px in a 1280x720 window and still 124px at 2560x1440, so a trace +// rendered into a 13px-wide box on a 2560px screen. + +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { minWorkbenchHeight } from '../src/controller/constants.js' +import { DragController, Direction } from '../src/utils/DragController.js' + +/** Position as a number, off the `flex-basis: Npx` the host renders. */ +const positionOf = (drag: DragController): number => + parseFloat(drag.getPosition().split(':')[1]) + +/** + * The controller only needs a ReactiveControllerHost to count updates and a + * shadow root to look for its handle in. No handle exists here, so the pointer + * tracker never attaches — position handling is what this covers. + */ +function fakeHost() { + const host = { + updates: 0, + addController: () => {}, + removeController: () => {}, + requestUpdate() { + host.updates++ + }, + updateComplete: Promise.resolve(true), + shadowRoot: { querySelector: () => null } + } + return host as unknown as ConstructorParameters[0] & { + updates: number + } +} + +const setWindowHeight = (height: number) => { + Object.defineProperty(window, 'innerHeight', { + value: height, + configurable: true, + writable: true + }) +} + +const resize = () => window.dispatchEvent(new Event('resize')) + +beforeEach(() => { + localStorage.clear() + setWindowHeight(720) +}) + +describe('a pane height derived from the window', () => { + const derived = (host: ReturnType) => + new DragController(host, { + localStorageKey: 'testPaneHeight', + minPosition: 50, + maxPosition: () => window.innerHeight * 0.9, + initialPosition: () => window.innerHeight * 0.5, + getContainerEl: () => Promise.resolve(null), + direction: Direction.vertical + }) + + it('resolves from the window as it is at construction', () => { + setWindowHeight(1000) + + expect(positionOf(derived(fakeHost()))).toBe(500) + }) + + it('follows the window when it changes', () => { + const host = fakeHost() + const drag = derived(host) + expect(positionOf(drag)).toBe(360) + + setWindowHeight(1440) + resize() + + // The whole bug: this used to stay at its construction-time value forever. + expect(positionOf(drag)).toBe(720) + expect(host.updates).toBeGreaterThan(0) + }) + + it('stays inside its own bounds while following', () => { + const drag = derived(fakeHost()) + + setWindowHeight(80) + resize() + + // 50% of 80 is under the 50px floor. + expect(positionOf(drag)).toBe(50) + }) +}) + +describe('a pane height the user chose', () => { + const chosen = (host: ReturnType) => + new DragController(host, { + localStorageKey: 'testPaneHeight', + minPosition: 50, + maxPosition: () => window.innerHeight * 0.5, + initialPosition: () => window.innerHeight * 0.9, + getContainerEl: () => Promise.resolve(null), + direction: Direction.vertical + }) + + it('wins over the derived default', () => { + localStorage.setItem('testPaneHeight', '300') + + expect(positionOf(chosen(fakeHost()))).toBe(300) + }) + + it('keeps winning when the window changes and it still fits', () => { + localStorage.setItem('testPaneHeight', '300') + const drag = chosen(fakeHost()) + + setWindowHeight(1440) + resize() + + // Not recomputed to 90% of the new window: the user picked this. + expect(positionOf(drag)).toBe(300) + }) + + it('is pulled back inside a window that no longer has room', () => { + localStorage.setItem('testPaneHeight', '300') + const drag = chosen(fakeHost()) + + setWindowHeight(400) + resize() + + // Max is half the window; a stored height outliving its room would push the + // handle off-screen. + expect(positionOf(drag)).toBe(200) + }) +}) + +describe('every controller on the page, not just the last one built', () => { + it('each follows the window independently', () => { + // `window.onresize` is a single slot, so five controllers assigning it left + // only the last one adjusting — which is why nothing re-fitted. + const build = (ratio: number) => + new DragController(fakeHost(), { + minPosition: 10, + initialPosition: () => window.innerHeight * ratio, + getContainerEl: () => Promise.resolve(null), + direction: Direction.vertical + }) + const first = build(0.25) + const second = build(0.5) + + setWindowHeight(1000) + resize() + + expect([positionOf(first), positionOf(second)]).toEqual([250, 500]) + }) +}) + +describe('minWorkbenchHeight', () => { + it('reads the window at call time, not at import time', () => { + setWindowHeight(2000) + expect(minWorkbenchHeight()).toBe(300) + + // The frozen value is what pinned the pane: loaded in a 413px window it is + // 124px, and it is also the pane's own minimum. + setWindowHeight(413) + expect(Math.round(minWorkbenchHeight())).toBe(124) + }) +}) + +describe('teardown', () => { + it('stops following once the host disconnects', () => { + const host = fakeHost() + const drag = new DragController(host, { + minPosition: 10, + initialPosition: () => window.innerHeight * 0.5, + getContainerEl: () => Promise.resolve(null), + direction: Direction.vertical + }) + const removeSpy = vi.spyOn(window, 'removeEventListener') + + drag.hostDisconnected() + setWindowHeight(1440) + resize() + + expect(removeSpy).toHaveBeenCalledWith('resize', expect.any(Function)) + expect(positionOf(drag)).toBe(360) + removeSpy.mockRestore() + }) +}) From c5b10e90ccfb4ab9ba81ecb309efce43623c5b19 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Tue, 8 Sep 2026 21:38:44 +0530 Subject: [PATCH 2/4] feat(app): give a device capture a column of its own --- .../src/components/browser/snapshot-styles.ts | 9 + .../app/src/components/browser/snapshot.ts | 4 + packages/app/src/components/workbench.ts | 188 ++++++++++++++++-- packages/app/src/controller/constants.ts | 14 ++ packages/app/src/utils/DragController.ts | 30 ++- .../test-ui/workbench/workbench-fixtures.ts | 5 +- .../app/test-ui/workbench/workbench.test.ts | 173 ++++++++++++++++ 7 files changed, 399 insertions(+), 24 deletions(-) diff --git a/packages/app/src/components/browser/snapshot-styles.ts b/packages/app/src/components/browser/snapshot-styles.ts index 9e74e324..c4069abc 100644 --- a/packages/app/src/components/browser/snapshot-styles.ts +++ b/packages/app/src/components/browser/snapshot-styles.ts @@ -16,6 +16,15 @@ export const snapshotStyles = css` background: ${unsafeCSS(BROWSER_BACKDROP_GRADIENT)}; } + /* A device frame owns its column, which already provides the backdrop and the + gap, so the host's own 1.25rem is 40px of height and width spent on + nothing. It is spent OUTSIDE the box deviceFrameSize measures, so the frame + came out 40px short on each axis — and because a portrait frame's width + follows its height, the lost height cost width a second time. */ + :host([device-frame]) { + padding: 0.25rem !important; + } + section { box-sizing: border-box; width: calc(100% - 0px); /* host padding already applied */ diff --git a/packages/app/src/components/browser/snapshot.ts b/packages/app/src/components/browser/snapshot.ts index b436d84d..0efd9d99 100644 --- a/packages/app/src/components/browser/snapshot.ts +++ b/packages/app/src/components/browser/snapshot.ts @@ -483,6 +483,10 @@ export class DevtoolsBrowser extends Element { // View-mode flips swap the iframe with /