fix: Size the expanded dialog and grid columns to their content (#2611)

- Closes #2607

docs/configuration/grid-layout-algorithm.md

@@ -27,3 +27,18 @@ grid_max_columns]` that will fit at least a `600px` column width.
 - Otherwise, use the largest number of columns in the range `[2 -
 grid_max_columns]` that will fit at least a `190px` column width.
 - Otherwise, there will be `1` column only.
+
+Unless `grid_columns` is specified, that number is then reduced to the number of
+columns the items actually need. Surplus columns would otherwise be left empty
+and every item shrunk to a width it did not need -- a single camera taking half
+the grid, for example.
+
+The number of columns needed is the sum of each item's
+[`width_factor`](cameras/README.md?id=dimensions), rounded up to a whole number
+of columns per item, plus enough extra columns for any one of them to be
+selected (see `grid_selected_width_factor` in [`live`](live.md?id=display) or
+[`media_viewer`](media-viewer.md?id=display)). That room is reserved whether or
+not there is a selection, so selecting an item never changes the number of
+columns -- which would otherwise resize the items the user did not interact
+with. A grid with a single item is an exception, as that item can never be wider
+than the whole grid.

src/card-controller/style-manager.ts

@@ -21,31 +21,31 @@ export class StyleManager {
     const card = this._api.getCardElementManager().getElement();
     const view = this._api.getViewManager().getView();
 
+    // A grid shows several media items at once, so no single item describes it.
+    const isSingleMediaView = !view?.isGrid() && !!view?.isAnyMediaView();
+
     // When a new media loads, set the aspect ratio for when the card is
     // expanded/popped-up. This is based exclusively on last media content,
     // as dimension configuration does not apply in fullscreen or expanded mode.
     const lastKnown = this._api.getMediaLoadedInfoManager().getLastKnown();
     card.style.setProperty(
       '--advanced-camera-card-expand-aspect-ratio',
-      view?.isAnyMediaView() && lastKnown
+      isSingleMediaView && lastKnown
         ? `${lastKnown.width} / ${lastKnown.height}`
         : 'unset',
     );
-    // Non-media may have no intrinsic dimensions (or multiple media items in a
-    // grid) and so we need to explicit request the dialog to use all available
-    // space.
-    const isGrid = view?.isGrid();
+    // Non-media and grids have no intrinsic width, so the dialog is asked to
+    // use all the width available.
     card.style.setProperty(
       '--advanced-camera-card-expand-width',
-      !isGrid && view?.isAnyMediaView()
-        ? 'none'
-        : 'var(--advanced-camera-card-expand-max-width)',
+      isSingleMediaView ? 'none' : 'var(--advanced-camera-card-expand-max-width)',
     );
+    // Non-media (e.g. the gallery) has no intrinsic height and fills the
+    // dialog. Media sizes the dialog to itself, up to the maximum height. A
+    // grid is media: it is as tall as the items it packs.
     card.style.setProperty(
       '--advanced-camera-card-expand-height',
-      !isGrid && view?.isAnyMediaView()
-        ? 'none'
-        : 'var(--advanced-camera-card-expand-max-height)',
+      view?.isAnyMediaView() ? 'none' : 'var(--advanced-camera-card-expand-max-height)',
     );
   }
 

src/components-lib/media-grid-controller.ts

@@ -416,7 +416,7 @@ export class MediaGridController {
       Math.floor(this._hostWidth / MEDIA_GRID_DEFAULT_IDEAL_CELL_WIDTH),
     );
     if (idealColumns > 1) {
-      return idealColumns;
+      return this._clampColumnsToDemand(idealColumns);
     }
 
     // If not, get a multi-column view using the minimum cell width.
@@ -425,7 +425,49 @@ export class MediaGridController {
     );
 
     // Last result use at least 1 column.
-    return Math.max(1, minColumns);
+    return this._clampColumnsToDemand(Math.max(1, minColumns));
+  }
+
+  private _clampColumnsToDemand(columns: number): number {
+    // Extra columns sit empty and make every item narrower than it needs to be.
+    // At least 1 column is used, as the grid may be empty.
+    return Math.max(1, Math.min(columns, this._getDemandedColumns()));
+  }
+
+  // The number of columns the grid items need: one or more per item, plus room
+  // for any one of them to be selected.
+  private _getDemandedColumns(): number {
+    let demand = 0;
+    let selectionAllowance = 0;
+
+    for (const element of this._gridContents.values()) {
+      const attribute = Number(element.getAttribute(this._widthFactorAttribute));
+
+      // An absent or invalid attribute means the item is one column wide.
+      const widthFactor = attribute > 0 ? attribute : 1;
+
+      // Width factors may be fractional, but an item occupies whole columns:
+      // two half-width items need two columns, not one.
+      const columns = Math.ceil(widthFactor);
+      demand += columns;
+
+      selectionAllowance = Math.max(
+        selectionAllowance,
+        Math.ceil(widthFactor * this._getSelectedWidthFactor()) - columns,
+      );
+    }
+
+    // The space a selection needs is reserved for any item, so that selecting
+    // one does not change the column count and resize the whole grid. A lone
+    // item cannot be wider than the grid, so it needs no reservation.
+    return demand + (this._gridContents.size > 1 ? selectionAllowance : 0);
+  }
+
+  private _getSelectedWidthFactor(): number {
+    return (
+      this._displayConfig?.grid_selected_width_factor ??
+      MEDIA_GRID_DEFAULT_SELECTED_WIDTH_FACTOR
+    );
   }
 
   private _setColumnSizeStyles(): void {
@@ -436,10 +478,7 @@ export class MediaGridController {
 
     this._host.style.setProperty(
       '--advanced-camera-card-grid-selected-width-factor',
-      `${
-        this._displayConfig?.grid_selected_width_factor ??
-        MEDIA_GRID_DEFAULT_SELECTED_WIDTH_FACTOR
-      }`,
+      `${this._getSelectedWidthFactor()}`,
     );
   }
 }

src/scss/card.scss

@@ -90,6 +90,10 @@ div.main::-webkit-scrollbar {
   display: none;
 }
 
+// Styling that must win over theme styling uses the `#ha-card` id selector
+// instead of this element selector, which a theme can match with the same
+// specificity and beat on order (e.g. a `ha-card { background: ... }` rule
+// injected by card-mod).
 ha-card {
   display: flex;
   flex-direction: column;
@@ -125,8 +129,6 @@ ha-card {
   margin: 0;
 }
 
-// Need to use an id selector here to overcome theme styling that may
-// incorrectly apply to ha-card style.
 :host(:fullscreen) #ha-card {
   @include fullscreen-ha-card;
 }
