From a6e2d2bd4b3a9953b0e4d6ea1e0e09c925b8b281 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 30 Sep 2026 11:02:05 +1000 Subject: [PATCH 1/4] fix(overlays): block confirm dismissal while its action runs Escape or a backdrop click during an action closed the dialog. Material then nulls componentInstance, and the next loading.set() threw. The modal now sets disableClose while loading is set, and wrappers that touch componentInstance after an await use optional chaining. Also rename ConfirmRepsonse to ConfirmResponse, type reason as 'done' | undefined (it is undefined on dismiss), and move the receiptToTsv JSDoc back above its function. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../cluster-task-list.component.ts | 2 +- src/app/admin/extensions.component.ts | 2 +- src/app/overlays/confirm-modal.component.ts | 39 ++++++++++++---- src/tests/groups/group-state.service.spec.ts | 6 ++- .../overlays/confirm-modal.component.spec.ts | 46 +++++++++++++++++++ src/tests/users/user-lifecycle.spec.ts | 2 +- 6 files changed, 84 insertions(+), 13 deletions(-) diff --git a/src/app/admin/cluster-details/cluster-task-list.component.ts b/src/app/admin/cluster-details/cluster-task-list.component.ts index 43ed72be2..587848bf2 100644 --- a/src/app/admin/cluster-details/cluster-task-list.component.ts +++ b/src/app/admin/cluster-details/cluster-task-list.component.ts @@ -270,7 +270,7 @@ export class PlaceClusterTaskListComponent ref.close(); }, (err) => { - ref.componentInstance.loading.set(null); + ref.componentInstance?.loading.set(''); this.killing.set(null); notifyError( i18n('ADMIN.CLUSTER_PROCESS_KILL_ERROR', { diff --git a/src/app/admin/extensions.component.ts b/src/app/admin/extensions.component.ts index 410b6e538..e4868ef7f 100644 --- a/src/app/admin/extensions.component.ts +++ b/src/app/admin/extensions.component.ts @@ -271,7 +271,7 @@ export class PlaceExtensionsComponent implements OnInit { await this.updateDomain(ext_list).catch((e) => notifyError(`Error removing extension: ${e}`), ); - ref.componentInstance.loading.set(''); + ref.componentInstance?.loading.set(''); ref.close(); }); } diff --git a/src/app/overlays/confirm-modal.component.ts b/src/app/overlays/confirm-modal.component.ts index 66f169fd8..0b4f1387c 100644 --- a/src/app/overlays/confirm-modal.component.ts +++ b/src/app/overlays/confirm-modal.component.ts @@ -4,6 +4,7 @@ import { OnInit, Output, computed, + effect, inject, signal, } from '@angular/core'; @@ -104,11 +105,6 @@ export interface ConfirmModalData { close_delay?: number; } -/** - * Renders a receipt as tab separated rows, for pasting into a ticket or - * spreadsheet. Failed rows are marked so a partial run is not mistaken for a - * complete one. - */ /** * Readable text for whatever an option's `details()` rejected with. * @@ -133,6 +129,11 @@ export function describeError(error: unknown): string { return 'Unknown error'; } +/** + * Renders a receipt as tab separated rows, for pasting into a ticket or + * spreadsheet. Failed rows are marked so a partial run is not mistaken for a + * complete one. + */ export function receiptToTsv(result: ConfirmModalResult): string { const rows = [ ...result.items.map((item) => [item.type, item.name, item.id]), @@ -156,17 +157,26 @@ export const CONFIRM_METADATA = { height: 'auto', }; -export interface ConfirmRepsonse { - reason: 'done' | '' | null; +export interface ConfirmResponse { + /** + * `'done'` when the user confirmed. `undefined` when the modal was + * dismissed (Cancel, Escape or backdrop click). + */ + reason?: 'done'; metadata?: { options?: ConfirmModalSelection }; loading: (_: string) => void; close: () => void; } +/** + * Opens a confirm modal and resolves when the user confirms or dismisses it. + * Always resolves to an object, so callers must check + * `details.reason !== 'done'` before they run the action. + */ export async function openConfirmModal( data: ConfirmModalData, dialog: MatDialog, -): Promise { +): Promise { const ref = dialog.open( ConfirmModalComponent, { @@ -182,7 +192,7 @@ export async function openConfirmModal( ), lastValueFrom(ref.afterClosed()), ])), - loading: (s) => ref.componentInstance.loading.set(s), + loading: (s) => ref.componentInstance?.loading.set(s), close: () => ref.close(), }; } @@ -552,6 +562,17 @@ export class ConfirmModalComponent extends AsyncHandler implements OnInit { /** Allow the user to close the modal */ public readonly enableClose = () => (this._dialog_ref.disableClose = false); + constructor() { + super(); + // Block Escape and backdrop dismissal while the action runs. Material + // nulls `componentInstance` on close, so a dismissal mid-action makes + // the caller's later `loading.set()` throw. The receipt stays + // dismissable. + effect(() => { + this._dialog_ref.disableClose = !!this.loading() && !this.result(); + }); + } + public ngOnInit() { // An option that starts enabled has never been toggled, so nothing has // asked it for a breakdown. Without this it would count as selected diff --git a/src/tests/groups/group-state.service.spec.ts b/src/tests/groups/group-state.service.spec.ts index 6b0e88bb7..bb3b41520 100644 --- a/src/tests/groups/group-state.service.spec.ts +++ b/src/tests/groups/group-state.service.spec.ts @@ -164,7 +164,11 @@ describe('group membership actions', () => { it.each(['user', 'zone'] as const)( 'does not remove a %s when confirmation is cancelled', async (kind) => { - mocks.confirm.mockResolvedValue({ reason: '', close, loading }); + mocks.confirm.mockResolvedValue({ + reason: undefined, + close, + loading, + }); if (kind === 'user') await service.removeUser(user); else await service.removeZone(zone); expect(mocks.removeGroupUser).not.toHaveBeenCalled(); diff --git a/src/tests/overlays/confirm-modal.component.spec.ts b/src/tests/overlays/confirm-modal.component.spec.ts index 292811502..14a18c84f 100644 --- a/src/tests/overlays/confirm-modal.component.spec.ts +++ b/src/tests/overlays/confirm-modal.component.spec.ts @@ -1,10 +1,13 @@ +import { EventEmitter, signal } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { MAT_DIALOG_DATA, + MatDialog, MatDialogModule, MatDialogRef, } from '@angular/material/dialog'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; +import { Subject } from 'rxjs'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // Mock the ts-client - factory must be self-contained @@ -30,11 +33,13 @@ vi.mock('../../app/common/notifications', () => ({ })); import { MockComponent } from 'ng-mocks'; +import { DialogEvent } from '../../app/common/types'; import { CONFIRM_METADATA, ConfirmModalComponent, ConfirmModalData, describeError, + openConfirmModal, receiptToTsv, } from '../../app/overlays/confirm-modal.component'; import { IconComponent } from '../../app/ui/icon.component'; @@ -572,6 +577,18 @@ describe('ConfirmModalComponent', () => { component.enableClose(); expect(dialog_ref_mock.disableClose).toBe(false); }); + + it('should block dismissal only while loading', () => { + // A dismissal mid-action tears down the instance the caller is + // still writing to. + component.loading.set('Deleting...'); + fixture.detectChanges(); + expect(dialog_ref_mock.disableClose).toBe(true); + + component.loading.set(''); + fixture.detectChanges(); + expect(dialog_ref_mock.disableClose).toBe(false); + }); }); describe('ngOnInit with close_delay', () => { @@ -681,6 +698,35 @@ describe('ConfirmModalComponent', () => { }); }); +describe('openConfirmModal', () => { + it('does not report a dismissed modal as confirmed', async () => { + const closed = new Subject(); + const ref = { + componentInstance: { + event: new EventEmitter(), + loading: signal(''), + } as Pick | null, + afterClosed: () => closed.asObservable(), + close: vi.fn(), + }; + const dialog = { open: vi.fn(() => ref) } as unknown as MatDialog; + + const pending = openConfirmModal( + { title: '', content: '', icon: { content: 'delete' } }, + dialog, + ); + // Cancel, Escape and backdrop all close without a result, and + // Material drops the component instance on close. + ref.componentInstance = null; + closed.next(undefined); + closed.complete(); + const details = await pending; + + expect(details.reason).not.toBe('done'); + expect(() => details.loading('Deleting...')).not.toThrow(); + }); +}); + describe('receiptToTsv', () => { it('renders one tab separated row per resource', () => { expect( diff --git a/src/tests/users/user-lifecycle.spec.ts b/src/tests/users/user-lifecycle.spec.ts index 708d73364..461891274 100644 --- a/src/tests/users/user-lifecycle.spec.ts +++ b/src/tests/users/user-lifecycle.spec.ts @@ -163,7 +163,7 @@ describe('user restoration', () => { it('does not restore after cancellation', async () => { vi.mocked(openConfirmModal).mockResolvedValue({ - reason: '', + reason: undefined, close, loading, }); From e8f2be053f0db6332f1ba5e8ab43440fdf191f73 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 30 Sep 2026 11:02:10 +1000 Subject: [PATCH 2/4] fix: stop cancelled confirm dialogs from running actions openConfirmModal always resolves to an object, so `if (!details) return` never fired. Cancel, Escape and backdrop clicks still ran the delete in domains (application), edge, build list, brokers, signage plugins and trigger reorder. All callers now use `details.reason !== 'done'`. Also close the modal on every path so no spinner is left stuck: - trigger reorder now closes on success and error, with a reorder icon - metadata removal now closes the modal after confirming - api keys, storage, upload library and resource imports close in a finally block when the request throws Co-Authored-By: Claude Opus 5.5 (1M context) --- src/app/admin/api-keys/api-keys.service.ts | 15 ++++++++----- src/app/admin/brokers.component.ts | 2 +- src/app/admin/build-list.component.ts | 2 +- src/app/admin/edge.component.ts | 2 +- src/app/admin/resource-imports.component.ts | 9 ++++++-- .../signage-plugins.component.ts | 2 +- src/app/admin/staff-api.component.ts | 2 +- src/app/admin/storage/storage.component.ts | 7 ++++-- src/app/admin/upload-library.component.ts | 15 ++++++++----- src/app/domains/domain-state.service.ts | 6 ++--- src/app/drivers/driver-state.service.ts | 8 +++---- src/app/systems/system-state.service.ts | 22 +++++++++---------- src/app/triggers/trigger-state.service.ts | 11 +++++----- src/app/ui/metadata-display.component.ts | 2 ++ src/app/zones/zones-state.service.ts | 4 ++-- 15 files changed, 63 insertions(+), 46 deletions(-) diff --git a/src/app/admin/api-keys/api-keys.service.ts b/src/app/admin/api-keys/api-keys.service.ts index e7dfe69da..6ed31c5b5 100644 --- a/src/app/admin/api-keys/api-keys.service.ts +++ b/src/app/admin/api-keys/api-keys.service.ts @@ -223,12 +223,15 @@ export class APIKeyService { ); if (details?.reason !== 'done') return; details.loading('Removing API key...'); - await remove({ - id: key.id, - query_params: {}, - path: 'api_keys', - }); - details.close(); + try { + await remove({ + id: key.id, + query_params: {}, + path: 'api_keys', + }); + } finally { + details.close(); + } notifySuccess('Successfully removed API key.'); this._change.set(Date.now()); } diff --git a/src/app/admin/brokers.component.ts b/src/app/admin/brokers.component.ts index 2f8bebdb5..234d0f34c 100644 --- a/src/app/admin/brokers.component.ts +++ b/src/app/admin/brokers.component.ts @@ -251,7 +251,7 @@ export class AdminBrokersComponent extends AsyncHandler implements OnInit { }, this._dialog, ); - if (!details) return; + if (details.reason !== 'done') return; details.loading('Deleting broker...'); const err = await removeBroker(item.id).catch((_) => _); details.close(); diff --git a/src/app/admin/build-list.component.ts b/src/app/admin/build-list.component.ts index 03f4e486c..da1942124 100644 --- a/src/app/admin/build-list.component.ts +++ b/src/app/admin/build-list.component.ts @@ -166,7 +166,7 @@ export class PlaceBuildListComponent implements OnInit { }, this._dialog, ); - if (!details) return; + if (details.reason !== 'done') return; details.loading(i18n('ADMIN.BUILD_LIST_REMOVE_LOADING')); const err = await cancelBuildJob(i.id).catch((_) => _); details.close(); diff --git a/src/app/admin/edge.component.ts b/src/app/admin/edge.component.ts index 8b5bb97fb..404b1f78a 100644 --- a/src/app/admin/edge.component.ts +++ b/src/app/admin/edge.component.ts @@ -233,7 +233,7 @@ export class PlaceEdgeComponent implements OnInit { }, this._dialog, ); - if (!details) return; + if (details.reason !== 'done') return; details.loading('Removing edge...'); const err = await removeEdge(i.id).catch((_) => _); details.close(); diff --git a/src/app/admin/resource-imports.component.ts b/src/app/admin/resource-imports.component.ts index d922a3188..45f7a8010 100644 --- a/src/app/admin/resource-imports.component.ts +++ b/src/app/admin/resource-imports.component.ts @@ -226,8 +226,13 @@ export class ResourceImportsComponent implements OnInit { if (resp?.reason !== 'done') return; resp.loading(i18n('ADMIN.RESOURCE_IMPORTS_ALL_LOADING')); - await Promise.all(missing.map((_) => this.importResource(_, false))); - resp.close(); + try { + await Promise.all( + missing.map((_) => this.importResource(_, false)), + ); + } finally { + resp.close(); + } notifySuccess( i18n('ADMIN.RESOURCE_IMPORTS_ALL_SUCCESS', { count: missing.length, diff --git a/src/app/admin/signage-plugins/signage-plugins.component.ts b/src/app/admin/signage-plugins/signage-plugins.component.ts index 159607ec4..1edcdd915 100644 --- a/src/app/admin/signage-plugins/signage-plugins.component.ts +++ b/src/app/admin/signage-plugins/signage-plugins.component.ts @@ -282,7 +282,7 @@ export class AdminSignagePluginsComponent }, this._dialog, ); - if (!details) return; + if (details.reason !== 'done') return; details.loading(i18n('ADMIN.SIGNAGE_PLUGINS_REMOVE_LOADING')); const err = await removeSignagePlugin(item.id).catch((_) => _); details.close(); diff --git a/src/app/admin/staff-api.component.ts b/src/app/admin/staff-api.component.ts index f39b68c95..544275828 100644 --- a/src/app/admin/staff-api.component.ts +++ b/src/app/admin/staff-api.component.ts @@ -260,7 +260,7 @@ export class PlaceStaffAPIComponent implements OnInit { }, this._dialog, ); - if (!details || !details.reason) return; + if (details.reason !== 'done') return; details.loading('Removing tenant from domain...'); const system = await del(`/api/staff/v1/tenants/${tenant.id}`).catch( (err) => { diff --git a/src/app/admin/storage/storage.component.ts b/src/app/admin/storage/storage.component.ts index 08be7e8c5..19efc24ec 100644 --- a/src/app/admin/storage/storage.component.ts +++ b/src/app/admin/storage/storage.component.ts @@ -229,8 +229,11 @@ export class StorageComponent implements OnInit { ); if (resp.reason !== 'done') return; resp.loading(i18n('ADMIN.STORAGE_REMOVE_LOADING')); - await removeStorage(item.id); - resp.close(); + try { + await removeStorage(item.id); + } finally { + resp.close(); + } this.loadStorage(); } diff --git a/src/app/admin/upload-library.component.ts b/src/app/admin/upload-library.component.ts index ad24f8f0e..43f6984fe 100644 --- a/src/app/admin/upload-library.component.ts +++ b/src/app/admin/upload-library.component.ts @@ -539,12 +539,15 @@ export class UploadLibraryComponent extends AsyncHandler implements OnInit { ); if (result?.reason !== 'done') return; result.loading(i18n('ADMIN.UPLOADS_LIB_REMOVE_LOADING')); - await remove({ - id: upload.id, - query_params: {}, - path: 'uploads', - }); - result.close(); + try { + await remove({ + id: upload.id, + query_params: {}, + path: 'uploads', + }); + } finally { + result.close(); + } this.refresh.update((value) => value + 1); } } diff --git a/src/app/domains/domain-state.service.ts b/src/app/domains/domain-state.service.ts index d594f52f8..b9f336794 100644 --- a/src/app/domains/domain-state.service.ts +++ b/src/app/domains/domain-state.service.ts @@ -246,7 +246,7 @@ export class DomainStateService { ), waitForEvent(ref.afterClosed()), ]); - if (!details) return; + if (details.reason !== 'done') return; this._changed.set(new Date().valueOf()); } @@ -263,7 +263,7 @@ export class DomainStateService { }, this._dialog, ); - if (!details) return; + if (details.reason !== 'done') return; details.loading('Deleting domain application...'); const err = await removeApplication(item.id).catch((_) => _); details.close(); @@ -294,7 +294,7 @@ export class DomainStateService { ), waitForEvent(ref.afterClosed()), ]); - if (!details) return; + if (details.reason !== 'done') return; this._changed.set(new Date().valueOf()); } diff --git a/src/app/drivers/driver-state.service.ts b/src/app/drivers/driver-state.service.ts index ec415b979..f352c50bf 100644 --- a/src/app/drivers/driver-state.service.ts +++ b/src/app/drivers/driver-state.service.ts @@ -111,7 +111,7 @@ export class DriverStateService { }, this._dialog, ); - if (!details?.reason) return details.close(); + if (details.reason !== 'done') return details.close(); details.loading('Updating driver...'); const success = await updateDriver(item.id, { ...item, @@ -133,7 +133,7 @@ export class DriverStateService { }, this._dialog, ); - if (!details?.reason) return details.close(); + if (details.reason !== 'done') return details.close(); details.loading('Recompiling driver... This may take a while.'); await recompileDriver(item.id).catch(async (e) => { console.log('Error:', e); @@ -160,7 +160,7 @@ export class DriverStateService { }, this._dialog, ); - if (!details?.reason) return details.close(); + if (details.reason !== 'done') return details.close(); details.loading('Reload driver... This may take a while.'); const success = await reloadDriver(item.id).catch(() => false); if (success === false) { @@ -186,7 +186,7 @@ export class DriverStateService { }, this._dialog, ); - if (!details?.reason) return; + if (details.reason !== 'done') return; const system = await removeSystemModule( this.active_item.id, device.id, diff --git a/src/app/systems/system-state.service.ts b/src/app/systems/system-state.service.ts index fd0a715d6..9ca858057 100644 --- a/src/app/systems/system-state.service.ts +++ b/src/app/systems/system-state.service.ts @@ -279,7 +279,7 @@ export class SystemStateService extends AsyncHandler { content: `Are you sure you want to start this system?
All stopped modules within the system will boot up.`, icon: { type: 'icon', class: 'backoffice-controller-play' }, }); - if (!details?.reason) return; + if (details.reason !== 'done') return; details.loading('Starting system...'); const error = await startSystem(this.active_item.id) .then(() => null) @@ -307,7 +307,7 @@ export class SystemStateService extends AsyncHandler { content: `Are you sure you want to stop this system?
All modules will be immediately stopped regardless of any other systems they may be in.`, icon: { type: 'icon', class: 'backoffice-controller-stop' }, }); - if (!details?.reason) return; + if (details.reason !== 'done') return; details.loading('Stopping system...'); const error = await stopSystem(this.active_item.id) .then(() => null) @@ -383,7 +383,7 @@ export class SystemStateService extends AsyncHandler { ), waitForEvent(ref.afterClosed()), ]); - if (!details?.reason) return ref.close(); + if (details.reason !== 'done') return ref.close(); const system = ref.componentInstance.item as PlaceSystem; if (!system) return ref.close(); await addSystemModule(system.id, device.id).catch((_e) => { @@ -424,7 +424,7 @@ export class SystemStateService extends AsyncHandler { ), waitForEvent(ref.afterClosed()), ]); - if (!details?.reason) return ref.close(); + if (details.reason !== 'done') return ref.close(); const t = await this.addTrigger( ref.componentInstance.item as PlaceTrigger, ); @@ -463,7 +463,7 @@ export class SystemStateService extends AsyncHandler { ), waitForEvent(ref.afterClosed()), ]); - if (!details?.reason) return; + if (details.reason !== 'done') return; instance.loading = 'Saving trigger settings...'; const url = `${apiEndpoint()}/systems/${ @@ -491,7 +491,7 @@ export class SystemStateService extends AsyncHandler { content: `

Are you sure you want remove trigger "${trigger.name}"?

Configuration will be updated immediately.

`, icon: { type: 'icon', content: 'delete' }, }); - if (!details?.reason) return; + if (details.reason !== 'done') return; await removeSystemTrigger(this.active_item.id, trigger.id).catch( (err) => { details.close(); @@ -514,7 +514,7 @@ export class SystemStateService extends AsyncHandler { content: `Are you sure you want to change the module priority?
Settings will be updated immediately for the system.`, icon: { type: 'icon', content: 'layers' }, }); - if (!details?.reason) return; + if (details.reason !== 'done') return; details.loading('Updating module order...'); const list: string[] = [...this.active_item.modules]; moveItemInArray(list, fst, snd); @@ -545,7 +545,7 @@ export class SystemStateService extends AsyncHandler { content: `Are you sure you want to sort modules by class?
Modules with the same class name will be grouped together.`, icon: { type: 'icon', content: 'sort' }, }); - if (!details?.reason) return; + if (details.reason !== 'done') return; details.loading('Sorting modules by class...'); let sorted_modules = []; if (alphabetical) { @@ -616,7 +616,7 @@ export class SystemStateService extends AsyncHandler { content: `Are you sure you want to change the zone priority?
Settings will be updated immediately for the system.`, icon: { type: 'icon', content: 'layers' }, }); - if (!details?.reason) return; + if (details.reason !== 'done') return; details.loading('Updating zone order...'); const resp = await updateSystem(this.active_item.id, { ...this.active_item, @@ -666,7 +666,7 @@ export class SystemStateService extends AsyncHandler { content: `Remove ${device.driver_id} from this system?
If this is not used elsewhere the associated data will be removed immediately.`, icon: { type: 'icon', content: 'delete' }, }); - if (!details?.reason) return; + if (details.reason !== 'done') return; const system = await removeSystemModule( this.active_item.id, device.id, @@ -715,7 +715,7 @@ export class SystemStateService extends AsyncHandler { content: `

Are you sure you want remove zone "${zone.name}" from the system?

Configuration will be updated immediately.`, icon: { type: 'icon', content: 'delete' }, }); - if (!details?.reason) return; + if (details.reason !== 'done') return; const zones = this.active_item.zones.filter((z) => z !== zone.id); const system = await updateSystem(this.active_item.id, { ...this.active_item, diff --git a/src/app/triggers/trigger-state.service.ts b/src/app/triggers/trigger-state.service.ts index bd7f2b69d..a9935a51a 100644 --- a/src/app/triggers/trigger-state.service.ts +++ b/src/app/triggers/trigger-state.service.ts @@ -144,11 +144,11 @@ export class TriggerStateService { { title: i18n('TRIGGERS.REORDER_CONFIRM_TITLE', { type }), content: i18n('TRIGGERS.REORDER_CONFIRM_MSG'), - icon: { type: 'icon', content: 'delete' }, + icon: { type: 'icon', content: 'reorder' }, }, this._dialog, ); - if (!details) return; + if (details.reason !== 'done') return; const list: Array = [ ...(type === 'function' ? this.active_item.actions.functions @@ -173,6 +173,7 @@ export class TriggerStateService { ...this.active_item.toJSON(), actions, }).catch((_) => _); + details.close(); if (!(resp instanceof PlaceTrigger)) { const error = resp as { response?: string; message?: string }; return notifyError( @@ -198,7 +199,7 @@ export class TriggerStateService { }, this._dialog, ); - if (!details?.reason) return; + if (details.reason !== 'done') return; details.loading(i18n('TRIGGERS.REMOVE_CONDITION_LOADING')); const item = this.active_item; const conditions = { @@ -240,7 +241,7 @@ export class TriggerStateService { }, this._dialog, ); - if (!details?.reason) return; + if (details.reason !== 'done') return; details.loading(i18n('TRIGGERS.REMOVE_ACTION_LOADING')); const item = this.active_item; const actions = { @@ -290,7 +291,7 @@ export class TriggerStateService { }, this._dialog, ); - if (!details?.reason) return; + if (details.reason !== 'done') return; details.loading(i18n('TRIGGERS.REMOVE_INSTANCE_LOADING', { type })); const method = type === 'zone' ? removeSystemTrigger : removeSystemTrigger; diff --git a/src/app/ui/metadata-display.component.ts b/src/app/ui/metadata-display.component.ts index 8c5dd0beb..78d603353 100644 --- a/src/app/ui/metadata-display.component.ts +++ b/src/app/ui/metadata-display.component.ts @@ -330,6 +330,7 @@ export class MetadataDisplayComponent ); if (result.reason !== 'done') return; await removeMetadata(this.item().id, { name: field }).catch((err) => { + result.close(); notifyError( `Error removing old "${field}" metadata. Error: ${ err.response || err.message || err @@ -337,6 +338,7 @@ export class MetadataDisplayComponent ); throw err; }); + result.close(); notifySuccess(`Successfully removed "${field}" metadata.`); this.metadata.set( this.metadata().filter((prop) => prop && prop.name !== field), diff --git a/src/app/zones/zones-state.service.ts b/src/app/zones/zones-state.service.ts index eb930b45c..38ef6260e 100644 --- a/src/app/zones/zones-state.service.ts +++ b/src/app/zones/zones-state.service.ts @@ -225,7 +225,7 @@ export class ZonesStateService { ), waitForEvent(ref.afterClosed()), ]); - if (!details?.reason) return ref.close(); + if (details.reason !== 'done') return ref.close(); const zone = await this.addTrigger( ref.componentInstance.item as PlaceTrigger, ); @@ -255,7 +255,7 @@ export class ZonesStateService { }, this._dialog, ); - if (!details?.reason) return; + if (details.reason !== 'done') return; const zone = await updateZone(this.active_item.id, { ...this.active_item, triggers: this.active_item.triggers.filter((t) => t !== trigger.id), From a43a0b2f1bd4ce168bccdf0d0b26410c7bc68f91 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 30 Sep 2026 11:35:03 +1000 Subject: [PATCH 3/4] fix: keep select dialogs working after confirm check change Select and trigger-settings dialogs resolve with reason 'action', not 'done', so the stricter check skipped the action every time. Dialogs that race afterClosed() can also resolve with undefined. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/app/domains/domain-state.service.ts | 4 ++-- src/app/systems/system-state.service.ts | 6 +++--- src/app/zones/zones-state.service.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/app/domains/domain-state.service.ts b/src/app/domains/domain-state.service.ts index b9f336794..6da59ddbf 100644 --- a/src/app/domains/domain-state.service.ts +++ b/src/app/domains/domain-state.service.ts @@ -246,7 +246,7 @@ export class DomainStateService { ), waitForEvent(ref.afterClosed()), ]); - if (details.reason !== 'done') return; + if (details?.reason !== 'done') return; this._changed.set(new Date().valueOf()); } @@ -294,7 +294,7 @@ export class DomainStateService { ), waitForEvent(ref.afterClosed()), ]); - if (details.reason !== 'done') return; + if (details?.reason !== 'done') return; this._changed.set(new Date().valueOf()); } diff --git a/src/app/systems/system-state.service.ts b/src/app/systems/system-state.service.ts index 9ca858057..bd97647ea 100644 --- a/src/app/systems/system-state.service.ts +++ b/src/app/systems/system-state.service.ts @@ -383,7 +383,7 @@ export class SystemStateService extends AsyncHandler { ), waitForEvent(ref.afterClosed()), ]); - if (details.reason !== 'done') return ref.close(); + if (details?.reason !== 'action') return ref.close(); const system = ref.componentInstance.item as PlaceSystem; if (!system) return ref.close(); await addSystemModule(system.id, device.id).catch((_e) => { @@ -424,7 +424,7 @@ export class SystemStateService extends AsyncHandler { ), waitForEvent(ref.afterClosed()), ]); - if (details.reason !== 'done') return ref.close(); + if (details?.reason !== 'action') return ref.close(); const t = await this.addTrigger( ref.componentInstance.item as PlaceTrigger, ); @@ -463,7 +463,7 @@ export class SystemStateService extends AsyncHandler { ), waitForEvent(ref.afterClosed()), ]); - if (details.reason !== 'done') return; + if (details?.reason !== 'action') return; instance.loading = 'Saving trigger settings...'; const url = `${apiEndpoint()}/systems/${ diff --git a/src/app/zones/zones-state.service.ts b/src/app/zones/zones-state.service.ts index 38ef6260e..7adf5f9d5 100644 --- a/src/app/zones/zones-state.service.ts +++ b/src/app/zones/zones-state.service.ts @@ -225,7 +225,7 @@ export class ZonesStateService { ), waitForEvent(ref.afterClosed()), ]); - if (details.reason !== 'done') return ref.close(); + if (details?.reason !== 'action') return ref.close(); const zone = await this.addTrigger( ref.componentInstance.item as PlaceTrigger, ); From 9e725c59fc83ef94e58dbacbeb828f908291b040 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 30 Sep 2026 12:17:07 +1000 Subject: [PATCH 4/4] fix(systems): show loading while removing a trigger or module Without it the confirm buttons stay live during the request. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/app/systems/system-state.service.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/app/systems/system-state.service.ts b/src/app/systems/system-state.service.ts index bd97647ea..441156b57 100644 --- a/src/app/systems/system-state.service.ts +++ b/src/app/systems/system-state.service.ts @@ -492,6 +492,7 @@ export class SystemStateService extends AsyncHandler { icon: { type: 'icon', content: 'delete' }, }); if (details.reason !== 'done') return; + details.loading('Removing trigger...'); await removeSystemTrigger(this.active_item.id, trigger.id).catch( (err) => { details.close(); @@ -667,6 +668,7 @@ export class SystemStateService extends AsyncHandler { icon: { type: 'icon', content: 'delete' }, }); if (details.reason !== 'done') return; + details.loading('Removing module...'); const system = await removeSystemModule( this.active_item.id, device.id,