From 155cd121b4af01fadb550e5cc1b867ce65369c08 Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Sun, 5 Jul 2026 01:26:55 +0300 Subject: [PATCH] feat(widget-config): always offer "Any" data source option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Data Source dropdown offered a literal "default" only when a path had a single source, and dropped it entirely once a second source appeared — even resetting a saved "default" selection. That hid the most useful option (server-merged, priority-following, failover-capable value) exactly when multiple sources make it valuable. Always list "Any" (the stored 'default' value) first, with concrete sources below, regardless of source count. A fresh path defaults to "Any"; an existing selection is preserved. Applies to both the widget path source and the data-chart source. Stored config value stays 'default'; "Any" is only the display label, so no migration is needed. Closes #203 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../dataset-chart-options.component.html | 2 +- .../dataset-chart-options.component.spec.ts | 40 +++++++++++++- .../dataset-chart-options.component.ts | 22 ++++---- .../path-control-config.component.html | 2 +- .../path-control-config.component.spec.ts | 54 +++++++++++++++++-- .../path-control-config.component.ts | 25 ++++----- 6 files changed, 112 insertions(+), 33 deletions(-) diff --git a/src/app/widget-config/dataset-chart-options/dataset-chart-options.component.html b/src/app/widget-config/dataset-chart-options/dataset-chart-options.component.html index 32946738..20c91c16 100644 --- a/src/app/widget-config/dataset-chart-options/dataset-chart-options.component.html +++ b/src/app/widget-config/dataset-chart-options/dataset-chart-options.component.html @@ -56,7 +56,7 @@ > @for (source of pathSources(); track source) { - {{source}} + {{source === 'default' ? 'Any' : source}} } diff --git a/src/app/widget-config/dataset-chart-options/dataset-chart-options.component.spec.ts b/src/app/widget-config/dataset-chart-options/dataset-chart-options.component.spec.ts index 3e6bef74..4089eb4d 100644 --- a/src/app/widget-config/dataset-chart-options/dataset-chart-options.component.spec.ts +++ b/src/app/widget-config/dataset-chart-options/dataset-chart-options.component.spec.ts @@ -4,12 +4,18 @@ import { UntypedFormControl } from '@angular/forms'; import { DatasetChartOptionsComponent } from './dataset-chart-options.component'; import { DataService } from '../../core/services/data.service'; import { UnitsService } from '../../core/services/units.service'; +import { ISkPathData } from '../../core/interfaces/app-interfaces'; + +const src = (...keys: string[]): ISkPathData['sources'] => + Object.fromEntries(keys.map(k => [k, { sourceTimestamp: '', sourceValue: 0 }])); describe('DatasetChartOptionsComponent', () => { let component: DatasetChartOptionsComponent; let fixture: ComponentFixture; + let pathObject: Partial | null; beforeEach(async () => { + pathObject = null; await TestBed.configureTestingModule({ imports: [DatasetChartOptionsComponent], providers: [ @@ -17,7 +23,7 @@ describe('DatasetChartOptionsComponent', () => { provide: DataService, useValue: { getPathsAndMetaByType: () => [], - getPathObject: () => null, + getPathObject: () => pathObject, }, }, { @@ -45,4 +51,36 @@ describe('DatasetChartOptionsComponent', () => { it('should create', () => { expect(component).toBeTruthy(); }); + + const setPathSources = (obj: Pick) => + (component as unknown as { setPathSources: (p: unknown) => void }).setPathSources(obj); + const changePath = (path: string) => + (component as unknown as { changePath: (e: unknown) => void }).changePath({ option: { value: path } }); + const sources = () => (component as unknown as { pathSources: () => string[] }).pathSources(); + + it('offers "Any" (default) plus the concrete source for a single-source path', () => { + setPathSources({ sources: src('gps.0') }); + expect(sources()).toEqual(['default', 'gps.0']); + expect(component.datachartSource().value).toBe('default'); + }); + + it('keeps "Any" (default) at the top and preserves the selection with multiple sources', () => { + component.datachartSource().setValue('gps.1'); + setPathSources({ sources: src('gps.1', 'gps.0') }); + expect(sources()).toEqual(['default', 'gps.0', 'gps.1']); + expect(component.datachartSource().value).toBe('gps.1'); + }); + + it('defaults to "Any" (default) when no source was selected', () => { + component.datachartSource().setValue(''); + setPathSources({ sources: src('gps.1', 'gps.0') }); + expect(component.datachartSource().value).toBe('default'); + }); + + it('resets a stale source to "Any" when switching to a path that lacks it', () => { + component.datachartSource().setValue('gps.9'); + pathObject = { path: 'navigation.speedThroughWater', sources: src('gps.0', 'gps.1') }; + changePath(pathObject.path as string); + expect(component.datachartSource().value).toBe('default'); + }); }); diff --git a/src/app/widget-config/dataset-chart-options/dataset-chart-options.component.ts b/src/app/widget-config/dataset-chart-options/dataset-chart-options.component.ts index fabc00a9..82cf5346 100644 --- a/src/app/widget-config/dataset-chart-options/dataset-chart-options.component.ts +++ b/src/app/widget-config/dataset-chart-options/dataset-chart-options.component.ts @@ -134,24 +134,24 @@ export class DatasetChartOptionsComponent implements OnInit { this.setPathUnits(); return; } + // A freshly chosen path must not carry over the previous path's concrete + // source, which may not exist here. Clear it so setPathSources falls back + // to "Any"; the ngOnInit load path calls setPathSources directly and keeps + // the saved selection. + this.datachartSource().reset(); this.setPathSources(pathObject); this.setPathUnits(pathObject.path); } private setPathSources(pathObject: ISkPathData): void { - if (Object.keys(pathObject.sources).length == 1) { - this.pathSources.set(['default']); + // 'default' (shown as "Any") always leads the list: it reads the server's + // merged, priority-selected value and follows source failover. Concrete + // sources follow. An existing selection is preserved; otherwise default to "Any". + this.pathSources.set(['default', ...Object.keys(pathObject.sources).sort()]); + if (!this.datachartSource().value) { this.datachartSource().setValue('default'); - this.datachartSource().enable(); - } else if (Object.keys(pathObject.sources).length > 1) { - this.pathSources.set(Object.keys(pathObject.sources).sort()); - if(this.datachartSource().value) { - this.datachartSource().setValue(this.datachartSource().value); - } else { - this.datachartSource().reset(); - } - this.datachartSource().enable(); } + this.datachartSource().enable(); } private setPathUnits(path?: string): void { diff --git a/src/app/widget-config/path-control-config/path-control-config.component.html b/src/app/widget-config/path-control-config/path-control-config.component.html index fcbb9950..c55f581a 100644 --- a/src/app/widget-config/path-control-config/path-control-config.component.html +++ b/src/app/widget-config/path-control-config/path-control-config.component.html @@ -82,7 +82,7 @@ required> @for (source of availableSources; track $index) { - {{source}} + {{source === 'default' ? 'Any' : source}} } diff --git a/src/app/widget-config/path-control-config/path-control-config.component.spec.ts b/src/app/widget-config/path-control-config/path-control-config.component.spec.ts index 9534b17a..3bde80d7 100644 --- a/src/app/widget-config/path-control-config/path-control-config.component.spec.ts +++ b/src/app/widget-config/path-control-config/path-control-config.component.spec.ts @@ -3,17 +3,24 @@ import { beforeEach, describe, expect, it } from 'vitest'; import { EMPTY } from 'rxjs'; import { UntypedFormControl, UntypedFormGroup } from '@angular/forms'; import { IDynamicControl } from '../../core/interfaces/widgets-interface'; +import { ISkPathData } from '../../core/interfaces/app-interfaces'; import { PathControlConfigComponent } from './path-control-config.component'; import { SignalKConnectionService } from '../../core/services/signalk-connection.service'; import { DataService } from '../../core/services/data.service'; import { UnitsService } from '../../core/services/units.service'; +const src = (...keys: string[]): ISkPathData['sources'] => + Object.fromEntries(keys.map(k => [k, { sourceTimestamp: '', sourceValue: 0 }])); + describe('PathControlConfigComponent', () => { let component: PathControlConfigComponent; let fixture: ComponentFixture; + let pathForm: UntypedFormGroup; + let pathObject: Partial; beforeEach(async () => { + pathObject = { sources: src('gps.0') }; await TestBed.configureTestingModule({ imports: [PathControlConfigComponent], providers: [ @@ -21,11 +28,11 @@ describe('PathControlConfigComponent', () => { { provide: DataService, useValue: { - getPathObject: () => ({ displayName: 'Speed', meta: { units: 'knots' } }), + getPathObject: () => pathObject, getPathsAndMetaByType: () => ([]) } }, - { provide: UnitsService, useValue: { skBaseUnits: [], getConversions: () => [] } } + { provide: UnitsService, useValue: { skBaseUnits: [], getConversions: () => [], getConversionsForPath: () => ({ base: '', conversions: [] }) } } ] }) .compileComponents(); @@ -35,7 +42,7 @@ describe('PathControlConfigComponent', () => { fixture = TestBed.createComponent(PathControlConfigComponent); component = fixture.componentInstance; // Provide required inputs before first detectChanges - const pathForm = new UntypedFormGroup({ + pathForm = new UntypedFormGroup({ description: new UntypedFormControl('Speed'), path: new UntypedFormControl('navigation.speedThroughWater'), pathID: new UntypedFormControl('uuid-1'), @@ -58,4 +65,45 @@ describe('PathControlConfigComponent', () => { it('should be created', () => { expect(component).toBeTruthy(); }); + + const enableFormFields = (setValues: boolean) => + (component as unknown as { enableFormFields: (v: boolean) => void }).enableFormFields(setValues); + + it('offers "Any" (default) as the only leading option for a single-source path', () => { + pathObject.sources = src('gps.0'); + pathForm.controls['source'].setValue('default'); + enableFormFields(false); + expect(component.availableSources).toEqual(['default', 'gps.0']); + expect(pathForm.controls['source'].value).toBe('default'); + }); + + it('keeps "Any" (default) available when a path gains a second source', () => { + pathObject.sources = src('gps.0', 'gps.1'); + pathForm.controls['source'].setValue('default'); + enableFormFields(false); + expect(component.availableSources).toEqual(['default', 'gps.0', 'gps.1']); + // Regression: a saved "Any" selection must not be reset when sources multiply. + expect(pathForm.controls['source'].value).toBe('default'); + }); + + it('preserves a concrete saved source on load', () => { + pathObject.sources = src('gps.0', 'gps.1'); + pathForm.controls['source'].setValue('gps.1'); + enableFormFields(false); + expect(pathForm.controls['source'].value).toBe('gps.1'); + }); + + it('defaults an empty saved source to "Any" on load', () => { + pathObject.sources = src('gps.0', 'gps.1'); + pathForm.controls['source'].setValue(''); + enableFormFields(false); + expect(pathForm.controls['source'].value).toBe('default'); + }); + + it('defaults a freshly selected path to "Any" (default)', () => { + pathObject.sources = src('gps.0', 'gps.1'); + pathForm.controls['source'].setValue('gps.1'); + enableFormFields(true); + expect(pathForm.controls['source'].value).toBe('default'); + }); }); diff --git a/src/app/widget-config/path-control-config/path-control-config.component.ts b/src/app/widget-config/path-control-config/path-control-config.component.ts index e1cc96fd..f53c4e30 100644 --- a/src/app/widget-config/path-control-config/path-control-config.component.ts +++ b/src/app/widget-config/path-control-config/path-control-config.component.ts @@ -216,23 +216,16 @@ export class PathControlConfigComponent implements OnInit, OnChanges { pathFormGroup.controls['convertUnitTo'].enable({onlySelf: false}); } - if (Object.keys(pathObject.sources).length == 1) { - this.availableSources = ['default']; - if (setValues) { - if (pathFormGroup.controls['source'].value != 'default') { - pathFormGroup.controls['source'].setValue('default', {onlySelf: true}); - } - } - else if (pathFormGroup.controls['source'].value != 'default') { - pathFormGroup.controls['source'].setValue('', {onlySelf: true}); - } - } else if (Object.keys(pathObject.sources).length > 1) { - this.availableSources = Object.keys(pathObject.sources); - if (pathFormGroup.controls['source'].value == 'default') { - pathFormGroup.controls['source'].reset(); - } + // 'default' (shown as "Any") always leads the list: it reads the server's + // merged, priority-selected value and follows source failover. Concrete + // sources follow, letting the user pin to one. A fresh path defaults to + // "Any"; an existing selection is preserved. + this.availableSources = ['default', ...Object.keys(pathObject.sources).sort()]; + const sourceControl = pathFormGroup.controls['source']; + if (setValues || !sourceControl.value) { + sourceControl.setValue('default', {onlySelf: true}); } - pathFormGroup.controls['source'].enable({onlySelf: false}); + sourceControl.enable({onlySelf: false}); } else { // we don't know this path. Maybe and old saved path... this.disablePathFields();