fix: Move upgrade button to top of editor (#2687)

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') {
+        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' },

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;
 

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.
+      //
+      // 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);

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()}
             ></advanced-camera-card-views>
-            ${fullCardIssue ? renderNotificationBlock(fullCardIssue.notification) : ''}
+            ${issueToRender ? renderNotificationBlock(issueToRender.notification) : ''}
           </div>
           ${this._renderMenuStatusContainer('bottom')}
           ${this._config?.elements &&

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 {
+  public isConfigValid(): boolean {
-    return this._configUpgradeable;
+    return this._configValid;
   }
 
   public upgrade(): void {

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()}
       </div>
     `;
   }
@@ -318,29 +319,29 @@ 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
+    return html`${this._controller.getNotices().map((notice) => {
-      .getNotices()
+      const button = notice.button;
-      .map(
+      return html`<ha-alert alert-type=${notice.type}>
-        (notice) =>
+        ${notice.message}
-          html`<ha-alert alert-type=${notice.type}>${notice.message}</ha-alert>`,
+        ${button
-      )}`;
-  }
-
-  private _renderActionButtons(): TemplateResult {
-    return html`
-      <div class="action-buttons">
-        ${this._controller.isConfigUpgradeable()
           ? html`<ha-button
-              appearance="filled"
+              slot="action"
-              variant="warning"
+              appearance="outlined"
-              title=${localize('editor.upgrade_available')}
+              variant=${ifDefined(notice.type === 'warning' ? 'warning' : undefined)}
-              aria-label=${localize('editor.upgrade_available')}
+              @click=${() => button.callback()}
-              @click=${() => this._controller.upgrade()}
             >
-              ${localize('editor.upgrade')}
+              ${button.label}
             </ha-button>`
           : ''}
-        <ha-button
+      </ha-alert>`;
+    })}`;
+  }
+
+  private _renderDiagnosticsButton(): TemplateResult {
+    return html`${
+      // Diagnostics won't render on a card with a broken config.
+      this._controller.isConfigValid()
+        ? html`<ha-button
             title=${localize('editor.toggle_diagnostics')}
             aria-label=${localize('editor.toggle_diagnostics')}
             @click=${() => {
@@ -348,9 +349,9 @@ export class AdvancedCameraCardEditor extends LitElement implements LovelaceCard
             }}
           >
             ${localize('editor.toggle_diagnostics')}
-        </ha-button>
+          </ha-button>`
-      </div>
+        : ''
-    `;
+    }`;
   }
 
   // `ha-form` and `ha-selector` lazily `import()` their per-type sub-elements

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"

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 {
+.card-config > ha-button {
-  display: flex;
-  flex-wrap: wrap;
-}
-
-ha-button {
   margin: 10px;
 }

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;

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<MountedCard> =>
+  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();
 

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();

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<ViewFactory>();
+    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>();
+    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>();
     viewFactory.getViewDefault.mockImplementation(() => {

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', () => {