From f924537025cea8f39873dad96327564d9cccb26d Mon Sep 17 00:00:00 2001 From: Dermot Duffy Date: Sun, 15 Sep 2024 17:29:05 -0700 Subject: [PATCH] fix: Replace `max_height` with `height` (#1539) * 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. * Test fixes --- docs/configuration/dimensions.md | 14 +++---- src/card-controller/style-manager.ts | 3 +- src/config/management.ts | 3 ++ src/config/types.ts | 6 +-- src/const.ts | 3 +- src/editor.ts | 6 +-- src/localize/languages/ca.json | 3 +- src/localize/languages/en.json | 3 +- src/localize/languages/fr.json | 3 +- src/localize/languages/it.json | 3 +- src/localize/languages/pt-BR.json | 3 +- src/localize/languages/pt-PT.json | 3 +- src/scss/card.scss | 9 +---- src/scss/gallery.scss | 9 +++-- tests/card-controller/style-manager.test.ts | 6 +-- tests/config/management.test.ts | 44 +++++++++++++++++++-- tests/config/types.test.ts | 3 +- 17 files changed, 72 insertions(+), 52 deletions(-) diff --git a/docs/configuration/dimensions.md b/docs/configuration/dimensions.md index a499c558..f9731286 100644 --- 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 index ff3245fd..e959e0f3 100644 --- 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 index 570b0f91..93a5e40c 100644 --- 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 index a87afa0d..0250a9b9 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -1857,8 +1857,7 @@ export type GalleryConfig = z.infer; const dimensionsConfigDefault = { aspect_ratio_mode: 'dynamic' as const, aspect_ratio: [16, 9], - max_height: '100vh', - min_height: '100px', + height: 'auto', }; export const dimensionsConfigSchema = z @@ -1867,8 +1866,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 index c9b1eb9f..2b851ffa 100644 --- 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 index 8b9c0da3..194fce21 100644 --- 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)} ` : ''} ${this._renderOptionSetHeader( diff --git a/src/localize/languages/ca.json b/src/localize/languages/ca.json index 01521890..56c21ac0 100644 --- 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 index 692edddf..334ad7f6 100644 --- 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 index 981d9524..4a78b072 100644 --- 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 index b0a0a09e..506b75ed 100644 --- 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 index c54f68fe..f06c7c59 100644 --- 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 index 10655859..f2015709 100644 --- 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 index dc37e1fb..1005e734 100644 --- 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 index 9f12b3be..958bf332 100644 --- 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 index c2920b70..f8323058 100644 --- a/tests/card-controller/style-manager.test.ts +++ b/tests/card-controller/style-manager.test.ts @@ -209,8 +209,7 @@ describe('StyleManager', () => { vi.mocked(api.getConfigManager().getConfig).mockReturnValue( createConfig({ dimensions: { - max_height: '800px', - min_height: '400px', + height: '800px', }, }), ); @@ -218,8 +217,7 @@ describe('StyleManager', () => { 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 index e819b80b..1c4bd532 100644 --- a/tests/config/management.test.ts +++ b/tests/config/management.test.ts @@ -1,4 +1,7 @@ -import { CallServiceActionConfig } from '@dermotduffy/custom-card-helpers'; +import { + CallServiceActionConfig, + PerformActionActionConfig, +} from '@dermotduffy/custom-card-helpers'; import { describe, expect, it } from 'vitest'; import { copyConfig, @@ -21,7 +24,6 @@ import { import { PTZControlAction } from '../../src/config/ptz'; import { Actions, - PerformActionActionConfig, RawFrigateCardConfig, frigateCardConfigSchema, } from '../../src/config/types'; @@ -3313,7 +3315,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 +3369,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); + }); }); }); diff --git a/tests/config/types.test.ts b/tests/config/types.test.ts index 6d7dadd2..23db7608 100644 --- a/tests/config/types.test.ts +++ b/tests/config/types.test.ts @@ -55,8 +55,7 @@ describe('config defaults', () => { dimensions: { aspect_ratio: [16, 9], aspect_ratio_mode: 'dynamic', - max_height: '100vh', - min_height: '100px', + height: 'auto', }, image: { mode: 'auto',