diff --git a/projects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.spec.ts b/projects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.spec.ts index 9c9b6e12f2..97796cee1c 100644 --- a/projects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.spec.ts +++ b/projects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.spec.ts @@ -44,7 +44,7 @@ describe('SiDashboardToolbarComponent', () => { await fixture.whenStable(); fixture.detectChanges(); - expect(component.editable(), 'Cancel shall not change editable state').toBe(true); + expect(component.editable()).toBe(false); }); it('#onSave() shall cancel editable mode and emit save', async () => { diff --git a/projects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.ts b/projects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.ts index a147234147..dc11876d12 100644 --- a/projects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.ts +++ b/projects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.ts @@ -97,12 +97,6 @@ export class SiDashboardToolbarComponent { */ readonly save = output(); - /** - * Emits on cancel button click. - */ - // eslint-disable-next-line @angular-eslint/no-output-native - readonly cancel = output(); - protected labelEdit = t(() => $localize`:@@DASHBOARD.EDIT:Edit`); protected labelCancel = t(() => $localize`:@@DASHBOARD.CANCEL:Cancel`); protected labelSave = t(() => $localize`:@@DASHBOARD.SAVE:Save`); @@ -140,7 +134,7 @@ export class SiDashboardToolbarComponent { protected onCancel(): void { if (this.editable()) { - this.cancel.emit(); + this.editable.set(false); } } diff --git a/projects/dashboards-ng/src/components/flexible-dashboard/si-flexible-dashboard.component.html b/projects/dashboards-ng/src/components/flexible-dashboard/si-flexible-dashboard.component.html index 5265d3040f..a1ee7edc22 100644 --- a/projects/dashboards-ng/src/components/flexible-dashboard/si-flexible-dashboard.component.html +++ b/projects/dashboards-ng/src/components/flexible-dashboard/si-flexible-dashboard.component.html @@ -5,14 +5,12 @@ [class.d-none]="!isDashboardVisible()" [primaryEditActions]="(primaryEditActions$ | async) ?? []" [secondaryEditActions]="(secondaryEditActions$ | async) ?? []" - [editable]="editable()" [disabled]="(grid.isLoading | async) ?? false" [hideEditButton]="hideEditButton()" [showEditButtonLabel]="showEditButtonLabel()" [grid]="grid" - (editableChange)="grid.edit()" + [(editable)]="editable" (save)="grid.save()" - (cancel)="grid.cancel()" > diff --git a/projects/dashboards-ng/src/components/flexible-dashboard/si-flexible-dashboard.component.spec.ts b/projects/dashboards-ng/src/components/flexible-dashboard/si-flexible-dashboard.component.spec.ts index aa6f317d20..f128bbea21 100644 --- a/projects/dashboards-ng/src/components/flexible-dashboard/si-flexible-dashboard.component.spec.ts +++ b/projects/dashboards-ng/src/components/flexible-dashboard/si-flexible-dashboard.component.spec.ts @@ -194,25 +194,6 @@ describe('SiFlexibleDashboardComponent', () => { expect(restoreSpy).toHaveBeenCalled(); }); - it('should call #grid.edit() on changing editable input to true', () => { - const spy = vi.spyOn(grid, 'edit'); - expect(component.editable()).toBe(false); - fixture.componentRef.setInput('editable', true); - fixture.detectChanges(); - expect(spy).toHaveBeenCalled(); - }); - - it('should call #grid.cancel() on changing editable input to false', () => { - expect(component.editable()).toBe(false); - grid.editable.set(true); - expect(component.editable()).toBe(true); - const spy = vi.spyOn(grid, 'cancel'); - - fixture.componentRef.setInput('editable', false); - fixture.detectChanges(); - expect(spy).toHaveBeenCalled(); - }); - it('should emit editableChange events on changing grid editable state', () => { grid.editable.set(true); expect(component.editable()).toBe(true); @@ -240,12 +221,11 @@ describe('SiFlexibleDashboardComponent', () => { }); it('should cancel edit state on dashboardId changes', () => { - const spy = vi.spyOn(component.grid(), 'cancel'); fixture.componentRef.setInput('editable', true); fixture.componentRef.setInput('dashboardId', '1'); fixture.detectChanges(); - expect(spy).toHaveBeenCalledTimes(1); + expect(component.editable()).toBe(false); }); }); diff --git a/projects/dashboards-ng/src/components/flexible-dashboard/si-flexible-dashboard.component.ts b/projects/dashboards-ng/src/components/flexible-dashboard/si-flexible-dashboard.component.ts index 9d2f5dbe61..71a0d1be97 100644 --- a/projects/dashboards-ng/src/components/flexible-dashboard/si-flexible-dashboard.component.ts +++ b/projects/dashboards-ng/src/components/flexible-dashboard/si-flexible-dashboard.component.ts @@ -238,7 +238,7 @@ export class SiFlexibleDashboardComponent implements OnInit, OnChanges, OnDestro dashboard.restore(); } if (this.editable()) { - this.grid().cancel(); + this.editable.set(false); this.viewState.set('dashboard'); this.catalogHost().clear(); } @@ -246,13 +246,6 @@ export class SiFlexibleDashboardComponent implements OnInit, OnChanges, OnDestro this.setupMenuItems(); } - if (changes.editable) { - if (changes.editable.currentValue) { - this.grid().edit(); - } else { - this.grid().cancel(); - } - } if (changes.hideAddWidgetInstanceButton) { this.hideAddWidgetInstanceButton$.next(changes.hideAddWidgetInstanceButton.currentValue); } @@ -276,7 +269,7 @@ export class SiFlexibleDashboardComponent implements OnInit, OnChanges, OnDestro dashboard.restore(); } if (!this.editable()) { - this.grid().edit(); + this.editable.set(true); } this.viewState.set('catalog'); const componentType = this.widgetCatalogComponent() ?? SiWidgetCatalogComponent; diff --git a/projects/dashboards-ng/src/components/grid/si-grid.component.spec.ts b/projects/dashboards-ng/src/components/grid/si-grid.component.spec.ts index a4044beae5..66097a3f3d 100644 --- a/projects/dashboards-ng/src/components/grid/si-grid.component.spec.ts +++ b/projects/dashboards-ng/src/components/grid/si-grid.component.spec.ts @@ -100,26 +100,68 @@ describe('SiGridComponent', () => { }); }); - it('should call edit() on setting editable to true', () => { - const spy = vi.spyOn(component, 'edit'); + it('should resetEditState on setting editable to true', () => { + component.transientWidgetInstances = [{ id: '1', widgetId: 'w1' }]; + component.markedForRemoval = [{ id: '2', widgetId: 'w2' }]; expect(component.editable()).toBe(false); fixture.componentRef.setInput('editable', true); fixture.detectChanges(); - expect(spy).toHaveBeenCalled(); expect(component.editable()).toBe(true); + expect(component.transientWidgetInstances).toEqual([]); + expect(component.markedForRemoval).toEqual([]); }); - it('should call cancel() on setting editable to false', () => { - const spy = vi.spyOn(component, 'cancel'); - fixture.componentRef.setInput('editable', true); + it('should resetEditState on calling edit()', () => { + component.transientWidgetInstances = [{ id: '1', widgetId: 'w1' }]; + component.markedForRemoval = [{ id: '2', widgetId: 'w2' }]; + expect(component.editable()).toBe(false); + + component.edit(); + fixture.detectChanges(); expect(component.editable()).toBe(true); - expect(spy).not.toHaveBeenCalled(); + expect(component.transientWidgetInstances).toEqual([]); + expect(component.markedForRemoval).toEqual([]); + }); + + it('should restoreSavedState on setting editable to false', () => { + vi.useFakeTimers(); + fixture.componentRef.setInput('editable', true); + + const savedWidgets = [...component.persistedWidgetInstances]; + component.addWidgetInstance({ widgetId: 'id' }); + expect(component.visibleWidgetInstances$.value.length).toBe(savedWidgets.length + 1); + + // Simulate grid modification so restoreSavedState actually restores + fixture.debugElement + .query(By.css('si-gridstack-wrapper')) + .triggerEventHandler('gridEvent', { event: { type: 'added' } }); + vi.advanceTimersByTime(0); fixture.componentRef.setInput('editable', false); fixture.detectChanges(); - expect(spy).toHaveBeenCalled(); - expect(component.editable()).toBe(false); + expect(component.visibleWidgetInstances$.value).toEqual(savedWidgets); + vi.useRealTimers(); + }); + + it('should restoreSavedState on calling cancel()', () => { + vi.useFakeTimers(); + component.edit(); + + const savedWidgets = [...component.persistedWidgetInstances]; + component.addWidgetInstance({ widgetId: 'id' }); + expect(component.visibleWidgetInstances$.value.length).toBe(savedWidgets.length + 1); + + // Simulate grid modification so restoreSavedState actually restores + fixture.debugElement + .query(By.css('si-gridstack-wrapper')) + .triggerEventHandler('gridEvent', { event: { type: 'added' } }); + vi.advanceTimersByTime(0); + + component.cancel(); + fixture.detectChanges(); + expect(component.visibleWidgetInstances$.value).toEqual(savedWidgets); + vi.useRealTimers(); }); it('#addWidget() shall add a new WidgetConfig to the visible widgets of the grid and assign unique ids', () => { diff --git a/projects/dashboards-ng/src/components/grid/si-grid.component.ts b/projects/dashboards-ng/src/components/grid/si-grid.component.ts index 9a93f26bc0..e2d1d7b649 100644 --- a/projects/dashboards-ng/src/components/grid/si-grid.component.ts +++ b/projects/dashboards-ng/src/components/grid/si-grid.component.ts @@ -66,14 +66,6 @@ export class SiGridComponent implements OnInit, OnChanges, OnDestroy { * @defaultValue false */ readonly editable = model(false); - /** - * This is the internal owner of the current editable state and is used to track if - * editable or not. Not editable can be changed by either calling the `edit()` api - * method or by setting the `editable` input. When setting the input, the `ngOnChanges(...)` - * hook is used to call the `edit()` method. Similar, to get from editable to not editable, - * `cancel()` or `save()` is used and can be triggered from `ngOnChanges(...)`. - */ - private editableInternal = false; /** * An optional, but recommended dashboard id that is used in persistence and passed @@ -193,9 +185,9 @@ export class SiGridComponent implements OnInit, OnChanges, OnDestroy { if (changes.editable) { if (changes.editable.currentValue) { - this.edit(); + this.resetEditState(); } else { - this.cancel(); + this.restoreSavedState(); } } @@ -218,13 +210,9 @@ export class SiGridComponent implements OnInit, OnChanges, OnDestroy { * Set dashboard grid in editable mode to modify widget instances. */ edit(): void { - if (!this.editableInternal) { - this.transientWidgetInstances = []; - this.markedForRemoval = []; - this.setModified(false); - this.editableInternal = true; + if (!this.editable()) { this.editable.set(true); - this.gridService.editable$.next(this.editableInternal); + this.resetEditState(); } } @@ -233,7 +221,7 @@ export class SiGridComponent implements OnInit, OnChanges, OnDestroy { * changes the editable and isModified modes. */ save(): void { - if (!this.editableInternal) { + if (!this.editable()) { return; } @@ -250,9 +238,7 @@ export class SiGridComponent implements OnInit, OnChanges, OnDestroy { .subscribe({ next: (value: WidgetConfig[]) => { this.setModified(false); - this.editableInternal = false; this.editable.set(false); - this.gridService.editable$.next(this.editableInternal); this.isLoading.next(false); }, error: (err: any) => { @@ -266,19 +252,10 @@ export class SiGridComponent implements OnInit, OnChanges, OnDestroy { * Cancel current changes and restore last saved state. */ cancel(): void { - if (!this.editableInternal) { - return; - } - - if (this.modified) { - this.visibleWidgetInstances$.next([...this.persistedWidgetInstances]); - this.setModified(false); - } - this.editableInternal = false; if (this.editable()) { this.editable.set(false); + this.restoreSavedState(); } - this.gridService.editable$.next(this.editableInternal); } /** @@ -440,6 +417,19 @@ export class SiGridComponent implements OnInit, OnChanges, OnDestroy { return widgets || []; } + private resetEditState(): void { + this.transientWidgetInstances = []; + this.markedForRemoval = []; + this.setModified(false); + } + + private restoreSavedState(): void { + if (this.modified) { + this.visibleWidgetInstances$.next([...this.persistedWidgetInstances]); + this.setModified(false); + } + } + private setModified(modified: boolean): void { if (this.modified !== modified) { this.modified = modified; diff --git a/projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.ts b/projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.ts index f6f0d803e1..0dfb1e6876 100644 --- a/projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.ts +++ b/projects/dashboards-ng/src/components/gridstack-wrapper/si-gridstack-wrapper.component.ts @@ -201,6 +201,7 @@ export class SiGridstackWrapperComponent implements OnInit, OnChanges { const componentRef = this.gridstackContainer()!.createComponent(SiWidgetHostComponent, { bindings: [ inputBinding('widgetConfig', configSignal), + inputBinding('editable', this.editable), outputBinding('remove', widgetId => { this.widgetInstanceRemove.emit(widgetId); }), diff --git a/projects/dashboards-ng/src/components/widget-host/si-widget-host.component.html b/projects/dashboards-ng/src/components/widget-host/si-widget-host.component.html index aeb6507e4f..4150fe40a2 100644 --- a/projects/dashboards-ng/src/components/widget-host/si-widget-host.component.html +++ b/projects/dashboards-ng/src/components/widget-host/si-widget-host.component.html @@ -1,5 +1,5 @@
- @if (editable$ | async) { + @if (editable()) {
(); + /** + * Sets the widget host into editable mode. + * + * @defaultValue false + */ + readonly editable = input(false); + readonly remove = output(); readonly edit = output(); @@ -86,8 +93,6 @@ export class SiWidgetHostComponent implements OnInit, OnChanges { secondaryActions: (MenuItemLegacy | MenuItem)[] = []; /** @defaultValue 'expanded' */ actionBarViewType: ViewType = 'expanded'; - editable$ = this.gridService.editable$; - /** @defaultValue [] */ editablePrimaryActions: (MenuItemLegacy | ContentActionBarMainItem)[] = []; /** @defaultValue [] */ @@ -148,13 +153,14 @@ export class SiWidgetHostComponent implements OnInit, OnChanges { } } } + + if (changes.editable && !changes.editable.firstChange) { + this.setupEditable(this.editable()); + } } ngOnInit(): void { this.attachWidgetInstance(); - this.editable$ - .pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe(editable => this.setupEditable(editable)); } private attachWidgetInstance(): void { @@ -175,9 +181,7 @@ export class SiWidgetHostComponent implements OnInit, OnChanges { // to the DOM. this.widgetInstance.configChange .pipe(takeUntilDestroyed(this.destroyRef)) - .subscribe(event => - setTimeout(() => this.setupEditable(this.editable$.value, event)) - ); + .subscribe(event => setTimeout(() => this.setupEditable(this.editable(), event))); } if (isSignal(this.widgetInstance.config)) { this.widgetRef.setInput('config', this.widgetConfig()); @@ -185,7 +189,7 @@ export class SiWidgetHostComponent implements OnInit, OnChanges { this.widgetInstance.config = this.widgetConfig(); } this.widgetInstanceFooter = this.widgetInstance.footer; - this.setupEditable(this.gridService.editable$.value); + this.setupEditable(this.editable()); }, error: error => console.error('Error: ', error) }); diff --git a/projects/dashboards-ng/src/services/si-grid.service.ts b/projects/dashboards-ng/src/services/si-grid.service.ts index 763ae05907..d37f609b14 100644 --- a/projects/dashboards-ng/src/services/si-grid.service.ts +++ b/projects/dashboards-ng/src/services/si-grid.service.ts @@ -3,7 +3,6 @@ * SPDX-License-Identifier: MIT */ import { computed, Injectable, signal } from '@angular/core'; -import { BehaviorSubject } from 'rxjs'; import { Widget } from '../model/widgets.model'; @@ -16,14 +15,6 @@ export class SiGridService { () => new Map(this.widgetCatalog().map(widget => [widget.id, widget])) ); - /** - * Observable that emits true if si-grid is set to editable. - * The owner of the editable state is the si-grid component. - * This subject is used to propagate these state changes via - * the si-widget-host components to the widgets. - */ - editable$ = new BehaviorSubject(false); - getWidget(id: string): Widget | undefined { return this.widgetCatalogMap().get(id); }