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 <dermot.duffy@gmail.com>
This commit is contained in:
@@ -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<HTMLElement> = 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();
|
||||
|
||||
@@ -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}
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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}
|
||||
|
||||
@@ -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<HTMLElement>(
|
||||
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,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -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<MountedCard> =>
|
||||
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<void> =>
|
||||
await new Promise<void>((resolve) => requestAnimationFrame(() => resolve()));
|
||||
|
||||
const getCells = (card: MountedCard): HTMLElement[] =>
|
||||
deepQueryAll<HTMLElement>(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<HTMLElement[]> => {
|
||||
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(() => ''),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -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<HTMLElement>(
|
||||
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,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user