From c9ce28d5e5524cd47055e88f1940154f3ab9250d Mon Sep 17 00:00:00 2001 From: Arlo White Date: Wed, 19 Nov 2025 14:06:39 +1000 Subject: [PATCH 01/25] extract BaseWorkspacePersistenceService --- .../model-workflow.component.ts | 1 - .../services/workspace-persistence.service.ts | 274 ++---------------- .../services/workspace-persistence.types.ts | 28 ++ .../base-workspace-persistence.service.ts | 233 +++++++++++++++ 4 files changed, 292 insertions(+), 244 deletions(-) create mode 100644 packages/app/src/app/projects/services/base-workspace-persistence.service.ts diff --git a/packages/app/src/app/model-workflow/model-workflow.component.ts b/packages/app/src/app/model-workflow/model-workflow.component.ts index dae6f829..d40623d7 100644 --- a/packages/app/src/app/model-workflow/model-workflow.component.ts +++ b/packages/app/src/app/model-workflow/model-workflow.component.ts @@ -1,4 +1,3 @@ -// src/app/model-workflow/model-workflow.component.ts import { Component, computed, diff --git a/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts b/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts index a7b6e53e..6a95c67e 100644 --- a/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts +++ b/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts @@ -1,80 +1,39 @@ -// src/app/model-workflow/services/workspace-persistence.service.ts -import { Injectable, inject } from '@angular/core'; -import { Observable, of, throwError } from 'rxjs'; -import { map, catchError, tap, switchMap } from 'rxjs/operators'; -import { ModelParameters } from '../parameter-config/parameter-config.component'; -import { WebApiService } from '../../../api/web-api.service'; - -export interface PersistedWorkspace { - id: string; - name: string; - parameters: ModelParameters | null; - createdAt: string; // ISO string - lastModified: string; // ISO string -} - +import { Injectable } from '@angular/core'; +import { map, Observable, of, switchMap } from 'rxjs'; +import { BaseWorkspacePersistenceService } from '../../projects/services/base-workspace-persistence.service'; +import { isValidPersistedWorkspace, PersistedWorkspace } from './workspace-persistence.types'; + +/** + * All projects have this base workspaces structure. + * hmmm + * Workspaces correspond to tabs in ADRIA Analysis projects. + * Currently, Site Assessment has a single workspace. + */ export interface WorkspaceState { workspaces: PersistedWorkspace[]; activeWorkspaceId: string | null; workspaceCounter: number; } +/** + * Workspace persistence service for ADRIA Analysis projects. + */ @Injectable({ providedIn: 'root' }) -export class WorkspacePersistenceService { - private readonly api = inject(WebApiService); - - private readonly STORAGE_KEY = 'reef-guide-workspaces'; - private readonly VERSION = '1.0'; - private readonly VERSION_KEY = 'reef-guide-workspaces-version'; - - private projectId: number | null = null; +export class WorkspacePersistenceService extends BaseWorkspacePersistenceService { + protected readonly STORAGE_KEY = 'reef-guide-workspaces'; + protected readonly VERSION = '1.0'; + protected readonly VERSION_KEY = 'reef-guide-workspaces-version'; constructor() { - this.migrateIfNeeded(); - } - - // Set the project ID for persistence operations - setProjectId(projectId: number): void { - this.projectId = projectId; - } - - // Get current project ID - getProjectId(): number | null { - return this.projectId; - } - - // Save complete workspace state - saveWorkspaceState(state: WorkspaceState): Observable { - // If we have a project ID, save to backend; otherwise use localStorage - if (this.projectId) { - return this.saveToBackend(state); - } else { - return this.saveToLocalStorage(state); - } - } - - // Load complete workspace state - loadWorkspaceState(): Observable { - // If we have a project ID, load from backend; otherwise use localStorage - if (this.projectId) { - return this.loadFromBackend(); - } else { - return this.loadFromLocalStorage(); - } - } - - // Clear all workspace state - clearWorkspaceState(): Observable { - if (this.projectId) { - return this.clearFromBackend(); - } else { - return this.clearFromLocalStorage(); - } + super(); } - // Save a single workspace + /** + * Save a single workspace. + * Updates existing entry with same id or pushes new workspace. + */ saveWorkspace(workspace: PersistedWorkspace): Observable { return this.loadWorkspaceState().pipe( map(currentState => { @@ -141,200 +100,29 @@ export class WorkspacePersistenceService { ); } - // Check if storage is available - isStorageAvailable(): boolean { - try { - const testKey = '__storage_test__'; - localStorage.setItem(testKey, 'test'); - localStorage.removeItem(testKey); - return true; - } catch { - return false; - } - } - - // ================== - // BACKEND PERSISTENCE METHODS - // ================== - - private saveToBackend(state: WorkspaceState): Observable { - if (!this.projectId) { - return throwError(() => new Error('Project ID not set')); - } - - const projectState = { - workspaces: state - }; - - return this.api - .updateProject(this.projectId, { - project_state: projectState - }) - .pipe( - map(() => void 0), - catchError(error => { - console.warn('Failed to save workspace state to backend:', error); - // Fallback to localStorage - return this.saveToLocalStorage(state); - }) - ); - } - - private loadFromBackend(): Observable { - if (!this.projectId) { - return throwError(() => new Error('Project ID not set')); - } - - return this.api.getProject(this.projectId).pipe( - map(response => { - const projectState = response.project.project_state as any; - - if (projectState && projectState.workspaces) { - const state = projectState.workspaces as WorkspaceState; - - // Validate the loaded state - if (!this.isValidWorkspaceState(state)) { - console.warn('Invalid workspace state found in project, returning null'); - return null; - } - - return state; - } - - return null; - }), - catchError(error => { - console.warn('Failed to load workspace state from backend:', error); - // Fallback to localStorage - return this.loadFromLocalStorage(); - }) - ); - } - - private clearFromBackend(): Observable { - if (!this.projectId) { - return throwError(() => new Error('Project ID not set')); - } - - return this.api - .updateProject(this.projectId, { - project_state: {} - }) - .pipe( - map(() => void 0), - catchError(error => { - console.warn('Failed to clear workspace state from backend:', error); - // Fallback to localStorage - return this.clearFromLocalStorage(); - }) - ); - } - - // ================== - // LOCAL STORAGE METHODS (FALLBACK) - // ================== - - private saveToLocalStorage(state: WorkspaceState): Observable { - try { - const serializedState = JSON.stringify(state); - localStorage.setItem(this.STORAGE_KEY, serializedState); - localStorage.setItem(this.VERSION_KEY, this.VERSION); - return of(void 0); - } catch (error) { - console.warn('Failed to save workspace state to localStorage:', error); - return throwError(() => error); - } - } - - private loadFromLocalStorage(): Observable { - try { - const serializedState = localStorage.getItem(this.STORAGE_KEY); - if (!serializedState) { - return of(null); - } - - const state: WorkspaceState = JSON.parse(serializedState); - - // Validate the loaded state - if (!this.isValidWorkspaceState(state)) { - console.warn('Invalid workspace state found in localStorage, clearing storage'); - this.clearFromLocalStorageSync(); - return of(null); - } - - return of(state); - } catch (error) { - console.warn('Failed to load workspace state from localStorage:', error); - this.clearFromLocalStorageSync(); - return of(null); - } - } - - private clearFromLocalStorage(): Observable { - try { - this.clearFromLocalStorageSync(); - return of(void 0); - } catch (error) { - console.warn('Failed to clear workspace state from localStorage:', error); - return throwError(() => error); - } - } - - private clearFromLocalStorageSync(): void { - try { - localStorage.removeItem(this.STORAGE_KEY); - localStorage.removeItem(this.VERSION_KEY); - } catch (error) { - console.warn('Failed to clear workspace state from localStorage:', error); - } - } - // ================== // VALIDATION METHODS // ================== - // Validate workspace state structure - private isValidWorkspaceState(state: any): state is WorkspaceState { + public isValidWorkspaceState(state: any): state is WorkspaceState { + // TODO delete invalid workspaces rather than invalidate whole state return ( state && typeof state === 'object' && Array.isArray(state.workspaces) && typeof state.workspaceCounter === 'number' && (state.activeWorkspaceId === null || typeof state.activeWorkspaceId === 'string') && - state.workspaces.every((w: any) => this.isValidPersistedWorkspace(w)) - ); - } - - // Validate individual workspace structure - private isValidPersistedWorkspace(workspace: any): workspace is PersistedWorkspace { - return ( - workspace && - typeof workspace === 'object' && - typeof workspace.id === 'string' && - typeof workspace.name === 'string' && - (workspace.parameters === null || typeof workspace.parameters === 'object') && - typeof workspace.createdAt === 'string' && - typeof workspace.lastModified === 'string' && - (workspace.submittedJobId === undefined || typeof workspace.submittedJobId === 'number') && - (workspace.activeCharts === undefined || Array.isArray(workspace.activeCharts)) + state.workspaces.every((w: any) => isValidPersistedWorkspace(w)) ); } - // Handle version migrations for localStorage - private migrateIfNeeded(): void { - const currentVersion = localStorage.getItem(this.VERSION_KEY); + protected validateAndMigrateWorkspaceState(state: unknown): WorkspaceState | undefined { + // FUTURE check version and migrate - if (!currentVersion) { - // First time or old version without versioning - const existingData = localStorage.getItem(this.STORAGE_KEY); - if (existingData) { - console.log('Migrating workspace data to new version'); - // Could add migration logic here if needed - } - localStorage.setItem(this.VERSION_KEY, this.VERSION); + if (this.isValidWorkspaceState(state)) { + return state; } - // Future migrations can be added here - // if (currentVersion === '1.0' && this.VERSION === '1.1') { ... } + return undefined; } } diff --git a/packages/app/src/app/model-workflow/services/workspace-persistence.types.ts b/packages/app/src/app/model-workflow/services/workspace-persistence.types.ts index d75bf5fa..86ff1865 100644 --- a/packages/app/src/app/model-workflow/services/workspace-persistence.types.ts +++ b/packages/app/src/app/model-workflow/services/workspace-persistence.types.ts @@ -2,6 +2,9 @@ import { JobDetailsResponse } from '@reefguide/types'; import { ModelParameters } from '../parameter-config/parameter-config.component'; +/** + * Object generated by app at runtime corresponding to PersistedWorkspace + */ export interface RuntimeWorkspace { id: string; name: string; @@ -18,6 +21,9 @@ export interface RuntimeWorkspace { mapConfig?: any; } +/** + * Single ADRIA Analysis workspace state (tab in UI) + */ export interface PersistedWorkspace { id: string; name: string; @@ -34,6 +40,28 @@ export interface PersistedWorkspace { mapConfig?: any; } +/** + * Validate individual workspace structure + */ +export function isValidPersistedWorkspace(workspace: any): workspace is PersistedWorkspace { + const isValid = + workspace && + typeof workspace === 'object' && + typeof workspace.id === 'string' && + typeof workspace.name === 'string' && + (workspace.parameters === null || typeof workspace.parameters === 'object') && + typeof workspace.createdAt === 'string' && + typeof workspace.lastModified === 'string' && + (workspace.submittedJobId === undefined || typeof workspace.submittedJobId === 'number') && + (workspace.activeCharts === undefined || Array.isArray(workspace.activeCharts)); + + if (!isValid) { + console.warn(`workspace ${workspace.id} is invalid`, workspace); + } + + return isValid; +} + // Convert runtime workspace to persisted format export function toPersistedWorkspace(workspace: RuntimeWorkspace): PersistedWorkspace { return { diff --git a/packages/app/src/app/projects/services/base-workspace-persistence.service.ts b/packages/app/src/app/projects/services/base-workspace-persistence.service.ts new file mode 100644 index 00000000..bf1a294b --- /dev/null +++ b/packages/app/src/app/projects/services/base-workspace-persistence.service.ts @@ -0,0 +1,233 @@ +import { inject, Injectable } from '@angular/core'; +import { Observable, of, throwError } from 'rxjs'; +import { catchError, map } from 'rxjs/operators'; +import { WebApiService } from '../../../api/web-api.service'; + +/** + * Base service that manages a project's persisted state. + */ +@Injectable({ + providedIn: 'root' +}) +export abstract class BaseWorkspacePersistenceService { + private readonly api = inject(WebApiService); + + /** + * local storage key for workspace state. + */ + protected abstract readonly STORAGE_KEY: string; + /** + * @deprecated version should be in workspace state objects + */ + protected abstract readonly VERSION: string; + /** + * @deprecated version should be in workspace state objects + */ + protected abstract readonly VERSION_KEY: string; + + private projectId: number | null = null; + + /** + * Set the project ID for persistence operations + * if projectId set, workspace state will be saved to API instead of local storage. + */ + setProjectId(projectId: number): void { + this.projectId = projectId; + } + + // Get current project ID + getProjectId(): number | null { + return this.projectId; + } + + // Save complete workspace state + saveWorkspaceState(state: T): Observable { + if (!this.isValidWorkspaceState(state)) { + console.error('invalid state', state); + throw new Error('App generated invalid workspace state'); + } + + // If we have a project ID, save to backend; otherwise use localStorage + if (this.projectId) { + return this.saveToBackend(state); + } else { + return this.saveToLocalStorage(state); + } + } + + // Load complete workspace state + loadWorkspaceState(): Observable { + // If we have a project ID, load from backend; otherwise use localStorage + if (this.projectId) { + return this.loadFromBackend(); + } else { + return this.loadFromLocalStorage(); + } + } + + // Clear all workspace state + clearWorkspaceState(): Observable { + if (this.projectId) { + return this.clearFromBackend(); + } else { + return this.clearFromLocalStorage(); + } + } + + // Check if storage is available + isStorageAvailable(): boolean { + try { + const testKey = '__storage_test__'; + localStorage.setItem(testKey, 'test'); + localStorage.removeItem(testKey); + return true; + } catch { + return false; + } + } + + // ================== + // BACKEND PERSISTENCE METHODS + // ================== + + private saveToBackend(state: T): Observable { + if (!this.projectId) { + return throwError(() => new Error('Project ID not set')); + } + + return this.api + .updateProject(this.projectId, { + project_state: state + }) + .pipe( + map(() => void 0), + catchError(error => { + console.warn('Failed to save workspace state to backend:', error); + // Fallback to localStorage + return this.saveToLocalStorage(state); + }) + ); + } + + private loadFromBackend(): Observable { + if (!this.projectId) { + return throwError(() => new Error('Project ID not set')); + } + + return this.api.getProject(this.projectId).pipe( + map(response => { + const projectState = response.project.project_state as unknown; + const validMigrated = this.validateAndMigrateWorkspaceState(projectState); + if (validMigrated) { + return validMigrated; + } else { + console.warn('Invalid workspace state found in project, returning null'); + return null; + } + }), + catchError(error => { + console.warn('Failed to load workspace state from backend:', error); + // Fallback to localStorage + return this.loadFromLocalStorage(); + }) + ); + } + + private clearFromBackend(): Observable { + if (!this.projectId) { + return throwError(() => new Error('Project ID not set')); + } + + return this.api + .updateProject(this.projectId, { + project_state: {} + }) + .pipe( + map(() => void 0), + catchError(error => { + console.warn('Failed to clear workspace state from backend:', error); + // Fallback to localStorage + return this.clearFromLocalStorage(); + }) + ); + } + + // ================== + // LOCAL STORAGE METHODS (FALLBACK) + // ================== + + private saveToLocalStorage(state: T): Observable { + try { + const serializedState = JSON.stringify(state); + localStorage.setItem(this.STORAGE_KEY, serializedState); + localStorage.setItem(this.VERSION_KEY, this.VERSION); + return of(void 0); + } catch (error) { + console.warn('Failed to save workspace state to localStorage:', error); + return throwError(() => error); + } + } + + private loadFromLocalStorage(): Observable { + try { + const serializedState = localStorage.getItem(this.STORAGE_KEY); + + if (!serializedState) { + return of(null); + } + + const state = this.validateAndMigrateWorkspaceState(JSON.parse(serializedState)); + if (!state) { + return of(null); + } + + // Validate the loaded state + if (!this.isValidWorkspaceState(state)) { + console.warn('Invalid workspace state found in localStorage, clearing storage'); + this.clearFromLocalStorageSync(); + return of(null); + } + + return of(state); + } catch (error) { + console.warn('Failed to load workspace state from localStorage:', error); + this.clearFromLocalStorageSync(); + return of(null); + } + } + + private clearFromLocalStorage(): Observable { + try { + this.clearFromLocalStorageSync(); + return of(void 0); + } catch (error) { + console.warn('Failed to clear workspace state from localStorage:', error); + return throwError(() => error); + } + } + + private clearFromLocalStorageSync(): void { + try { + localStorage.removeItem(this.STORAGE_KEY); + localStorage.removeItem(this.VERSION_KEY); + } catch (error) { + console.warn('Failed to clear workspace state from localStorage:', error); + } + } + + // ================== + // VALIDATION METHODS + // ================== + + /** + * Migrate the workspace state if needed and ensure the state is valid. + * @param state workspace state, which may be an old version + * @returns T if migrated and valid, undefined if not. + */ + protected abstract validateAndMigrateWorkspaceState(state: unknown): T | undefined; + + /** + * Validate workspace state structure is valid and the latest version. + */ + protected abstract isValidWorkspaceState(state: any): state is T; +} From 2bb62dcc08f946f1ba0d3a289dd861cea4aa31e2 Mon Sep 17 00:00:00 2001 From: Arlo White Date: Wed, 19 Nov 2025 14:25:09 +1000 Subject: [PATCH 02/25] display errors, minor repairs ok (consider individual tabs) --- .../services/workspace-persistence.service.ts | 27 ++++++++++++++----- .../base-workspace-persistence.service.ts | 21 +++++++++++---- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts b/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts index 6a95c67e..65d927f1 100644 --- a/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts +++ b/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts @@ -104,22 +104,35 @@ export class WorkspacePersistenceService extends BaseWorkspacePersistenceService // VALIDATION METHODS // ================== - public isValidWorkspaceState(state: any): state is WorkspaceState { - // TODO delete invalid workspaces rather than invalidate whole state - return ( + public isValidWorkspaceState(state: any, repair: boolean): state is WorkspaceState { + const isRootValid = state && typeof state === 'object' && Array.isArray(state.workspaces) && typeof state.workspaceCounter === 'number' && - (state.activeWorkspaceId === null || typeof state.activeWorkspaceId === 'string') && - state.workspaces.every((w: any) => isValidPersistedWorkspace(w)) - ); + (state.activeWorkspaceId === null || typeof state.activeWorkspaceId === 'string'); + + if (!isRootValid) { + return false; + } + + if (repair) { + state.workspaces = state.workspaces.filter(isValidPersistedWorkspace); + } else { + if (!state.workspaces.every(isValidPersistedWorkspace)) { + console.warn('invalid workspace invalidated entire workspace state'); + this.showUserErrorMessage('Invalid workspaces were discarded'); + return false; + } + } + + return true; } protected validateAndMigrateWorkspaceState(state: unknown): WorkspaceState | undefined { // FUTURE check version and migrate - if (this.isValidWorkspaceState(state)) { + if (this.isValidWorkspaceState(state, true)) { return state; } diff --git a/packages/app/src/app/projects/services/base-workspace-persistence.service.ts b/packages/app/src/app/projects/services/base-workspace-persistence.service.ts index bf1a294b..b4a47177 100644 --- a/packages/app/src/app/projects/services/base-workspace-persistence.service.ts +++ b/packages/app/src/app/projects/services/base-workspace-persistence.service.ts @@ -2,6 +2,7 @@ import { inject, Injectable } from '@angular/core'; import { Observable, of, throwError } from 'rxjs'; import { catchError, map } from 'rxjs/operators'; import { WebApiService } from '../../../api/web-api.service'; +import { MatSnackBar } from '@angular/material/snack-bar'; /** * Base service that manages a project's persisted state. @@ -10,7 +11,8 @@ import { WebApiService } from '../../../api/web-api.service'; providedIn: 'root' }) export abstract class BaseWorkspacePersistenceService { - private readonly api = inject(WebApiService); + protected readonly api = inject(WebApiService); + protected readonly snackbar = inject(MatSnackBar); /** * local storage key for workspace state. @@ -42,8 +44,9 @@ export abstract class BaseWorkspacePersistenceService { // Save complete workspace state saveWorkspaceState(state: T): Observable { - if (!this.isValidWorkspaceState(state)) { + if (!this.isValidWorkspaceState(state, false)) { console.error('invalid state', state); + this.showUserErrorMessage('Failed to save project state'); throw new Error('App generated invalid workspace state'); } @@ -86,6 +89,11 @@ export abstract class BaseWorkspacePersistenceService { } } + // TODO standardize app user messaging system + protected showUserErrorMessage(message: string): void { + this.snackbar.open(`ERROR: ${message}`); + } + // ================== // BACKEND PERSISTENCE METHODS // ================== @@ -122,6 +130,7 @@ export abstract class BaseWorkspacePersistenceService { return validMigrated; } else { console.warn('Invalid workspace state found in project, returning null'); + this.showUserErrorMessage('Workspace state invalid and reset'); return null; } }), @@ -181,8 +190,8 @@ export abstract class BaseWorkspacePersistenceService { return of(null); } - // Validate the loaded state - if (!this.isValidWorkspaceState(state)) { + // Validate the loaded state, allow repairs + if (!this.isValidWorkspaceState(state, true)) { console.warn('Invalid workspace state found in localStorage, clearing storage'); this.clearFromLocalStorageSync(); return of(null); @@ -228,6 +237,8 @@ export abstract class BaseWorkspacePersistenceService { /** * Validate workspace state structure is valid and the latest version. + * @param state workspace state object + * @param repair make minor repairs (mutations) to make state valid */ - protected abstract isValidWorkspaceState(state: any): state is T; + protected abstract isValidWorkspaceState(state: any, repair: boolean): state is T; } From 55f48df176ec0275eba2f67ddafcf22a4d52802b Mon Sep 17 00:00:00 2001 From: Arlo White Date: Wed, 19 Nov 2025 14:35:24 +1000 Subject: [PATCH 03/25] move version to property --- .../model-workflow.component.ts | 1 + .../services/workspace-persistence.service.ts | 19 +++++++++---------- .../base-workspace-persistence.service.ts | 10 ---------- 3 files changed, 10 insertions(+), 20 deletions(-) diff --git a/packages/app/src/app/model-workflow/model-workflow.component.ts b/packages/app/src/app/model-workflow/model-workflow.component.ts index d40623d7..f45ee4a7 100644 --- a/packages/app/src/app/model-workflow/model-workflow.component.ts +++ b/packages/app/src/app/model-workflow/model-workflow.component.ts @@ -397,6 +397,7 @@ export class ModelWorkflowComponent implements OnInit, OnDestroy { const persistedWorkspaces = workspaces.map(w => toPersistedWorkspace(w)); const state: WorkspaceState = { + version: '1.0', workspaces: persistedWorkspaces, activeWorkspaceId: this.activeWorkspaceId(), workspaceCounter: this.workspaceCounter() diff --git a/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts b/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts index 65d927f1..68e0e367 100644 --- a/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts +++ b/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts @@ -4,12 +4,11 @@ import { BaseWorkspacePersistenceService } from '../../projects/services/base-wo import { isValidPersistedWorkspace, PersistedWorkspace } from './workspace-persistence.types'; /** - * All projects have this base workspaces structure. - * hmmm - * Workspaces correspond to tabs in ADRIA Analysis projects. - * Currently, Site Assessment has a single workspace. + * Workspace state for ADRIA Analysis projects. + * workspaces property correspond to tabs in the UI. */ export interface WorkspaceState { + version: '1.0'; workspaces: PersistedWorkspace[]; activeWorkspaceId: string | null; workspaceCounter: number; @@ -23,12 +22,7 @@ export interface WorkspaceState { }) export class WorkspacePersistenceService extends BaseWorkspacePersistenceService { protected readonly STORAGE_KEY = 'reef-guide-workspaces'; - protected readonly VERSION = '1.0'; - protected readonly VERSION_KEY = 'reef-guide-workspaces-version'; - - constructor() { - super(); - } + private readonly LATEST_VERSION: WorkspaceState['version'] = '1.0'; /** * Save a single workspace. @@ -118,6 +112,11 @@ export class WorkspacePersistenceService extends BaseWorkspacePersistenceService if (repair) { state.workspaces = state.workspaces.filter(isValidPersistedWorkspace); + + if (state.version === undefined) { + // original state did not have version property within object + state.version = '1.0'; + } } else { if (!state.workspaces.every(isValidPersistedWorkspace)) { console.warn('invalid workspace invalidated entire workspace state'); diff --git a/packages/app/src/app/projects/services/base-workspace-persistence.service.ts b/packages/app/src/app/projects/services/base-workspace-persistence.service.ts index b4a47177..b9391d4d 100644 --- a/packages/app/src/app/projects/services/base-workspace-persistence.service.ts +++ b/packages/app/src/app/projects/services/base-workspace-persistence.service.ts @@ -18,14 +18,6 @@ export abstract class BaseWorkspacePersistenceService { * local storage key for workspace state. */ protected abstract readonly STORAGE_KEY: string; - /** - * @deprecated version should be in workspace state objects - */ - protected abstract readonly VERSION: string; - /** - * @deprecated version should be in workspace state objects - */ - protected abstract readonly VERSION_KEY: string; private projectId: number | null = null; @@ -169,7 +161,6 @@ export abstract class BaseWorkspacePersistenceService { try { const serializedState = JSON.stringify(state); localStorage.setItem(this.STORAGE_KEY, serializedState); - localStorage.setItem(this.VERSION_KEY, this.VERSION); return of(void 0); } catch (error) { console.warn('Failed to save workspace state to localStorage:', error); @@ -218,7 +209,6 @@ export abstract class BaseWorkspacePersistenceService { private clearFromLocalStorageSync(): void { try { localStorage.removeItem(this.STORAGE_KEY); - localStorage.removeItem(this.VERSION_KEY); } catch (error) { console.warn('Failed to clear workspace state from localStorage:', error); } From f59533afc1925b6d08bc93d8f6c30226cf115d18 Mon Sep 17 00:00:00 2001 From: Arlo White Date: Wed, 19 Nov 2025 16:17:13 +1000 Subject: [PATCH 04/25] fix error on new project, improve design --- .../services/workspace-persistence.service.ts | 32 +++++++------- .../base-workspace-persistence.service.ts | 42 +++++++++++++++---- 2 files changed, 49 insertions(+), 25 deletions(-) diff --git a/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts b/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts index 68e0e367..e642e7c5 100644 --- a/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts +++ b/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts @@ -94,14 +94,25 @@ export class WorkspacePersistenceService extends BaseWorkspacePersistenceService ); } - // ================== - // VALIDATION METHODS - // ================== + protected generateDefaultWorkspaceState(): WorkspaceState { + return { + version: '1.0', + activeWorkspaceId: null, + workspaces: [], + workspaceCounter: 0 + }; + } + + protected migrateWorkspaceState(state: unknown): WorkspaceState | undefined { + console.warn('Migration not implemented'); + return undefined; + } public isValidWorkspaceState(state: any, repair: boolean): state is WorkspaceState { const isRootValid = state && typeof state === 'object' && + typeof state.version === 'string' && Array.isArray(state.workspaces) && typeof state.workspaceCounter === 'number' && (state.activeWorkspaceId === null || typeof state.activeWorkspaceId === 'string'); @@ -112,11 +123,6 @@ export class WorkspacePersistenceService extends BaseWorkspacePersistenceService if (repair) { state.workspaces = state.workspaces.filter(isValidPersistedWorkspace); - - if (state.version === undefined) { - // original state did not have version property within object - state.version = '1.0'; - } } else { if (!state.workspaces.every(isValidPersistedWorkspace)) { console.warn('invalid workspace invalidated entire workspace state'); @@ -127,14 +133,4 @@ export class WorkspacePersistenceService extends BaseWorkspacePersistenceService return true; } - - protected validateAndMigrateWorkspaceState(state: unknown): WorkspaceState | undefined { - // FUTURE check version and migrate - - if (this.isValidWorkspaceState(state, true)) { - return state; - } - - return undefined; - } } diff --git a/packages/app/src/app/projects/services/base-workspace-persistence.service.ts b/packages/app/src/app/projects/services/base-workspace-persistence.service.ts index b9391d4d..4428b65c 100644 --- a/packages/app/src/app/projects/services/base-workspace-persistence.service.ts +++ b/packages/app/src/app/projects/services/base-workspace-persistence.service.ts @@ -214,16 +214,44 @@ export abstract class BaseWorkspacePersistenceService { } } - // ================== - // VALIDATION METHODS - // ================== - /** - * Migrate the workspace state if needed and ensure the state is valid. + * Procedure: + * 1. If empty, generate default state + * 2. If valid, return current state (with repair) + * 3. Attempt to migrate and return the migrated state + * 4. return undefined + * @param state workspace state, which may be an old version - * @returns T if migrated and valid, undefined if not. */ - protected abstract validateAndMigrateWorkspaceState(state: unknown): T | undefined; + protected validateAndMigrateWorkspaceState(state: unknown): T | undefined { + if (this.isEmptyWorkspaceState(state)) { + return this.generateDefaultWorkspaceState(); + } else if (this.isValidWorkspaceState(state, true)) { + return state; + } else { + return this.migrateWorkspaceState(state); + } + } + + /** + * Check if the workspace state is undefined or empty. + * @param state + */ + protected isEmptyWorkspaceState(state: unknown): boolean { + return state == null || Object.keys(state).length === 0; + } + + /** + * Generate default workspace state to use. + */ + protected abstract generateDefaultWorkspaceState(): T; + + /** + * Attempt to migrate old/invalid workspace state. + * @param state old state + * @returns T migrated state otherwise undefined. + */ + protected abstract migrateWorkspaceState(state: unknown): T | undefined; /** * Validate workspace state structure is valid and the latest version. From 571d6f0c26eaa921d716707e21729fea5987f495 Mon Sep 17 00:00:00 2001 From: Arlo White Date: Wed, 19 Nov 2025 16:38:21 +1000 Subject: [PATCH 05/25] fix save project on delete/close tab --- packages/app/src/app/model-workflow/model-workflow.component.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/app/src/app/model-workflow/model-workflow.component.ts b/packages/app/src/app/model-workflow/model-workflow.component.ts index f45ee4a7..95941954 100644 --- a/packages/app/src/app/model-workflow/model-workflow.component.ts +++ b/packages/app/src/app/model-workflow/model-workflow.component.ts @@ -590,6 +590,8 @@ export class ModelWorkflowComponent implements OnInit, OnDestroy { this.activeWorkspaceId.set(updatedWorkspaces[newIndex].id); } } + + this.triggerSave(); } // Select a workspace (when tab is clicked) From 1911c67089e00942dacb993e4c2e06a75a72b36c Mon Sep 17 00:00:00 2001 From: Arlo White Date: Fri, 21 Nov 2025 10:24:09 +1000 Subject: [PATCH 06/25] improve projectId management and service providing --- packages/app/src/app/app.config.ts | 5 ++- .../location-selection.component.ts | 34 +++++++++++----- .../workspace-persistence.service.ts | 26 ++++++++++++ .../model-workflow.component.ts | 14 +++++-- .../services/workspace-persistence.service.ts | 6 +-- .../base-workspace-persistence.service.ts | 40 ++++++++++++------- 6 files changed, 93 insertions(+), 32 deletions(-) create mode 100644 packages/app/src/app/location-selection/persistence/workspace-persistence.service.ts diff --git a/packages/app/src/app/app.config.ts b/packages/app/src/app/app.config.ts index 6943585b..99aa0731 100644 --- a/packages/app/src/app/app.config.ts +++ b/packages/app/src/app/app.config.ts @@ -1,7 +1,7 @@ import { provideHttpClient, withFetch, withInterceptors } from '@angular/common/http'; import { ApplicationConfig, ErrorHandler, provideZonelessChangeDetection } from '@angular/core'; import { provideAnimationsAsync } from '@angular/platform-browser/animations/async'; -import { provideRouter, withComponentInputBinding } from '@angular/router'; +import { provideRouter, withComponentInputBinding, withViewTransitions } from '@angular/router'; import { routes } from './app.routes'; import { authInterceptor } from './auth/auth-http-interceptor'; import { MAT_FORM_FIELD_DEFAULT_OPTIONS } from '@angular/material/form-field'; @@ -26,7 +26,8 @@ export const appConfig: ApplicationConfig = { provideZonelessChangeDetection(), // For debugging change detection, exhaustive false by default // provideCheckNoChangesConfig({ exhaustive: true, interval: 1_000}), - provideRouter(routes, withComponentInputBinding()), + provideRouter(routes, withComponentInputBinding(), withViewTransitions()), + // TODO remove uses of old animations system provideAnimationsAsync(), provideHttpClient(withFetch(), withInterceptors([authInterceptor])), diff --git a/packages/app/src/app/location-selection/location-selection.component.ts b/packages/app/src/app/location-selection/location-selection.component.ts index 7d5cf199..da13e2d6 100644 --- a/packages/app/src/app/location-selection/location-selection.component.ts +++ b/packages/app/src/app/location-selection/location-selection.component.ts @@ -1,6 +1,15 @@ import { AsyncPipe, CommonModule } from '@angular/common'; -import { Component, effect, inject, signal, viewChild, ViewChild } from '@angular/core'; -import { toObservable, toSignal } from '@angular/core/rxjs-interop'; +import { + Component, + effect, + inject, + input, + numberAttribute, + signal, + viewChild, + ViewChild +} from '@angular/core'; +import { toObservable } from '@angular/core/rxjs-interop'; import { MatButtonModule } from '@angular/material/button'; import { MatExpansionModule } from '@angular/material/expansion'; import { MatIconModule } from '@angular/material/icon'; @@ -23,8 +32,8 @@ import { MAP_UI, MapUI, ReefGuideMapService } from './reef-guide-map.service'; import { SelectionCriteriaComponent } from './selection-criteria/selection-criteria.component'; import { MapToolbarComponent } from './map-toolbar/map-toolbar.component'; import { PolygonMapService } from './polygon-map.service'; -import { ActivatedRoute } from '@angular/router'; import BaseLayer from 'ol/layer/Base'; +import { WorkspacePersistenceService } from './persistence/workspace-persistence.service'; type DrawerModes = 'criteria' | 'style'; @@ -56,7 +65,8 @@ type DrawerModes = 'criteria' | 'style'; providers: [ ReefGuideMapService, PolygonMapService, - { provide: MAP_UI, useExisting: LocationSelectionComponent } + { provide: MAP_UI, useExisting: LocationSelectionComponent }, + WorkspacePersistenceService ], templateUrl: './location-selection.component.html', styleUrl: './location-selection.component.scss' @@ -66,9 +76,14 @@ export class LocationSelectionComponent implements MapUI { readonly authService = inject(AuthService); readonly api = inject(WebApiService); readonly mapService = inject(ReefGuideMapService); + readonly persistenceService = inject(WorkspacePersistenceService); private readonly snackbar = inject(MatSnackBar); - private activatedRoute = inject(ActivatedRoute); - private projectId = toSignal(this.activatedRoute.params.pipe(map(p => p['projectId']))); + + /** + * Current project ID + * via route param + */ + public readonly projectId = input(undefined, { transform: numberAttribute }); map = viewChild.required(ReefMapComponent); @@ -176,10 +191,11 @@ export class LocationSelectionComponent implements MapUI { */ onPolygonDrawn(geojson: string): void { try { + const projectId = this.projectId(); const polygon = JSON.parse(geojson); // Create the polygon via API - if (!this.projectId()) { + if (projectId == null) { // Routing stuffed up here! console.error('There is no project ID, this route should not have loaded!'); this.snackbar.open( @@ -190,14 +206,14 @@ export class LocationSelectionComponent implements MapUI { } ); } else { - this.api.createPolygon({ polygon, projectId: parseInt(this.projectId()!) }).subscribe({ + this.api.createPolygon({ polygon, projectId }).subscribe({ next: () => { this.snackbar.open('Polygon saved successfully', 'OK', { duration: 3000 }); // Refresh the polygon layer on the map - this.mapService.polygonMapService.refresh(parseInt(this.projectId()!)); + this.mapService.polygonMapService.refresh(projectId); }, error: error => { console.error('Error creating polygon:', error); diff --git a/packages/app/src/app/location-selection/persistence/workspace-persistence.service.ts b/packages/app/src/app/location-selection/persistence/workspace-persistence.service.ts new file mode 100644 index 00000000..353a1211 --- /dev/null +++ b/packages/app/src/app/location-selection/persistence/workspace-persistence.service.ts @@ -0,0 +1,26 @@ +import { BaseWorkspacePersistenceService } from '../../projects/services/base-workspace-persistence.service'; +import { Injectable } from '@angular/core'; + +export interface WorkspaceState { + version: '1.0'; +} + +/** + * Site Assessment project workspace persistence. + * + * Provided by the project component. + */ +@Injectable() +export class WorkspacePersistenceService extends BaseWorkspacePersistenceService { + protected override STORAGE_KEY = 'site-assessment-workspace'; + + protected override generateDefaultWorkspaceState(): WorkspaceState { + throw new Error('Method not implemented.'); + } + protected override migrateWorkspaceState(state: unknown): WorkspaceState | undefined { + throw new Error('Method not implemented.'); + } + protected override isValidWorkspaceState(state: any, repair: boolean): state is WorkspaceState { + throw new Error('Method not implemented.'); + } +} diff --git a/packages/app/src/app/model-workflow/model-workflow.component.ts b/packages/app/src/app/model-workflow/model-workflow.component.ts index 95941954..8f1fa838 100644 --- a/packages/app/src/app/model-workflow/model-workflow.component.ts +++ b/packages/app/src/app/model-workflow/model-workflow.component.ts @@ -7,7 +7,9 @@ import { HostListener, ElementRef, OnInit, - OnDestroy + OnDestroy, + input, + numberAttribute } from '@angular/core'; import { CommonModule } from '@angular/common'; import { Router, ActivatedRoute } from '@angular/router'; @@ -208,7 +210,8 @@ class WorkspaceService { MapResultsViewComponent ], templateUrl: './model-workflow.component.html', - styleUrl: './model-workflow.component.scss' + styleUrl: './model-workflow.component.scss', + providers: [WorkspacePersistenceService] }) export class ModelWorkflowComponent implements OnInit, OnDestroy { private readonly api = inject(WebApiService); @@ -220,7 +223,12 @@ export class ModelWorkflowComponent implements OnInit, OnDestroy { private readonly destroy$ = new Subject(); // Project management - private projectId = signal(null); + + /** + * Current project ID + * via route param + */ + public readonly projectId = input(undefined, { transform: numberAttribute }); private isLoading = signal(true); // Workspace management diff --git a/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts b/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts index e642e7c5..54586c6c 100644 --- a/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts +++ b/packages/app/src/app/model-workflow/services/workspace-persistence.service.ts @@ -16,10 +16,10 @@ export interface WorkspaceState { /** * Workspace persistence service for ADRIA Analysis projects. + * + * Provided by project components. */ -@Injectable({ - providedIn: 'root' -}) +@Injectable() export class WorkspacePersistenceService extends BaseWorkspacePersistenceService { protected readonly STORAGE_KEY = 'reef-guide-workspaces'; private readonly LATEST_VERSION: WorkspaceState['version'] = '1.0'; diff --git a/packages/app/src/app/projects/services/base-workspace-persistence.service.ts b/packages/app/src/app/projects/services/base-workspace-persistence.service.ts index 4428b65c..3030cd9c 100644 --- a/packages/app/src/app/projects/services/base-workspace-persistence.service.ts +++ b/packages/app/src/app/projects/services/base-workspace-persistence.service.ts @@ -1,37 +1,47 @@ -import { inject, Injectable } from '@angular/core'; +import { inject, numberAttribute } from '@angular/core'; import { Observable, of, throwError } from 'rxjs'; import { catchError, map } from 'rxjs/operators'; import { WebApiService } from '../../../api/web-api.service'; import { MatSnackBar } from '@angular/material/snack-bar'; +import { ActivatedRoute } from '@angular/router'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; /** * Base service that manages a project's persisted state. */ -@Injectable({ - providedIn: 'root' -}) export abstract class BaseWorkspacePersistenceService { protected readonly api = inject(WebApiService); protected readonly snackbar = inject(MatSnackBar); + private readonly route = inject(ActivatedRoute); /** * local storage key for workspace state. */ protected abstract readonly STORAGE_KEY: string; - private projectId: number | null = null; + public readonly projectId: number | null; - /** - * Set the project ID for persistence operations - * if projectId set, workspace state will be saved to API instead of local storage. - */ - setProjectId(projectId: number): void { - this.projectId = projectId; - } + constructor() { + // route param available now since the routed project components provide this service. + const initialProjectId = numberAttribute(this.route.snapshot.params['projectId']); + + if (!isNaN(initialProjectId)) { + this.projectId = initialProjectId; - // Get current project ID - getProjectId(): number | null { - return this.projectId; + // important sanity check to verify service is not being reused with different projects. + // if route directly between projects, Angular will reuse the component, which may trigger this. + // configure Angular to create always create new components with projectId changes in route + // https://angular.dev/guide/routing/customizing-route-behavior#route-reuse-strategy + this.route.params.pipe(takeUntilDestroyed()).subscribe(params => { + const projectId = numberAttribute(params['projectId']); + if (projectId !== this.projectId) { + throw new Error(`projectId changed from ${this.projectId} to ${projectId}`); + } + }); + } else { + // no project id + this.projectId = null; + } } // Save complete workspace state From 985cd32fef7dbd51a286d14e6d94b65885c9ba76 Mon Sep 17 00:00:00 2001 From: Arlo White Date: Tue, 25 Nov 2025 17:02:54 +1000 Subject: [PATCH 07/25] Show dialog when project fails to load, no longer fallback to local storage. document the local storage issues. fix observable errors. dev_tips doc update --- packages/app/docs/dev_tips.md | 19 +++ .../model-workflow.component.ts | 160 +++++++++--------- .../base-workspace-persistence.service.ts | 42 +++-- .../failed-project-load-dialog.component.html | 11 ++ .../failed-project-load-dialog.component.scss | 5 + ...iled-project-load-dialog.component.spec.ts | 22 +++ .../failed-project-load-dialog.component.ts | 19 +++ .../user-message.service.spec.ts | 16 ++ .../app/user-messages/user-message.service.ts | 34 ++++ 9 files changed, 230 insertions(+), 98 deletions(-) create mode 100644 packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.html create mode 100644 packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.scss create mode 100644 packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.spec.ts create mode 100644 packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.ts create mode 100644 packages/app/src/app/user-messages/user-message.service.spec.ts create mode 100644 packages/app/src/app/user-messages/user-message.service.ts diff --git a/packages/app/docs/dev_tips.md b/packages/app/docs/dev_tips.md index 6e13a9b6..6bf87334 100644 --- a/packages/app/docs/dev_tips.md +++ b/packages/app/docs/dev_tips.md @@ -15,5 +15,24 @@ which is a good guideline for modern Angular. ### Debugging +[Angular DevTools Extenion](https://angular.dev/tools/devtools) for Chrome and Firefox. + Chrome Dev Tools now has support for [Angular performance profiling](https://angular.dev/best-practices/profiling-with-chrome-devtools). + +## RxJS + +### Debugging + +The `rxjs-util.ts` file has a `tapDebug` function that can be helpful. + +### Error Management +A function that returns an `Observable` should throw errors via an `Observable`; otherwise there are two error paths: synchronous, and via the observable, which is confusing. + +```javascript +return throwError(() => new Error('my error')); +``` + +### Subscription Cleanup + +Cleanup un-tracked subscriptions tied to the life of a component using [takeUntilDestroyed](https://angular.dev/ecosystem/rxjs-interop/take-until-destroyed) diff --git a/packages/app/src/app/model-workflow/model-workflow.component.ts b/packages/app/src/app/model-workflow/model-workflow.component.ts index 8f1fa838..6c2aaf4c 100644 --- a/packages/app/src/app/model-workflow/model-workflow.component.ts +++ b/packages/app/src/app/model-workflow/model-workflow.component.ts @@ -1,18 +1,19 @@ import { Component, computed, - inject, - signal, + DestroyRef, effect, - HostListener, ElementRef, - OnInit, - OnDestroy, + HostListener, + inject, input, - numberAttribute + numberAttribute, + OnDestroy, + OnInit, + signal } from '@angular/core'; import { CommonModule } from '@angular/common'; -import { Router, ActivatedRoute } from '@angular/router'; +import { Router } from '@angular/router'; import { MatToolbarModule } from '@angular/material/toolbar'; import { MatButtonModule } from '@angular/material/button'; import { MatIconModule } from '@angular/material/icon'; @@ -21,14 +22,14 @@ import { MatTabsModule } from '@angular/material/tabs'; import { MatTooltipModule } from '@angular/material/tooltip'; import { MatMenuModule } from '@angular/material/menu'; import { MatDialog, MatDialogModule } from '@angular/material/dialog'; -import { Subject, takeUntil, switchMap, tap, of } from 'rxjs'; +import { tap } from 'rxjs'; import { JobDetailsResponse } from '@reefguide/types'; import { WebApiService } from '../../api/web-api.service'; import { JobStatusComponent } from '../jobs/job-status/job-status.component'; import { JobStatusConfig, mergeJobConfig } from '../jobs/job-status/job-status.types'; import { - ParameterConfigComponent, - ModelParameters + ModelParameters, + ParameterConfigComponent } from './parameter-config/parameter-config.component'; import { ResultsViewComponent } from './results-view/results-view.component'; import { MapResultsViewComponent } from './map-results-view/map-results-view.component'; @@ -38,6 +39,8 @@ import { WorkspaceState } from './services/workspace-persistence.service'; import { toPersistedWorkspace, toRuntimeWorkspace } from './services/workspace-persistence.types'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; +import { UserMessageService } from '../user-messages/user-message.service'; type WorkflowState = 'configuring' | 'submitting' | 'monitoring' | 'viewing'; @@ -216,11 +219,11 @@ class WorkspaceService { export class ModelWorkflowComponent implements OnInit, OnDestroy { private readonly api = inject(WebApiService); private readonly router = inject(Router); - private readonly route = inject(ActivatedRoute); private readonly dialog = inject(MatDialog); private readonly persistenceService = inject(WorkspacePersistenceService); + private readonly userMessageService = inject(UserMessageService); private readonly elementRef = inject(ElementRef); - private readonly destroy$ = new Subject(); + private readonly destroyRef = inject(DestroyRef); // Project management @@ -281,32 +284,16 @@ export class ModelWorkflowComponent implements OnInit, OnDestroy { } ngOnInit(): void { - // Extract project ID from route and initialize workspace state - this.route.params - .pipe( - takeUntil(this.destroy$), - tap(params => { - const projectId = params['projectId'] ? parseInt(params['projectId'], 10) : null; - this.projectId.set(projectId); - - if (projectId) { - this.persistenceService.setProjectId(projectId); - } - }), - switchMap(() => this.loadWorkspacesFromPersistence()) - ) - .subscribe({ - next: () => { - this.isLoading.set(false); - console.debug('Workspace initialization complete'); - }, - error: error => { - console.error('Failed to initialize workspaces:', error); - this.isLoading.set(false); - // Create default workspace as fallback - this.createWorkspaceWithName('Workspace 1'); - } - }); + this.loadWorkspacesFromPersistence().subscribe({ + next: () => { + console.debug('Workspace initialization complete'); + }, + error: error => { + console.error('Failed to initialize workspaces:', error); + // Create default workspace as fallback + this.userMessageService.showProjectLoadFailed('Failed to initialize project workspace'); + } + }); } ngOnDestroy(): void { @@ -314,9 +301,6 @@ export class ModelWorkflowComponent implements OnInit, OnDestroy { if (this.saveTimeout) { clearTimeout(this.saveTimeout); } - - this.destroy$.next(); - this.destroy$.complete(); } // Panel management methods @@ -355,38 +339,48 @@ export class ModelWorkflowComponent implements OnInit, OnDestroy { // Load workspaces from persistence private loadWorkspacesFromPersistence() { + this.isLoading.set(true); return this.persistenceService.loadWorkspaceState().pipe( - tap(savedState => { - if (savedState && savedState.workspaces.length > 0) { - // Restore from saved state - this.workspaceCounter.set(savedState.workspaceCounter); - - const runtimeWorkspaces = savedState.workspaces.map(pw => toRuntimeWorkspace(pw)); - this.workspaces.set(runtimeWorkspaces); - - // Restore active workspace (with fallback) - const activeId = - savedState.activeWorkspaceId && - runtimeWorkspaces.find(w => w.id === savedState.activeWorkspaceId) - ? savedState.activeWorkspaceId - : runtimeWorkspaces[0].id; - - this.activeWorkspaceId.set(activeId); - - // Initialize services for restored workspaces - runtimeWorkspaces.forEach(workspace => { - this.getWorkspaceService(workspace.id); - }); - - // Restore jobs for workspaces that have submitted job IDs - setTimeout(() => { - this.restoreJobsForWorkspaces(); - }, 100); // Small delay to ensure services are initialized - - console.debug(`Restored ${runtimeWorkspaces.length} workspaces from persistence`); - } else { - // No saved state, create default workspace - this.createWorkspaceWithName('Workspace 1'); + // cancel request if navigate away + takeUntilDestroyed(this.destroyRef), + tap({ + next: savedState => { + if (savedState && savedState.workspaces.length > 0) { + // Restore from saved state + this.workspaceCounter.set(savedState.workspaceCounter); + + const runtimeWorkspaces = savedState.workspaces.map(pw => toRuntimeWorkspace(pw)); + this.workspaces.set(runtimeWorkspaces); + + // Restore active workspace (with fallback) + const activeId = + savedState.activeWorkspaceId && + runtimeWorkspaces.find(w => w.id === savedState.activeWorkspaceId) + ? savedState.activeWorkspaceId + : runtimeWorkspaces[0].id; + + this.activeWorkspaceId.set(activeId); + + // Initialize services for restored workspaces + runtimeWorkspaces.forEach(workspace => { + this.getWorkspaceService(workspace.id); + }); + + // REVIEW should this be moved to WorkspaceService? consider RxJS + // Restore jobs for workspaces that have submitted job IDs + setTimeout(() => { + this.restoreJobsForWorkspaces(); + }, 100); // Small delay to ensure services are initialized + + console.debug(`Restored ${runtimeWorkspaces.length} workspaces from persistence`); + } else { + console.info('Project has no workspaces, creating one'); + // No saved state, create default workspace + this.createWorkspaceWithName('Workspace 1'); + } + }, + finalize: () => { + this.isLoading.set(false); } }) ); @@ -394,6 +388,7 @@ export class ModelWorkflowComponent implements OnInit, OnDestroy { // Save workspaces to persistence with debouncing private saveWorkspacesToPersistence(): void { + // TODO ideally this should be throttled subject that replaces any existing save request // Clear any existing timeout if (this.saveTimeout) { clearTimeout(this.saveTimeout); @@ -411,17 +406,15 @@ export class ModelWorkflowComponent implements OnInit, OnDestroy { workspaceCounter: this.workspaceCounter() }; - this.persistenceService - .saveWorkspaceState(state) - .pipe(takeUntil(this.destroy$)) - .subscribe({ - next: () => { - console.debug('Workspace state saved successfully'); - }, - error: error => { - console.warn('Failed to save workspace state:', error); - } - }); + this.persistenceService.saveWorkspaceState(state).subscribe({ + next: () => { + console.debug('Workspace state saved successfully'); + }, + error: error => { + console.warn('Failed to save workspace state:', error); + this.userMessageService.error('Failed to save workspace'); + } + }); }, 500); // 500ms debounce } @@ -826,6 +819,7 @@ export class ModelWorkflowComponent implements OnInit, OnDestroy { private restoreJobsForWorkspaces(): void { const workspaces = this.workspaces(); + // REVIEW does every workspace job need to be restored? or just active tab? workspaces.forEach(workspace => { if (workspace.submittedJobId) { const service = this.getWorkspaceService(workspace.id); diff --git a/packages/app/src/app/projects/services/base-workspace-persistence.service.ts b/packages/app/src/app/projects/services/base-workspace-persistence.service.ts index 3030cd9c..eac68021 100644 --- a/packages/app/src/app/projects/services/base-workspace-persistence.service.ts +++ b/packages/app/src/app/projects/services/base-workspace-persistence.service.ts @@ -1,5 +1,5 @@ import { inject, numberAttribute } from '@angular/core'; -import { Observable, of, throwError } from 'rxjs'; +import { Observable, of, tap, throwError } from 'rxjs'; import { catchError, map } from 'rxjs/operators'; import { WebApiService } from '../../../api/web-api.service'; import { MatSnackBar } from '@angular/material/snack-bar'; @@ -49,7 +49,7 @@ export abstract class BaseWorkspacePersistenceService { if (!this.isValidWorkspaceState(state, false)) { console.error('invalid state', state); this.showUserErrorMessage('Failed to save project state'); - throw new Error('App generated invalid workspace state'); + throwError(() => new Error('App generated invalid workspace state')); } // If we have a project ID, save to backend; otherwise use localStorage @@ -105,21 +105,31 @@ export abstract class BaseWorkspacePersistenceService { return throwError(() => new Error('Project ID not set')); } + if (!this.isValidWorkspaceState(state, false)) { + return throwError(() => new Error('Cannot save invalid workspace state')); + } + return this.api .updateProject(this.projectId, { project_state: state }) .pipe( map(() => void 0), - catchError(error => { - console.warn('Failed to save workspace state to backend:', error); - // Fallback to localStorage - return this.saveToLocalStorage(state); + tap({ + error: error => { + console.warn('Failed to save workspace state to backend:', error); + // Fallback to localStorage + // FIXME there is no recovery mechanism since local storage is only loaded when no projectId + // Also, all projects share the same local storage key + // Should always save to local storage!? + // GitHub issue: https://github.com/open-AIMS/reefguide/issues/232 + return this.saveToLocalStorage(state); + } }) ); } - private loadFromBackend(): Observable { + private loadFromBackend(): Observable { if (!this.projectId) { return throwError(() => new Error('Project ID not set')); } @@ -131,16 +141,18 @@ export abstract class BaseWorkspacePersistenceService { if (validMigrated) { return validMigrated; } else { - console.warn('Invalid workspace state found in project, returning null'); - this.showUserErrorMessage('Workspace state invalid and reset'); - return null; + throw new Error('Project failed to validate/migrate'); } - }), - catchError(error => { - console.warn('Failed to load workspace state from backend:', error); - // Fallback to localStorage - return this.loadFromLocalStorage(); }) + // FIXME Relates to https://github.com/open-AIMS/reefguide/issues/232 + // disabled because I disagree with silently loading from local storage when there's an error; + // this fallback, should be explicit. + // + // catchError(error => { + // console.warn('Failed to load workspace state from backend:', error); + // // Fallback to localStorage + // return this.loadFromLocalStorage(); + // }) ); } diff --git a/packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.html b/packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.html new file mode 100644 index 00000000..ac95f544 --- /dev/null +++ b/packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.html @@ -0,0 +1,11 @@ +

