fix: Replace max_height with height

BREAKING CHANGE: This entirely removes `min_height`, and replaces `max_height` with `height`. The behavior is obviously not exactly the same, but the prior behavior did not actually work correctly. CSS does not limit the height of child elements that are `100%` of their parents height if the parent does not have an explicit height set (this caused spillage over the set max height). As such, it's simpler to just allow the user to set the actual height of the card should they need to do so, with a (hopefully) minor loss of flexibility.
dermotduffy · Sep 15, 2024 · d3b9c41 · d3b9c41
1 parent 9766185
commit d3b9c41
Show file tree

Hide file tree

Showing 16 changed files with 67 additions and 49 deletions.
diff --git a/docs/configuration/dimensions.md b/docs/configuration/dimensions.md
@@ -13,12 +13,11 @@ dimensions:
   # [...]
 ```
 
-| Option              | Default   | Description                                                                                                                                                                                                                                                            |
-| ------------------- | --------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `aspect_ratio_mode` | `dynamic` | The aspect ratio mode to use. Acceptable values: `dynamic`, `static`, `unconstrained`. See below.                                                                                                                                                                      |
-| `aspect_ratio`      | `16:9`    | The aspect ratio to use. Acceptable values: `[W]:[H]` or `[W]/[H]`. See below.                                                                                                                                                                                         |
-| `max_height`        | `100vh`   | The maximum allowable height for the card. Specified in [CSS units](https://developer.mozilla.org/en-US/docs/Learn/CSS/Building_blocks/Values_and_units). Generally users should not need to change this setting unless they have set an `unconstrained` aspect ratio. |
-| `min_height`        | `100px`   | The minimum allowable height for the card. Specified in [CSS units](https://developer.mozilla.org/en-US/docs/Learn/CSS/Building_blocks/Values_and_units). Generally users should not need to change this setting.                                                      |
+| Option              | Default   | Description                                                                                                                                                                             |
+| ------------------- | --------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `aspect_ratio_mode` | `dynamic` | The aspect ratio mode to use. Acceptable values: `dynamic`, `static`, `unconstrained`. See below.                                                                                       |
+| `aspect_ratio`      | `16:9`    | The aspect ratio to use. Acceptable values: `[W]:[H]` or `[W]/[H]`. See below.                                                                                                          |
+| `height`            | `auto`    | The height for the card. Specified in [CSS units](https://developer.mozilla.org/en-US/docs/Learn/CSS/Building_blocks/Values_and_units). Generally users should not need to change this. |
 
 ### `aspect_ratio_mode`
 
@@ -42,6 +41,5 @@ dimensions:
 dimensions:
   aspect_ratio_mode: dynamic
   aspect_ratio: 16:9
-  max_height: 100vh
-  min_height: 100px
+  height: auto
 ```
diff --git a/src/card-controller/style-manager.ts b/src/card-controller/style-manager.ts
@@ -62,8 +62,7 @@ export class StyleManager {
     const config = this._api.getConfigManager().getConfig();
     if (config) {
       const card = this._api.getCardElementManager().getElement();
-      card.style.setProperty('--frigate-card-min-height', config.dimensions.min_height);
-      card.style.setProperty('--frigate-card-max-height', config.dimensions.max_height);
+      card.style.setProperty('--frigate-card-height', config.dimensions.height);
     }
   }
 

diff --git a/src/config/management.ts b/src/config/management.ts
@@ -11,6 +11,7 @@ import {
   CONF_CAMERAS_GLOBAL_JSMPEG,
   CONF_CAMERAS_GLOBAL_PTZ,
   CONF_CAMERAS_GLOBAL_WEBRTC_CARD,
+  CONF_DIMENSIONS_HEIGHT,
   CONF_ELEMENTS,
   CONF_LIVE_CONTROLS_THUMBNAILS_EVENTS_MEDIA_TYPE,
   CONF_LIVE_CONTROLS_TIMELINE_EVENTS_MEDIA_TYPE,
@@ -878,4 +879,6 @@ const UPGRADES = [
       typeof data === 'object' && data ? (data as RawFrigateCardConfig) : {},
     );
   },
+  upgradeMoveToWithOverrides('dimensions.max_height', CONF_DIMENSIONS_HEIGHT),
+  deleteWithOverrides('dimensions.min_height'),
 ];
diff --git a/src/config/types.ts b/src/config/types.ts
@@ -1856,8 +1856,7 @@ export type GalleryConfig = z.infer<typeof galleryConfigSchema>;
 const dimensionsConfigDefault = {
   aspect_ratio_mode: 'dynamic' as const,
   aspect_ratio: [16, 9],
-  max_height: '100vh',
-  min_height: '100px',
+  height: 'auto',
 };
 
 export const dimensionsConfigSchema = z
