From 245462c1c0350e130e960e7ec6a85e929e40a691 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 21 Oct 2026 15:09:03 +1100 Subject: [PATCH 1/5] fix: repair zone bulk-add, import links, edge keys and driver modules - Zones: bulk add groups read the modal result as an array and did nothing - Resource imports: the view link was always disabled by disabled="false" - Edges: editing an edge hid the one-time API key of the last created edge - Drivers: the Modules tab only listed the first page of modules Co-Authored-By: Claude Opus 5.5 --- src/app/admin/edge.component.ts | 10 ++- src/app/admin/resource-imports.component.ts | 33 +++++--- src/app/drivers/driver-state.service.ts | 10 ++- src/app/zones/zones-state.service.ts | 15 +++- src/tests/zones/zones-state.service.spec.ts | 87 +++++++++++++++++++++ 5 files changed, 134 insertions(+), 21 deletions(-) create mode 100644 src/tests/zones/zones-state.service.spec.ts diff --git a/src/app/admin/edge.component.ts b/src/app/admin/edge.component.ts index b6409e251..3a485fddb 100644 --- a/src/app/admin/edge.component.ts +++ b/src/app/admin/edge.component.ts @@ -202,8 +202,14 @@ export class PlaceEdgeComponent implements OnInit { ref.afterClosed().subscribe((_) => { // Cancel keeps the last saved edge and its one-time API key if (!_) return; - sessionStorage.setItem('BACKOFFICE.last_edge', JSON.stringify(_)); - this.last_change.set(_); + // Only creates return a key; edits must not hide the last one + if (_.x_api_key) { + sessionStorage.setItem( + 'BACKOFFICE.last_edge', + JSON.stringify(_), + ); + this.last_change.set(_); + } this.loadEdges(); }); } diff --git a/src/app/admin/resource-imports.component.ts b/src/app/admin/resource-imports.component.ts index 81ff49dd2..d7190d390 100644 --- a/src/app/admin/resource-imports.component.ts +++ b/src/app/admin/resource-imports.component.ts @@ -164,18 +164,27 @@ export interface ExternalResource { > publish - - visibility - + @if (row.system_id) { + + visibility + + } @else { + + } diff --git a/src/app/drivers/driver-state.service.ts b/src/app/drivers/driver-state.service.ts index 1545fa4bd..1518dd8e5 100644 --- a/src/app/drivers/driver-state.service.ts +++ b/src/app/drivers/driver-state.service.ts @@ -14,6 +14,7 @@ import { import { describeError } from '../common/errors'; import { ActiveItemService } from '../common/item.service'; import { notifyError, notifySuccess } from '../common/notifications'; +import { collectPages } from '../common/support-access'; import { HashMap, Identity } from '../common/types'; import { openConfirmModal } from '../overlays/confirm-modal.component'; import { ViewResponseModalComponent } from '../overlays/view-response-modal.component'; @@ -58,10 +59,11 @@ export class DriverStateService { loader: async ({ params: { item } }) => { if (!(item instanceof PlaceDriver)) return { data: [] as PlaceModule[], total: 0 }; - const response = await queryModules({ - driver_id: item.id, - }).catch(() => ({ data: [] as PlaceModule[], total: 0 })); - return { data: response.data, total: response.total }; + // Load every page so the list matches the tab count + const data = await collectPages( + queryModules({ driver_id: item.id, limit: 500 }), + ).catch(() => [] as PlaceModule[]); + return { data, total: data.length }; }, }); diff --git a/src/app/zones/zones-state.service.ts b/src/app/zones/zones-state.service.ts index a78e4d5e0..688a4ac9f 100644 --- a/src/app/zones/zones-state.service.ts +++ b/src/app/zones/zones-state.service.ts @@ -31,7 +31,11 @@ import { querySupportZones as queryZones, } from '../common/support-access'; import { DialogEvent, Identity } from '../common/types'; -import { GroupBulkAddModalComponent } from '../groups/group-bulk-add-modal.component'; +import { + GroupBulkAddModalComponent, + GroupBulkAddModalData, + GroupBulkAddResult, +} from '../groups/group-bulk-add-modal.component'; import { GroupPermissionsModalComponent } from '../groups/group-permissions-modal.component'; import { openConfirmModal } from '../overlays/confirm-modal.component'; import { @@ -306,9 +310,13 @@ export class ZonesStateService { } public async bulkAddGroups(existing_groups: PlaceGroupZone[] = []) { - const groups = await waitForEvent( + const result = await waitForEvent( this._dialog - .open(GroupBulkAddModalComponent, { + .open< + GroupBulkAddModalComponent, + GroupBulkAddModalData, + GroupBulkAddResult + >(GroupBulkAddModalComponent, { data: { title: 'ZONES.GROUPS_BULK', placeholder: 'GROUPS.SEARCH', @@ -339,6 +347,7 @@ export class ZonesStateService { }) .afterClosed(), ); + const groups = result?.items; if (!groups?.length) return; this._saving.set(true); const results = await Promise.allSettled( diff --git a/src/tests/zones/zones-state.service.spec.ts b/src/tests/zones/zones-state.service.spec.ts new file mode 100644 index 000000000..282c7d8a2 --- /dev/null +++ b/src/tests/zones/zones-state.service.spec.ts @@ -0,0 +1,87 @@ +import { provideZonelessChangeDetection, signal } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; +import { MatDialog } from '@angular/material/dialog'; +import { NavigationEnd, Router } from '@angular/router'; +import { PlaceGroup, PlaceZone } from '@placeos/ts-client'; +import { of, Subject } from 'rxjs'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { ActiveItemService } from '../../app/common/item.service'; +import { ZonesStateService } from '../../app/zones/zones-state.service'; + +const mocks = vi.hoisted(() => ({ + addGroupZone: vi.fn(), + open: vi.fn(), + notifyError: vi.fn(), + notifySuccess: vi.fn(), +})); +vi.mock('@placeos/ts-client', async () => ({ + ...(await vi.importActual( + '@placeos/ts-client/dist/index.es.js', + )), + addGroupZone: mocks.addGroupZone, +})); +vi.mock('../../app/common/item.service', () => ({ + ActiveItemService: class {}, +})); +vi.mock('../../app/common/locale.service', () => ({ + i18n: (key: string, params?: { count?: number }) => + params?.count === undefined ? key : `${key}:${params.count}`, +})); +vi.mock('../../app/common/notifications', () => ({ + notifyError: mocks.notifyError, + notifySuccess: mocks.notifySuccess, +})); +vi.mock('../../app/groups/group-bulk-add-modal.component', () => ({ + GroupBulkAddModalComponent: class {}, +})); + +describe('zone group bulk add', () => { + const zone = new PlaceZone({ id: 'zone-1' }); + let service: ZonesStateService; + + beforeEach(() => { + vi.resetAllMocks(); + mocks.addGroupZone.mockResolvedValue({}); + TestBed.configureTestingModule({ + providers: [ + provideZonelessChangeDetection(), + ZonesStateService, + { + provide: ActiveItemService, + useValue: { item: signal(null), active_item: zone }, + }, + { provide: MatDialog, useValue: { open: mocks.open } }, + { + provide: Router, + useValue: { + url: '/zones/zone-1/about', + events: new Subject(), + }, + }, + ], + }); + service = TestBed.inject(ZonesStateService); + }); + + it('adds each group picked in the bulk modal', async () => { + mocks.open.mockReturnValue({ + afterClosed: () => + of({ + items: [ + new PlaceGroup({ id: 'group-1' }), + new PlaceGroup({ id: 'group-2' }), + ], + permissions: 0, + }), + }); + await service.bulkAddGroups(); + expect(mocks.addGroupZone).toHaveBeenCalledTimes(2); + expect(mocks.addGroupZone).toHaveBeenCalledWith({ + group_id: 'group-2', + zone_id: 'zone-1', + }); + expect(mocks.notifySuccess).toHaveBeenCalledExactlyOnceWith( + 'ZONES.GROUPS_BULK_SUCCESS:2', + ); + }); +}); From 51e4bad156912440d040100f51df139013be9533 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 21 Oct 2026 16:10:19 +1100 Subject: [PATCH 2/5] fix(ui): stop edit modals and forms dropping unsaved work - Edit modals ask before Escape, backdrop or X close them with unsaved changes, block closing while saving, and show the saving text - Saving one settings level or metadata block keeps edits on the others - Zone tree CSV import confirms before it creates zones - Bulk driver update starts with no drivers selected Co-Authored-By: Claude Opus 5.5 --- public/assets/locale/en-AU.json | 4 + .../admin/booking-limits-modal.component.ts | 2 - src/app/admin/broker-form.component.ts | 3 - src/app/admin/database-details.component.ts | 11 ++ src/app/admin/edge-modal.component.ts | 2 - .../signage-ai-provider-modal.component.ts | 5 +- .../signage-plugin-modal.component.ts | 3 - src/app/admin/staff-tenant-modal.component.ts | 2 - .../storage-provider-modal.component.ts | 2 - src/app/domains/application-form.component.ts | 3 - src/app/domains/domain-form.component.ts | 3 - src/app/drivers/driver-form.component.ts | 3 - .../driver-update-list-modal.component.ts | 12 +- src/app/groups/group-form.component.ts | 3 - src/app/modules/module-form.component.ts | 3 - .../repositories/repository-form.component.ts | 3 - src/app/systems/system-form.component.ts | 3 - src/app/triggers/trigger-form.component.ts | 3 - src/app/ui/forms/settings-form.component.ts | 50 ++++-- .../ui/fullscreen-modal-shell.component.ts | 123 ++++++++++++-- src/app/ui/metadata-display.component.ts | 25 ++- src/app/users/user-form.component.ts | 3 - src/app/zones/zone-form.component.ts | 3 - ...ignage-ai-provider-modal.component.spec.ts | 8 +- .../signage-plugin-modal.component.spec.ts | 8 +- .../staff-tenant-modal.component.spec.ts | 8 +- .../modules/module-form.component.spec.ts | 19 ++- .../repository-form.component.spec.ts | 10 +- .../system-trigger-form.component.spec.ts | 10 +- .../fullscreen-modal-shell.component.spec.ts | 151 ++++++++++++++++++ .../settings-form.component.spec.ts | 12 ++ src/tests/zones/zone-form.component.spec.ts | 18 ++- 32 files changed, 425 insertions(+), 93 deletions(-) create mode 100644 src/tests/ui/components/fullscreen-modal-shell.component.spec.ts diff --git a/public/assets/locale/en-AU.json b/public/assets/locale/en-AU.json index 531cf1b6a..4647318c8 100644 --- a/public/assets/locale/en-AU.json +++ b/public/assets/locale/en-AU.json @@ -48,6 +48,9 @@ "NONE": "None", "CANCEL": "Cancel", "CLOSE": "Close", + "DISCARD": "Discard", + "DISCARD_CHANGES": "Discard changes?", + "DISCARD_CHANGES_MSG": "You have unsaved changes. Close and lose them?", "CONFIRM": "Confirm", "ITEM_REMOVE": "Remove {{ item }}", "FIELD_REQUIRED": "This field is required", @@ -476,6 +479,7 @@ "UPDATE": "Update Drivers", "UPDATE_COUNT": "{{ count }} updates available", "UPDATE_SELECTED": "Update selected drivers", + "UPDATING": "Updating drivers...", "NO_UPDATES": "No drivers require updating.", "TAB_ABOUT": "About", "TAB_DOCS": "Readme", diff --git a/src/app/admin/booking-limits-modal.component.ts b/src/app/admin/booking-limits-modal.component.ts index 9db2f2519..5e0a6188a 100644 --- a/src/app/admin/booking-limits-modal.component.ts +++ b/src/app/admin/booking-limits-modal.component.ts @@ -61,7 +61,6 @@ export class BookingLimitsModalComponent { public async save() { await submit(this.form, async () => { - this._dialog_ref.disableClose = true; this.loading.set(true); const limits = this.formModel().booking_limits || []; const booking_limits = {}; @@ -74,7 +73,6 @@ export class BookingLimitsModalComponent { ); const resp = await call.catch(() => null); this.loading.set(false); - this._dialog_ref.disableClose = false; if (!resp) return notifyError('Error adding new tenant.'); notifySuccess('Successfully added new tenant.'); this._dialog_ref.close({ ...this.tenant, booking_limits: resp }); diff --git a/src/app/admin/broker-form.component.ts b/src/app/admin/broker-form.component.ts index f386c64fe..cffd5a02f 100644 --- a/src/app/admin/broker-form.component.ts +++ b/src/app/admin/broker-form.component.ts @@ -426,7 +426,6 @@ export class BrokerFormComponent extends AsyncHandler implements OnInit { await submit(this.form, async () => { const item = this._data.item; this.loading.set(i18n(`${this._name}_SAVING`)); - this._dialog_ref.disableClose = true; const item_json = item.toJSON ? item.toJSON() : item; const form_item = ( item.id @@ -444,7 +443,6 @@ export class BrokerFormComponent extends AsyncHandler implements OnInit { : addBroker(form_item as unknown as PlaceMQTTBroker) ).catch(async (err) => { this.loading.set(null); - this._dialog_ref.disableClose = false; notifyError( i18n(`${this._name}_SAVE_ERROR`, { error: await readError(err), @@ -453,7 +451,6 @@ export class BrokerFormComponent extends AsyncHandler implements OnInit { return null; }); if (!result) return; - this._dialog_ref.disableClose = false; this.event.emit({ reason: 'done', metadata: { item: result } }); notifySuccess(i18n(`${this._name}_SAVE_SUCCESS`)); this._dialog_ref.close(); diff --git a/src/app/admin/database-details.component.ts b/src/app/admin/database-details.component.ts index 0974d2e1a..cff0d2193 100644 --- a/src/app/admin/database-details.component.ts +++ b/src/app/admin/database-details.component.ts @@ -452,6 +452,17 @@ export class PlaceDatabaseDetailsComponent let level = pending.filter( (zone) => !zone.parent_id || !file_ids.has(zone.parent_id), ); + const details = await openConfirmModal( + { + title: 'Import Zone Tree', + content: `Create ${pending.length} new zones? ${level.length} of them have no parent in the file, so they go in at the top level. Zones are not removed if the import fails.`, + icon: { type: 'icon', content: 'account_tree' }, + confirm_text: 'Import', + }, + this._dialog, + ); + if (details.reason !== 'done') return; + details.close(); const id_map = new Map(); let created = 0; try { diff --git a/src/app/admin/edge-modal.component.ts b/src/app/admin/edge-modal.component.ts index 5be45a2b7..31e61337a 100644 --- a/src/app/admin/edge-modal.component.ts +++ b/src/app/admin/edge-modal.component.ts @@ -104,13 +104,11 @@ export class EdgeModalComponent implements OnInit { public async save() { await submit(this.form, async () => undefined); if (this.form().invalid()) return; - this._dialog_ref.disableClose = true; this.loading.set('Saving edge node...'); const edge = { ...this.edge, ...this.formModel() }; const method = edge.id ? updateEdge(edge.id, edge) : addEdge(edge); const new_edge = await method.catch(() => null); this.loading.set(''); - this._dialog_ref.disableClose = false; if (!new_edge) return notifyError(i18n('ADMIN.EDGE_ERROR')); if (edge.id) { notifySuccess(i18n('ADMIN.EDGE_NEW_SUCCESS')); diff --git a/src/app/admin/signage-ai/signage-ai-provider-modal.component.ts b/src/app/admin/signage-ai/signage-ai-provider-modal.component.ts index 788d0108a..a332210fa 100644 --- a/src/app/admin/signage-ai/signage-ai-provider-modal.component.ts +++ b/src/app/admin/signage-ai/signage-ai-provider-modal.component.ts @@ -233,7 +233,8 @@ export class SignageAIProviderModalComponent { provider: this._data.item?.provider || 'OPENAI', endpoint: this._data.item?.endpoint || '', location: this._data.item?.location || '', - default_model: this._data.item?.default_model || 'gpt-image-2.5-sunburst', + default_model: + this._data.item?.default_model || 'gpt-image-2.5-sunburst', enabled: this._data.item?.enabled ?? true, is_default: this._data.item?.is_default ?? false, user_per_day: this._data.item?.quotas?.user_per_day ?? 60, @@ -277,7 +278,6 @@ export class SignageAIProviderModalComponent { } this.loading.set(i18n('ADMIN.AI_PROVIDER_SAVING')); - this._dialog_ref.disableClose = true; const body: Partial & { credentials?: Record; @@ -309,7 +309,6 @@ export class SignageAIProviderModalComponent { ); } finally { this.loading.set(''); - this._dialog_ref.disableClose = false; } notifySuccess(i18n('ADMIN.AI_PROVIDER_SAVE_SUCCESS')); this._dialog_ref.close(true); diff --git a/src/app/admin/signage-plugins/signage-plugin-modal.component.ts b/src/app/admin/signage-plugins/signage-plugin-modal.component.ts index 4926687da..74f4ca219 100644 --- a/src/app/admin/signage-plugins/signage-plugin-modal.component.ts +++ b/src/app/admin/signage-plugins/signage-plugin-modal.component.ts @@ -355,7 +355,6 @@ export class SignagePluginModalComponent } await submit(this.form, async () => { this.loading.set(i18n('ADMIN.SIGNAGE_PLUGINS_SAVING')); - this._dialog_ref.disableClose = true; const schema = this.schema(); const values = this.formModel(); const payload: Partial = { @@ -376,7 +375,6 @@ export class SignagePluginModalComponent }; try { const result = await this._data.save(payload); - this._dialog_ref.disableClose = false; this.event.emit({ reason: 'done', metadata: { item: result }, @@ -388,7 +386,6 @@ export class SignagePluginModalComponent }); } catch (err) { this.loading.set(null); - this._dialog_ref.disableClose = false; notifyError( i18n('ADMIN.SIGNAGE_PLUGINS_SAVE_ERROR', { error: await readError(err), diff --git a/src/app/admin/staff-tenant-modal.component.ts b/src/app/admin/staff-tenant-modal.component.ts index 3e37fb6e0..aae6e230c 100644 --- a/src/app/admin/staff-tenant-modal.component.ts +++ b/src/app/admin/staff-tenant-modal.component.ts @@ -536,7 +536,6 @@ export class StaffTenantModalComponent { }), ); } - this._dialog_ref.disableClose = true; this.loading.set('Saving staff API tenant...'); const model = this.formModel(); const limits = model.booking_limits || []; @@ -590,7 +589,6 @@ export class StaffTenantModalComponent { : post('/api/staff/v1/tenants', data); const tenant = await call.catch((__) => null); this.loading.set(''); - this._dialog_ref.disableClose = false; if (!tenant) return notifyError(i18n('ADMIN.TENANTS_SAVE_ERROR')); notifySuccess(i18n('ADMIN.TENANTS_SAVE_SUCCESS')); this._dialog_ref.close(); diff --git a/src/app/admin/storage/storage-provider-modal.component.ts b/src/app/admin/storage/storage-provider-modal.component.ts index c04441c72..70f4a070b 100644 --- a/src/app/admin/storage/storage-provider-modal.component.ts +++ b/src/app/admin/storage/storage-provider-modal.component.ts @@ -257,7 +257,6 @@ export class StorageProviderModalComponent { await submit(this.form, async () => undefined); if (this.form().invalid()) return; this.loading.set('Saving storage provider...'); - this._dialog_ref.disableClose = true; const details = { ...this.formModel() } as PlaceStorage; if (details.id && !details.access_secret) { delete (details as PlaceStorage & { access_secret?: unknown }) @@ -271,7 +270,6 @@ export class StorageProviderModalComponent { ); } finally { this.loading.set(''); - this._dialog_ref.disableClose = false; } notifySuccess(i18n('ADMIN.STORAGE_SAVE_SUCCESS')); this._dialog_ref.close(); diff --git a/src/app/domains/application-form.component.ts b/src/app/domains/application-form.component.ts index b67eff3b5..ab1f1529b 100644 --- a/src/app/domains/application-form.component.ts +++ b/src/app/domains/application-form.component.ts @@ -299,7 +299,6 @@ export class ApplicationFormComponent extends AsyncHandler implements OnInit { await submit(this.form, async () => { const item = this._data.item as unknown as PlaceResource; this.loading.set(i18n(`${this._name}_SAVING`)); - this._dialog_ref.disableClose = true; const item_json = item.toJSON ? item.toJSON() : item; const form_item = ( item.id @@ -317,13 +316,11 @@ export class ApplicationFormComponent extends AsyncHandler implements OnInit { save_item as unknown as PlaceApplication, ) : addApplication(save_item as unknown as PlaceApplication)); - this._dialog_ref.disableClose = false; this.event.emit({ reason: 'done', metadata: { item: _item } }); notifySuccess(i18n(`${this._name}_SAVE_SUCCESS`)); this._dialog_ref.close(); } catch (err) { this.loading.set(null); - this._dialog_ref.disableClose = false; notifyError( i18n(`${this._name}_SAVE_ERROR`, { error: await readError(err), diff --git a/src/app/domains/domain-form.component.ts b/src/app/domains/domain-form.component.ts index c0e0cab14..4bb56f570 100644 --- a/src/app/domains/domain-form.component.ts +++ b/src/app/domains/domain-form.component.ts @@ -331,7 +331,6 @@ export class DomainFormComponent extends AsyncHandler implements OnInit { await submit(this.form, async () => { const item = this._data.item; this.loading.set(i18n(`${this._name}.SAVING`)); - this._dialog_ref.disableClose = true; const item_json = item.toJSON ? item.toJSON() : item; const form_item = ( item.id @@ -347,13 +346,11 @@ export class DomainFormComponent extends AsyncHandler implements OnInit { form_item as unknown as PlaceDomain, ) : addDomain(form_item as unknown as PlaceDomain)); - this._dialog_ref.disableClose = false; this.event.emit({ reason: 'done', metadata: { item: _item } }); notifySuccess(i18n(`${this._name}.SAVE_SUCCESS`)); this._dialog_ref.close(); } catch (err) { this.loading.set(null); - this._dialog_ref.disableClose = false; notifyError( i18n(`${this._name}.SAVE_ERROR`, { error: await readError(err), diff --git a/src/app/drivers/driver-form.component.ts b/src/app/drivers/driver-form.component.ts index 05267e5a1..5514ea09d 100644 --- a/src/app/drivers/driver-form.component.ts +++ b/src/app/drivers/driver-form.component.ts @@ -480,7 +480,6 @@ export class DriverFormComponent extends AsyncHandler implements OnInit { await submit(this.form, async () => { const item = this._data.item; this.saving.set(i18n(`${this._name}.SAVING`)); - this._dialog_ref.disableClose = true; const item_json = item.toJSON ? item.toJSON() : item; const form_value = { ...this.formModel() }; if (item.id) { @@ -499,7 +498,6 @@ export class DriverFormComponent extends AsyncHandler implements OnInit { form_item as unknown as PlaceDriver, ) : addDriver(form_item as unknown as PlaceDriver)); - this._dialog_ref.disableClose = false; this.event.emit({ reason: 'done', metadata: { item: _item as unknown as Identity }, @@ -514,7 +512,6 @@ export class DriverFormComponent extends AsyncHandler implements OnInit { this._dialog_ref.close(); } catch (err) { this.saving.set(null); - this._dialog_ref.disableClose = false; notifyError( i18n(`${this._name}.SAVE_ERROR`, { error: await readError(err), diff --git a/src/app/drivers/driver-update-list-modal.component.ts b/src/app/drivers/driver-update-list-modal.component.ts index be7e392a6..c8b7b42ed 100644 --- a/src/app/drivers/driver-update-list-modal.component.ts +++ b/src/app/drivers/driver-update-list-modal.component.ts @@ -8,6 +8,7 @@ import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; import { MatTooltipModule } from '@angular/material/tooltip'; import { queryDrivers, updateDriver } from '@placeos/ts-client'; import { describeError } from '../common/errors'; +import { i18n } from '../common/locale.service'; import { notifyError, notifySuccess } from '../common/notifications'; import { IconComponent } from '../ui/icon.component'; import { DateFromPipe } from '../ui/pipes/date-from.pipe'; @@ -121,7 +122,7 @@ import { TranslatePipe } from '../ui/translate.pipe'; class="flex h-48 w-[20rem] flex-col items-center justify-center space-y-2" > -

{{ 'DRIVERS.LOADING' | translate }}

+

{{ loading() }}

} @if (!loading()) { @@ -165,12 +166,12 @@ export class DriverUpdateListModalComponent { private _dialog_ref = inject>(MatDialogRef); - public readonly loading = signal('Loading drivers...'); + public readonly loading = signal(i18n('DRIVERS.LOADING')); private readonly _change = signal(0); private readonly _drivers_with_updates = resource({ params: () => this._change(), loader: async () => { - this.loading.set('Loading drivers...'); + this.loading.set(i18n('DRIVERS.LOADING')); const response = await queryDrivers({ update_available: true, limit: 1000, @@ -183,7 +184,8 @@ export class DriverUpdateListModalComponent { ); // Keep total in sync with the filtered list for select all response.total = response.data.length; - this.selected_drivers.set(response.data.map((d) => d.id)); + // Updates reload live modules, so the user picks them on purpose + this.selected_drivers.set([]); this.loading.set(''); return response; }, @@ -220,7 +222,7 @@ export class DriverUpdateListModalComponent { } public async updateDrivers() { - this.loading.set('Updating drivers...'); + this.loading.set(i18n('DRIVERS.UPDATING')); this._dialog_ref.disableClose = true; const drivers = this.drivers_with_updates(); const selected = drivers.data.filter((_) => diff --git a/src/app/groups/group-form.component.ts b/src/app/groups/group-form.component.ts index aa7562054..51688bef9 100644 --- a/src/app/groups/group-form.component.ts +++ b/src/app/groups/group-form.component.ts @@ -364,7 +364,6 @@ export class GroupFormComponent extends AsyncHandler implements OnInit { await submit(this.form, async () => { const item = this._data.item; this.loading.set(i18n(`${this._name}.SAVING`)); - this._dialog_ref.disableClose = true; const form_value = this.formModel(); const form_item = cleanObject( { @@ -381,13 +380,11 @@ export class GroupFormComponent extends AsyncHandler implements OnInit { form_item as unknown as PlaceGroup, ) : addGroup(form_item as unknown as PlaceGroup)); - this._dialog_ref.disableClose = false; this.event.emit({ reason: 'done', metadata: { item: _item } }); notifySuccess(i18n(`${this._name}.SAVE_SUCCESS`)); this._dialog_ref.close(); } catch (err) { this.loading.set(null); - this._dialog_ref.disableClose = false; notifyError( i18n(`${this._name}.SAVE_ERROR`, { error: await readError(err), diff --git a/src/app/modules/module-form.component.ts b/src/app/modules/module-form.component.ts index 209faf599..808fef283 100644 --- a/src/app/modules/module-form.component.ts +++ b/src/app/modules/module-form.component.ts @@ -503,7 +503,6 @@ export class ModuleFormComponent extends AsyncHandler implements OnInit { await submit(this.form, async () => { const item = this._data.item; this.loading.set(i18n(`${this._name}.SAVING`)); - this._dialog_ref.disableClose = true; const item_json = item.toJSON ? item.toJSON() : item; const form_value: Partial = { ...this.formModel(), @@ -530,13 +529,11 @@ export class ModuleFormComponent extends AsyncHandler implements OnInit { ) : addModule(form_item as unknown as PlaceModule)); clearSupportModuleCache(); - this._dialog_ref.disableClose = false; this.event.emit({ reason: 'done', metadata: { item: _item } }); notifySuccess(i18n(`${this._name}.SAVE_SUCCESS`)); this._dialog_ref.close(); } catch (err) { this.loading.set(null); - this._dialog_ref.disableClose = false; notifyError( i18n(`${this._name}.SAVE_ERROR`, { error: await readError(err), diff --git a/src/app/repositories/repository-form.component.ts b/src/app/repositories/repository-form.component.ts index 43f921cb1..362bdc2bf 100644 --- a/src/app/repositories/repository-form.component.ts +++ b/src/app/repositories/repository-form.component.ts @@ -637,7 +637,6 @@ export class RepositoryFormComponent extends AsyncHandler implements OnInit { } const item = this._data.item; this.saving.set(i18n(`${this._name}.SAVING`)); - this._dialog_ref.disableClose = true; const item_json = item.toJSON ? item.toJSON() : item; const form_value = { ...this.formModel() }; if (form_value.root_path) { @@ -661,13 +660,11 @@ export class RepositoryFormComponent extends AsyncHandler implements OnInit { form_item as unknown as PlaceRepository, ) : addRepository(form_item as unknown as PlaceRepository)); - this._dialog_ref.disableClose = false; this.event.emit({ reason: 'done', metadata: { item: _item } }); notifySuccess(i18n(`${this._name}.SAVE_SUCCESS`)); this._dialog_ref.close(); } catch (err) { this.saving.set(null); - this._dialog_ref.disableClose = false; notifyError( i18n(`${this._name}.SAVE_ERROR`, { error: await readError(err), diff --git a/src/app/systems/system-form.component.ts b/src/app/systems/system-form.component.ts index 65bc6727f..426762360 100644 --- a/src/app/systems/system-form.component.ts +++ b/src/app/systems/system-form.component.ts @@ -703,7 +703,6 @@ export class SystemFormComponent extends AsyncHandler implements OnInit { await submit(this.form, async () => { const item = this._data.item as unknown as PlaceResource; this.loading.set(i18n(`${this._name}.SAVING`)); - this._dialog_ref.disableClose = true; const item_json = item.toJSON ? item.toJSON() : item; // `zone` is UI-only; the backend uses `zones`, which the effect // keeps in sync. Strip it so we never POST `zone: null`. @@ -728,13 +727,11 @@ export class SystemFormComponent extends AsyncHandler implements OnInit { processed_item as unknown as PlaceSystem, ) : addSystem(processed_item as unknown as PlaceSystem)); - this._dialog_ref.disableClose = false; this.event.emit({ reason: 'done', metadata: { item: _item } }); notifySuccess(i18n(`${this._name}.SAVE_SUCCESS`)); this._dialog_ref.close(); } catch (err) { this.loading.set(null); - this._dialog_ref.disableClose = false; notifyError( i18n(`${this._name}.SAVE_ERROR`, { error: await readError(err), diff --git a/src/app/triggers/trigger-form.component.ts b/src/app/triggers/trigger-form.component.ts index 8deb34690..56c5232e3 100644 --- a/src/app/triggers/trigger-form.component.ts +++ b/src/app/triggers/trigger-form.component.ts @@ -215,7 +215,6 @@ export class TriggerFormComponent extends AsyncHandler implements OnInit { } const item = this._data.item; this.loading.set(i18n(`${this._name}.SAVING`)); - this._dialog_ref.disableClose = true; const item_json = item.toJSON ? item.toJSON() : item; const form_item = ( item.id @@ -228,13 +227,11 @@ export class TriggerFormComponent extends AsyncHandler implements OnInit { const _item = await (form_item.id ? updateTrigger(form_item.id, form_item) : addTrigger(form_item)); - this._dialog_ref.disableClose = false; this.event.emit({ reason: 'done', metadata: { item: _item } }); notifySuccess(i18n(`${this._name}.SAVE_SUCCESS`)); this._dialog_ref.close(); } catch (err) { this.loading.set(null); - this._dialog_ref.disableClose = false; notifyError( i18n(`${this._name}.SAVE_ERROR`, { error: await readError(err), diff --git a/src/app/ui/forms/settings-form.component.ts b/src/app/ui/forms/settings-form.component.ts index d6a24f833..58b1a4c68 100644 --- a/src/app/ui/forms/settings-form.component.ts +++ b/src/app/ui/forms/settings-form.component.ts @@ -8,6 +8,7 @@ import { model, OnInit, signal, + untracked, WritableSignal, } from '@angular/core'; import { FormsModule } from '@angular/forms'; @@ -247,6 +248,8 @@ export class SettingsFormComponent extends AsyncHandler implements OnInit { settings4: '', }); public readonly dirty = signal>({}); + /** Item the form was last loaded for */ + private _form_id?: string; /** Whether a setting is being saved */ public readonly saving = signal<[boolean, boolean, boolean, boolean]>([ false, @@ -376,12 +379,13 @@ export class SettingsFormComponent extends AsyncHandler implements OnInit { }); effect(() => { if (this.merge_settings() !== undefined) { - this.timeout('update_merge', () => this.clearChanges(), 50); + this.timeout('update_merge', () => this._reloadForm(), 50); } }); effect(() => { if (this.settings()) { - this.clearChanges(); + // Only new settings should re-run this, not the form it writes + untracked(() => this._reloadForm()); } }); } @@ -453,13 +457,13 @@ export class SettingsFormComponent extends AsyncHandler implements OnInit { ).then( (new_settings: PlaceSettings) => { this._setSaving(level, false); + this._clearDirty([level]); this._setSettings(level, new_settings); notifySuccess( i18n('COMMON.SETTINGS_SAVE_SUCCESS', { type: this.type(level), }), ); - this.clearChanges(); }, async (err) => { this._setSaving(level, false); @@ -499,12 +503,12 @@ export class SettingsFormComponent extends AsyncHandler implements OnInit { if (promises.length) { Promise.all(promises).then( (results: PlaceSettings[]) => { + this._clearDirty(saved_levels); results.forEach((result, index) => { this._setSaving(saved_levels[index], false); this._setSettings(saved_levels[index], result); }); notifySuccess(i18n('COMMON.SETTINGS_SAVE_SUCCESS_ALL')); - this.clearChanges(); }, async (err) => { for (const level of saved_levels) { @@ -560,24 +564,40 @@ export class SettingsFormComponent extends AsyncHandler implements OnInit { }); } + /** Discard all unsaved changes */ public clearChanges() { - this.used_settings.set(this._processSettings(this.settings() || [])); - this._initForm(); + this.dirty.set({}); + this._reloadForm(); } - private _initForm() { + /** + * Reload the form from the saved settings. Unsaved edits are kept, unless + * the settings now belong to a different item. + */ + private _reloadForm() { + if (this._form_id !== this.id()) this.dirty.set({}); + this._form_id = this.id(); + this.used_settings.set(this._processSettings(this.settings() || [])); const used = this.used_settings(); - this.form.set({ - settings0: used[0].settings_string, - settings1: used[1].settings_string, - settings2: used[2].settings_string, - settings3: used[3].settings_string, - settings4: used[4].settings_string, - }); - this.dirty.set({}); + const form = this.form(); + const dirty = this.dirty(); + const next: HashMap = {}; + for (let i = 0; i <= EncryptionLevel.NeverDisplay + 1; i++) { + const key = `settings${i}`; + next[key] = dirty[key] ? form[key] : used[i]?.settings_string; + } + this.form.set(next); this.changed.set(Date.now()); } + private _clearDirty(levels: number[]) { + this.dirty.update((dirty) => { + const next = { ...dirty }; + for (const level of levels) delete next[`settings${level}`]; + return next; + }); + } + private _processSettings(settings: PlaceSettings[]): PlaceSettings[] { if (!settings.length) return []; const processed_settings = []; diff --git a/src/app/ui/fullscreen-modal-shell.component.ts b/src/app/ui/fullscreen-modal-shell.component.ts index a23c5e7b0..73a92aa91 100644 --- a/src/app/ui/fullscreen-modal-shell.component.ts +++ b/src/app/ui/fullscreen-modal-shell.component.ts @@ -1,10 +1,33 @@ -import { Component, input, output } from '@angular/core'; +import { + Component, + DestroyRef, + ElementRef, + inject, + input, + output, +} from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { MatRippleModule } from '@angular/material/core'; -import { MatDialogModule } from '@angular/material/dialog'; +import { MatDialog, MatDialogRef } from '@angular/material/dialog'; import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; +import { filter, merge } from 'rxjs'; +import { i18n } from '../common/locale.service'; +import { openConfirmModal } from '../overlays/confirm-modal.component'; import { IconComponent } from './icon.component'; import { TranslatePipe } from './translate.pipe'; +/** Events in the form that may change a value */ +const EDIT_EVENTS = ['input', 'change', 'click', 'keydown']; +/** Keys that move around the form without changing it */ +const NAVIGATION_KEYS = new Set([ + 'Tab', + 'Escape', + 'Shift', + 'Control', + 'Alt', + 'Meta', +]); + @Component({ selector: 'fullscreen-modal-shell,[fs-modal-shell]', template: ` @@ -21,7 +44,12 @@ import { TranslatePipe } from './translate.pipe'; {{ heading() }} @if (!loading()) { - } @@ -60,18 +88,17 @@ import { TranslatePipe } from './translate.pipe'; } + @if (loading()) { +
+ +

{{ loading() }}

+
+ } - -
- -

{{ loading() }}

-
- `, imports: [ - MatDialogModule, MatProgressSpinnerModule, TranslatePipe, IconComponent, @@ -79,6 +106,31 @@ import { TranslatePipe } from './translate.pipe'; ], }) export class FullscreenModalShellComponent { + private _dialog_ref = inject(MatDialogRef, { optional: true }); + private _dialog = inject(MatDialog); + private _confirming = false; + + /** Set once the user edits a field, so closing asks before it drops them */ + public touched = false; + + /** + * Whether a form event may have changed a value. Selects, chips and +/- + * counters change values without input events, so clicks on buttons or + * selects and non-navigation keys count as edits too. + */ + private _isEdit(event: Event) { + const target = event.target as Element | null; + if (!target?.closest?.('main')) return false; + if (event instanceof KeyboardEvent) { + return !NAVIGATION_KEYS.has(event.key); + } + if (event.type === 'click') { + return !!target.closest('button, [role="combobox"]'); + } + return true; + } + + /** Text shown over the form while it saves. Closing is blocked meanwhile */ public readonly loading = input(''); public readonly heading = input('Fullscreen Modal'); public readonly confirm_text = input(''); @@ -86,4 +138,51 @@ export class FullscreenModalShellComponent { /** Disable the save button, e.g. until required choices are made */ public readonly disable_confirm = input(false); public readonly save = output(); + + constructor() { + // Capture phase, as Material stops some events (e.g. chip remove) + const host: HTMLElement = inject(ElementRef).nativeElement; + const mark = (event: Event) => { + if (this._isEdit(event)) this.touched = true; + }; + for (const type of EDIT_EVENTS) { + host.addEventListener(type, mark, true); + } + inject(DestroyRef).onDestroy(() => { + for (const type of EDIT_EVENTS) { + host.removeEventListener(type, mark, true); + } + }); + const ref = this._dialog_ref; + if (!ref) return; + // The shell owns closing so Escape and backdrop clicks can be guarded + ref.disableClose = true; + merge( + ref.backdropClick(), + ref.keydownEvents().pipe(filter((e) => e.key === 'Escape')), + ) + .pipe(takeUntilDestroyed()) + .subscribe(() => this.close()); + } + + /** Close the modal, asking first if there are unsaved changes */ + public async close() { + if (!this._dialog_ref || this.loading() || this._confirming) return; + if (this.touched) { + this._confirming = true; + const details = await openConfirmModal( + { + title: i18n('COMMON.DISCARD_CHANGES'), + content: i18n('COMMON.DISCARD_CHANGES_MSG'), + icon: { type: 'icon', content: 'edit_off' }, + confirm_text: i18n('COMMON.DISCARD'), + }, + this._dialog, + ); + this._confirming = false; + if (details.reason !== 'done') return; + details.close(); + } + this._dialog_ref.close(); + } } diff --git a/src/app/ui/metadata-display.component.ts b/src/app/ui/metadata-display.component.ts index 6e83618a7..cae34f47f 100644 --- a/src/app/ui/metadata-display.component.ts +++ b/src/app/ui/metadata-display.component.ts @@ -125,7 +125,10 @@ function replaceDescTag(inputString, newContent) { icon matRipple (click)="saveMetadata(item)" - [disabled]="!edited()[item.name]" + [disabled]=" + !edited()[item.name] || + loading()[item.name] + " [matTooltip]="'COMMON.SAVE' | translate" > save @@ -336,8 +339,11 @@ export class MetadataDisplayComponent this._dialog, ); if (result.reason !== 'done') return; + const is_new = !!this.metadata().find((prop) => prop?.name === field) + ?.new; try { - await removeMetadata(this.item().id, { name: field }); + // Blocks that were never saved only exist locally + if (!is_new) await removeMetadata(this.item().id, { name: field }); } catch (err) { return notifyError( `Error removing old "${field}" metadata. Error: ${await readError( @@ -442,10 +448,21 @@ export class MetadataDisplayComponent }); } - private generateForms() { + /** Build a form for each block. Unsaved edits are kept unless `reset` is set */ + private generateForms(reset = false) { + const forms = this.form_map(); + const edited = reset ? {} : this.edited(); this.form_map.set({}); this.edited.set({}); this.metadata().forEach((group) => { + if (edited[group.name] && forms[group.name]) { + this.form_map.update((m) => { + m[group.name] = forms[group.name]; + return m; + }); + this.edited.update((m) => ({ ...m, [group.name]: true })); + return; + } const details = typeof group.details === 'string' ? JSON.parse(group.details) @@ -514,7 +531,7 @@ export class MetadataDisplayComponent .sort((a, b) => a.name.localeCompare(b.name)) .filter((m) => !!m), ); - this.generateForms(); + this.generateForms(true); }) .catch((err) => notifyError( diff --git a/src/app/users/user-form.component.ts b/src/app/users/user-form.component.ts index d35dc1600..a89bd48e5 100644 --- a/src/app/users/user-form.component.ts +++ b/src/app/users/user-form.component.ts @@ -483,7 +483,6 @@ export class UserFormComponent extends AsyncHandler implements OnInit { await submit(this.form, async () => { const item = this._data.item; this.loading.set(i18n(`${this._name}.SAVING`)); - this._dialog_ref.disableClose = true; const item_json = item.toJSON ? item.toJSON() : item; const form_item = ( item.id @@ -511,13 +510,11 @@ export class UserFormComponent extends AsyncHandler implements OnInit { form_item as unknown as PlaceUser, ) : addUser(form_item as unknown as PlaceUser)); - this._dialog_ref.disableClose = false; this.event.emit({ reason: 'done', metadata: { item: _item } }); notifySuccess(i18n(`${this._name}.SAVE_SUCCESS`)); this._dialog_ref.close(); } catch (err) { this.loading.set(null); - this._dialog_ref.disableClose = false; notifyError( i18n(`${this._name}.SAVE_ERROR`, { error: await readError(err), diff --git a/src/app/zones/zone-form.component.ts b/src/app/zones/zone-form.component.ts index e1a257f5b..f1a14116d 100644 --- a/src/app/zones/zone-form.component.ts +++ b/src/app/zones/zone-form.component.ts @@ -424,7 +424,6 @@ export class ZoneFormComponent extends AsyncHandler implements OnInit { await submit(this.form, async () => { const item: PlaceZone = this._data.item; this.loading.set(i18n(`${this._name}.SAVING`)); - this._dialog_ref.disableClose = true; const item_json = item.toJSON ? item.toJSON() : item; // `parent_zone` is UI-only; the backend uses `parent_id`, which the // effect keeps in sync. Strip it so we never POST `parent_zone: null`. @@ -442,13 +441,11 @@ export class ZoneFormComponent extends AsyncHandler implements OnInit { form_item as unknown as PlaceZone, ) : addZoneRequest(form_item as unknown as PlaceZone)); - this._dialog_ref.disableClose = false; this.event.emit({ reason: 'done', metadata: { item: _item } }); notifySuccess(i18n(`${this._name}.SAVE_SUCCESS`)); this._dialog_ref.close(); } catch (err) { this.loading.set(null); - this._dialog_ref.disableClose = false; notifyError( i18n(`${this._name}.SAVE_ERROR`, { error: await readError(err), diff --git a/src/tests/admin/signage-ai-provider-modal.component.spec.ts b/src/tests/admin/signage-ai-provider-modal.component.spec.ts index d00b82160..dcea8274f 100644 --- a/src/tests/admin/signage-ai-provider-modal.component.spec.ts +++ b/src/tests/admin/signage-ai-provider-modal.component.spec.ts @@ -1,5 +1,6 @@ import { TestBed } from '@angular/core/testing'; import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; +import { EMPTY } from 'rxjs'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { SignageAIProviderModalComponent } from '../../app/admin/signage-ai/signage-ai-provider-modal.component'; @@ -20,7 +21,12 @@ describe('SignageAIProviderModalComponent', () => { { provide: MAT_DIALOG_DATA, useValue: {} }, { provide: MatDialogRef, - useValue: { close: vi.fn(), disableClose: false }, + useValue: { + close: vi.fn(), + disableClose: false, + backdropClick: () => EMPTY, + keydownEvents: () => EMPTY, + }, }, ], }) diff --git a/src/tests/admin/signage-plugin-modal.component.spec.ts b/src/tests/admin/signage-plugin-modal.component.spec.ts index ee624266d..cc45f3e9e 100644 --- a/src/tests/admin/signage-plugin-modal.component.spec.ts +++ b/src/tests/admin/signage-plugin-modal.component.spec.ts @@ -1,6 +1,7 @@ import { TestBed } from '@angular/core/testing'; import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; import { SignagePlugin } from '@placeos/ts-client'; +import { EMPTY } from 'rxjs'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { SignagePluginModalComponent, @@ -47,7 +48,12 @@ describe('SignagePluginModalComponent', () => { }, { provide: MatDialogRef, - useValue: { close: vi.fn(), disableClose: false }, + useValue: { + close: vi.fn(), + disableClose: false, + backdropClick: () => EMPTY, + keydownEvents: () => EMPTY, + }, }, { provide: HotkeysService, diff --git a/src/tests/admin/staff-tenant-modal.component.spec.ts b/src/tests/admin/staff-tenant-modal.component.spec.ts index 4ec77c167..042852b2e 100644 --- a/src/tests/admin/staff-tenant-modal.component.spec.ts +++ b/src/tests/admin/staff-tenant-modal.component.spec.ts @@ -1,6 +1,7 @@ import { TestBed } from '@angular/core/testing'; import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; import { post, put } from '@placeos/ts-client'; +import { EMPTY } from 'rxjs'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { StaffTenantModalComponent, @@ -38,7 +39,12 @@ describe('Staff tenant advance booking limits', () => { { provide: MAT_DIALOG_DATA, useValue: data }, { provide: MatDialogRef, - useValue: { close: vi.fn(), disableClose: false }, + useValue: { + close: vi.fn(), + disableClose: false, + backdropClick: () => EMPTY, + keydownEvents: () => EMPTY, + }, }, ], }) diff --git a/src/tests/modules/module-form.component.spec.ts b/src/tests/modules/module-form.component.spec.ts index 3ecba6e38..0668d135a 100644 --- a/src/tests/modules/module-form.component.spec.ts +++ b/src/tests/modules/module-form.component.spec.ts @@ -3,6 +3,7 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; import { provideNoopAnimations } from '@angular/platform-browser/animations'; import { PlaceDriverRole, type PlaceDriver } from '@placeos/ts-client'; +import { EMPTY } from 'rxjs'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ModuleFormComponent } from '../../app/modules/module-form.component'; @@ -60,7 +61,14 @@ describe('ModuleFormComponent', () => { providers: [ provideZonelessChangeDetection(), provideNoopAnimations(), - { provide: MatDialogRef, useValue: { close: vi.fn() } }, + { + provide: MatDialogRef, + useValue: { + close: vi.fn(), + backdropClick: () => EMPTY, + keydownEvents: () => EMPTY, + }, + }, { provide: MAT_DIALOG_DATA, useValue: { item } }, ], }).compileComponents(); @@ -137,7 +145,14 @@ describe('creating a module', () => { providers: [ provideZonelessChangeDetection(), provideNoopAnimations(), - { provide: MatDialogRef, useValue: { close: vi.fn() } }, + { + provide: MatDialogRef, + useValue: { + close: vi.fn(), + backdropClick: () => EMPTY, + keydownEvents: () => EMPTY, + }, + }, { provide: MAT_DIALOG_DATA, useValue: { item } }, ], }).compileComponents(); diff --git a/src/tests/repositories/repository-form.component.spec.ts b/src/tests/repositories/repository-form.component.spec.ts index d6fca41cb..d2fb76657 100644 --- a/src/tests/repositories/repository-form.component.spec.ts +++ b/src/tests/repositories/repository-form.component.spec.ts @@ -7,6 +7,7 @@ import { listRepositoryBranches, updateRepository, } from '@placeos/ts-client'; +import { EMPTY } from 'rxjs'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { notifyError } from '../../app/common/notifications'; import { RepositoryFormComponent } from '../../app/repositories/repository-form.component'; @@ -44,7 +45,14 @@ describe('RepositoryFormComponent', () => { providers: [ provideZonelessChangeDetection(), provideNoopAnimations(), - { provide: MatDialogRef, useValue: { close: vi.fn() } }, + { + provide: MatDialogRef, + useValue: { + close: vi.fn(), + backdropClick: () => EMPTY, + keydownEvents: () => EMPTY, + }, + }, { provide: MAT_DIALOG_DATA, useValue: { diff --git a/src/tests/systems/system-trigger-form.component.spec.ts b/src/tests/systems/system-trigger-form.component.spec.ts index ccc1f9570..3e2954918 100644 --- a/src/tests/systems/system-trigger-form.component.spec.ts +++ b/src/tests/systems/system-trigger-form.component.spec.ts @@ -2,6 +2,7 @@ import { provideZonelessChangeDetection } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; import { PlaceTrigger, SignagePlaylist } from '@placeos/ts-client'; +import { EMPTY } from 'rxjs'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { SystemTriggerFormComponent } from '../../app/systems/system-trigger-form.component'; @@ -37,7 +38,14 @@ describe('system trigger playlists', () => { imports: [SystemTriggerFormComponent], providers: [ provideZonelessChangeDetection(), - { provide: MatDialogRef, useValue: { close: vi.fn() } }, + { + provide: MatDialogRef, + useValue: { + close: vi.fn(), + backdropClick: () => EMPTY, + keydownEvents: () => EMPTY, + }, + }, { provide: MAT_DIALOG_DATA, useValue: { diff --git a/src/tests/ui/components/fullscreen-modal-shell.component.spec.ts b/src/tests/ui/components/fullscreen-modal-shell.component.spec.ts new file mode 100644 index 000000000..9944d8e23 --- /dev/null +++ b/src/tests/ui/components/fullscreen-modal-shell.component.spec.ts @@ -0,0 +1,151 @@ +import { + Component, + Pipe, + PipeTransform, + provideZonelessChangeDetection, + signal, +} from '@angular/core'; +import { TestBed } from '@angular/core/testing'; +import { MatChipsModule } from '@angular/material/chips'; +import { MatDialog, MatDialogRef } from '@angular/material/dialog'; +import { MatSelectModule } from '@angular/material/select'; +import { provideNoopAnimations } from '@angular/platform-browser/animations'; +import { Subject } from 'rxjs'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { openConfirmModal } from '../../../app/overlays/confirm-modal.component'; +import { FullscreenModalShellComponent } from '../../../app/ui/fullscreen-modal-shell.component'; +import { TranslatePipe } from '../../../app/ui/translate.pipe'; + +vi.mock('@placeos/ts-client', () => + vi.importActual('@placeos/ts-client/dist/index.es.js'), +); +vi.mock('../../../app/overlays/confirm-modal.component', () => ({ + openConfirmModal: vi.fn(), +})); + +@Pipe({ name: 'translate' }) +class TestTranslatePipe implements PipeTransform { + transform(value: string) { + return value; + } +} + +@Component({ + imports: [FullscreenModalShellComponent, MatChipsModule, MatSelectModule], + template: ` + + + + Tag + + + + + A + B + + + `, +}) +class TestFormComponent { + public readonly loading = signal(''); +} + +describe('FullscreenModalShellComponent', () => { + const escape = new Subject(); + const ref = { + close: vi.fn(), + disableClose: false, + backdropClick: () => new Subject(), + keydownEvents: () => escape, + }; + + function createForm() { + TestBed.configureTestingModule({ + imports: [TestFormComponent], + providers: [ + provideZonelessChangeDetection(), + provideNoopAnimations(), + { provide: MatDialogRef, useValue: ref }, + { provide: MatDialog, useValue: {} }, + ], + }).overrideComponent(FullscreenModalShellComponent, { + remove: { imports: [TranslatePipe] }, + add: { imports: [TestTranslatePipe] }, + }); + const fixture = TestBed.createComponent(TestFormComponent); + fixture.detectChanges(); + const element: HTMLElement = fixture.nativeElement; + const shell = fixture.debugElement.children[0] + .componentInstance as FullscreenModalShellComponent; + return { fixture, element, shell }; + } + + beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(openConfirmModal).mockResolvedValue({ + reason: undefined, + loading: vi.fn(), + close: vi.fn(), + }); + }); + + it('closes on Escape when nothing was edited', async () => { + createForm(); + expect(ref.disableClose).toBe(true); + escape.next(new KeyboardEvent('keydown', { key: 'Escape' })); + await Promise.resolve(); + expect(openConfirmModal).not.toHaveBeenCalled(); + expect(ref.close).toHaveBeenCalled(); + }); + + it('asks before closing once a field was typed in', async () => { + const { element } = createForm(); + element + .querySelector('main') + .dispatchEvent(new Event('input', { bubbles: true })); + escape.next(new KeyboardEvent('keydown', { key: 'Escape' })); + await Promise.resolve(); + expect(openConfirmModal).toHaveBeenCalled(); + expect(ref.close).not.toHaveBeenCalled(); + }); + + it('asks before closing after a chip was removed', async () => { + const { element, shell } = createForm(); + element.querySelector('[matChipRemove]').click(); + await shell.close(); + expect(openConfirmModal).toHaveBeenCalled(); + expect(ref.close).not.toHaveBeenCalled(); + }); + + it('asks before closing after a select was changed by keyboard', async () => { + const { element, shell } = createForm(); + const select = element.querySelector('mat-select'); + select.focus(); + select.dispatchEvent( + new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true }), + ); + await shell.close(); + expect(openConfirmModal).toHaveBeenCalled(); + }); + + it('does not count moving between fields as an edit', async () => { + const { element, shell } = createForm(); + element + .querySelector('mat-select') + .dispatchEvent( + new KeyboardEvent('keydown', { key: 'Tab', bubbles: true }), + ); + await shell.close(); + expect(openConfirmModal).not.toHaveBeenCalled(); + expect(ref.close).toHaveBeenCalled(); + }); + + it('does not close while saving', async () => { + const { fixture, shell } = createForm(); + fixture.componentInstance.loading.set('Saving...'); + await fixture.whenStable(); + await shell.close(); + expect(ref.close).not.toHaveBeenCalled(); + }); +}); diff --git a/src/tests/ui/components/settings-form.component.spec.ts b/src/tests/ui/components/settings-form.component.spec.ts index 37a8545bc..3b5edae87 100644 --- a/src/tests/ui/components/settings-form.component.spec.ts +++ b/src/tests/ui/components/settings-form.component.spec.ts @@ -98,6 +98,18 @@ describe('SettingsFormComponent', () => { ); }); + it('keeps unsaved edits on other levels after saving one', async () => { + const component = createForm(); + component.updateSetting(0, 'level0: changed'); + component.updateSetting(1, 'level1: changed'); + component.save(0); + await Promise.resolve(); + TestBed.tick(); + expect(component.settingDirty(0)).toBe(false); + expect(component.settingValue(1)).toBe('level1: changed'); + expect(component.settingDirty(1)).toBe(true); + }); + it('masks admin settings for support users', () => { mocks.user = { sys_admin: false, support: true }; const component = createForm(); diff --git a/src/tests/zones/zone-form.component.spec.ts b/src/tests/zones/zone-form.component.spec.ts index 6aabc4bf5..f8cfc1bad 100644 --- a/src/tests/zones/zone-form.component.spec.ts +++ b/src/tests/zones/zone-form.component.spec.ts @@ -2,6 +2,7 @@ import { provideZonelessChangeDetection } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; import { PlaceZone } from '@placeos/ts-client'; +import { EMPTY } from 'rxjs'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ZoneFormComponent } from '../../app/zones/zone-form.component'; @@ -32,7 +33,12 @@ vi.mock('../../app/common/notifications', () => ({ describe('saving zones', () => { let fixture: ComponentFixture; - const dialog = { close: vi.fn(), disableClose: false }; + const dialog = { + close: vi.fn(), + disableClose: false, + backdropClick: () => EMPTY, + keydownEvents: () => EMPTY, + }; beforeEach(async () => { vi.resetAllMocks(); @@ -112,7 +118,8 @@ describe('saving zones', () => { 'Renamed floor', ); expect(fixture.componentInstance.loading()).toBeNull(); - expect(dialog.disableClose).toBe(false); + // The modal shell keeps guarding Escape and backdrop clicks + expect(dialog.disableClose).toBe(true); expect(dialog.close).not.toHaveBeenCalled(); expect(mocks.notifyError).toHaveBeenCalled(); expect(mocks.notifySuccess).not.toHaveBeenCalled(); @@ -123,7 +130,12 @@ describe('saving zones', () => { }); describe('saving zones with a stored parent', () => { - const dialog = { close: vi.fn(), disableClose: false }; + const dialog = { + close: vi.fn(), + disableClose: false, + backdropClick: () => EMPTY, + keydownEvents: () => EMPTY, + }; async function createForm() { TestBed.configureTestingModule({ From 1193c005141f352b8efdc5fe1da6cc7779336129 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 21 Oct 2026 16:16:36 +1100 Subject: [PATCH 3/5] fix(ui): name delete targets and report failed list loads - Delete confirmations name the item and use a red Delete button - Start and stop system confirmations name the system - Error toasts stay open until dismissed - Failed list loads show an error instead of an empty list - The sidebar no longer says a list is empty while it loads Co-Authored-By: Claude Opus 5.5 --- public/assets/locale/en-AU.json | 20 +++++----- public/assets/locale/en-GB.json | 16 ++++---- public/assets/locale/en-US.json | 16 ++++---- .../cluster-task-list.component.ts | 16 +++++++- src/app/admin/staff-api.component.ts | 9 +++-- src/app/admin/storage/storage.component.ts | 5 ++- src/app/common/item.service.ts | 1 + src/app/common/load-error.ts | 17 ++++++++ src/app/common/notifications.ts | 3 +- src/app/domains/domain-state.service.ts | 7 +++- src/app/drivers/driver-state.service.ts | 3 +- .../driver-update-list-modal.component.ts | 3 +- src/app/modules/module-state.service.ts | 9 +++-- src/app/overlays/confirm-modal.component.ts | 8 +++- src/app/systems/system-state.service.ts | 20 ++++++---- src/app/triggers/trigger-state.service.ts | 5 ++- src/app/ui/item-sidebar.component.ts | 14 +++++-- src/app/zones/zones-state.service.ts | 9 +++-- src/tests/common/notifications.spec.ts | 8 ++++ .../overlays/confirm-modal.component.spec.ts | 39 ++++++++++++++++++- 20 files changed, 170 insertions(+), 58 deletions(-) create mode 100644 src/app/common/load-error.ts diff --git a/public/assets/locale/en-AU.json b/public/assets/locale/en-AU.json index 4647318c8..74e75c4ac 100644 --- a/public/assets/locale/en-AU.json +++ b/public/assets/locale/en-AU.json @@ -52,6 +52,7 @@ "DISCARD_CHANGES": "Discard changes?", "DISCARD_CHANGES_MSG": "You have unsaved changes. Close and lose them?", "CONFIRM": "Confirm", + "DELETE": "Delete", "ITEM_REMOVE": "Remove {{ item }}", "FIELD_REQUIRED": "This field is required", "DATE_EMPTY": "No date selected", @@ -70,6 +71,7 @@ "COPY_LINK": "Copy Link", "RETRY": "Retry", "EDITOR_LOAD_ERROR": "Could not load the editor. Check your connection and retry.", + "LOAD_ERROR": "Failed to load data. Error: {{ error }}", "IMAGES": "Images", "NEW_ITEM": "New item", "ITEM_ADD": "Add {{ item }}", @@ -247,7 +249,7 @@ "ADD": "Add system", "EDIT": "Edit system", "DELETE": "Delete system", - "DELETE_MSG": "

Are you sure you want delete this system({{ name }})?

Deleting this will immediately delete modules that are not in another system

", + "DELETE_MSG": "

Delete the system {{ name }}?

Modules that are not in another system are deleted immediately.

", "DELETE_LOADING": "Deleting system...", "DELETE_SUCCESS": "Successfully deleted system.", "DELETE_ERROR": "Failed to delete system. Error: {{ error }}", @@ -335,7 +337,7 @@ "SEARCH": "Search for modules...", "ADD": "Add module", "DELETE": "Delete module", - "DELETE_MSG": "

Are you sure you want delete this module?

Deleting this will module immediately remove it from any system associated with it

", + "DELETE_MSG": "

Delete the module {{ name }}?

It is removed immediately from every system that uses it.

", "DELETE_LOADING": "Deleting module...", "DELETE_SUCCESS": "Successfully deleted module.", "DELETE_ERROR": "Failed to delete module. Error: {{ error }}", @@ -392,7 +394,7 @@ "ADD": "Add zone", "EDIT": "Edit zone", "DELETE": "Delete zone", - "DELETE_MSG": "

Are you sure you want delete this zone?

Zones beneath it, its triggers, metadata and settings are removed immediately. Systems in this zone are kept unless you also remove associated resources.

", + "DELETE_MSG": "

Delete the zone {{ name }}?

Zones beneath it, its triggers, metadata and settings are removed immediately. Systems in this zone are kept unless you also remove associated resources.

", "DELETE_LOADING": "Deleting zone...", "DELETE_SUCCESS": "Successfully deleted zone.", "DELETE_ERROR": "Failed to delete zone. Error: {{ error }}", @@ -466,7 +468,7 @@ "ADD": "Add driver", "EDIT": "Edit driver", "DELETE": "Delete driver", - "DELETE_MSG": "

Are you sure you want delete this driver?

", + "DELETE_MSG": "

Delete the driver {{ name }}?

", "DELETE_LOADING": "Deleting driver...", "DELETE_SUCCESS": "Successfully deleted driver.", "DELETE_ERROR": "Failed to delete driver. Error: {{ error }}", @@ -522,7 +524,7 @@ "DELETE": "Delete repository", "REMOVE": "Remove repository", "NEW": "New repository", - "DELETE_MSG": "

Deleting this repository will immediately remove assoicated drivers and modules

", + "DELETE_MSG": "

Delete the repository {{ name }}?

Its drivers and their modules are removed immediately.

", "DELETE_LOADING": "Deleting repository...", "DELETE_SUCCESS": "Successfully deleted repository.", "DELETE_ERROR": "Failed to delete repository. Error: {{ error }}", @@ -571,7 +573,7 @@ "DELETE": "Delete trigger", "DELETE_INSTANCE": "Delete instance", "REMOVE": "Remove trigger", - "DELETE_MSG": "

Are you sure you want delete this trigger?

Deleting this trigger will immediately remove it from all associated systems and zones

", + "DELETE_MSG": "

Delete the trigger {{ name }}?

It is removed immediately from all systems and zones that use it.

", "DELETE_LOADING": "Deleting trigger...", "DELETE_SUCCESS": "Successfully deleted trigger.", "DELETE_ERROR": "Failed to delete trigger. Error: {{ error }}", @@ -688,7 +690,7 @@ "BULK": "Bulk add users", "REMOVE": "Remove user", "DELETE": "Delete user", - "DELETE_MSG": "

Are you sure you want delete this user?

The user will be removed from the system within 24 hours

", + "DELETE_MSG": "

Delete the user {{ name }}?

The user is removed within 24 hours.

", "DELETE_LOADING": "Deleting user...", "DELETE_SUCCESS": "Successfully deleted user.", "DELETE_ERROR": "Failed to delete user. Error: {{ error }}", @@ -752,7 +754,7 @@ "NEW": "New group", "REMOVE": "Remove group", "DELETE": "Delete group", - "DELETE_MSG": "

Are you sure you want delete this group?

The group will be deleted immediately.

", + "DELETE_MSG": "

Delete the group {{ name }}?

The group is deleted immediately.

", "DELETE_LOADING": "Deleting group...", "DELETE_SUCCESS": "Successfully deleted group.", "DELETE_ERROR": "Failed to delete group. Error: {{ error }}", @@ -846,7 +848,7 @@ "BULK": "Bulk add domains", "REMOVE": "Remove domain", "DELETE": "Delete domain", - "DELETE_MSG": "

Are you sure you want delete this domain?

Its users, auth sources and groups are removed immediately.

", + "DELETE_MSG": "

Delete the domain {{ name }}?

Its users, auth sources and groups are removed immediately.

", "DELETE_LOADING": "Deleting domain...", "DELETE_SUCCESS": "Successfully deleted domain.", "DELETE_ERROR": "Failed to delete domain. Error: {{ error }}", diff --git a/public/assets/locale/en-GB.json b/public/assets/locale/en-GB.json index 0d0cca2ad..84a3d9676 100644 --- a/public/assets/locale/en-GB.json +++ b/public/assets/locale/en-GB.json @@ -213,7 +213,7 @@ "ADD": "Add system", "EDIT": "Edit system", "DELETE": "Delete system", - "DELETE_MSG": "

Are you sure you want delete this system({{ name }})?

Deleting this will immediately delete modules that are not in another system

", + "DELETE_MSG": "

Delete the system {{ name }}?

Modules that are not in another system are deleted immediately.

", "DELETE_LOADING": "Deleting system...", "DELETE_SUCCESS": "Successfully deleted system.", "DELETE_ERROR": "Failed to delete system. Error: {{ error }}", @@ -292,7 +292,7 @@ "SEARCH": "Search for modules...", "ADD": "Add module", "DELETE": "Delete module", - "DELETE_MSG": "

Are you sure you want delete this module?

Deleting this will module immediately remove it from any system associated with it

", + "DELETE_MSG": "

Delete the module {{ name }}?

It is removed immediately from every system that uses it.

", "DELETE_LOADING": "Deleting module...", "DELETE_SUCCESS": "Successfully deleted module.", "DELETE_ERROR": "Failed to delete module. Error: {{ error }}", @@ -348,7 +348,7 @@ "ADD": "Add zone", "EDIT": "Edit zone", "DELETE": "Delete zone", - "DELETE_MSG": "

Are you sure you want delete this zone?

Zones beneath it, its triggers, metadata and settings are removed immediately. Systems in this zone are kept unless you also remove associated resources.

", + "DELETE_MSG": "

Delete the zone {{ name }}?

Zones beneath it, its triggers, metadata and settings are removed immediately. Systems in this zone are kept unless you also remove associated resources.

", "DELETE_LOADING": "Deleting zone...", "DELETE_SUCCESS": "Successfully deleted zone.", "DELETE_ERROR": "Failed to delete zone. Error: {{ error }}", @@ -403,7 +403,7 @@ "ADD": "Add driver", "EDIT": "Edit driver", "DELETE": "Delete driver", - "DELETE_MSG": "

Are you sure you want delete this driver?

", + "DELETE_MSG": "

Delete the driver {{ name }}?

", "DELETE_LOADING": "Deleting driver...", "DELETE_SUCCESS": "Successfully deleted driver.", "DELETE_ERROR": "Failed to delete driver. Error: {{ error }}", @@ -457,7 +457,7 @@ "DELETE": "Delete repository", "REMOVE": "Remove repository", "NEW": "New repository", - "DELETE_MSG": "

Deleting this repository will immediately remove assoicated drivers and modules

", + "DELETE_MSG": "

Delete the repository {{ name }}?

Its drivers and their modules are removed immediately.

", "DELETE_LOADING": "Deleting repository...", "DELETE_SUCCESS": "Successfully deleted repository.", "DELETE_ERROR": "Failed to delete repository. Error: {{ error }}", @@ -500,7 +500,7 @@ "DELETE": "Delete trigger", "DELETE_INSTANCE": "Delete instance", "REMOVE": "Remove trigger", - "DELETE_MSG": "

Are you sure you want delete this trigger?

Deleting this trigger will immediately remove it from all associated systems and zones

", + "DELETE_MSG": "

Delete the trigger {{ name }}?

It is removed immediately from all systems and zones that use it.

", "DELETE_LOADING": "Deleting trigger...", "DELETE_SUCCESS": "Successfully deleted trigger.", "DELETE_ERROR": "Failed to delete trigger. Error: {{ error }}", @@ -607,7 +607,7 @@ "NEW": "New user", "BULK": "Bulk add users", "REMOVE": "Remove user", - "DELETE_MSG": "

Are you sure you want delete this user?

The user will be removed from the system within 24 hours

", + "DELETE_MSG": "

Delete the user {{ name }}?

The user is removed within 24 hours.

", "DELETE_LOADING": "Deleting user...", "DELETE_SUCCESS": "Successfully deleted user.", "DELETE_ERROR": "Failed to delete user. Error: {{ error }}", @@ -651,7 +651,7 @@ "BULK": "Bulk add domains", "REMOVE": "Remove domain", "DELETE": "Delete domain", - "DELETE_MSG": "

Are you sure you want delete this domain?

Its users, auth sources and groups are removed immediately.

", + "DELETE_MSG": "

Delete the domain {{ name }}?

Its users, auth sources and groups are removed immediately.

", "DELETE_LOADING": "Deleting domain...", "DELETE_SUCCESS": "Successfully deleted domain.", "DELETE_ERROR": "Failed to delete domain. Error: {{ error }}", diff --git a/public/assets/locale/en-US.json b/public/assets/locale/en-US.json index 74f37f815..d85c6cf89 100644 --- a/public/assets/locale/en-US.json +++ b/public/assets/locale/en-US.json @@ -210,7 +210,7 @@ "ADD": "Add system", "EDIT": "Edit system", "DELETE": "Delete system", - "DELETE_MSG": "

Are you sure you want delete this system({{ name }})?

Deleting this will immediately delete modules that are not in another system

", + "DELETE_MSG": "

Delete the system {{ name }}?

Modules that are not in another system are deleted immediately.

", "DELETE_LOADING": "Deleting system...", "DELETE_SUCCESS": "Successfully deleted system.", "DELETE_ERROR": "Failed to delete system. Error: {{ error }}", @@ -289,7 +289,7 @@ "SEARCH": "Search for modules...", "ADD": "Add module", "DELETE": "Delete module", - "DELETE_MSG": "

Are you sure you want delete this module?

Deleting this will module immediately remove it from any system associated with it

", + "DELETE_MSG": "

Delete the module {{ name }}?

It is removed immediately from every system that uses it.

", "DELETE_LOADING": "Deleting module...", "DELETE_SUCCESS": "Successfully deleted module.", "DELETE_ERROR": "Failed to delete module. Error: {{ error }}", @@ -345,7 +345,7 @@ "ADD": "Add zone", "EDIT": "Edit zone", "DELETE": "Delete zone", - "DELETE_MSG": "

Are you sure you want delete this zone?

Zones beneath it, its triggers, metadata and settings are removed immediately. Systems in this zone are kept unless you also remove associated resources.

", + "DELETE_MSG": "

Delete the zone {{ name }}?

Zones beneath it, its triggers, metadata and settings are removed immediately. Systems in this zone are kept unless you also remove associated resources.

", "DELETE_LOADING": "Deleting zone...", "DELETE_SUCCESS": "Successfully deleted zone.", "DELETE_ERROR": "Failed to delete zone. Error: {{ error }}", @@ -400,7 +400,7 @@ "ADD": "Add driver", "EDIT": "Edit driver", "DELETE": "Delete driver", - "DELETE_MSG": "

Are you sure you want delete this driver?

", + "DELETE_MSG": "

Delete the driver {{ name }}?

", "DELETE_LOADING": "Deleting driver...", "DELETE_SUCCESS": "Successfully deleted driver.", "DELETE_ERROR": "Failed to delete driver. Error: {{ error }}", @@ -454,7 +454,7 @@ "DELETE": "Delete repository", "REMOVE": "Remove repository", "NEW": "New repository", - "DELETE_MSG": "

Deleting this repository will immediately remove assoicated drivers and modules

", + "DELETE_MSG": "

Delete the repository {{ name }}?

Its drivers and their modules are removed immediately.

", "DELETE_LOADING": "Deleting repository...", "DELETE_SUCCESS": "Successfully deleted repository.", "DELETE_ERROR": "Failed to delete repository. Error: {{ error }}", @@ -497,7 +497,7 @@ "DELETE": "Delete trigger", "DELETE_INSTANCE": "Delete instance", "REMOVE": "Remove trigger", - "DELETE_MSG": "

Are you sure you want delete this trigger?

Deleting this trigger will immediately remove it from all associated systems and zones

", + "DELETE_MSG": "

Delete the trigger {{ name }}?

It is removed immediately from all systems and zones that use it.

", "DELETE_LOADING": "Deleting trigger...", "DELETE_SUCCESS": "Successfully deleted trigger.", "DELETE_ERROR": "Failed to delete trigger. Error: {{ error }}", @@ -604,7 +604,7 @@ "NEW": "New user", "BULK": "Bulk add users", "REMOVE": "Remove user", - "DELETE_MSG": "

Are you sure you want delete this user?

The user will be removed from the system within 24 hours

", + "DELETE_MSG": "

Delete the user {{ name }}?

The user is removed within 24 hours.

", "DELETE_LOADING": "Deleting user...", "DELETE_SUCCESS": "Successfully deleted user.", "DELETE_ERROR": "Failed to delete user. Error: {{ error }}", @@ -648,7 +648,7 @@ "BULK": "Bulk add domains", "REMOVE": "Remove domain", "DELETE": "Delete domain", - "DELETE_MSG": "

Are you sure you want delete this domain?

Its users, auth sources and groups are removed immediately.

", + "DELETE_MSG": "

Delete the domain {{ name }}?

Its users, auth sources and groups are removed immediately.

", "DELETE_LOADING": "Deleting domain...", "DELETE_SUCCESS": "Successfully deleted domain.", "DELETE_ERROR": "Failed to delete domain. Error: {{ error }}", 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 5b4085291..2ff4f45eb 100644 --- a/src/app/admin/cluster-details/cluster-task-list.component.ts +++ b/src/app/admin/cluster-details/cluster-task-list.component.ts @@ -24,6 +24,7 @@ import { MatTooltipModule } from '@angular/material/tooltip'; import { ActivatedRoute, RouterModule } from '@angular/router'; import { AsyncHandler } from '../../common/async-handler.class'; import { describeError } from '../../common/errors'; +import { reportLoadError } from '../../common/load-error'; import { i18n } from '../../common/locale.service'; import { notifyError } from '../../common/notifications'; import { signalFromSubscribable } from '../../common/signals'; @@ -316,13 +317,26 @@ export class PlaceClusterTaskListComponent this.updateProcessList(); } + /** Whether the last process list load failed */ + private _load_failed = false; + public async updateProcessList() { const cluster = this.cluster(); if (!cluster) return; this.loading.set(true); + // This polls, so only report the first failure in a row const list = await queryProcesses(cluster.id, { include_status: true, - }).catch(() => []); + }) + .then((list) => { + this._load_failed = false; + return list; + }) + .catch((error) => { + if (this._load_failed) return []; + this._load_failed = true; + return reportLoadError([])(error); + }); // The page moved to another cluster while this was loading if (cluster !== this.cluster()) return; this.process_list.set( diff --git a/src/app/admin/staff-api.component.ts b/src/app/admin/staff-api.component.ts index 9747c9039..a7e8bf743 100644 --- a/src/app/admin/staff-api.component.ts +++ b/src/app/admin/staff-api.component.ts @@ -8,8 +8,9 @@ import { MatProgressBarModule } from '@angular/material/progress-bar'; import { MatSelectModule } from '@angular/material/select'; import { MatTooltipModule } from '@angular/material/tooltip'; import { del, get, PlaceDomain } from '@placeos/ts-client'; -import { escapeHtml } from '../common/general'; import { readError } from '../common/errors'; +import { escapeHtml } from '../common/general'; +import { reportLoadError } from '../common/load-error'; import { notifyError, notifySuccess } from '../common/notifications'; import { HashMap } from '../common/types'; import { openConfirmModal } from '../overlays/confirm-modal.component'; @@ -18,8 +19,8 @@ import { SimpleTableComponent } from '../ui/simple-table.component'; import { TranslatePipe } from '../ui/translate.pipe'; import { AdminDataService } from './admin-data.service'; import { BookingLimitsModalComponent } from './booking-limits-modal.component'; -import { StaffTenantModalComponent } from './staff-tenant-modal.component'; import { isSecretExpired, isSecretExpiring } from './staff-api.utilities'; +import { StaffTenantModalComponent } from './staff-tenant-modal.component'; export interface PlaceTenant { id: string; @@ -278,7 +279,9 @@ export class PlaceStaffAPIComponent implements OnInit { return; } this.loading.set('Loading tenants for domain...'); - const tenants = await get('/api/staff/v1/tenants').catch(() => []); + const tenants = await get('/api/staff/v1/tenants').catch( + reportLoadError([]), + ); this.tenants.set( (tenants || []).filter((t) => t.domain === this.domain()?.domain), ); diff --git a/src/app/admin/storage/storage.component.ts b/src/app/admin/storage/storage.component.ts index a383613be..ac54395ad 100644 --- a/src/app/admin/storage/storage.component.ts +++ b/src/app/admin/storage/storage.component.ts @@ -6,8 +6,9 @@ import { MatFormFieldModule } from '@angular/material/form-field'; import { MatProgressBarModule } from '@angular/material/progress-bar'; import { MatSelectModule } from '@angular/material/select'; import { MatTooltipModule } from '@angular/material/tooltip'; -import { escapeHtml } from '../../common/general'; import { describeError } from '../../common/errors'; +import { escapeHtml } from '../../common/general'; +import { reportLoadError } from '../../common/load-error'; import { i18n } from '../../common/locale.service'; import { notifyError } from '../../common/notifications'; import { openConfirmModal } from '../../overlays/confirm-modal.component'; @@ -248,7 +249,7 @@ export class StorageComponent implements OnInit { this.loading.set('Loading storage...'); const { data } = await queryStorage({ auth_id: this.domain()?.id, - }).catch(() => ({ data: [] })); + }).catch(reportLoadError({ data: [] })); this.store_list.set(data); this.loading.set(''); } diff --git a/src/app/common/item.service.ts b/src/app/common/item.service.ts index 30e3a878b..ac96e7f81 100644 --- a/src/app/common/item.service.ts +++ b/src/app/common/item.service.ts @@ -409,6 +409,7 @@ export class ActiveItemService extends AsyncHandler { ] : undefined, icon: { type: 'icon', content: 'delete' }, + destructive: true, }, }, ); diff --git a/src/app/common/load-error.ts b/src/app/common/load-error.ts new file mode 100644 index 000000000..e4206d511 --- /dev/null +++ b/src/app/common/load-error.ts @@ -0,0 +1,17 @@ +import { readError } from './errors'; +import { i18n } from './locale.service'; +import { notifyError } from './notifications'; + +/** + * Catch handler for list loads. Tells the user the load failed, then returns + * `empty` so the page still renders. Without it a failed load looks like a + * list with nothing in it. + */ +export function reportLoadError(empty: T) { + return async (error: unknown): Promise => { + notifyError( + i18n('COMMON.LOAD_ERROR', { error: await readError(error) }), + ); + return empty; + }; +} diff --git a/src/app/common/notifications.ts b/src/app/common/notifications.ts index b6e6709c4..1df181cd0 100644 --- a/src/app/common/notifications.ts +++ b/src/app/common/notifications.ts @@ -58,7 +58,8 @@ function openNotification( ) { const snackbar_ref = service.open(message, action, { panelClass: [type], - duration: 5000, + // Errors stay until dismissed so they can be read and acted on + duration: type === 'error' ? undefined : 5000, }); if (action) { on_action = on_action || (() => snackbar_ref.dismiss()); diff --git a/src/app/domains/domain-state.service.ts b/src/app/domains/domain-state.service.ts index 5bf3636d1..9776dc1d6 100644 --- a/src/app/domains/domain-state.service.ts +++ b/src/app/domains/domain-state.service.ts @@ -34,6 +34,7 @@ import { filter, map } from 'rxjs'; import { describeError } from '../common/errors'; import { escapeHtml } from '../common/general'; import { ActiveItemService } from '../common/item.service'; +import { reportLoadError } from '../common/load-error'; import { i18n } from '../common/locale.service'; import { notifyError, notifySuccess } from '../common/notifications'; import { waitForEvent } from '../common/signals'; @@ -116,7 +117,7 @@ export class DomainStateService { const response = await queryUsers({ authority_id: params.id, limit: params.full ? 1000 : 1, - }).catch(() => ({ data: [] as PlaceUser[], total: 0 })); + }).catch(reportLoadError({ data: [] as PlaceUser[], total: 0 })); return { ...response, data: [...response.data].sort((a, b) => @@ -179,7 +180,9 @@ export class DomainStateService { const response = await queryApplications({ authority_id: params.id, ...(params.full ? {} : { limit: 1 }), - }).catch(() => ({ data: [] as PlaceApplication[], total: 0 })); + }).catch( + reportLoadError({ data: [] as PlaceApplication[], total: 0 }), + ); return { ...response, data: [...response.data].sort((a, b) => diff --git a/src/app/drivers/driver-state.service.ts b/src/app/drivers/driver-state.service.ts index 1518dd8e5..ae04af08b 100644 --- a/src/app/drivers/driver-state.service.ts +++ b/src/app/drivers/driver-state.service.ts @@ -13,6 +13,7 @@ import { } from '@placeos/ts-client'; import { describeError } from '../common/errors'; import { ActiveItemService } from '../common/item.service'; +import { reportLoadError } from '../common/load-error'; import { notifyError, notifySuccess } from '../common/notifications'; import { collectPages } from '../common/support-access'; import { HashMap, Identity } from '../common/types'; @@ -62,7 +63,7 @@ export class DriverStateService { // Load every page so the list matches the tab count const data = await collectPages( queryModules({ driver_id: item.id, limit: 500 }), - ).catch(() => [] as PlaceModule[]); + ).catch(reportLoadError([] as PlaceModule[])); return { data, total: data.length }; }, }); diff --git a/src/app/drivers/driver-update-list-modal.component.ts b/src/app/drivers/driver-update-list-modal.component.ts index c8b7b42ed..0d9f209e6 100644 --- a/src/app/drivers/driver-update-list-modal.component.ts +++ b/src/app/drivers/driver-update-list-modal.component.ts @@ -8,6 +8,7 @@ import { MatProgressSpinnerModule } from '@angular/material/progress-spinner'; import { MatTooltipModule } from '@angular/material/tooltip'; import { queryDrivers, updateDriver } from '@placeos/ts-client'; import { describeError } from '../common/errors'; +import { reportLoadError } from '../common/load-error'; import { i18n } from '../common/locale.service'; import { notifyError, notifySuccess } from '../common/notifications'; import { IconComponent } from '../ui/icon.component'; @@ -175,7 +176,7 @@ export class DriverUpdateListModalComponent { const response = await queryDrivers({ update_available: true, limit: 1000, - }).catch(() => ({ data: [], total: 0 })); + }).catch(reportLoadError({ data: [], total: 0 })); response.data = response.data.filter( (_) => _.commit !== _.update_info.commit, ); diff --git a/src/app/modules/module-state.service.ts b/src/app/modules/module-state.service.ts index f7d9ebf11..b7e1af1c9 100644 --- a/src/app/modules/module-state.service.ts +++ b/src/app/modules/module-state.service.ts @@ -11,6 +11,7 @@ import { stopModule, } from '@placeos/ts-client'; import { ActiveItemService } from '../common/item.service'; +import { reportLoadError } from '../common/load-error'; import { notifyError, notifySuccess } from '../common/notifications'; import { querySupportSystems as querySystems } from '../common/support-access'; import { ViewResponseModalComponent } from '../overlays/view-response-modal.component'; @@ -79,7 +80,9 @@ export class ModuleStateService { try { const response = await querySystems({ module_id: item.id, - }).catch(() => ({ data: [] as PlaceSystem[], total: 0 })); + }).catch( + reportLoadError({ data: [] as PlaceSystem[], total: 0 }), + ); return { data: response.data, total: response.total }; } finally { this._loading.set(false); @@ -123,9 +126,7 @@ export class ModuleStateService { } return; } - notifySuccess( - `Module successfully ${running ? 'stopped' : 'started'}`, - ); + notifySuccess(`Module successfully ${running ? 'stopped' : 'started'}`); item.running = !running; } diff --git a/src/app/overlays/confirm-modal.component.ts b/src/app/overlays/confirm-modal.component.ts index 52deaa594..5116fe456 100644 --- a/src/app/overlays/confirm-modal.component.ts +++ b/src/app/overlays/confirm-modal.component.ts @@ -97,6 +97,8 @@ export interface ConfirmModalData { options?: ConfirmModalOption[]; /** Text displaed on the confirmation button */ confirm_text?: string; + /** Whether confirming deletes or loses data. Styles the button as an error */ + destructive?: boolean; /** Text displaed on the confirmation button */ cancel_text?: string; /** Icon to display on the modal */ @@ -386,6 +388,7 @@ export async function openConfirmModal( matRipple name="accept" class="flex-1" + [class.error]="destructive" [disabled]="resolving() || blocked()" (click)="onConfirm()" > @@ -426,9 +429,12 @@ export class ConfirmModalComponent implements OnInit { public readonly content: string = this._data.content || 'Are you sure?'; /** Body of the confirm modal */ public readonly extra: [string, string] = this._data.extra || ['', '']; + /** Whether the confirm button deletes or loses data */ + public readonly destructive = !!this._data.destructive; /** Display text on the confirm button */ public readonly confirm_text: string = - this._data.confirm_text || 'COMMON.ACCEPT'; + this._data.confirm_text || + (this.destructive ? 'COMMON.DELETE' : 'COMMON.ACCEPT'); /** Display text on the cancel button */ public readonly cancel_text: string = this._data.cancel_text || 'COMMON.CANCEL'; diff --git a/src/app/systems/system-state.service.ts b/src/app/systems/system-state.service.ts index 36c0bd45e..f3c81b3c9 100644 --- a/src/app/systems/system-state.service.ts +++ b/src/app/systems/system-state.service.ts @@ -39,6 +39,7 @@ import { import { describeError, readError } from '../common/errors'; import { ActiveItemService } from '../common/item.service'; +import { reportLoadError } from '../common/load-error'; import { notifyError, notifySuccess } from '../common/notifications'; import { waitForEvent } from '../common/signals'; import { DialogEvent, HashMap, Identity } from '../common/types'; @@ -161,7 +162,9 @@ export class SystemStateService extends AsyncHandler { control_system_id: item.id, complete: true, limit: 200, - } as Record).catch(() => ({ data: [] })); + } as Record).catch( + reportLoadError({ data: [] }), + ); // A newer load or another system took over. Leave its state alone. if (abortSignal.aborted || this.item()?.id !== item.id) { return [] as PlaceModule[]; @@ -244,9 +247,9 @@ export class SystemStateService extends AsyncHandler { ) ).filter((zone) => !!zone), } - : await listSystemZones(item.id).catch(() => ({ - data: [], - })); + : await listSystemZones(item.id).catch( + reportLoadError({ data: [] }), + ); const zones = [...response.data]; zones.sort( (a, b) => @@ -276,7 +279,7 @@ export class SystemStateService extends AsyncHandler { const response = await listSystemTriggers( item.id, full ? {} : { limit: 1 }, - ).catch(() => empty); + ).catch(full ? reportLoadError(empty) : () => empty); return full ? response : { ...empty, total: response.total }; } finally { this.setLoading('triggers', false); @@ -303,8 +306,9 @@ export class SystemStateService extends AsyncHandler { public async startSystem() { const details = await this.confirm({ title: 'Start system?', - content: `Are you sure you want to start this system?
All stopped modules within the system will boot up.`, + content: `Start ${escapeHtml(this.active_item.name)}?
All stopped modules within the system will boot up.`, icon: { type: 'icon', class: 'backoffice-controller-play' }, + confirm_text: 'Start system', }); if (details.reason !== 'done') return; details.loading('Starting system...'); @@ -327,8 +331,10 @@ export class SystemStateService extends AsyncHandler { public async stopSystem() { const details = await this.confirm({ title: 'Stop system?', - 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.`, + content: `Stop ${escapeHtml(this.active_item.name)}?
All modules will be immediately stopped regardless of any other systems they may be in.`, icon: { type: 'icon', class: 'backoffice-controller-stop' }, + confirm_text: 'Stop system', + destructive: true, }); if (details.reason !== 'done') return; details.loading('Stopping system...'); diff --git a/src/app/triggers/trigger-state.service.ts b/src/app/triggers/trigger-state.service.ts index 8f303d6a5..2181e1a73 100644 --- a/src/app/triggers/trigger-state.service.ts +++ b/src/app/triggers/trigger-state.service.ts @@ -18,6 +18,7 @@ import { import { describeError } from '../common/errors'; import { escapeHtml } from '../common/general'; import { ActiveItemService } from '../common/item.service'; +import { reportLoadError } from '../common/load-error'; import { i18n } from '../common/locale.service'; import { notifyError, notifySuccess } from '../common/notifications'; import { waitForEvent } from '../common/signals'; @@ -49,7 +50,9 @@ export class TriggerStateService { loader: async ({ params }) => { const { item } = params; if (!(item instanceof PlaceTrigger)) return [] as PlaceTrigger[]; - return listTriggerInstances(item.id).catch(() => []); + return listTriggerInstances(item.id).catch( + reportLoadError([] as PlaceTrigger[]), + ); }, }); diff --git a/src/app/ui/item-sidebar.component.ts b/src/app/ui/item-sidebar.component.ts index 82e780f47..446e67c27 100644 --- a/src/app/ui/item-sidebar.component.ts +++ b/src/app/ui/item-sidebar.component.ts @@ -30,6 +30,7 @@ import { } from '@placeos/ts-client'; import { AsyncHandler } from '../common/async-handler.class'; import { ActiveItemService } from '../common/item.service'; +import { reportLoadError } from '../common/load-error'; import { toSignal } from '../common/signals'; import { IconComponent } from './icon.component'; import { SettingsToggleComponent } from './settings-toggle.component'; @@ -67,7 +68,7 @@ interface GroupTreeItem { 'COMMON.SEARCH_FOR' | translate: { name: title() } " /> - @if (loading()) { + @if (loading() || group_hierarchy_loading()) { - } @else { + } @else if (!group_hierarchy_loading()) {
@@ -322,7 +323,8 @@ interface GroupTreeItem { } - } @else { + } @else if (!loading()) { +
@@ -387,6 +389,8 @@ export class ItemSidebarComponent public readonly filter_options = input([]); public readonly show = signal(true); public readonly group_hierarchy = signal([]); + /** Whether the group tree is loading */ + public readonly group_hierarchy_loading = signal(false); public readonly expanded_groups = signal>({}); public readonly include_deleted = this._service.include_deleted; @@ -614,6 +618,7 @@ export class ItemSidebarComponent } private async loadGroupHierarchy() { + this.group_hierarchy_loading.set(true); const response = await queryGroups({ limit: 2500, fields: [ @@ -624,11 +629,12 @@ export class ItemSidebarComponent 'parent_id', 'children_count', ].join(','), - }).catch(() => ({ data: [] })); + }).catch(reportLoadError({ data: [] })); const groups = response.data.sort((a, b) => this.displayName(a).localeCompare(this.displayName(b)), ); this.group_hierarchy.set(groups); + this.group_hierarchy_loading.set(false); } private expandGroupPath(group_id: string) { diff --git a/src/app/zones/zones-state.service.ts b/src/app/zones/zones-state.service.ts index 688a4ac9f..078275799 100644 --- a/src/app/zones/zones-state.service.ts +++ b/src/app/zones/zones-state.service.ts @@ -20,6 +20,7 @@ import { import { describeError, readError } from '../common/errors'; import { escapeHtml, unique } from '../common/general'; import { ActiveItemService } from '../common/item.service'; +import { reportLoadError } from '../common/load-error'; import { i18n } from '../common/locale.service'; import { notifyError, notifySuccess } from '../common/notifications'; import { openedTabs } from '../common/opened-tabs'; @@ -115,7 +116,7 @@ export class ZonesStateService { // Load every page so the list matches the tab count const data = await collectPages( querySystems({ zone_id: item.id, limit: 500 }), - ).catch(() => [] as PlaceSystem[]); + ).catch(reportLoadError([] as PlaceSystem[])); return { data, total: data.length }; }, }); @@ -128,7 +129,7 @@ export class ZonesStateService { const { item } = params; const empty = { data: [] as PlaceTrigger[], total: 0 }; if (!(item instanceof PlaceZone) || isSubsystemUser()) return empty; - return listZoneTriggers(item.id).catch(() => empty); + return listZoneTriggers(item.id).catch(reportLoadError(empty)); }, }); @@ -166,7 +167,7 @@ export class ZonesStateService { // Load every page so the list matches the tab count const data = await collectPages( queryZones({ parent_id: item.id, limit: 500 }), - ).catch(() => [] as PlaceZone[]); + ).catch(reportLoadError([] as PlaceZone[])); return { data, total: data.length }; }, }); @@ -188,7 +189,7 @@ export class ZonesStateService { const response = await queryGroupZones({ zone_id: item.id, limit: full ? 1000 : 1, - }).catch(() => empty); + }).catch(reportLoadError(empty)); if (!full) return { ...empty, total: response.total }; return { total: response.total, diff --git a/src/tests/common/notifications.spec.ts b/src/tests/common/notifications.spec.ts index 53ec32850..d88771612 100644 --- a/src/tests/common/notifications.spec.ts +++ b/src/tests/common/notifications.spec.ts @@ -181,6 +181,14 @@ describe('notifications.ts', () => { ); }); + it('should stay open until dismissed', () => { + notifyError('Error message'); + + expect( + mock_snackbar.open.mock.calls[0][2].duration, + ).toBeUndefined(); + }); + it('should log to console.error', () => { notifyError('Error message'); diff --git a/src/tests/overlays/confirm-modal.component.spec.ts b/src/tests/overlays/confirm-modal.component.spec.ts index d97dfe75d..a37f7667d 100644 --- a/src/tests/overlays/confirm-modal.component.spec.ts +++ b/src/tests/overlays/confirm-modal.component.spec.ts @@ -8,7 +8,7 @@ import { } from '@angular/material/dialog'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; import { Subject } from 'rxjs'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; // Mock the ts-client - factory must be self-contained vi.mock('@placeos/ts-client', () => { @@ -109,6 +109,43 @@ describe('ConfirmModalComponent', () => { }); }); + describe('destructive confirmations', () => { + it('label the confirm button Delete and style it as an error', async () => { + await TestBed.resetTestingModule(); + await TestBed.configureTestingModule({ + imports: [ + ConfirmModalComponent, + MatDialogModule, + NoopAnimationsModule, + ], + providers: [ + { provide: MatDialogRef, useValue: dialog_ref_mock }, + { + provide: MAT_DIALOG_DATA, + useValue: { ...default_data, destructive: true }, + }, + ], + }) + .overrideComponent(ConfirmModalComponent, { + remove: { imports: [IconComponent] }, + add: { imports: [MockComponent(IconComponent)] }, + }) + .compileComponents(); + const destructive_fixture = TestBed.createComponent( + ConfirmModalComponent, + ); + destructive_fixture.detectChanges(); + expect(destructive_fixture.componentInstance.confirm_text).toBe( + 'COMMON.DELETE', + ); + const accept: HTMLElement = + destructive_fixture.nativeElement.querySelector( + 'button[name="accept"]', + ); + expect(accept.classList).toContain('error'); + }); + }); + describe('with default values when data is missing', () => { let minimal_component: ConfirmModalComponent; From 19ea2e1af63f388a1fa704a54a3f65fe89cc3119 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Wed, 21 Oct 2026 16:28:25 +1100 Subject: [PATCH 4/5] fix(ui): fit tablet layouts, readable dark placeholders, closable menus - Below 1024px the list sidebar gives way to the item picker, so the details pane keeps its width on tablets - Placeholders are readable in dark mode - Popover menus close on Escape and on navigation - Logout moves to the bottom of the user menu Co-Authored-By: Claude Opus 5.5 --- src/app/domains/domains.component.ts | 6 ++--- src/app/drivers/drivers.component.ts | 8 +++---- src/app/groups/groups.component.ts | 6 ++--- src/app/modules/modules.component.ts | 6 ++--- .../repositories/repositories.component.ts | 6 ++--- src/app/systems/systems.component.ts | 8 +++---- src/app/triggers/triggers.component.ts | 6 ++--- src/app/ui/custom-tooltip.component.ts | 21 ++++++++++++++++++ src/app/ui/item-selection.component.ts | 2 +- src/app/ui/user-menu-tooltip.component.ts | 20 ++++++++--------- src/app/users/users.component.ts | 8 +++---- src/app/zones/zones.component.ts | 8 +++---- src/styles.css | 7 ++---- .../custom-tooltip.component.spec.ts | 22 ++++++++++++++----- 14 files changed, 82 insertions(+), 52 deletions(-) diff --git a/src/app/domains/domains.component.ts b/src/app/domains/domains.component.ts index caad62351..93cd8c370 100644 --- a/src/app/domains/domains.component.ts +++ b/src/app/domains/domains.component.ts @@ -24,13 +24,13 @@ import { DomainStateService } from './domain-state.service'; >
`, styles: [ diff --git a/src/app/users/users.component.ts b/src/app/users/users.component.ts index e46fbcf77..0643ef16e 100644 --- a/src/app/users/users.component.ts +++ b/src/app/users/users.component.ts @@ -27,13 +27,13 @@ import { UsersStateService } from './users-state.service'; >
diff --git a/src/app/admin/signage-plugins/signage-plugin-import-modal.component.ts b/src/app/admin/signage-plugins/signage-plugin-import-modal.component.ts index e7fa54a7c..59bbd9b8a 100644 --- a/src/app/admin/signage-plugins/signage-plugin-import-modal.component.ts +++ b/src/app/admin/signage-plugins/signage-plugin-import-modal.component.ts @@ -56,7 +56,12 @@ interface PluginProbe {

{{ 'ADMIN.SIGNAGE_PLUGINS_IMPORT' | translate }}

- diff --git a/src/app/admin/signage-plugins/signage-plugin-test-modal.component.ts b/src/app/admin/signage-plugins/signage-plugin-test-modal.component.ts index c72b0dd9f..fc5c235a3 100644 --- a/src/app/admin/signage-plugins/signage-plugin-test-modal.component.ts +++ b/src/app/admin/signage-plugins/signage-plugin-test-modal.component.ts @@ -188,7 +188,12 @@ export interface SignagePluginTestModalData { {{ 'ADMIN.SIGNAGE_PLUGINS_TEST_HEADING' | translate }} — {{ plugin().name }} - diff --git a/src/app/admin/staff-tenant-modal.component.ts b/src/app/admin/staff-tenant-modal.component.ts index aae6e230c..5662c1446 100644 --- a/src/app/admin/staff-tenant-modal.component.ts +++ b/src/app/admin/staff-tenant-modal.component.ts @@ -525,7 +525,7 @@ export class StaffTenantModalComponent { const invalid_credentials = this.credential_fields() .filter((field) => field.required) .filter((field) => !this.formModel().credentials[field.key]) - .map((field) => field.key); + .map((field) => FIELD_NAME_MAPPING[field.key] || field.key); if (this.form().invalid() || invalid_credentials.length) { return notifyError( i18n('COMMON.INVALID_FIELDS', { diff --git a/src/app/admin/upload-library.component.ts b/src/app/admin/upload-library.component.ts index b482b42e3..a6c8f1543 100644 --- a/src/app/admin/upload-library.component.ts +++ b/src/app/admin/upload-library.component.ts @@ -500,7 +500,7 @@ export class UploadLibraryComponent extends AsyncHandler implements OnInit { if (failed) { notifyError('Failed to upload files.'); } else if (success && finished) { - notifySuccess('Succesfully uploaded files.'); + notifySuccess('Successfully uploaded files.'); } this.clearTimeout('upload_list'); this.loading.set(false); diff --git a/src/app/admin/view-upload-modal.component.ts b/src/app/admin/view-upload-modal.component.ts index 3c36bf892..9da5f5a4c 100644 --- a/src/app/admin/view-upload-modal.component.ts +++ b/src/app/admin/view-upload-modal.component.ts @@ -9,10 +9,10 @@ import { UploadInfo } from './upload-library.component'; selector: 'view-upload-modal', template: `

View Upload

-
diff --git a/src/app/admin/zone-tree-export-modal.component.ts b/src/app/admin/zone-tree-export-modal.component.ts index 43849e161..4bee61d05 100644 --- a/src/app/admin/zone-tree-export-modal.component.ts +++ b/src/app/admin/zone-tree-export-modal.component.ts @@ -16,7 +16,7 @@ import { IconComponent } from '../ui/icon.component'; template: `

Export Zone Tree

-
@@ -59,6 +59,7 @@ import { IconComponent } from '../ui/icon.component'; icon matRipple (click)="removeSelected(zone)" + aria-label="Close" > close diff --git a/src/app/common/forms.ts b/src/app/common/forms.ts index 2e333cc6d..7c2fa29d1 100644 --- a/src/app/common/forms.ts +++ b/src/app/common/forms.ts @@ -17,12 +17,21 @@ export function removeSignalChipItem(items: T[], item: T): T[] { return items.filter((existing) => existing !== item); } +/** Readable name for a form field key, e.g. `service_account` to "Service account" */ +export function fieldLabel(key: string): string { + const words = key.replace(/_/g, ' ').trim(); + return words.charAt(0).toUpperCase() + words.slice(1); +} + +/** Readable names of the invalid top-level fields, for error messages */ export function getInvalidSignalFields( form: FieldTree, ): string[] { const value = form().value() as object; - return Object.keys(value).filter((key) => { - const field = form[key as keyof typeof form]; - return typeof field === 'function' && field().invalid(); - }); + return Object.keys(value) + .filter((key) => { + const field = form[key as keyof typeof form]; + return typeof field === 'function' && field().invalid(); + }) + .map(fieldLabel); } diff --git a/src/app/common/hotkeys.service.ts b/src/app/common/hotkeys.service.ts index 82fc3b3de..24e6f87a8 100644 --- a/src/app/common/hotkeys.service.ts +++ b/src/app/common/hotkeys.service.ts @@ -20,6 +20,17 @@ const INVALID_STANDALONE_KEYS: string[] = [ /** Modifier keys whose held state is read from the key event flags */ const MODIFIER_KEYS: string[] = ['control', 'shift', 'alt', 'meta']; +/** Focused controls that handle letter and arrow keys themselves */ +const KEYBOARD_WIDGETS = [ + '[role="combobox"]', + '[role="listbox"]', + '[role="option"]', + '[role="menu"]', + '[role="menuitem"]', + '[role="radiogroup"]', + '[role="slider"]', + '[role="spinbutton"]', +].join(', '); @Service() export class HotkeysService { @@ -149,6 +160,11 @@ export class HotkeysService { return true; } + // Selects, menus and similar controls use letters and arrow keys + if (active.closest(KEYBOARD_WIDGETS)) { + return true; + } + return false; } diff --git a/src/app/domains/domains.component.ts b/src/app/domains/domains.component.ts index 93cd8c370..e3819e064 100644 --- a/src/app/domains/domains.component.ts +++ b/src/app/domains/domains.component.ts @@ -39,6 +39,7 @@ import { DomainStateService } from './domain-state.service'; icon class="mr-2 sm:hidden" (click)="open_menu = true" + [attr.aria-label]="'COMMON.MENU' | translate" > menu diff --git a/src/app/drivers/driver-update-list-modal.component.ts b/src/app/drivers/driver-update-list-modal.component.ts index 0d9f209e6..1badf8ef5 100644 --- a/src/app/drivers/driver-update-list-modal.component.ts +++ b/src/app/drivers/driver-update-list-modal.component.ts @@ -37,7 +37,12 @@ import { TranslatePipe } from '../ui/translate.pipe'; } @if (!loading()) { - } diff --git a/src/app/drivers/drivers.component.ts b/src/app/drivers/drivers.component.ts index 91bbb5a95..883ce94a7 100644 --- a/src/app/drivers/drivers.component.ts +++ b/src/app/drivers/drivers.component.ts @@ -42,6 +42,7 @@ import { DriverStateService } from './driver-state.service'; icon class="mr-2 sm:hidden" (click)="open_menu.set(true)" + [attr.aria-label]="'COMMON.MENU' | translate" > menu diff --git a/src/app/groups/group-bulk-add-modal.component.ts b/src/app/groups/group-bulk-add-modal.component.ts index 28122c047..da79639c0 100644 --- a/src/app/groups/group-bulk-add-modal.component.ts +++ b/src/app/groups/group-bulk-add-modal.component.ts @@ -52,7 +52,12 @@ export interface GroupBulkAddResult {

{{ title | translate }}

- @@ -66,7 +71,9 @@ export interface GroupBulkAddResult { } -
+
@if (!show_permissions || tab() === 0) { { }) export class GroupBulkAddModalComponent { private _dialog_ref = - inject, GroupBulkAddResult>>( - MatDialogRef, - ); + inject< + MatDialogRef, GroupBulkAddResult> + >(MatDialogRef); private _data = inject>(MAT_DIALOG_DATA); public readonly selected = signal([]); diff --git a/src/app/groups/groups.component.ts b/src/app/groups/groups.component.ts index 06be881a8..13d1277bc 100644 --- a/src/app/groups/groups.component.ts +++ b/src/app/groups/groups.component.ts @@ -41,6 +41,7 @@ import { GroupStateService } from './group-state.service'; icon class="mr-2 sm:hidden" (click)="open_menu = true" + [attr.aria-label]="'COMMON.MENU' | translate" > menu diff --git a/src/app/modules/modules.component.ts b/src/app/modules/modules.component.ts index 7a0871df9..2674b5b84 100644 --- a/src/app/modules/modules.component.ts +++ b/src/app/modules/modules.component.ts @@ -44,6 +44,7 @@ import { ModuleStateService } from './module-state.service'; matRipple class="mr-2 sm:hidden" (click)="open_menu.set(true)" + [attr.aria-label]="'COMMON.MENU' | translate" > menu diff --git a/src/app/overlays/bulk-item-modal/bulk-item-modal.component.ts b/src/app/overlays/bulk-item-modal/bulk-item-modal.component.ts index 2c53b8670..e82330c09 100644 --- a/src/app/overlays/bulk-item-modal/bulk-item-modal.component.ts +++ b/src/app/overlays/bulk-item-modal/bulk-item-modal.component.ts @@ -66,7 +66,12 @@ export interface BulkItemModalData> {

@if (!loading) { - } diff --git a/src/app/overlays/bulk-item-modal/list.component.ts b/src/app/overlays/bulk-item-modal/list.component.ts index 728e6056e..cabd416cf 100644 --- a/src/app/overlays/bulk-item-modal/list.component.ts +++ b/src/app/overlays/bulk-item-modal/list.component.ts @@ -167,6 +167,7 @@ import { TranslatePipe } from '../../ui/translate.pipe'; class="text-error mx-auto" title="Remove row" (click)="removeItem(index); $event.stopPropagation()" + [attr.aria-label]="'COMMON.DELETE' | translate" > delete diff --git a/src/app/overlays/metadata-details-modal.component.ts b/src/app/overlays/metadata-details-modal.component.ts index af5b1b12b..52eeb2ade 100644 --- a/src/app/overlays/metadata-details-modal.component.ts +++ b/src/app/overlays/metadata-details-modal.component.ts @@ -1,7 +1,13 @@ import { COMMA, ENTER, SPACE } from '@angular/cdk/keycodes'; -import { Component, OnInit, inject, signal } from '@angular/core'; +import { Component, inject, OnInit, signal } from '@angular/core'; import { FormsModule } from '@angular/forms'; -import { form, FormField, required, submit, validate } from '@angular/forms/signals'; +import { + form, + FormField, + required, + submit, + validate, +} from '@angular/forms/signals'; import { MatChipInputEvent, MatChipsModule } from '@angular/material/chips'; import { MatRippleModule } from '@angular/material/core'; import { @@ -14,6 +20,7 @@ import { MatInputModule } from '@angular/material/input'; import { addSignalChipItem, removeSignalChipItem } from '../common/forms'; import { IconComponent } from '../ui/icon.component'; import type { MetadataFormModel } from '../ui/metadata-display.component'; +import { TranslatePipe } from '../ui/translate.pipe'; export interface MetadataDetailsModalData { value: MetadataFormModel; @@ -29,7 +36,12 @@ export interface MetadataDetailsModalData { class="border-base-100 bg-base-200 z-10 mx-auto my-2 flex w-[calc(100%-1rem)] items-center justify-between rounded-sm border px-4 py-2" >

Update metadata details

- @@ -115,6 +127,7 @@ export interface MetadataDetailsModalData { `, styles: [``], imports: [ + TranslatePipe, MatDialogModule, MatRippleModule, MatFormFieldModule, diff --git a/src/app/overlays/metadata-history-modal.component.ts b/src/app/overlays/metadata-history-modal.component.ts index 444f5b7fe..b12281b83 100644 --- a/src/app/overlays/metadata-history-modal.component.ts +++ b/src/app/overlays/metadata-history-modal.component.ts @@ -30,7 +30,12 @@ import { TranslatePipe } from '../ui/translate.pipe';

{{ 'COMMON.METADATA_HISTORY' | translate }}

- diff --git a/src/app/overlays/select-item-modal.component.ts b/src/app/overlays/select-item-modal.component.ts index 955996f76..ccde63e53 100644 --- a/src/app/overlays/select-item-modal.component.ts +++ b/src/app/overlays/select-item-modal.component.ts @@ -25,7 +25,12 @@ export interface SelectItemModalData { {{ 'COMMON.ITEM_ADD' | translate: { item: name } }} @if (!loading) { - } diff --git a/src/app/overlays/view-module-state.component.ts b/src/app/overlays/view-module-state.component.ts index 9283f2c33..6cffc099f 100644 --- a/src/app/overlays/view-module-state.component.ts +++ b/src/app/overlays/view-module-state.component.ts @@ -35,7 +35,12 @@ export interface ModuleStateModalData {

{{ 'MODULES.STATE' | translate }}

- diff --git a/src/app/repositories/repositories.component.ts b/src/app/repositories/repositories.component.ts index 87095766f..fbb7a0b38 100644 --- a/src/app/repositories/repositories.component.ts +++ b/src/app/repositories/repositories.component.ts @@ -39,6 +39,7 @@ import { RepositoriesStateService } from './repositories-state.service'; matRipple class="mr-2 sm:hidden" (click)="open_menu.set(true)" + [attr.aria-label]="'COMMON.MENU' | translate" > menu diff --git a/src/app/systems/system-modules.component.ts b/src/app/systems/system-modules.component.ts index a14a502f1..9595125f7 100644 --- a/src/app/systems/system-modules.component.ts +++ b/src/app/systems/system-modules.component.ts @@ -307,6 +307,7 @@ import { SystemStateService } from './system-state.service'; default matRipple (click)="editModule(row)" + [attr.aria-label]="'COMMON.EDIT' | translate" > edit @@ -316,6 +317,9 @@ import { SystemStateService } from './system-state.service'; matRipple (click)="setActive(i)" [matMenuTriggerFor]="context_menu" + [attr.aria-label]=" + 'COMMON.MORE_ACTIONS' | translate + " > more_vert diff --git a/src/app/systems/systems.component.ts b/src/app/systems/systems.component.ts index 77a948796..e988df778 100644 --- a/src/app/systems/systems.component.ts +++ b/src/app/systems/systems.component.ts @@ -42,6 +42,7 @@ import { SystemStateService } from './system-state.service'; icon class="mr-2 sm:hidden" (click)="open_menu.set(true)" + [attr.aria-label]="'COMMON.MENU' | translate" > menu diff --git a/src/app/triggers/trigger-about.component.ts b/src/app/triggers/trigger-about.component.ts index 8abbe89cb..933f8b76a 100644 --- a/src/app/triggers/trigger-about.component.ts +++ b/src/app/triggers/trigger-about.component.ts @@ -182,6 +182,7 @@ import { TriggerStateService } from './trigger-state.service'; matRipple [disabled]="!template_system()" (click)="editCondition(row)" + [attr.aria-label]="'COMMON.EDIT' | translate" > edit @@ -191,6 +192,7 @@ import { TriggerStateService } from './trigger-state.service'; error matRipple (click)="removeCondition(row)" + [attr.aria-label]="'COMMON.DELETE' | translate" > delete @@ -281,6 +283,7 @@ import { TriggerStateService } from './trigger-state.service'; matRipple [disabled]="!template_system()" (click)="editAction(row)" + [attr.aria-label]="'COMMON.EDIT' | translate" > edit @@ -290,6 +293,7 @@ import { TriggerStateService } from './trigger-state.service'; error matRipple (click)="removeAction(row)" + [attr.aria-label]="'COMMON.DELETE' | translate" > delete diff --git a/src/app/triggers/triggers.component.ts b/src/app/triggers/triggers.component.ts index fd493c7b9..05f270f34 100644 --- a/src/app/triggers/triggers.component.ts +++ b/src/app/triggers/triggers.component.ts @@ -45,6 +45,7 @@ import { TriggerStateService } from './trigger-state.service'; icon class="mr-2 sm:hidden" (click)="open_menu = true" + [attr.aria-label]="'COMMON.MENU' | translate" > menu diff --git a/src/app/ui/counter.component.ts b/src/app/ui/counter.component.ts index 7fb833e63..292a073b2 100644 --- a/src/app/ui/counter.component.ts +++ b/src/app/ui/counter.component.ts @@ -12,6 +12,7 @@ import { } from '@angular/forms'; import { MatRippleModule } from '@angular/material/core'; import { IconComponent } from './icon.component'; +import { TranslatePipe } from './translate.pipe'; @Component({ selector: 'a-counter', @@ -25,6 +26,7 @@ import { IconComponent } from './icon.component'; class="border-secondary text-secondary z-10 h-12 w-12 rounded-l rounded-r-none border" [disabled]="disabled() || !value() || value() === min()" (click)="remove()" + [attr.aria-label]="'COMMON.DECREASE' | translate" > remove @@ -58,6 +60,7 @@ import { IconComponent } from './icon.component'; class="border-secondary text-secondary z-10 h-12 w-12 rounded-l-none rounded-r border" [disabled]="disabled() || value() === max()" (click)="add()" + [attr.aria-label]="'COMMON.INCREASE' | translate" > add @@ -72,7 +75,7 @@ import { IconComponent } from './icon.component'; multi: true, }, ], - imports: [IconComponent, MatRippleModule, FormsModule], + imports: [TranslatePipe, IconComponent, MatRippleModule, FormsModule], }) export class CounterComponent implements ControlValueAccessor { /** Size of a single step */ diff --git a/src/app/ui/custom-fields/image-list-field.component.ts b/src/app/ui/custom-fields/image-list-field.component.ts index 2c01b3b27..ec77ec003 100644 --- a/src/app/ui/custom-fields/image-list-field.component.ts +++ b/src/app/ui/custom-fields/image-list-field.component.ts @@ -75,10 +75,22 @@ import { TranslatePipe } from '../translate.pipe'; actions class="absolute top-0 right-0 left-0 flex items-center justify-center space-x-2 opacity-0" > - - @@ -101,6 +114,9 @@ import { TranslatePipe } from '../translate.pipe'; [style.transform]="'translate(-' + offset() + '00%)'" [matTooltip]="item.error" (click)="retryUpload(item)" + [attr.aria-label]=" + item.error ? ('COMMON.RETRY' | translate) : null + " > @if (!item.error) { delete diff --git a/src/app/ui/debug-info.component.ts b/src/app/ui/debug-info.component.ts index 29fb5d840..2e25d5f3a 100644 --- a/src/app/ui/debug-info.component.ts +++ b/src/app/ui/debug-info.component.ts @@ -79,7 +79,13 @@ import { TranslatePipe } from './translate.pipe';

} @if (compact()) { - diff --git a/src/app/ui/global-banner.component.ts b/src/app/ui/global-banner.component.ts index 334e6dc91..2379ae4b1 100644 --- a/src/app/ui/global-banner.component.ts +++ b/src/app/ui/global-banner.component.ts @@ -2,6 +2,7 @@ import { Component, inject } from '@angular/core'; import { MatRipple } from '@angular/material/core'; import { SettingsService } from '../common/settings.service'; import { IconComponent } from './icon.component'; +import { TranslatePipe } from './translate.pipe'; export interface BannerDetails { id: string; @@ -25,7 +26,12 @@ export interface BannerDetails { [class.text-error-content]="banner.type === 'error'" >
{{ banner?.content }}
- @@ -39,7 +45,7 @@ export interface BannerDetails { } `, ], - imports: [IconComponent, MatRipple], + imports: [TranslatePipe, IconComponent, MatRipple], }) export class GlobalBannerComponent { private _settings = inject(SettingsService); diff --git a/src/app/ui/icon.component.ts b/src/app/ui/icon.component.ts index 83ad4a704..0cb0ee668 100644 --- a/src/app/ui/icon.component.ts +++ b/src/app/ui/icon.component.ts @@ -1,7 +1,24 @@ -import { ChangeDetectionStrategy, Component, input } from '@angular/core'; +import { + afterEveryRender, + afterNextRender, + ChangeDetectionStrategy, + Component, + ElementRef, + inject, + Injector, + input, +} from '@angular/core'; +import { MatTooltip } from '@angular/material/tooltip'; import { ApplicationIcon } from '../common/types'; import { SafePipe } from './pipes/safe.pipe'; +/** Text of an element, leaving out the names of any icons inside it */ +function textWithoutIcons(element: HTMLElement) { + const copy = element.cloneNode(true) as HTMLElement; + for (const icon of copy.querySelectorAll('icon, app-icon')) icon.remove(); + return copy.textContent.trim(); +} + @Component({ selector: 'app-icon,icon', template: ` @@ -35,4 +52,55 @@ export class IconComponent { public readonly className = input('material-symbols-rounded'); /** Icon details */ public readonly icon = input(undefined); + + private readonly _element = inject>(ElementRef); + private readonly _tooltip = inject(MatTooltip, { optional: true }); + private readonly _injector = inject(Injector); + + constructor() { + afterNextRender(() => this._labelControl()); + } + + /** + * Icon fonts render names like "more_vert", which screen readers read + * aloud. Inside a button or link, hide the icon when the control has its + * own text or label. For icon-only controls with a tooltip, use the + * tooltip as the control's label. + */ + private _labelControl() { + const host = this._element.nativeElement; + const control = host.parentElement?.closest('button, a'); + if (!control) return; + const has_text = !!textWithoutIcons(control); + if (control.hasAttribute('aria-label') || has_text) { + host.setAttribute('aria-hidden', 'true'); + return; + } + // The tooltip must be on the control itself, not an outer element + const tooltip = this._tooltip; + if ( + !tooltip || + !control.classList.contains('mat-mdc-tooltip-trigger') + ) { + return; + } + // Tooltip text can change, e.g. "Enable" and "Disable" + afterEveryRender( + () => { + const label = `${tooltip.message || ''}`.trim(); + if (control.getAttribute('aria-label') === (label || null)) { + return; + } + // Without tooltip text the icon name is the only label left + if (label) { + control.setAttribute('aria-label', label); + host.setAttribute('aria-hidden', 'true'); + } else { + control.removeAttribute('aria-label'); + host.removeAttribute('aria-hidden'); + } + }, + { injector: this._injector }, + ); + } } diff --git a/src/app/ui/item-details.component.ts b/src/app/ui/item-details.component.ts index 3bfd1cc1b..23a8c6eb1 100644 --- a/src/app/ui/item-details.component.ts +++ b/src/app/ui/item-details.component.ts @@ -101,7 +101,13 @@ export interface DisplayItem { } - diff --git a/src/app/ui/item-sidebar.component.ts b/src/app/ui/item-sidebar.component.ts index 446e67c27..16e04b088 100644 --- a/src/app/ui/item-sidebar.component.ts +++ b/src/app/ui/item-sidebar.component.ts @@ -84,6 +84,7 @@ interface GroupTreeItem { class="h-11 w-11" [class.border-info]="selected_filters.length" [class.text-info]="selected_filters.length" + [attr.aria-label]="'COMMON.FILTER' | translate" > filter_list diff --git a/src/app/ui/module-runtime-errors-modal.component.ts b/src/app/ui/module-runtime-errors-modal.component.ts index ed5243403..b0b85c31a 100644 --- a/src/app/ui/module-runtime-errors-modal.component.ts +++ b/src/app/ui/module-runtime-errors-modal.component.ts @@ -25,7 +25,12 @@ import { TranslatePipe } from './translate.pipe'; @if (!loading()) { - } diff --git a/src/app/ui/sidebar-menu.component.ts b/src/app/ui/sidebar-menu.component.ts index d38e3f727..8092e556a 100644 --- a/src/app/ui/sidebar-menu.component.ts +++ b/src/app/ui/sidebar-menu.component.ts @@ -80,6 +80,7 @@ import { UserMenuTooltipComponent } from './user-menu-tooltip.component'; compact() ? (link.name | translate) : '' " matTooltipPosition="right" + [attr.aria-label]="link.name | translate" >
+ [attr.aria-label]="'COMMON.CLOSE' | translate"> close
@@ -164,6 +166,7 @@ import { UserMenuTooltipComponent } from './user-menu-tooltip.component'; [content]="application_picker" [matTooltip]="compact() ? 'Applications' : ''" matTooltipPosition="right" + aria-label="Applications" yPosition="bottom" > diff --git a/src/app/ui/upload-permissions-modal.component.ts b/src/app/ui/upload-permissions-modal.component.ts index 9a5d28a1f..961c51098 100644 --- a/src/app/ui/upload-permissions-modal.component.ts +++ b/src/app/ui/upload-permissions-modal.component.ts @@ -23,7 +23,12 @@ import { TranslatePipe } from './translate.pipe';

{{ 'COMMON.UPLOAD_FILE' | translate }}

- diff --git a/src/app/users/users.component.ts b/src/app/users/users.component.ts index 0643ef16e..d5e02b5b5 100644 --- a/src/app/users/users.component.ts +++ b/src/app/users/users.component.ts @@ -42,6 +42,7 @@ import { UsersStateService } from './users-state.service'; icon class="mr-2 sm:hidden" (click)="open_menu = true" + [attr.aria-label]="'COMMON.MENU' | translate" > menu diff --git a/src/app/zones/zones.component.ts b/src/app/zones/zones.component.ts index 152d7d69f..1144fc81a 100644 --- a/src/app/zones/zones.component.ts +++ b/src/app/zones/zones.component.ts @@ -47,6 +47,7 @@ import { ZonesStateService } from './zones-state.service'; matRipple class="mr-2 sm:hidden" (click)="open_menu.set(true)" + [attr.aria-label]="'COMMON.MENU' | translate" > menu diff --git a/src/tests/common/forms.spec.ts b/src/tests/common/forms.spec.ts new file mode 100644 index 000000000..14b321f6d --- /dev/null +++ b/src/tests/common/forms.spec.ts @@ -0,0 +1,9 @@ +import { describe, expect, it } from 'vitest'; +import { fieldLabel } from '../../app/common/forms'; + +describe('fieldLabel', () => { + it('turns form keys into readable names', () => { + expect(fieldLabel('zone')).toBe('Zone'); + expect(fieldLabel('service_account')).toBe('Service account'); + }); +}); diff --git a/src/tests/common/hotkeys.service.spec.ts b/src/tests/common/hotkeys.service.spec.ts index b79baa4fc..fcf010055 100644 --- a/src/tests/common/hotkeys.service.spec.ts +++ b/src/tests/common/hotkeys.service.spec.ts @@ -159,6 +159,23 @@ describe('HotkeysService', () => { }); }); + describe('focused controls', () => { + it('ignores keys while a select has focus', () => { + const callback = vi.fn(); + const subscription = service.listen(['ArrowRight'], callback); + const select = document.createElement('div'); + select.setAttribute('role', 'combobox'); + select.tabIndex = 0; + document.body.appendChild(select); + select.focus(); + keyEvent('keydown', 'ArrowRight'); + keyEvent('keyup', 'ArrowRight'); + expect(callback).not.toHaveBeenCalled(); + select.remove(); + subscription?.unsubscribe(); + }); + }); + describe('modifier keys', () => { it('ignores a single key combination while Control or Meta is held', () => { const callback = vi.fn(); diff --git a/src/tests/ui/components/icon.component.spec.ts b/src/tests/ui/components/icon.component.spec.ts index a2b102aa2..c996a91cd 100644 --- a/src/tests/ui/components/icon.component.spec.ts +++ b/src/tests/ui/components/icon.component.spec.ts @@ -1,4 +1,6 @@ +import { Component } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { MatTooltip } from '@angular/material/tooltip'; import { beforeEach, describe, expect, it } from 'vitest'; import { ApplicationIcon } from '../../../app/common/types'; import { IconComponent } from '../../../app/ui/icon.component'; @@ -123,4 +125,38 @@ describe('IconComponent', () => { expect(wrapper.className).toContain('justify-center'); }); }); + + describe('inside buttons', () => { + @Component({ + imports: [IconComponent, MatTooltip], + template: ` + + + + `, + }) + class TestButtonsComponent {} + + it('labels icon-only buttons from their tooltip and hides icon names', async () => { + const host = TestBed.createComponent(TestButtonsComponent); + host.detectChanges(); + await host.whenStable(); + const [icon_only, with_text, two_icons] = + host.nativeElement.querySelectorAll('button'); + expect(icon_only.getAttribute('aria-label')).toBe('More actions'); + expect( + icon_only.querySelector('icon').getAttribute('aria-hidden'), + ).toBe('true'); + // Other icons are not text, so these stay readable + expect( + two_icons.querySelector('icon').hasAttribute('aria-hidden'), + ).toBe(false); + expect(with_text.hasAttribute('aria-label')).toBe(false); + expect( + with_text.querySelector('icon').getAttribute('aria-hidden'), + ).toBe('true'); + }); + }); });