diff --git a/src/card-controller/card-element-manager.ts b/src/card-controller/card-element-manager.ts index 365a079b..ed373b9a 100644 --- a/src/card-controller/card-element-manager.ts +++ b/src/card-controller/card-element-manager.ts @@ -277,7 +277,15 @@ export class CardElementManager { const toggleDiagnostics = (): void => { const viewManager = this._api.getViewManager(); if (viewManager.getView()?.view === 'diagnostics') { - viewManager.setViewDefault(); + if (viewManager.canSetViewDefault()) { + // Clear the view when no default one can be built, to return the card + // to what it showed before diagnostics (e.g. an initialization error). + viewManager.setViewDefault(); + } else { + // Won't be a blank card: will show the issue that stopped camera + // initialization, or the loading indicator if that is still running. + viewManager.reset(); + } } else { viewManager.setViewByParameters({ params: { view: 'diagnostics' }, diff --git a/src/card-controller/view/types.ts b/src/card-controller/view/types.ts index 0bf60dcd..f5de74f5 100644 --- a/src/card-controller/view/types.ts +++ b/src/card-controller/view/types.ts @@ -74,6 +74,7 @@ export interface ViewManagerInterface { hasView(): boolean; reset(): void; + canSetViewDefault(): boolean; setViewDefault(options?: ViewFactoryOptions): void; setViewByParameters(options?: ViewFactoryOptions): void; diff --git a/src/card-controller/view/view-manager.ts b/src/card-controller/view/view-manager.ts index 83916ab3..3f485e15 100644 --- a/src/card-controller/view/view-manager.ts +++ b/src/card-controller/view/view-manager.ts @@ -62,6 +62,12 @@ export class ViewManager implements ViewManagerInterface { } } + // Whether a default view will be built at all. A view-creation request made + // while this is false is silently ignored. + public canSetViewDefault(): boolean { + return this._isAllowedToProposeView(); + } + setViewDefault = (options?: ViewFactoryOptions): void => this._setViewGeneric( this._viewFactory.getViewDefault.bind(this._viewFactory), @@ -116,7 +122,11 @@ export class ViewManager implements ViewManagerInterface { viewFactoryFunc: (options?: ViewFactoryOptions) => View | null, options?: ViewFactoryOptions, ): void { - if (!this._isAllowedToProposeView()) { + const isDiagnosticsRequest = options?.params?.view === 'diagnostics'; + + // Diagnostics requires no camera, and is what the user is asked to open + // when the card is broken. + if (!isDiagnosticsRequest && !this._isAllowedToProposeView()) { return; } @@ -131,8 +141,13 @@ export class ViewManager implements ViewManagerInterface { // does not linger invisibly and re-pop on the next evaluation cycle, // and that a stale media_query failure from an abandoned gallery / // viewer doesn't follow the user into an unrelated view. - this._api.getIssueManager().reset('view_incompatible'); - this._api.getIssueManager().reset('media_query'); + // + // Diagnostics is the exception: those issues are part of what its report + // shows, so opening it must not erase them. + if (!isDiagnosticsRequest) { + this._api.getIssueManager().reset('view_incompatible'); + this._api.getIssueManager().reset('media_query'); + } } catch (e) { if (!this._view) { view = this._getFailSafeView(viewFactoryFunc); diff --git a/src/card.ts b/src/card.ts index 7190c5d3..6212d9fc 100644 --- a/src/card.ts +++ b/src/card.ts @@ -413,6 +413,12 @@ export class AdvancedCameraCard extends LitElement { this._config?.performance?.features.card_loading_indicator !== false && !fullCardIssue; + // Always render diagnostics. The issue itself remains and will be rendered + // outside of the diagostics view. + const issueToRender = this._controller.getViewManager().getView()?.is('diagnostics') + ? null + : fullCardIssue; + // Caution: Keep the main div and the menu next to one another in order to // ensure the hover menu styling continues to work. return this._renderInDialogIfNecessary( @@ -485,7 +491,7 @@ export class AdvancedCameraCard extends LitElement { .cardWideConfig=${this._controller.getConfigManager().getCardWideConfig()} .rawConfig=${this._controller.getConfigManager().getRawConfig()} .configManager=${this._controller.getConfigManager()} - .hide=${!!fullCardIssue} + .hide=${!!issueToRender} .microphoneManager=${this._controller.getMicrophoneManager()} .microphoneState=${this._controller.getMicrophoneManager().getState()} .call=${this._controller.getCallManager().getCall() ?? undefined} @@ -500,7 +506,7 @@ export class AdvancedCameraCard extends LitElement { .getStateManager() .getIssuePresence()} > - ${fullCardIssue ? renderNotificationBlock(fullCardIssue.notification) : ''} + ${issueToRender ? renderNotificationBlock(issueToRender.notification) : ''} ${this._renderMenuStatusContainer('bottom')} ${this._config?.elements && diff --git a/src/components-lib/editor/controller.ts b/src/components-lib/editor/controller.ts index 74aaa8c6..065c841f 100644 --- a/src/components-lib/editor/controller.ts +++ b/src/components-lib/editor/controller.ts @@ -13,7 +13,10 @@ import { import { setProfiles } from '../../config/profiles/set-profiles'; import type { EditorMode } from '../../config/schema/editor'; import { profilesSchema, type ProfileType } from '../../config/schema/profiles'; -import { configDefaults } from '../../config/schema/types'; +import { + advancedCameraCardConfigSchema, + configDefaults, +} from '../../config/schema/types'; import type { RawAdvancedCameraCardConfig, RawAdvancedCameraCardConfigArray, @@ -35,9 +38,14 @@ import { getEditorCameraTitle, getEditorFolderTitle } from './titles'; type EditorControllerHost = ReactiveControllerHost & EventTarget; // A card-state notice the editor shows as a banner above the sections. -interface EditorNotice { +export interface EditorNotice { type: 'info' | 'warning'; message: string; + + button?: { + label: string; + callback: () => void; + }; } // Interpret a raw configuration value as an array of configuration objects. @@ -65,6 +73,7 @@ export class EditorController implements ReactiveController { private _defaults = copyConfig(configDefaults); private _configUpgradeable = false; + private _configValid = false; private _initialized = false; private _hass?: HomeAssistant; @@ -108,6 +117,7 @@ export class EditorController implements ReactiveController { private _applyConfig(config: RawAdvancedCameraCardConfig): void { this._config = config; this._configUpgradeable = isConfigUpgradeable(config); + this._configValid = advancedCameraCardConfigSchema.safeParse(config).success; this._editorMode = getEditorMode(config); // The defaults are rebuilt from scratch so that removing (or breaking) a @@ -124,6 +134,17 @@ export class EditorController implements ReactiveController { public getNotices(): EditorNotice[] { const notices: EditorNotice[] = []; + if (this._configUpgradeable) { + notices.push({ + type: 'warning', + message: localize('editor.upgrade_available'), + button: { + label: localize('editor.upgrade'), + callback: () => this.upgrade(), + }, + }); + } + if (this._profiles.includes('low-performance')) { notices.push({ type: 'warning', @@ -196,8 +217,8 @@ export class EditorController implements ReactiveController { return this._hass; } - public isConfigUpgradeable(): boolean { - return this._configUpgradeable; + public isConfigValid(): boolean { + return this._configValid; } public upgrade(): void { diff --git a/src/editor.ts b/src/editor.ts index 8b90418e..7cc357f1 100644 --- a/src/editor.ts +++ b/src/editor.ts @@ -7,6 +7,7 @@ import { type TemplateResult, } from 'lit'; import { customElement, property } from 'lit/decorators.js'; +import { ifDefined } from 'lit/directives/if-defined.js'; import './components/editor/cameras.js'; import './components/editor/doc-link.js'; @@ -248,7 +249,7 @@ export class AdvancedCameraCardEditor extends LitElement implements LovelaceCard ${this._controller.getEditorMode() === 'simple' ? this._renderSimple(hass, input) : this._renderFull(hass, input)} - ${this._renderActionButtons()} + ${this._renderDiagnosticsButton()} `; } @@ -318,39 +319,39 @@ export class AdvancedCameraCardEditor extends LitElement implements LovelaceCard // Card-state notices rendered as banners above the sections (matching how // native Home Assistant editors surface such notes). private _renderNotices(): TemplateResult { - return html`${this._controller - .getNotices() - .map( - (notice) => - html`${notice.message}`, - )}`; - } - - private _renderActionButtons(): TemplateResult { - return html` -
- ${this._controller.isConfigUpgradeable() + return html`${this._controller.getNotices().map((notice) => { + const button = notice.button; + return html` + ${notice.message} + ${button ? html` this._controller.upgrade()} + slot="action" + appearance="outlined" + variant=${ifDefined(notice.type === 'warning' ? 'warning' : undefined)} + @click=${() => button.callback()} > - ${localize('editor.upgrade')} + ${button.label} ` : ''} - { - fireAdvancedCameraCardEvent(this, 'editor:diagnostics'); - }} - > - ${localize('editor.toggle_diagnostics')} - -
- `; + `; + })}`; + } + + private _renderDiagnosticsButton(): TemplateResult { + return html`${ + // Diagnostics won't render on a card with a broken config. + this._controller.isConfigValid() + ? html` { + fireAdvancedCameraCardEvent(this, 'editor:diagnostics'); + }} + > + ${localize('editor.toggle_diagnostics')} + ` + : '' + }`; } // `ha-form` and `ha-selector` lazily `import()` their per-type sub-elements diff --git a/src/localize/languages/en.json b/src/localize/languages/en.json index 92d59e0a..838e8a75 100644 --- a/src/localize/languages/en.json +++ b/src/localize/languages/en.json @@ -784,7 +784,7 @@ "timeline": "Timeline", "timeline_secondary": "Event timeline options", "toggle_diagnostics": "Toggle diagnostics", - "upgrade": "Automatic Upgrade", + "upgrade": "Upgrade", "upgrade_available": "An automatic card configuration upgrade is available", "view": "View", "view_secondary": "What the card should show and how to show it" diff --git a/src/scss/editor.scss b/src/scss/editor.scss index 9f7036f6..9393fd2d 100644 --- a/src/scss/editor.scss +++ b/src/scss/editor.scss @@ -13,6 +13,13 @@ margin-bottom: 8px; } +// The alert sizes the slot its button sits in to `min-content`, which would +// otherwise break the label across lines. +.card-config > ha-alert ha-button { + width: max-content; + white-space: nowrap; +} + // The switch between the editors, set apart from what it switches: it is a // control over the editor rather than one of the card settings below it. Its // icon sits beside the form so the row reads as one of the editor's rows, each @@ -60,11 +67,6 @@ margin-bottom: 8px; } -.action-buttons { - display: flex; - flex-wrap: wrap; -} - -ha-button { +.card-config > ha-button { margin: 10px; } diff --git a/tests/browser/mounted-card.ts b/tests/browser/mounted-card.ts index d5ca97bf..8ccaa745 100644 --- a/tests/browser/mounted-card.ts +++ b/tests/browser/mounted-card.ts @@ -370,6 +370,11 @@ export interface MountOptions { width?: string; height?: string; + // The element the container is created as, for a test about behavior the card + // offers only *within* a particular Home Assistant element (e.g. its card + // edit dialog, which the card looks for among its ancestors). + containerTagName?: string; + // Where that container is placed, as CSS lengths from the page's top left // corner. The page grows to reach it, so a card put beyond the window can // only be brought into view by scrolling. @@ -424,7 +429,7 @@ export class MountedCard { ) { this._hass = hass; - this._container = document.createElement('div'); + this._container = document.createElement(options?.containerTagName ?? 'div'); this._container.style.width = options?.width ?? DEFAULT_CONTAINER_WIDTH; if (options?.height) { this._container.style.height = options.height; diff --git a/tests/card-controller/card-element-manager.browser.test.ts b/tests/card-controller/card-element-manager.browser.test.ts index daa7afdb..3628732d 100644 --- a/tests/card-controller/card-element-manager.browser.test.ts +++ b/tests/card-controller/card-element-manager.browser.test.ts @@ -1,13 +1,23 @@ import { describe, expect, it, onTestFinished } from 'vitest'; +import type { RawAdvancedCameraCardConfig } from '../../src/config/types'; import { createLogAction } from '../../src/utils/action'; -import { deepQueryAll, getFocusedElement, pressKey, pressTab } from '../browser/dom'; +import { + deepQuery, + deepQueryAll, + getFocusedElement, + pressKey, + pressTab, +} from '../browser/dom'; import { MountedCardFactory, type MountedCard } from '../browser/mounted-card'; import { CARD_INITIALIZED_MESSAGE, createGenericCameraHASS, createInitializedAutomation, + createStillImageCameraConfig, createStillImageCardConfig, + getBlockNotificationText, + isLiveMediaShowing, } from '../browser/test-utils'; // What the automation writes when it runs, written as the pattern the console @@ -68,7 +78,100 @@ const addTrailingControl = (): HTMLElement => { return control; }; +// The Home Assistant dialog the card is previewed in while its configuration is +// edited, which is what the card looks for before answering the editor's +// diagnostics button. +const EDIT_DIALOG_TAG_NAME = 'hui-dialog-edit-card'; + +const INIT_FAILED_ISSUE_HEADING = 'Initialization failed'; + +const DIAGNOSTICS_SELECTOR = 'advanced-camera-card-diagnostics'; + +const mountCardInEditDialog = async ( + config: RawAdvancedCameraCardConfig, +): Promise => + await MountedCardFactory.createFromSource(config, createGenericCameraHASS(), { + containerTagName: EDIT_DIALOG_TAG_NAME, + }); + +/** + * Press the editor's diagnostics button. The editor is elsewhere in the dialog + * rather than within the card, so the event is fired from a sibling of it. + */ +const toggleDiagnostics = (card: MountedCard): void => { + const editor = document.createElement('div'); + card.card.parentElement?.append(editor); + + editor.dispatchEvent( + new CustomEvent('advanced-camera-card:editor:diagnostics', { + bubbles: true, + composed: true, + }), + ); + + editor.remove(); +}; + +const isDiagnosticsShowing = (card: MountedCard): boolean => + !!deepQuery(card.card, DIAGNOSTICS_SELECTOR); + describe('CardElementManager', () => { + describe('should toggle diagnostics from the editor', () => { + it('should show diagnostics over a card that could not be started', async () => { + // A camera Home Assistant has never heard of, so the card cannot start + // and shows an issue in place of its views. + const card = await mountCardInEditDialog( + createStillImageCardConfig({ + cameras: [createStillImageCameraConfig('camera.missing')], + view: { issues: { retry_seconds: 0 } }, + }), + ); + await card.waitForRender( + () => + getBlockNotificationText(card.card).includes(INIT_FAILED_ISSUE_HEADING) || + null, + 'the initialization issue', + ); + + toggleDiagnostics(card); + + // Diagnostics is what the user is asked for when the card is broken, so + // it must be reachable in the state the issue describes. + await card.waitForSelector(DIAGNOSTICS_SELECTOR); + expect(getBlockNotificationText(card.card)).not.toContain( + INIT_FAILED_ISSUE_HEADING, + ); + + toggleDiagnostics(card); + + // Toggling diagnostics again just puts the issue back in front of the + // user. + await card.waitForRender( + () => + getBlockNotificationText(card.card).includes(INIT_FAILED_ISSUE_HEADING) || + null, + 'the initialization issue', + ); + expect(isDiagnosticsShowing(card)).toBe(false); + }); + + it('should return a started card to its default view', async () => { + const card = await mountCardInEditDialog(createStillImageCardConfig()); + await card.events.waitForFirst('advanced-camera-card:media:loaded'); + + toggleDiagnostics(card); + await card.waitForSelector(DIAGNOSTICS_SELECTOR); + + toggleDiagnostics(card); + + await card.waitForRender( + () => isLiveMediaShowing(card.card) || null, + 'the live view', + ); + expect(isDiagnosticsShowing(card)).toBe(false); + }); + }); + it('should be reachable by tabbing', async () => { const card = await mountCard(); diff --git a/tests/card-controller/card-element-manager.test.ts b/tests/card-controller/card-element-manager.test.ts index e93371ad..c185bbce 100644 --- a/tests/card-controller/card-element-manager.test.ts +++ b/tests/card-controller/card-element-manager.test.ts @@ -439,6 +439,7 @@ describe('CardElementManager', () => { vi.mocked(api.getViewManager().getView).mockReturnValue( new View({ view: 'diagnostics' }), ); + vi.mocked(api.getViewManager().canSetViewDefault).mockReturnValue(true); const dialog = createDialogWithCard(element); document.body.append(dialog); @@ -449,6 +450,31 @@ describe('CardElementManager', () => { expect(api.getViewManager().setViewDefault).toHaveBeenCalled(); }); + it('should reset the view when leaving diagnostics with no default view available', () => { + const api = createCardAPI(); + const element = createCardHTMLElement(); + const manager = new CardElementManager( + api, + element, + () => undefined, + () => undefined, + ); + + vi.mocked(api.getViewManager().getView).mockReturnValue( + new View({ view: 'diagnostics' }), + ); + vi.mocked(api.getViewManager().canSetViewDefault).mockReturnValue(false); + + const dialog = createDialogWithCard(element); + document.body.append(dialog); + manager.elementConnected(); + + fireFromDialog(dialog); + + expect(api.getViewManager().reset).toHaveBeenCalled(); + expect(api.getViewManager().setViewDefault).not.toHaveBeenCalled(); + }); + it('does not set view to diagnostics if card is not in editor', () => { const api = createCardAPI(); const element = createCardHTMLElement(); diff --git a/tests/card-controller/view/view-manager.test.ts b/tests/card-controller/view/view-manager.test.ts index d401d226..cb83dd04 100644 --- a/tests/card-controller/view/view-manager.test.ts +++ b/tests/card-controller/view/view-manager.test.ts @@ -203,6 +203,26 @@ describe('should not set view without cameras being initialized', () => { expect(manager.getView()).toBeNull(); }); + + it('should report whether a default view can be set', () => { + expect(new ViewManager(createInitializedCardAPI()).canSetViewDefault()).toBe(true); + expect(new ViewManager(createInitializedCardAPI(false)).canSetViewDefault()).toBe( + false, + ); + }); + + it('should set the diagnostics view without cameras being initialized', () => { + const view = createView({ view: 'diagnostics' }); + const factory = mock(); + factory.getViewByParameters.mockReturnValue(view); + + const manager = new ViewManager(createInitializedCardAPI(false), { + viewFactory: factory, + }); + manager.setViewByParameters({ params: { view: 'diagnostics' } }); + + expect(manager.getView()).toBe(view); + }); }); describe('should respect microphone navigation lock', () => { @@ -494,6 +514,19 @@ describe('should handle exceptions', () => { expect(api.getIssueManager().reset).toHaveBeenCalledWith('view_incompatible'); }); + it('should retain issues when the diagnostics view is requested', () => { + const viewFactory = mock(); + viewFactory.getViewByParameters.mockReturnValue(createView({ view: 'diagnostics' })); + + const api = createInitializedCardAPI(); + const manager = new ViewManager(api, { viewFactory }); + manager.setViewByParameters({ params: { view: 'diagnostics' } }); + + expect(manager.getView()?.is('diagnostics')).toBeTruthy(); + expect(api.getIssueManager().reset).not.toHaveBeenCalledWith('view_incompatible'); + expect(api.getIssueManager().reset).not.toHaveBeenCalledWith('media_query'); + }); + it('should return null when failSafe view factory also throws', () => { const viewFactory = mock(); viewFactory.getViewDefault.mockImplementation(() => { diff --git a/tests/components-lib/editor/controller.test.ts b/tests/components-lib/editor/controller.test.ts index a94ecbc4..baffb35d 100644 --- a/tests/components-lib/editor/controller.test.ts +++ b/tests/components-lib/editor/controller.test.ts @@ -10,7 +10,10 @@ import { type Mock, } from 'vitest'; -import { EditorController } from '../../../src/components-lib/editor/controller'; +import { + EditorController, + type EditorNotice, +} from '../../../src/components-lib/editor/controller'; import { getConfigValue } from '../../../src/config/management'; import { configDefaults } from '../../../src/config/schema/types'; import type { RawAdvancedCameraCardConfig } from '../../../src/config/types'; @@ -62,6 +65,11 @@ const createUpgradeableConfig = (): RawAdvancedCameraCardConfig => ({ ], }); +const getUpgradeNotice = (controller: EditorController): EditorNotice | null => + controller + .getNotices() + .find((notice) => notice.message === localize('editor.upgrade_available')) ?? null; + // @vitest-environment jsdom describe('EditorController', () => { beforeEach(() => { @@ -118,13 +126,24 @@ describe('EditorController', () => { it('should detect an upgradeable configuration', () => { const { controller } = createController(); - expect(controller.isConfigUpgradeable()).toBeFalsy(); + expect(getUpgradeNotice(controller)).toBeNull(); controller.setConfig(createUpgradeableConfig()); - expect(controller.isConfigUpgradeable()).toBeTruthy(); + expect(getUpgradeNotice(controller)).toBeTruthy(); controller.setConfig({ cameras: [] }); - expect(controller.isConfigUpgradeable()).toBeFalsy(); + expect(getUpgradeNotice(controller)).toBeNull(); + }); + + it('should detect an invalid configuration', () => { + const { controller } = createController(); + expect(controller.isConfigValid()).toBeFalsy(); + + controller.setConfig({ type: 'custom:advanced-camera-card', cameras: [] }); + expect(controller.isConfigValid()).toBeTruthy(); + + controller.setConfig({ type: 'custom:advanced-camera-card', cameras: 'nope' }); + expect(controller.isConfigValid()).toBeFalsy(); }); it('should apply profile defaults', () => { @@ -278,13 +297,15 @@ describe('EditorController', () => { const { controller, configListener } = createController(); controller.setConfig(createUpgradeableConfig()); - controller.upgrade(); + const notice = getUpgradeNotice(controller); + assert(notice?.button); + notice.button.callback(); const config = getLastConfig(configListener); expect(getConfigValue(config, 'elements.0.tap_action.data')).toEqual({ message: 'Hello', }); - expect(controller.isConfigUpgradeable()).toBeFalsy(); + expect(getUpgradeNotice(controller)).toBeNull(); }); it('should do nothing without a configuration', () => {