@@ -1866,8 +1865,7 @@ export const dimensionsConfigSchema = z
       .enum(['dynamic', 'static', 'unconstrained'])
       .default(dimensionsConfigDefault.aspect_ratio_mode),
     aspect_ratio: aspectRatioSchema.default(dimensionsConfigDefault.aspect_ratio),
-    max_height: z.string().default(dimensionsConfigDefault.max_height),
-    min_height: z.string().default(dimensionsConfigDefault.min_height),
+    height: z.string().default(dimensionsConfigDefault.height),
   })
   .default(dimensionsConfigDefault);
 

diff --git a/src/const.ts b/src/const.ts
@@ -346,8 +346,7 @@ const CONF_DIMENSIONS = 'dimensions' as const;
 export const CONF_DIMENSIONS_ASPECT_RATIO = `${CONF_DIMENSIONS}.aspect_ratio` as const;
 export const CONF_DIMENSIONS_ASPECT_RATIO_MODE =
   `${CONF_DIMENSIONS}.aspect_ratio_mode` as const;
-export const CONF_DIMENSIONS_MAX_HEIGHT = `${CONF_DIMENSIONS}.max_height` as const;
-export const CONF_DIMENSIONS_MIN_HEIGHT = `${CONF_DIMENSIONS}.min_height` as const;
+export const CONF_DIMENSIONS_HEIGHT = `${CONF_DIMENSIONS}.height` as const;
 
 export const CONF_OVERRIDES = 'overrides' as const;
 

diff --git a/src/editor.ts b/src/editor.ts
@@ -86,8 +86,7 @@ import {
   CONF_CAMERAS_ARRAY_WEBRTC_CARD_URL,
   CONF_DIMENSIONS_ASPECT_RATIO,
   CONF_DIMENSIONS_ASPECT_RATIO_MODE,
-  CONF_DIMENSIONS_MAX_HEIGHT,
-  CONF_DIMENSIONS_MIN_HEIGHT,
+  CONF_DIMENSIONS_HEIGHT,
   CONF_IMAGE_ENTITY,
   CONF_IMAGE_ENTITY_PARAMETERS,
   CONF_IMAGE_MODE,
@@ -2805,8 +2804,7 @@ export class FrigateCardEditor extends LitElement implements LovelaceCardEditor
                 this._aspectRatioModes,
               )}
               ${this._renderStringInput(CONF_DIMENSIONS_ASPECT_RATIO)}