@@ -178,6 +180,20 @@ web-dialog::part(dialog) {
   background: transparent;
 }
 
+// The card renders inside the dialog when expanded, leaving the opaque
+// background on `:host` behind in the dashboard. Without the background, a view
+// that does not fill the dialog (e.g. the timeline) shows the dashboard through
+// the gap.
+//
+// The card is a flex item of the dialog, and a flex item is by default at least
+// as tall as its own content. A view taller than the dialog is allowed to be
+// (e.g. a grid of many cameras) would overflow it, so the minimum is removed
+// and the card shrinks to the dialog instead, leaving the view to scroll.
+:host([expanded]) #ha-card {
+  background-color: var(--advanced-camera-card-background);
+  min-height: 0;
+}
+
 /*******************
  * Menu hover styles
  *******************/

tests/card-controller/style-manager.test.ts

@@ -133,7 +133,32 @@ describe('StyleManager', () => {
 
       expect(
         element.style.getPropertyValue('--advanced-camera-card-expand-aspect-ratio'),
-      ).toBe('800 / 600');
+      ).toBe('unset');
+      expect(element.style.getPropertyValue('--advanced-camera-card-expand-width')).toBe(
+        'var(--advanced-camera-card-expand-max-width)',
+      );
+      expect(
+        element.style.getPropertyValue('--advanced-camera-card-expand-height'),
+      ).toBe('none');
+    });
+
+    it('with a non-media view', () => {
+      const api = createCardAPI();
+      const element = document.createElement('div');
+      vi.mocked(api.getCardElementManager().getElement).mockReturnValue(element);
+      const view = createView({ view: 'timeline' });
+      vi.mocked(api.getViewManager().getView).mockReturnValue(view);
+      vi.mocked(api.getMediaLoadedInfoManager().getLastKnown).mockReturnValue({
+        width: 800,
+        height: 600,
+      });
+      const manager = new StyleManager(api);
+
+      manager.setExpandedMode();
+
+      expect(
+        element.style.getPropertyValue('--advanced-camera-card-expand-aspect-ratio'),
+      ).toBe('unset');
       expect(element.style.getPropertyValue('--advanced-camera-card-expand-width')).toBe(
         'var(--advanced-camera-card-expand-max-width)',
       );

tests/components-lib/media-grid-controller.test.ts

@@ -423,6 +423,190 @@ describe('MediaGridController', () => {
     expect(masonry.destroy).toBeCalledTimes(1);
   });
 
+  it('should not use more columns than the items ask for', () => {
+    const parent = createParent({ children: createChildren(['0']), width: 3000 });
+    createController(parent);
+
+    // The lone item takes the whole grid. Sizing from the width alone would
+    // give it 1 of 5 columns, with the other 4 left empty.
+    expect(Masonry).toBeCalledWith(
+      parent,
+      expect.objectContaining({
+        columnWidth: 3000,
+      }),
+    );
+    expect(
+      parent.style.getPropertyValue('--advanced-camera-card-grid-column-size'),
+    ).toBe('3000px');
+  });
+
+  it('should not use more columns than the items ask for on a narrow host', () => {
+    const parent = createParent({ children: createChildren(['0']) });
+    createController(parent);
+
+    // Sizing from the width alone would give the lone item half of a default
+    // width card.
+    expect(Masonry).toBeCalledWith(
+      parent,
+      expect.objectContaining({
+        columnWidth: 492,
+      }),
+    );
+    expect(
+      parent.style.getPropertyValue('--advanced-camera-card-grid-column-size'),
+    ).toBe('492px');
+  });
+
+  it('should use a single column when there are no items', () => {
+    const parent = createParent({ width: 3000 });
+    createController(parent);
+
+    expect(Masonry).toBeCalledWith(
+      parent,
+      expect.objectContaining({
+        columnWidth: 3000,
+      }),
+    );
+    expect(
+      parent.style.getPropertyValue('--advanced-camera-card-grid-column-size'),
+    ).toBe('3000px');
+  });
+
+  it('should respect exact columns even with fewer items', () => {
+    const parent = createParent({ children: createChildren(['0']), width: 3000 });
+    const controller = createController(parent);
+    controller.setDisplayConfig({ mode: 'grid', grid_columns: 4 });
+
+    expect(masonry.option).toBeCalledWith(
+      expect.objectContaining({
+        columnWidth: 749,
+      }),
+    );
+    expect(
+      parent.style.getPropertyValue('--advanced-camera-card-grid-column-size'),
+    ).toBe('749px');
+  });
+
+  it('should size columns so a selected item and its siblings fit a single row', () => {
+    const parent = createParent({
+      children: createChildren(['0', '1', '2']),
+      width: 3000,
+    });
+    createController(parent, { selected: '1' });
+
+    // The items ask for 4 columns: 2 for the selection (the default
+    // `grid_selected_width_factor`) and 1 for each of its siblings.
+    expect(Masonry).toBeCalledWith(
+      parent,
+      expect.objectContaining({
+        columnWidth: 749,
+      }),
+    );
+
+    const selectedWidth = 2 * 749;
+    expect(selectedWidth + 749 + 749).toBeLessThanOrEqual(3000);
+  });
+
+  it('should give a lone selected item the whole grid', () => {
+    const parent = createParent({ children: createChildren(['0']), width: 3000 });
+    createController(parent, { selected: '0' });
+
+    // A selection is normally reserved extra columns, but a lone item cannot be
+    // wider than the grid and so cannot use them.
+    expect(Masonry).toBeCalledWith(
+      parent,
+      expect.objectContaining({
+        columnWidth: 3000,
+      }),
+    );
+  });
+
+  it('should count a custom selected width factor towards the columns asked for', () => {
+    const parent = createParent({
+      children: createChildren(['0', '1', '2']),
+      width: 3000,
+    });
+    const controller = createController(parent, { selected: '1' });
+    controller.setDisplayConfig({ mode: 'grid', grid_selected_width_factor: 3 });
+
+    // 3 columns for the selection and 1 for each sibling exhausts the 5
+    // columns the width allows.
+    expect(masonry.option).toBeCalledWith(
+      expect.objectContaining({
+        columnWidth: 599,
+      }),
+    );
+    expect(
+      parent.style.getPropertyValue('--advanced-camera-card-grid-column-size'),
+    ).toBe('599px');
+  });
+
+  it('should count item width factors towards the columns asked for', () => {
+    const children = createChildren(['0', '1', '2']);
+    children[0].setAttribute('grid-width-factor', '2');
+    const parent = createParent({ children: children, width: 4200 });
+    createController(parent);
+
+    // The items span 4 columns, and the widest needs 2 more when selected.
+    // Ignoring the width factor would give 4 columns of 1049px.
+    expect(Masonry).toBeCalledWith(
+      parent,
+      expect.objectContaining({
+        columnWidth: 699,
+      }),
+    );
+  });
+
+  it('should give an item that is narrower than a column a column of its own', () => {
+    const children = createChildren(['0', '1', '2']);
+    for (const child of children) {
+      child.setAttribute('grid-width-factor', '0.5');
+    }
+    const parent = createParent({ children: children, width: 1800 });
+    createController(parent, { selected: '0' });
+
+    // Each item asks for one column: the selection fills exactly one at 0.5 x
+    // 2, and a half-width sibling still occupies a whole one.
+    expect(Masonry).toBeCalledWith(
+      parent,
+      expect.objectContaining({
+        columnWidth: 599,
+      }),
+    );
+  });
+
+  it('should not resize columns when a selection is made or removed', () => {
+    const children = createChildren(['0', '1', '2']);
+    const parent = createParent({ children: children, width: 3000 });
+    const controller = createController(parent);
+
+    // Room for a selection is reserved whether or not there is one, so the
+    // three items ask for 4 columns either way.
+    expect(Masonry).toBeCalledWith(
+      parent,
+      expect.objectContaining({
+        columnWidth: 749,
+      }),
+    );
+
+    vi.mocked(masonry.option)?.mockClear();
+    controller.selectCell('1');
+
+    // Selecting an item would otherwise resize the items the user did not
+    // interact with.
+    expect(masonry.option).not.toBeCalled();
+    expect(
+      parent.style.getPropertyValue('--advanced-camera-card-grid-column-size'),
+    ).toBe('749px');
+
+    controller.unselectAll();
+
+    expect(masonry.option).not.toBeCalled();
+    expect(
+      parent.style.getPropertyValue('--advanced-camera-card-grid-column-size'),
+    ).toBe('749px');
+  });
+
   it('should respect selected width factor', () => {
     const parent = createParent({ children: createChildren(), width: 2000 });
     const controller = createController(parent);
@@ -511,10 +695,10 @@ describe('MediaGridController', () => {
     // Masonry should not be recreated, but column width should be updated
     // via option() and layout should be called.
     expect(Masonry).not.toBeCalled();
-    expect(masonry.option).toBeCalledWith({ columnWidth: 599 });
+    expect(masonry.option).toBeCalledWith({ columnWidth: 749 });
     expect(
       parent.style.getPropertyValue('--advanced-camera-card-grid-column-size'),
-    ).toBe('599px');
+    ).toBe('749px');
     expect(masonry.layout).toBeCalled();
 
     // Clear mock state.