From 299973a1fa7d28dbc1b5a9caa65733d76a63da63 Mon Sep 17 00:00:00 2001 From: Matthijs <115901851+tieskuh@users.noreply.github.com> Date: Mon, 31 Aug 2026 02:33:48 +0200 Subject: [PATCH] fix: size a newly selected grid cell in a single layout pass (#2730) Selecting a camera in `live.display.mode: grid` lands in two visible passes: the cell gets its 2-column width immediately but keeps its 1-row height for ~400 ms, and the rest of the grid only settles ~700 ms after the click (measured per-frame on `main`; v7.27.4 has the same two passes, v8's early position resolution just leaves the late height correction standing out as a lone vertical jump). **Cause:** a grid cell's height follows its content in the same style recalculation as its width (intrinsic media ratio, `dimensions.aspect_ratio`, or the unsized 16:9 reservation), but the slotted carousel carries an inline `max-height` from `MediaHeightController` (debounced 0.3 s + 0.1 s transition; it sizes the card outside grids). In a grid that cap can only ever delay growth: it pins the freshly widened cell at its old height until the debounce fires, and the correction then also rides the 300 ms `_throttledLayout`. `selectCell()`'s existing `forceReflow()` + `layout()` was already positioned to do this in one pass; the cap starves it of the final height. **Fix:** neutralize the cap on grid cells (`max-height: none !important`; important is needed to beat the inline style). Covers the live and viewer grids; non-grid behaviour is untouched. This also stops the cap clipping cells by their border width (it was measured on the slide's content box but applied to the cell's border box). **Trade-off worth flagging:** in the *viewer* grid a cell's carousel holds all of one camera's media. If those slides do not share one ratio, the cell now sizes to its tallest slide (letterboxing shorter ones, no re-layout per swipe) instead of tracking the selected one after a debounce. If you would rather leave the viewer untouched, the rule can be scoped to `::slotted(advanced-camera-card-live-carousel)` instead (live grid cells always hold exactly one slide); say so and I will rework the PR that way. **Verification:** - New browser test `tests/components/live/grid.browser.test.ts`: fails on any frame where the newly selected cell is selected-wide but still unselected-high. On `main` it fails with 22 such frames; with this change there are none: the click settles in a single frame (~28 ms), every cell at its final size and position. - `yarn run test`, `yarn run test:browser` (chromium, firefox and webkit for the new test), `yarn run lint`, `yarn run typecheck` pass. - No resize oscillation in a cramped viewport with an appearing/disappearing ancestor scrollbar (the #2306 scenario), on window resizes, or under a narrow-screen `grid_columns: 2` override; verified against a live HA 2026.8.3 dashboard (6 go2rtc cameras). --------- Co-authored-by: dermotduffy --- src/components/live/carousel.ts | 10 +- src/components/live/grid.ts | 1 + src/components/viewer/carousel.ts | 10 +- src/components/viewer/grid.ts | 1 + .../components/live/carousel.browser.test.ts | 35 ++++ tests/components/live/grid.browser.test.ts | 175 ++++++++++++++++++ .../viewer/carousel.browser.test.ts | 21 +++ 7 files changed, 251 insertions(+), 2 deletions(-) create mode 100644 tests/components/live/carousel.browser.test.ts create mode 100644 tests/components/live/grid.browser.test.ts diff --git a/src/components/live/carousel.ts b/src/components/live/carousel.ts index 116ff682..4963e048 100644 --- a/src/components/live/carousel.ts +++ b/src/components/live/carousel.ts @@ -88,6 +88,12 @@ export class AdvancedCameraCardLiveCarousel extends LitElement { @property({ attribute: false }) public viewFilterCameraID?: string; + // Whether this carousel automatically sets its own height (via + // MediaHeightController) to fit the media it shows. Counter-example: A grid + // cell is sized by its grid instead. + @property({ attribute: false }) + public autoHeight = true; + private _refCarousel: Ref = createRef(); private _mediaActionsController = new MediaActionsController(); @@ -103,7 +109,9 @@ export class AdvancedCameraCardLiveCarousel extends LitElement { public connectedCallback(): void { super.connectedCallback(); - this._mediaHeightController.setRoot(this.renderRoot); + if (this.autoHeight) { + this._mediaHeightController.setRoot(this.renderRoot); + } // Request update in order to reinitialize the media action controller. this.requestUpdate(); diff --git a/src/components/live/grid.ts b/src/components/live/grid.ts index a81865a9..787573f0 100644 --- a/src/components/live/grid.ts +++ b/src/components/live/grid.ts @@ -74,6 +74,7 @@ export class AdvancedCameraCardLiveGrid extends LitElement { .stateWatcher=${this.stateWatcher} .viewManagerEpoch=${this.viewManagerEpoch} .viewFilterCameraID=${cameraID} + .autoHeight=${!cameraID} .liveConfig=${this.liveConfig} .cardWideConfig=${this.cardWideConfig} .cameraManager=${this.cameraManager} diff --git a/src/components/viewer/carousel.ts b/src/components/viewer/carousel.ts index 7981b47a..5cd3b1be 100644 --- a/src/components/viewer/carousel.ts +++ b/src/components/viewer/carousel.ts @@ -86,6 +86,12 @@ export class AdvancedCameraCardViewerCarousel extends LitElement { @property({ attribute: false }) public showControls = true; + // Whether this carousel automatically sets its own height (via + // MediaHeightController) to fit the media it shows. Counter-example: A grid + // cell is sized by its grid instead. + @property({ attribute: false }) + public autoHeight = true; + @state() private _selected: number | null = null; @@ -106,7 +112,9 @@ export class AdvancedCameraCardViewerCarousel extends LitElement { public connectedCallback(): void { super.connectedCallback(); - this._mediaHeightController.setRoot(this.renderRoot); + if (this.autoHeight) { + this._mediaHeightController.setRoot(this.renderRoot); + } // Request update in order to reinitialize the media action controller. this.requestUpdate(); diff --git a/src/components/viewer/grid.ts b/src/components/viewer/grid.ts index eed0f59d..3a81cfb1 100644 --- a/src/components/viewer/grid.ts +++ b/src/components/viewer/grid.ts @@ -64,6 +64,7 @@ export class AdvancedCameraCardViewerGrid extends LitElement { .hass=${this.hass} .viewManagerEpoch=${this.viewManagerEpoch} .viewFilterCameraID=${filterCamera} + .autoHeight=${!filterCamera} .viewerConfig=${this.viewerConfig} .resolvedMediaCache=${this.resolvedMediaCache} .cameraManager=${this.cameraManager} diff --git a/tests/components/live/carousel.browser.test.ts b/tests/components/live/carousel.browser.test.ts new file mode 100644 index 00000000..a724731d --- /dev/null +++ b/tests/components/live/carousel.browser.test.ts @@ -0,0 +1,35 @@ +import { assert, describe, expect, it } from 'vitest'; + +import { deepQuery } from '../../browser/dom'; +import { MountedCardFactory } from '../../browser/mounted-card'; +import { + createGenericCameraHASS, + createStillImageCardConfig, +} from '../../browser/test-utils'; + +describe('AdvancedCameraCardLiveCarousel', () => { + it('should cap its own height to fit the media it shows', async () => { + const card = await MountedCardFactory.createFromSource( + createStillImageCardConfig(), + createGenericCameraHASS(), + ); + await card.events.waitForFirst('advanced-camera-card:media:loaded'); + + const carousel = deepQuery( + card.card, + 'advanced-camera-card-live-carousel', + ); + assert(carousel); + + await card.waitForRender( + () => carousel.style.maxHeight || null, + 'the carousel capping its own height', + ); + + // Outside a grid the carousel fills the card, so the cap is what gives the + // card the height of its media rather than of whatever contains it. + expect(parseFloat(carousel.style.maxHeight)).toBe( + carousel.getBoundingClientRect().height, + ); + }); +}); diff --git a/tests/components/live/grid.browser.test.ts b/tests/components/live/grid.browser.test.ts new file mode 100644 index 00000000..6c4ef636 --- /dev/null +++ b/tests/components/live/grid.browser.test.ts @@ -0,0 +1,175 @@ +import { assert, describe, expect, it } from 'vitest'; + +import type { PartialAdvancedCameraCardConfig } from '../../../src/config/types'; +import { clickElement, deepQueryAll } from '../../browser/dom'; +import { MountedCardFactory, type MountedCard } from '../../browser/mounted-card'; +import { + CAMERA_ENTITY, + createGenericCameraHASS, + createStillImageCameraConfig, + createStillImageCardConfig, +} from '../../browser/test-utils'; + +// Enough cameras to fill a 2-column grid beyond a single row. +const CAMERA_ENTITIES = [CAMERA_ENTITY, 'camera.two', 'camera.three']; + +// Every camera declares the shape of its media, so a cell's height follows +// from its width alone and never depends on when media happens to load. +const CELL_RATIO_PARTS = [16, 9]; +const CELL_RATIO = CELL_RATIO_PARTS[0] / CELL_RATIO_PARTS[1]; + +// How many frames a wait may take before the grid is called stuck. Frames +// rather than time, so a slow machine gets proportionally more patience. +const STUCK_FRAME_BUDGET = 600; + +// How many consecutive frames of unchanged geometry mean the grid has stopped +// moving. Any change restarts the count, so work the cells defer is waited out +// however late it lands. +const QUIET_FRAME_COUNT = 20; + +// How many frames to watch a selection transition for intermediate states. The +// regression this guards against parked the selected cell at the wrong height +// for several hundred milliseconds, so it spans comfortably more than that. +const TRANSITION_FRAME_SAMPLES = 40; + +const GRID_CONFIG: PartialAdvancedCameraCardConfig = { + live: { + display: { + mode: 'grid', + grid_columns: 2, + grid_selected_position: 'first', + grid_selected_width_factor: 2, + }, + }, +}; + +const mountGrid = async (): Promise => + await MountedCardFactory.createFromSource( + createStillImageCardConfig({ + cameras: CAMERA_ENTITIES.map((cameraEntity) => ({ + ...createStillImageCameraConfig(cameraEntity), + dimensions: { aspect_ratio: CELL_RATIO_PARTS }, + })), + ...GRID_CONFIG, + }), + createGenericCameraHASS({ cameras: CAMERA_ENTITIES.slice(1) }), + { + width: '600px', + + // A cell resize can resize the grid host and vice versa, and a browser + // reports each round it has to defer as an uncaught error. How many + // rounds that takes follows the browser's frame scheduling, not the card, + // so it cannot be counted, only tolerated. + toleratedConsoleErrors: [/ResizeObserver loop completed/], + }, + ); + +const nextFrame = async (): Promise => + await new Promise((resolve) => requestAnimationFrame(() => resolve())); + +const getCells = (card: MountedCard): HTMLElement[] => + deepQueryAll(card.card, 'advanced-camera-card-live-carousel'); + +const getCell = (card: MountedCard, cameraEntity: string): HTMLElement | null => + getCells(card).find((cell) => cell.getAttribute('grid-id') === cameraEntity) ?? null; + +// The height a cell at the configured ratio should have at a given width. The +// ratio applies to the content box; the grid draws its border around it. +const getExpectedCellHeight = (cell: HTMLElement, width: number): number => { + const styles = getComputedStyle(cell); + const vertical = + parseFloat(styles.borderTopWidth) + parseFloat(styles.borderBottomWidth); + const horizontal = + parseFloat(styles.borderLeftWidth) + parseFloat(styles.borderRightWidth); + return (width - horizontal) / CELL_RATIO + vertical; +}; + +const isCellAtRatio = (cell: HTMLElement): boolean => { + const rect = cell.getBoundingClientRect(); + return ( + rect.width > 0 && Math.abs(rect.height - getExpectedCellHeight(cell, rect.width)) < 2 + ); +}; + +const getGeometry = (card: MountedCard): string => + getCells(card) + .map((cell) => { + const rect = cell.getBoundingClientRect(); + return `${cell.getAttribute('grid-id')}:${rect.width}x${rect.height}`; + }) + .join(' '); + +// Wait for the cells to stop changing size. Geometry can change with no DOM +// mutation, so this polls frames rather than waiting on the card to render. +const waitForQuietGrid = async (card: MountedCard): Promise => { + let previous: string | null = null; + let quietFrames = 0; + + for (let frame = 0; frame < STUCK_FRAME_BUDGET; frame++) { + const geometry = getGeometry(card); + quietFrames = geometry === previous ? quietFrames + 1 : 0; + previous = geometry; + + if (quietFrames >= QUIET_FRAME_COUNT) { + return getCells(card); + } + await nextFrame(); + } + throw new Error('The grid cells never stopped changing size'); +}; + +describe('AdvancedCameraCardLiveGrid', () => { + it('should apply the height of a newly selected cell in the same pass as its width', async () => { + const card = await mountGrid(); + const cells = await waitForQuietGrid(card); + expect(cells).toHaveLength(CAMERA_ENTITIES.length); + expect(cells.every(isCellAtRatio)).toBe(true); + + const selected = getCell(card, CAMERA_ENTITY); + const target = getCell(card, 'camera.two'); + assert(selected && target); + + // Thresholds derived from the two laid out sizes rather than constants, so + // a changed gutter or border width does not invalidate the test. + const selectedWidth = selected.getBoundingClientRect().width; + const unselectedRect = target.getBoundingClientRect(); + + // Every cell spans the full width until the grid controller applies column + // sizes, so a quiet grid is not necessarily a laid out one. + expect(unselectedRect.width).toBeLessThan(selectedWidth); + const wideThreshold = (selectedWidth + unselectedRect.width) / 2; + const shortThreshold = + (getExpectedCellHeight(target, selectedWidth) + unselectedRect.height) / 2; + + await clickElement(target); + await card.waitForRender( + () => target.hasAttribute('selected') || null, + 'the clicked cell being selected', + ); + + // A cell that is already at its selected width but still at an unselected + // height is the intermediate state the user sees as a two-step layout. + const wideButShort: { width: number; height: number }[] = []; + for (let frame = 0; frame < TRANSITION_FRAME_SAMPLES; frame++) { + await nextFrame(); + const rect = target.getBoundingClientRect(); + if (rect.width > wideThreshold && rect.height < shortThreshold) { + wideButShort.push({ width: rect.width, height: rect.height }); + } + } + expect(wideButShort).toEqual([]); + + // The transition did complete: the clicked cell holds the selected size. + const finalRect = target.getBoundingClientRect(); + expect(Math.abs(finalRect.width - selectedWidth)).toBeLessThan(2); + expect( + Math.abs(finalRect.height - getExpectedCellHeight(target, finalRect.width)), + ).toBeLessThan(2); + + // Outside a grid a carousel caps its own height with an inline + // `max-height`. A grid cell must not carry that cap. + expect(getCells(card).map((cell) => cell.style.maxHeight)).toEqual( + CAMERA_ENTITIES.map(() => ''), + ); + }); +}); diff --git a/tests/components/viewer/carousel.browser.test.ts b/tests/components/viewer/carousel.browser.test.ts index ca8dca1a..af9da1d1 100644 --- a/tests/components/viewer/carousel.browser.test.ts +++ b/tests/components/viewer/carousel.browser.test.ts @@ -226,4 +226,25 @@ describe('AdvancedCameraCardViewerCarousel', () => { await card.waitForSelector('advanced-camera-card-gallery'); expect(deepQuery(card.card, 'advanced-camera-card-viewer-carousel')).toBeNull(); }); + + it('should cap its own height to fit the media it shows', async () => { + const card = await mountViewer(); + + const carousel = deepQuery( + card.card, + 'advanced-camera-card-viewer-carousel', + ); + assert(carousel); + + await card.waitForRender( + () => carousel.style.maxHeight || null, + 'the carousel capping its own height', + ); + + // Outside a grid the carousel fills the card, so the cap is what gives the + // card the height of its media rather than of whatever contains it. + expect(parseFloat(carousel.style.maxHeight)).toBe( + carousel.getBoundingClientRect().height, + ); + }); });