From c1e9a6d88cda8eb176bef2fa510eac74f27666bb Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Sat, 4 Jul 2026 23:57:23 +0300 Subject: [PATCH] feat(nav): align dashboards editor to the single-tap action menu (PR-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Settings → Dashboards editor still used the retired gesture vocabulary — double-tap a tile to edit, long-press for a Duplicate/Delete bottom sheet. Bring it in line with the rest of the redesign: a single tap on a page tile opens one responsive ActionMenuComponent (Edit / Duplicate / Delete) at the tap point, reusing the same menu the widget grid uses. Delete now routes through a confirmation dialog before removing the page and its widgets. The bespoke DashboardsBottomSheetComponent and the press/drag-arbitration state it needed are removed, along with two dead fields (pageTitle, DomSanitizer). Closes #188. Refs #183. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01VDu7zsKECA9SvBR5DLdUex --- .../dashboards-bottom-sheet.component.html | 16 -- .../dashboards-bottom-sheet.component.scss | 0 .../dashboards-bottom-sheet.component.spec.ts | 24 --- .../dashboards-bottom-sheet.component.ts | 22 --- .../dashboards-editor.component.html | 16 +- .../dashboards-editor.component.spec.ts | 135 ++++++++++++++- .../dashboards-editor.component.ts | 162 ++++++++---------- 7 files changed, 207 insertions(+), 168 deletions(-) delete mode 100644 src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.html delete mode 100644 src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.scss delete mode 100644 src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.spec.ts delete mode 100644 src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.ts diff --git a/src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.html b/src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.html deleted file mode 100644 index e9c4c21a..00000000 --- a/src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.html +++ /dev/null @@ -1,16 +0,0 @@ - - - - @if (showCancel) { - - } - diff --git a/src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.scss b/src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.scss deleted file mode 100644 index e69de29b..00000000 diff --git a/src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.spec.ts b/src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.spec.ts deleted file mode 100644 index 8d9c1f77..00000000 --- a/src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.spec.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { beforeEach, describe, expect, it } from 'vitest'; - -import { DashboardsBottomSheetComponent } from './dashboards-bottom-sheet.component'; - -describe('DashboardsBottomSheetComponent', () => { - let component: DashboardsBottomSheetComponent; - let fixture: ComponentFixture; - - beforeEach(async () => { - await TestBed.configureTestingModule({ - imports: [DashboardsBottomSheetComponent] - }) - .compileComponents(); - - fixture = TestBed.createComponent(DashboardsBottomSheetComponent); - component = fixture.componentInstance; - fixture.detectChanges(); - }); - - it('should create', () => { - expect(component).toBeTruthy(); - }); -}); diff --git a/src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.ts b/src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.ts deleted file mode 100644 index 7b1c8179..00000000 --- a/src/app/core/components/dashboards-bottom-sheet/dashboards-bottom-sheet.component.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { Component, inject } from '@angular/core'; -import { MatBottomSheetRef, MAT_BOTTOM_SHEET_DATA } from '@angular/material/bottom-sheet'; -import { MatIconModule } from '@angular/material/icon'; -import { MatListModule } from '@angular/material/list'; - -@Component({ - selector: 'dashboards-bottom-sheet', - imports: [ MatListModule, MatIconModule ], - templateUrl: './dashboards-bottom-sheet.component.html', - styleUrl: './dashboards-bottom-sheet.component.scss' -}) -export class DashboardsBottomSheetComponent { - private _bottomSheetRef = - inject>(MatBottomSheetRef); - public data: { showCancel?: boolean } = inject<{ showCancel?: boolean }>(MAT_BOTTOM_SHEET_DATA); - showCancel = !!(this.data && this.data.showCancel); - - clickAction(action: string) { - this._bottomSheetRef.dismiss(action); - } - -} diff --git a/src/app/core/components/dashboards-editor/dashboards-editor.component.html b/src/app/core/components/dashboards-editor/dashboards-editor.component.html index 8ace451f..1898f692 100644 --- a/src/app/core/components/dashboards-editor/dashboards-editor.component.html +++ b/src/app/core/components/dashboards-editor/dashboards-editor.component.html @@ -3,18 +3,18 @@
+ (keydown.enter)="onTileKey(index, $event)" + (keydown.space)="onTileKey(index, $event)">
{{index + 1}}
Add Page

+ + diff --git a/src/app/core/components/dashboards-editor/dashboards-editor.component.spec.ts b/src/app/core/components/dashboards-editor/dashboards-editor.component.spec.ts index 4c1204f2..86b11455 100644 --- a/src/app/core/components/dashboards-editor/dashboards-editor.component.spec.ts +++ b/src/app/core/components/dashboards-editor/dashboards-editor.component.spec.ts @@ -1,23 +1,148 @@ +import { signal } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { of, Subject } from 'rxjs'; import { DashboardsEditorComponent } from './dashboards-editor.component'; +import { ActionMenuComponent } from '../action-menu/action-menu.component'; +import { Dashboard, DashboardService } from '../../services/dashboard.service'; +import { DialogService } from '../../services/dialog.service'; +import { uiEventService } from '../../services/uiEvent.service'; -describe('DashboardsManageComponent', () => { +const pages: Dashboard[] = [ + { id: 'a', name: 'Sailing', icon: 'dashboard-sailing', configuration: [] }, + { id: 'b', name: 'Engine', icon: 'dashboard-engine', configuration: [] } +]; + +describe('DashboardsEditorComponent', () => { let component: DashboardsEditorComponent; let fixture: ComponentFixture; + let dashboard: { + dashboards: ReturnType>; + delete: ReturnType; + duplicate: ReturnType; + update: ReturnType; + add: ReturnType; + }; + let dialog: { + openDashboardPageEditorDialog: ReturnType; + openConfirmationDialog: ReturnType; + }; + let confirm$: Subject; + /** Result the (mocked) page-editor dialog emits on close; null = cancelled. */ + let editorResult: { name: string; icon: string } | null; + let open: ReturnType; beforeEach(async () => { + editorResult = null; + dashboard = { + dashboards: signal([...pages]), + delete: vi.fn(), + duplicate: vi.fn(), + update: vi.fn(), + add: vi.fn() + }; + confirm$ = new Subject(); + dialog = { + openDashboardPageEditorDialog: vi.fn(() => ({ afterClosed: () => of(editorResult) })), + openConfirmationDialog: vi.fn(() => confirm$) + }; + // Isolate the editor's routing logic from the real menu overlay (and stop + // CDK overlays leaking across tests). + open = vi.spyOn(ActionMenuComponent.prototype, 'open').mockImplementation(() => undefined); + await TestBed.configureTestingModule({ - imports: [DashboardsEditorComponent] - }) - .compileComponents(); + imports: [DashboardsEditorComponent], + providers: [ + { provide: DashboardService, useValue: dashboard }, + { provide: DialogService, useValue: dialog }, + { provide: uiEventService, useValue: { isDragging: signal(false) } } + ] + }).compileComponents(); fixture = TestBed.createComponent(DashboardsEditorComponent); component = fixture.componentInstance; fixture.detectChanges(); }); + afterEach(() => fixture.destroy()); + + const call = (name: string, ...args: unknown[]) => + (component as unknown as Record void>)[name](...args); + const tapEvent = (center: { x: number; y: number }) => ({ detail: { center } }); + it('should create', () => { expect(component).toBeTruthy(); }); + + it('offers exactly the edit, duplicate and delete actions', () => { + const ids = (component as unknown as { pageActions: { id: string }[] }).pageActions.map(a => a.id); + expect(ids).toEqual(['edit', 'duplicate', 'delete']); + }); + + it('opens the action menu at the tap point on a single tap', () => { + call('onTileTap', 1, tapEvent({ x: 42, y: 24 })); + expect(open).toHaveBeenCalledWith(42, 24); + }); + + it('opens the action menu at the tile centre on keyboard activation', () => { + const preventDefault = vi.fn(); + call('onTileKey', 0, { + preventDefault, + target: { getBoundingClientRect: () => ({ left: 10, top: 20, width: 100, height: 40 }) } + }); + expect(preventDefault).toHaveBeenCalled(); + expect(open).toHaveBeenCalledWith(60, 40); + }); + + it('routes the edit action to the page editor dialog and applies the result', () => { + editorResult = { name: 'Sailing (renamed)', icon: 'dashboard-map' }; + call('onTileTap', 0, tapEvent({ x: 0, y: 0 })); + call('onPageAction', 'edit'); + expect(dialog.openDashboardPageEditorDialog).toHaveBeenCalledWith( + expect.objectContaining({ title: 'Page Options', name: 'Sailing' }) + ); + expect(dashboard.update).toHaveBeenCalledWith(0, 'Sailing (renamed)', 'dashboard-map'); + }); + + it('routes the duplicate action and applies the result', () => { + editorResult = { name: 'Engine copy', icon: 'dashboard-engine' }; + call('onTileTap', 1, tapEvent({ x: 0, y: 0 })); + call('onPageAction', 'duplicate'); + expect(dialog.openDashboardPageEditorDialog).toHaveBeenCalledWith( + expect.objectContaining({ title: 'Duplicate Page', name: 'Engine copy' }) + ); + expect(dashboard.duplicate).toHaveBeenCalledWith(1, 'Engine copy', 'dashboard-engine'); + }); + + it('does not mutate when the editor dialog is cancelled', () => { + editorResult = null; + call('onTileTap', 0, tapEvent({ x: 0, y: 0 })); + call('onPageAction', 'edit'); + expect(dashboard.update).not.toHaveBeenCalled(); + }); + + it('confirms before deleting and deletes only when confirmed', () => { + call('onTileTap', 1, tapEvent({ x: 0, y: 0 })); + call('onPageAction', 'delete'); + expect(dialog.openConfirmationDialog).toHaveBeenCalledWith( + expect.objectContaining({ title: 'Delete Page', confirmBtnText: 'Delete' }) + ); + expect(dashboard.delete).not.toHaveBeenCalled(); + + confirm$.next(true); + expect(dashboard.delete).toHaveBeenCalledWith(1); + }); + + it('does not delete when the confirmation is cancelled', () => { + call('onTileTap', 0, tapEvent({ x: 0, y: 0 })); + call('onPageAction', 'delete'); + confirm$.next(false); + expect(dashboard.delete).not.toHaveBeenCalled(); + }); + + it('ignores an action when no tile menu is open (stale index)', () => { + call('onPageAction', 'delete'); + expect(dialog.openConfirmationDialog).not.toHaveBeenCalled(); + expect(dashboard.delete).not.toHaveBeenCalled(); + }); }); diff --git a/src/app/core/components/dashboards-editor/dashboards-editor.component.ts b/src/app/core/components/dashboards-editor/dashboards-editor.component.ts index 70423241..964cb427 100644 --- a/src/app/core/components/dashboards-editor/dashboards-editor.component.ts +++ b/src/app/core/components/dashboards-editor/dashboards-editor.component.ts @@ -1,44 +1,37 @@ -import { ChangeDetectionStrategy, Component, inject } from '@angular/core'; +import { ChangeDetectionStrategy, Component, inject, viewChild } from '@angular/core'; import { GestureDirective } from '../../directives/gesture.directive'; import { Dashboard, DashboardService } from '../../services/dashboard.service'; import { MatButtonModule } from '@angular/material/button'; import { MatIconModule } from '@angular/material/icon'; import { DialogService } from '../../services/dialog.service'; -import { CdkDropList, CdkDrag, CdkDragDrop, CdkDragMove, moveItemInArray } from '@angular/cdk/drag-drop'; -import { DashboardsBottomSheetComponent } from '../dashboards-bottom-sheet/dashboards-bottom-sheet.component'; -import { MatBottomSheet, MatBottomSheetModule } from '@angular/material/bottom-sheet'; +import { CdkDropList, CdkDrag, CdkDragDrop, moveItemInArray } from '@angular/cdk/drag-drop'; import { uiEventService } from '../../services/uiEvent.service'; import { MatRippleModule } from '@angular/material/core'; -import { DomSanitizer } from '@angular/platform-browser'; +import { ActionMenuComponent } from '../action-menu/action-menu.component'; +import { ActionMenuItem } from '../action-menu/action-menu-item'; @Component({ selector: 'dashboards-editor', changeDetection: ChangeDetectionStrategy.OnPush, - imports: [MatBottomSheetModule, MatButtonModule, MatIconModule, CdkDropList, CdkDrag, MatRippleModule, GestureDirective], + imports: [MatButtonModule, MatIconModule, CdkDropList, CdkDrag, MatRippleModule, GestureDirective, ActionMenuComponent], templateUrl: './dashboards-editor.component.html', - styleUrl: './dashboards-editor.component.scss', - host: { - '(document:mouseup)': 'onPointerRelease()', - '(document:touchend)': 'onPointerRelease()' - } + styleUrl: './dashboards-editor.component.scss' }) export class DashboardsEditorComponent { - protected readonly pageTitle = 'Dashboards'; - private _bottomSheet = inject(MatBottomSheet); protected _dashboard = inject(DashboardService); private _uiEvent = inject(uiEventService); private _dialog = inject(DialogService); - private _sanitizer = inject(DomSanitizer); - /** True while bottom sheet open */ - protected _sheetOpen = false; - /** Suppress starting a drag after a press consumed the pointer */ - protected suppressDrag = false; - // Drag / press coordination flags - private _dragActive = false; // true between dragStart and dragEnd - private _dragMoved = false; // becomes true once movement surpasses threshold - private readonly _dragSuppressThresholdPx = 4; // movement to treat as a real drag + private readonly _actionMenu = viewChild.required(ActionMenuComponent); + /** The tile whose action menu is currently open. */ + private _menuIndex = -1; + + protected readonly pageActions: ActionMenuItem[] = [ + { id: 'edit', label: 'Edit', icon: 'edit' }, + { id: 'duplicate', label: 'Duplicate', icon: 'content_copy' }, + { id: 'delete', label: 'Delete', icon: 'delete_forever' } + ]; protected addDashboard(): void { this._dialog.openDashboardPageEditorDialog({ @@ -53,34 +46,49 @@ export class DashboardsEditorComponent { }); } - protected openBottomSheet(index: number): void { - if (this._sheetOpen) { - return; // guard against re-entrancy / duplicate opens + /** + * A single tap on a page tile opens its action menu at the tap point. Tap vs. + * drag is arbitrated by the gesture directive's movement threshold — a reorder + * moves past the tap slop and emits no tap, so it never reaches here. (Gating + * on the shared isDragging signal instead would swallow a legitimate tap whose + * minor travel already tripped cdkDrag's lower start threshold.) + */ + protected onTileTap(index: number, e: Event | CustomEvent): void { + const center = (e as CustomEvent).detail?.center as { x: number; y: number } | undefined; + this.openMenu(index, center?.x ?? 0, center?.y ?? 0); + } + + /** Keyboard equivalent: open the action menu centered on the focused tile. */ + protected onTileKey(index: number, e: Event): void { + e.preventDefault(); + const rect = (e.target as HTMLElement).getBoundingClientRect(); + this.openMenu(index, rect.left + rect.width / 2, rect.top + rect.height / 2); + } + + private openMenu(index: number, x: number, y: number): void { + this._menuIndex = index; + this._actionMenu().open(x, y); + } + + protected onPageAction(id: string): void { + const index = this._menuIndex; + if (index < 0 || index >= this._dashboard.dashboards().length) return; + switch (id) { + case 'edit': + this.editDashboard(index); + break; + case 'duplicate': + this.duplicateDashboard(index, this._dashboard.dashboards()[index].name); + break; + case 'delete': + this.confirmDelete(index); + break; + default: + break; } - this._sheetOpen = true; - // Detect Linux Firefox for workaround - const isLinuxFirefox = typeof navigator !== 'undefined' && - /Linux/.test(navigator.platform) && - /Firefox/.test(navigator.userAgent); - const sheetRef = this._bottomSheet.open(DashboardsBottomSheetComponent, isLinuxFirefox ? { disableClose: true, data: { showCancel: true } } : {}); - sheetRef.afterDismissed().subscribe((action) => { - this._sheetOpen = false; - switch (action) { - case 'delete': - this.deleteDashboard(index); - break; - - case 'duplicate': - this.duplicateDashboard(index, `${this._dashboard.dashboards()[index].name}`); - break; - - default: - break; - } - }); } - protected editDashboard(itemIndex: number): void { + private editDashboard(itemIndex: number): void { const dashboard = this._dashboard.dashboards()[itemIndex]; this._dialog.openDashboardPageEditorDialog({ title: 'Page Options', @@ -94,11 +102,7 @@ export class DashboardsEditorComponent { }); } - protected deleteDashboard(index: number): void { - this._dashboard.delete(index); - } - - protected duplicateDashboard(itemIndex: number, currentName: string): void { + private duplicateDashboard(itemIndex: number, currentName: string): void { const originalDashboard = this._dashboard.dashboards()[itemIndex]; this._dialog.openDashboardPageEditorDialog({ title: 'Duplicate Page', @@ -112,6 +116,18 @@ export class DashboardsEditorComponent { }); } + private confirmDelete(itemIndex: number): void { + const name = this._dashboard.dashboards()[itemIndex].name; + this._dialog.openConfirmationDialog({ + title: 'Delete Page', + message: `Delete the "${name}" page and all its widgets?`, + confirmBtnText: 'Delete', + cancelBtnText: 'Cancel' + }).subscribe(confirmed => { + if (confirmed) this._dashboard.delete(itemIndex); + }); + } + protected drop(event: CdkDragDrop): void { this._dashboard.dashboards.update(dashboards => { const updatedDashboards = [...dashboards]; @@ -134,52 +150,10 @@ export class DashboardsEditorComponent { } protected dragStart(): void { - if (this._sheetOpen || this.suppressDrag) return; // block drag while sheet open or suppressed this._uiEvent.isDragging.set(true); - this._dragActive = true; - this._dragMoved = false; // reset for new gesture } protected dragEnd(): void { this._uiEvent.isDragging.set(false); - this._dragActive = false; - // Reset movement flag shortly after drag end so future presses work. - // Timeout lets finish any internal gesture state before we allow a new press. - setTimeout(() => { this._dragMoved = false; }, 60); - } - - protected onDragMoved(ev: CdkDragMove): void { - if (!this._dragActive || this._dragMoved) return; - const dist = Math.hypot(ev.distance.x, ev.distance.y); - if (dist > this._dragSuppressThresholdPx) { - this._dragMoved = true; - } - } - - protected onPress(index: number): void { - // Avoid passive listener warnings and duplicate opens; no need to preventDefault/stopPropagation here - if (this._sheetOpen) return; - // Suppress press if an actual drag movement occurred - if (this._dragMoved) return; - // Cancel pointer sequence so moving while holding does not initiate drag - this.cancelPointerSequence(); - this.openBottomSheet(index); - } - - private cancelPointerSequence(): void { - this.suppressDrag = true; - this._dragActive = false; - this._dragMoved = false; - // Dispatch synthetic pointer end events to end any potential drag tracking - ['pointerup', 'mouseup', 'touchend'].forEach(type => { - document.dispatchEvent(new Event(type, { bubbles: true })); - }); - } - - protected onPointerRelease(): void { - // Allow future drags after actual release, but only if sheet not open - if (!this._sheetOpen) { - this.suppressDrag = false; - } } }