+ error + Failed to load project +

+ +

{{ data.message }}

+
+ + + + diff --git a/packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.scss b/packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.scss new file mode 100644 index 00000000..7d5040a9 --- /dev/null +++ b/packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.scss @@ -0,0 +1,5 @@ +mat-icon { + color: var(--mat-sys-error); + // TODO improve vertical centering + vertical-align: middle; +} diff --git a/packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.spec.ts b/packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.spec.ts new file mode 100644 index 00000000..bc4a1fb8 --- /dev/null +++ b/packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.spec.ts @@ -0,0 +1,22 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; + +import { FailedProjectLoadDialogComponent } from './failed-project-load-dialog.component'; + +describe('FailedProjectLoadDialogComponent', () => { + let component: FailedProjectLoadDialogComponent; + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [FailedProjectLoadDialogComponent] + }).compileComponents(); + + fixture = TestBed.createComponent(FailedProjectLoadDialogComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); +}); diff --git a/packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.ts b/packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.ts new file mode 100644 index 00000000..7cf96403 --- /dev/null +++ b/packages/app/src/app/user-messages/failed-project-load-dialog/failed-project-load-dialog.component.ts @@ -0,0 +1,19 @@ +import { Component, inject } from '@angular/core'; +import { MAT_DIALOG_DATA, MatDialogModule } from '@angular/material/dialog'; +import { MatIconModule } from '@angular/material/icon'; +import { MatButtonModule } from '@angular/material/button'; +import { RouterLink } from '@angular/router'; + +@Component({ + selector: 'app-failed-project-load-dialog', + imports: [MatDialogModule, MatIconModule, MatButtonModule, RouterLink], + templateUrl: './failed-project-load-dialog.component.html', + styleUrl: './failed-project-load-dialog.component.scss' +}) +export class FailedProjectLoadDialogComponent { + readonly data = inject<{ message: string }>(MAT_DIALOG_DATA); + + reload() { + window.location.reload(); + } +} diff --git a/packages/app/src/app/user-messages/user-message.service.spec.ts b/packages/app/src/app/user-messages/user-message.service.spec.ts new file mode 100644 index 00000000..a31524fb --- /dev/null +++ b/packages/app/src/app/user-messages/user-message.service.spec.ts @@ -0,0 +1,16 @@ +import { TestBed } from '@angular/core/testing'; + +import { UserMessageService } from './user-message.service'; + +describe('UserMessageService', () => { + let service: UserMessageService; + + beforeEach(() => { + TestBed.configureTestingModule({}); + service = TestBed.inject(UserMessageService); + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); +}); diff --git a/packages/app/src/app/user-messages/user-message.service.ts b/packages/app/src/app/user-messages/user-message.service.ts new file mode 100644 index 00000000..8606ce1b --- /dev/null +++ b/packages/app/src/app/user-messages/user-message.service.ts @@ -0,0 +1,34 @@ +import { inject, Injectable } from '@angular/core'; +import { MatDialog } from '@angular/material/dialog'; +import { FailedProjectLoadDialogComponent } from './failed-project-load-dialog/failed-project-load-dialog.component'; +import { MatSnackBar } from '@angular/material/snack-bar'; + +@Injectable({ + providedIn: 'root' +}) +export class UserMessageService { + dialog = inject(MatDialog); + snackbar = inject(MatSnackBar); + + /** + * Show blocking dialog indicating the project page/route failed to load. + * + * Prompts the user to reload or go back to projects + * @param message main content message to display + */ + showProjectLoadFailed(message: string): void { + this.dialog.open(FailedProjectLoadDialogComponent, { + disableClose: true, + closeOnNavigation: true, // no effect? + data: { message } + }); + } + + /** + * Show non-blocking error message to user. + * @param message + */ + error(message: string): void { + this.snackbar.open(message, 'OK'); + } +} From 0647736e07d3a3f9b2f1c3132d3ec541d96217b2 Mon Sep 17 00:00:00 2001 From: Arlo White Date: Tue, 25 Nov 2025 17:16:07 +1000 Subject: [PATCH 08/25] Add AI guides from ng new project --- packages/app/.claude/CLAUDE.md | 47 ++++++++++++++++++++ packages/app/.github/copilot-instructions.md | 47 ++++++++++++++++++++ 2 files changed, 94 insertions(+) create mode 100644 packages/app/.claude/CLAUDE.md create mode 100644 packages/app/.github/copilot-instructions.md diff --git a/packages/app/.claude/CLAUDE.md b/packages/app/.claude/CLAUDE.md new file mode 100644 index 00000000..9cb8b7f9 --- /dev/null +++ b/packages/app/.claude/CLAUDE.md @@ -0,0 +1,47 @@ +You are an expert in TypeScript, Angular, and scalable web application development. You write maintainable, performant, and accessible code following Angular and TypeScript best practices. + +## TypeScript Best Practices + +- Use strict type checking +- Prefer type inference when the type is obvious +- Avoid the `any` type; use `unknown` when type is uncertain + +## Angular Best Practices + +- Always use standalone components over NgModules +- Must NOT set `standalone: true` inside Angular decorators. It's the default. +- Use signals for state management +- Implement lazy loading for feature routes +- Do NOT use the `@HostBinding` and `@HostListener` decorators. Put host bindings inside the `host` object of the `@Component` or `@Directive` decorator instead +- Use `NgOptimizedImage` for all static images. + - `NgOptimizedImage` does not work for inline base64 images. + +## Components + +- Keep components small and focused on a single responsibility +- Use `input()` and `output()` functions instead of decorators +- Use `computed()` for derived state +- Set `changeDetection: ChangeDetectionStrategy.OnPush` in `@Component` decorator +- Prefer inline templates for small components +- Prefer Reactive forms instead of Template-driven ones +- Do NOT use `ngClass`, use `class` bindings instead +- Do NOT use `ngStyle`, use `style` bindings instead + +## State Management + +- Use signals for local component state +- Use `computed()` for derived state +- Keep state transformations pure and predictable +- Do NOT use `mutate` on signals, use `update` or `set` instead + +## Templates + +- Keep templates simple and avoid complex logic +- Use native control flow (`@if`, `@for`, `@switch`) instead of `*ngIf`, `*ngFor`, `*ngSwitch` +- Use the async pipe to handle observables + +## Services + +- Design services around a single responsibility +- Use the `providedIn: 'root'` option for singleton services +- Use the `inject()` function instead of constructor injection diff --git a/packages/app/.github/copilot-instructions.md b/packages/app/.github/copilot-instructions.md new file mode 100644 index 00000000..9cb8b7f9 --- /dev/null +++ b/packages/app/.github/copilot-instructions.md @@ -0,0 +1,47 @@ +You are an expert in TypeScript, Angular, and scalable web application development. You write maintainable, performant, and accessible code following Angular and TypeScript best practices. + +## TypeScript Best Practices + +- Use strict type checking +- Prefer type inference when the type is obvious +- Avoid the `any` type; use `unknown` when type is uncertain + +## Angular Best Practices + +- Always use standalone components over NgModules +- Must NOT set `standalone: true` inside Angular decorators. It's the default. +- Use signals for state management +- Implement lazy loading for feature routes +- Do NOT use the `@HostBinding` and `@HostListener` decorators. Put host bindings inside the `host` object of the `@Component` or `@Directive` decorator instead +- Use `NgOptimizedImage` for all static images. + - `NgOptimizedImage` does not work for inline base64 images. + +## Components + +- Keep components small and focused on a single responsibility +- Use `input()` and `output()` functions instead of decorators +- Use `computed()` for derived state +- Set `changeDetection: ChangeDetectionStrategy.OnPush` in `@Component` decorator +- Prefer inline templates for small components +- Prefer Reactive forms instead of Template-driven ones +- Do NOT use `ngClass`, use `class` bindings instead +- Do NOT use `ngStyle`, use `style` bindings instead + +## State Management + +- Use signals for local component state +- Use `computed()` for derived state +- Keep state transformations pure and predictable +- Do NOT use `mutate` on signals, use `update` or `set` instead + +## Templates + +- Keep templates simple and avoid complex logic +- Use native control flow (`@if`, `@for`, `@switch`) instead of `*ngIf`, `*ngFor`, `*ngSwitch` +- Use the async pipe to handle observables + +## Services + +- Design services around a single responsibility +- Use the `providedIn: 'root'` option for singleton services +- Use the `inject()` function instead of constructor injection From 1fb57d5c26d553263dcd25b958a1eb6cdac9f9cd Mon Sep 17 00:00:00 2001 From: Arlo White Date: Tue, 25 Nov 2025 17:26:25 +1000 Subject: [PATCH 09/25] Move delete workspace tab to menu --- .../model-workflow.component.html | 23 ++++---- .../model-workflow.component.scss | 53 ------------------- 2 files changed, 11 insertions(+), 65 deletions(-) diff --git a/packages/app/src/app/model-workflow/model-workflow.component.html b/packages/app/src/app/model-workflow/model-workflow.component.html index 1fdf59cd..7e4234a0 100644 --- a/packages/app/src/app/model-workflow/model-workflow.component.html +++ b/packages/app/src/app/model-workflow/model-workflow.component.html @@ -35,18 +35,6 @@ {{ getWorkspaceDisplayName(workspace) }} - - - @@ -67,6 +55,17 @@ content_copy Copy parameters to new workspace + @if (allWorkspaces().length > 1) { + + } diff --git a/packages/app/src/app/model-workflow/model-workflow.component.scss b/packages/app/src/app/model-workflow/model-workflow.component.scss index da6e4163..ecf3f1cb 100644 --- a/packages/app/src/app/model-workflow/model-workflow.component.scss +++ b/packages/app/src/app/model-workflow/model-workflow.component.scss @@ -169,59 +169,6 @@ order: 2; // Show second } -.close-button { - width: 22px !important; // Increased from 20px - height: 22px !important; // Increased from 20px - min-width: 22px !important; - max-width: 22px !important; - flex-shrink: 0; - border-radius: 50%; - padding: 0 !important; - margin: 0; - margin-left: auto; // Push to right edge - order: 3; // Show last - - // Override all Material button styling - :deep(.mat-mdc-button-base) { - width: 22px !important; - height: 22px !important; - padding: 0 !important; - border-radius: 50%; - display: flex; - align-items: center; - justify-content: center; - } - - :deep(.mat-mdc-button-touch-target) { - display: none !important; - } - - :deep(.mat-mdc-button-ripple), - :deep(.mat-mdc-button-persistent-ripple) { - border-radius: 50%; - } - - :deep(.mat-icon) { - font-size: 14px !important; // Increased from 12px - width: 14px !important; // Increased from 12px - height: 14px !important; // Increased from 12px - line-height: 14px !important; - } - - &:hover:not(:disabled) { - background-color: rgba(0, 0, 0, 0.08); - } - - &:disabled { - opacity: 0.4; - cursor: not-allowed; - - &:hover { - background-color: transparent; - } - } -} - .new-workspace-button { width: 40px; height: 40px; From befac4cd2c274dc5b896166cb8828c9283076a14 Mon Sep 17 00:00:00 2001 From: Arlo White Date: Tue, 25 Nov 2025 18:27:41 +1000 Subject: [PATCH 10/25] cleanup ADRIA project CSS --- .../model-workflow.component.html | 26 ++- .../model-workflow.component.scss | 170 ++---------------- packages/app/src/styles/mat-overrides.scss | 10 ++ 3 files changed, 39 insertions(+), 167 deletions(-) diff --git a/packages/app/src/app/model-workflow/model-workflow.component.html b/packages/app/src/app/model-workflow/model-workflow.component.html index 7e4234a0..0e80c102 100644 --- a/packages/app/src/app/model-workflow/model-workflow.component.html +++ b/packages/app/src/app/model-workflow/model-workflow.component.html @@ -20,22 +20,18 @@ > -
- - + + - - {{ getWorkspaceDisplayName(workspace) }} -
+ {{ getWorkspaceDisplayName(workspace) }}
diff --git a/packages/app/src/app/model-workflow/model-workflow.component.scss b/packages/app/src/app/model-workflow/model-workflow.component.scss index ecf3f1cb..23be653b 100644 --- a/packages/app/src/app/model-workflow/model-workflow.component.scss +++ b/packages/app/src/app/model-workflow/model-workflow.component.scss @@ -1,4 +1,5 @@ /* src/app/model-workflow/model-workflow.component.scss */ +@use '@angular/material' as mat; .workflow-container { height: 100vh; @@ -21,6 +22,12 @@ flex: 0 0 auto; // Don't expand to fill width max-width: calc(100% - 60px); // Leave space for the new workspace button + // Override M3 tabs component styles + // https://material.angular.dev/components/tabs/styling + //@include mat.tabs-overrides(( + // label-text-size: 1rem + //)); + // Override Material tabs styling :deep(.mat-mdc-tab-group) { --mdc-secondary-navigation-tab-container-height: 56px; // Increased from 48px @@ -36,39 +43,15 @@ display: none; // Hide pagination arrows for cleaner look } - :deep(.mat-mdc-tab) { - min-width: 180px; // Increased from 160px - max-width: 280px; // Increased from 240px - width: auto; - flex: 0 1 220px; // Increased preferred width from 200px - padding: 0 12px; // Increased from 8px - border-right: 1px solid rgba(0, 0, 0, 0.12); // Add border between tabs - position: relative; - - &:last-child { - border-right: none; // Remove border from last tab - } - - // Add subtle background on hover - &:hover { - background-color: rgba(0, 0, 0, 0.04); - } - - // Active tab styling - &.mat-mdc-tab-active { - background-color: rgba(25, 118, 210, 0.04); - } + ::ng-deep .mat-mdc-tab { + // left padding not as necessary with leading context-menu button + padding-left: 8px; } :deep(.mat-mdc-tab .mdc-tab__content) { padding: 0; } - :deep(.mat-mdc-tab .mdc-tab__text-label) { - padding: 0; - width: 100%; - } - // When there are multiple tabs, make them shrink proportionally like Chrome &:has(:deep(.mat-mdc-tab:nth-child(4))) :deep(.mat-mdc-tab) { flex: 1 1 200px; // Increased from 180px @@ -91,84 +74,6 @@ } } -.tab-label { - display: flex; - align-items: center; - width: 100%; - height: 40px; // Increased from 32px - padding: 0 4px; // Increased from 0 8px 0 4px - gap: 30px; // Increased from 8px -} - -.context-menu-button { - width: 20px !important; // Increased from 18px - height: 20px !important; // Increased from 18px - min-width: 20px !important; - max-width: 20px !important; - flex-shrink: 0; - border-radius: 50%; - padding: 0 !important; - margin: 0; - margin-left: 0; // Align to left edge - order: 1; // Show first - - // Override all Material button styling - :deep(.mat-mdc-button-base) { - width: 20px !important; - height: 20px !important; - padding: 0 !important; - border-radius: 50%; - display: flex; - align-items: center; - justify-content: center; - } - - :deep(.mat-mdc-button-touch-target) { - display: none !important; - } - - :deep(.mat-mdc-button-ripple), - :deep(.mat-mdc-button-persistent-ripple) { - border-radius: 50%; - } - - :deep(.mat-icon) { - font-size: 14px !important; // Increased from 12px - width: 14px !important; // Increased from 12px - height: 14px !important; // Increased from 12px - line-height: 14px !important; - color: rgba(0, 0, 0, 0.6); - } - - &:hover:not(:disabled) { - background-color: rgba(0, 0, 0, 0.08); - - :deep(.mat-icon) { - color: rgba(0, 0, 0, 0.87); - } - } - - &:disabled { - opacity: 0.4; - cursor: not-allowed; - - &:hover { - background-color: transparent; - } - } -} - -.tab-name { - flex: 1; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - font-size: 14px; // Increased from 13px - font-weight: 500; - min-width: 0; // Important for text truncation - order: 2; // Show second -} - .new-workspace-button { width: 40px; height: 40px; @@ -502,6 +407,14 @@ } .workspace-tabs { + // Override M3 tabs component styles + @include mat.tabs-overrides( + ( + // Slightly smaller on mobile + label-text-size: 13px + ) + ); + max-width: calc(100% - 50px); // Adjust for smaller new workspace button :deep(.mat-mdc-tab) { @@ -522,53 +435,6 @@ } } - .tab-label { - gap: 6px; // Further reduced gap for mobile - padding: 0 6px 0 2px; // Adjusted mobile padding for context menu - } - - .tab-name { - font-size: 13px; // Slightly smaller on mobile but still larger than before - } - - .context-menu-button { - width: 16px !important; // Smaller on mobile - height: 16px !important; - min-width: 16px !important; - max-width: 16px !important; - - :deep(.mat-mdc-button-base) { - width: 16px !important; - height: 16px !important; - } - - :deep(.mat-icon) { - font-size: 10px !important; // Smaller icon on mobile - width: 10px !important; - height: 10px !important; - line-height: 10px !important; - } - } - - .close-button { - width: 18px !important; // Smaller on mobile - height: 18px !important; - min-width: 18px !important; - max-width: 18px !important; - - :deep(.mat-mdc-button-base) { - width: 18px !important; - height: 18px !important; - } - - :deep(.mat-icon) { - font-size: 10px !important; // Smaller icon on mobile - width: 10px !important; - height: 10px !important; - line-height: 10px !important; - } - } - .right-panel { padding: 8px; } diff --git a/packages/app/src/styles/mat-overrides.scss b/packages/app/src/styles/mat-overrides.scss index e60a496c..36435c17 100644 --- a/packages/app/src/styles/mat-overrides.scss +++ b/packages/app/src/styles/mat-overrides.scss @@ -1,5 +1,15 @@ +@use '@angular/material' as mat; + mat-tab-group.fixed-height { .mat-mdc-tab-body-wrapper { height: 100%; } } + +// Note, the correct way to override Angular Material 3 theme is documented here: +// https://material.angular.dev/guide/theming#system-tokens + +// Component Overrides Example: +//@include mat.tabs-overrides(( +// label-text-size: 40px +//)); From 4c4bfd77e3179505a0e0a90934db4608a9a63981 Mon Sep 17 00:00:00 2001 From: Arlo White Date: Tue, 25 Nov 2025 18:29:00 +1000 Subject: [PATCH 11/25] cleanup :deep style halucinations that have no effect --- .../model-workflow.component.scss | 65 ------------------- 1 file changed, 65 deletions(-) diff --git a/packages/app/src/app/model-workflow/model-workflow.component.scss b/packages/app/src/app/model-workflow/model-workflow.component.scss index 23be653b..f8fe0a2f 100644 --- a/packages/app/src/app/model-workflow/model-workflow.component.scss +++ b/packages/app/src/app/model-workflow/model-workflow.component.scss @@ -28,60 +28,16 @@ // label-text-size: 1rem //)); - // Override Material tabs styling - :deep(.mat-mdc-tab-group) { - --mdc-secondary-navigation-tab-container-height: 56px; // Increased from 48px - width: auto; // Let tabs determine their own width - } - - :deep(.mat-mdc-tab-header) { - border-bottom: none; - width: auto; - } - - :deep(.mat-mdc-tab-header-pagination) { - display: none; // Hide pagination arrows for cleaner look - } - ::ng-deep .mat-mdc-tab { // left padding not as necessary with leading context-menu button padding-left: 8px; } - - :deep(.mat-mdc-tab .mdc-tab__content) { - padding: 0; - } - - // When there are multiple tabs, make them shrink proportionally like Chrome - &:has(:deep(.mat-mdc-tab:nth-child(4))) :deep(.mat-mdc-tab) { - flex: 1 1 200px; // Increased from 180px - max-width: 240px; // Increased from 200px - } - - &:has(:deep(.mat-mdc-tab:nth-child(6))) :deep(.mat-mdc-tab) { - flex: 1 1 180px; // Increased from 160px - max-width: 220px; // Increased from 180px - } - - &:has(:deep(.mat-mdc-tab:nth-child(8))) :deep(.mat-mdc-tab) { - flex: 1 1 160px; // Increased from 140px - max-width: 200px; // Increased from 160px - } - - // Hide the tab content since we're managing it separately - :deep(.mat-mdc-tab-body-wrapper) { - display: none; - } } .new-workspace-button { width: 40px; height: 40px; flex-shrink: 0; - - :deep(.mat-icon) { - font-size: 20px; - } } .workspace-content { @@ -400,10 +356,6 @@ .new-workspace-button { width: 36px; height: 36px; - - :deep(.mat-icon) { - font-size: 18px; - } } .workspace-tabs { @@ -416,23 +368,6 @@ ); max-width: calc(100% - 50px); // Adjust for smaller new workspace button - - :deep(.mat-mdc-tab) { - min-width: 120px; - max-width: 160px; - flex: 0 1 140px; - } - - // Adjust shrinking behavior for mobile - &:has(:deep(.mat-mdc-tab:nth-child(3))) :deep(.mat-mdc-tab) { - flex: 1 1 100px; - max-width: 120px; - } - - &:has(:deep(.mat-mdc-tab:nth-child(4))) :deep(.mat-mdc-tab) { - flex: 1 1 90px; - max-width: 110px; - } } .right-panel { From a986aa702e594b6af5cc872f3f57f5db63652291 Mon Sep 17 00:00:00 2001 From: Arlo White Date: Tue, 25 Nov 2025 18:41:06 +1000 Subject: [PATCH 12/25] improve new tab button css --- .../app/model-workflow/model-workflow.component.html | 3 +-- .../app/model-workflow/model-workflow.component.scss | 10 +++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/app/src/app/model-workflow/model-workflow.component.html b/packages/app/src/app/model-workflow/model-workflow.component.html index 0e80c102..55fb3727 100644 --- a/packages/app/src/app/model-workflow/model-workflow.component.html +++ b/packages/app/src/app/model-workflow/model-workflow.component.html @@ -67,8 +67,7 @@