-              ${this._renderStringInput(CONF_DIMENSIONS_MAX_HEIGHT)}
-              ${this._renderStringInput(CONF_DIMENSIONS_MIN_HEIGHT)}
+              ${this._renderStringInput(CONF_DIMENSIONS_HEIGHT)}
             </div>`
           : ''}
         ${this._renderOptionSetHeader(

diff --git a/src/localize/languages/ca.json b/src/localize/languages/ca.json
@@ -267,8 +267,7 @@
         "static": "Relació d'aspecte estàtica",
         "unconstrained": "Relació d'aspecte sense restriccions"
       },
-      "max_height": "Alçada màxima de la targeta en unitats CSS (p. ex., '100vh')",
-      "min_height": "Alçada mínima de la targeta en unitats CSS (p. ex., '100 px')"
+      "height": ""
     },
     "live": {
       "auto_mute": "Silencia automàticament les càmeres en directe",

diff --git a/src/localize/languages/en.json b/src/localize/languages/en.json
@@ -267,8 +267,7 @@
         "static": "Static aspect ratio",
         "unconstrained": "Unconstrained aspect ratio"
       },
-      "max_height": "Maximum card height in CSS units (e.g. '100vh')",
-      "min_height": "Minimum card height in CSS units (e.g. '100px')"
+      "height": "Card height in CSS units (e.g. '500px')"
     },
     "live": {
       "auto_mute": "Automatically mute live cameras",

diff --git a/src/localize/languages/fr.json b/src/localize/languages/fr.json
@@ -267,8 +267,7 @@
         "static": "Rapport hauteur/largeur statique",
         "unconstrained": "Rapport hauteur/largeur sans contrainte"
       },
-      "max_height": "Hauteur maximale de la carte en unités CSS (par exemple '100vh')",
-      "min_height": "Hauteur minimale de la carte en unités CSS (par exemple « 100 px »)"
+      "height": ""
     },
     "live": {
       "auto_mute": "Couper automatiquement le son des caméras en direct",

diff --git a/src/localize/languages/it.json b/src/localize/languages/it.json
@@ -267,8 +267,7 @@
         "static": "Proporzioni statiche",
         "unconstrained": "Proporzioni non vincolate"
       },
-      "max_height": "",
-      "min_height": ""
+      "height": ""
     },
     "live": {
       "auto_mute": "Muta automaticamente le telecamere in diretta",

diff --git a/src/localize/languages/pt-BR.json b/src/localize/languages/pt-BR.json
@@ -267,8 +267,7 @@
         "static": "Proporção estática",
         "unconstrained": "Proporção irrestrita"
       },
-      "max_height": "",
-      "min_height": ""
+      "height": ""
     },
     "live": {
       "auto_mute": "Silenciar câmeras ao vivo automaticamente",

diff --git a/src/localize/languages/pt-PT.json b/src/localize/languages/pt-PT.json
@@ -267,8 +267,7 @@
         "static": "Proporção estática",
         "unconstrained": "Proporção irrestrita"
       },
-      "max_height": "",
-      "min_height": ""
+      "height": ""
     },
     "live": {
       "auto_mute": "Silenciar câmeras ao vivo automaticamente",

diff --git a/src/scss/card.scss b/src/scss/card.scss
@@ -12,11 +12,7 @@
   // keeping the background-color within the radius.
   border-radius: var(--ha-card-border-radius, 4px);
 
-  // Necessary to ensure children adhere to height of outer container (without
-  // this gallery surround is not correctly positioned in the middle of the
-  // card, but rather the middle of the scrolling gallery container).
-  max-height: var(--frigate-card-max-height);
-  min-height: var(--frigate-card-min-height);
+  height: var(--frigate-card-height);
 
   // Ensure all clicks at the top level work.
   pointer-events: all;
@@ -30,8 +26,7 @@
   --frigate-card-expand-height: none;
   --frigate-card-expand-aspect-ratio: unset;
 
-  --frigate-card-max-height: none;
-  --frigate-card-min-height: none;
+  --frigate-card-height: auto;
 }
 
 :host([dark]) {

diff --git a/src/scss/gallery.scss b/src/scss/gallery.scss
@@ -5,8 +5,9 @@
 }
 
 frigate-card-surround-basic {
-  // This must be re-specified (in addition to on the top card element) to
-  // ensure the filter 'tab' on the gallery drawer is correctly included on the
-  // non-scrolling part of the gallery.
-  max-height: var(--frigate-card-max-height);
+  // The gallery will grow indefinitely in the `unconstrained` aspect ratio
+  // modes, the surround height needs to be limited to something reasonable in
+  // order for the media filter to still display somewhere the user can
+  // easily/continually access.
+  max-height: 100dvh;
 }
diff --git a/tests/card-controller/style-manager.test.ts b/tests/card-controller/style-manager.test.ts
@@ -209,17 +209,15 @@ describe('StyleManager', () => {
       vi.mocked(api.getConfigManager().getConfig).mockReturnValue(
         createConfig({
           dimensions: {
-            max_height: '800px',
-            min_height: '400px',
+            height: '800px',
           },
         }),
       );
       const manager = new StyleManager(api);
 
       manager.setMinMaxHeight();
 
-      expect(element.style.getPropertyValue('--frigate-card-min-height')).toBe('400px');
-      expect(element.style.getPropertyValue('--frigate-card-max-height')).toBe('800px');
+      expect(element.style.getPropertyValue('--frigate-card-height')).toBe('800px');
     });
   });
 

diff --git a/tests/config/management.test.ts b/tests/config/management.test.ts
@@ -21,7 +21,6 @@ import {
 import { PTZControlAction } from '../../src/config/ptz';
 import {
   Actions,
-  PerformActionActionConfig,
   RawFrigateCardConfig,
   frigateCardConfigSchema,
 } from '../../src/config/types';
@@ -3313,7 +3312,7 @@ describe('should handle version specific upgrades', () => {
       });
     });
 
-    it('call-service -> perform-action', () => {
+    it('rename call-service -> perform-action', () => {
       const config = {
         type: 'custom:frigate-card',
         cameras: [{ camera_entity: 'camera.office' }],
@@ -3367,5 +3366,41 @@ describe('should handle version specific upgrades', () => {
       });
       postUpgradeChecks(config);
     });
+
+    it('rename dimensions.max_height -> dimensions.height', () => {
+      const config = {
+        type: 'custom:frigate-card',
+        cameras: [{ camera_entity: 'camera.office' }],
+        dimensions: {
+          max_height: '500px',
+        },
+      };
+      expect(upgradeConfig(config)).toBeTruthy();
+      expect(config).toEqual({
+        type: 'custom:frigate-card',
+        cameras: [{ camera_entity: 'camera.office' }],
+        dimensions: {
+          height: '500px',
+        },
+      });
+      postUpgradeChecks(config);
+    });
+
+    it('delete dimensions.min_height', () => {
+      const config = {
+        type: 'custom:frigate-card',
+        cameras: [{ camera_entity: 'camera.office' }],
+        dimensions: {
+          min_height: '100px',
+        },
+      };
+      expect(upgradeConfig(config)).toBeTruthy();
+      expect(config).toEqual({
+        type: 'custom:frigate-card',
+        cameras: [{ camera_entity: 'camera.office' }],
+        dimensions: {},
+      });
+      postUpgradeChecks(config);
+    });
   });
 });