mirror of
https://github.com/cpvalente/ontime.git
synced 2026-08-14 11:53:49 +00:00
refactor: address review feedback on aux timer naming
- Revert the automation action labels back to "Aux N: action" (no functional reason to change this wording). - Simplify the aux timers header in the playback control: the flex container now only handles layout, and the label text reuses the same font-size/color as the per-timer labels below it, instead of a bespoke style block duplicating those values. - Replace the generic, length-driven normaliser with an explicit sanitiseAuxTimerNames() that always deals with exactly three timers, and rename it away from "normalise" (which didn't convey that it trims, caps length and fills in missing entries). Static defaults now use a plain ['', '', ''] literal instead of calling the sanitiser with no input to sanitise. - Settings.type.ts and AuxTimerSettings.tsx no longer generate their three fields from a loop; the form mirrors the same explicit, one-field-per-row style already used by GeneralSettings.tsx. - Trim comments that only restated what the following line already says, keeping the ones that explain non-obvious behaviour (why AuxTimerService needs to be resynced separately from the data provider, why SimpleTimer.reset() preserves the name). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WCZejVTzuAY3tHTE6nB3JH
This commit is contained in:
@@ -23,7 +23,6 @@ import {
|
||||
customFieldLabelToKey,
|
||||
eventDef as eventModel,
|
||||
isKnownTimerType,
|
||||
normaliseAuxTimerNames,
|
||||
validateEndAction,
|
||||
} from 'ontime-utils';
|
||||
|
||||
@@ -82,7 +81,7 @@ export function migrateSettings(jsonData: object): (Settings & { serverPort: num
|
||||
operatorKey,
|
||||
timeFormat,
|
||||
language,
|
||||
auxTimerNames: normaliseAuxTimerNames(),
|
||||
auxTimerNames: ['', '', ''],
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { DatabaseModel, Settings } from 'ontime-types';
|
||||
import { normaliseAuxTimerNames } from 'ontime-utils';
|
||||
import { sanitiseAuxTimerNames } from 'ontime-utils';
|
||||
|
||||
import { is } from '../../../utils/is.js';
|
||||
|
||||
@@ -24,7 +24,7 @@ export function migrateServerPort(jsonData: Partial<DatabaseModel>): {
|
||||
const operatorKey = settings?.operatorKey;
|
||||
const timeFormat = settings?.timeFormat;
|
||||
const language = settings?.language;
|
||||
const auxTimerNames = normaliseAuxTimerNames(settings?.auxTimerNames);
|
||||
const auxTimerNames = sanitiseAuxTimerNames(settings?.auxTimerNames);
|
||||
const version = '4.5.0';
|
||||
db.settings = {
|
||||
version,
|
||||
|
||||
@@ -20,7 +20,7 @@ describe('parseSettings()', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('carries custom aux timer names through and normalises to a length-3 array', () => {
|
||||
it('carries custom aux timer names through and pads to a length-3 array', () => {
|
||||
const result = parseSettings({
|
||||
settings: { version: '1', auxTimerNames: ['Speaker'] } as unknown as Settings,
|
||||
});
|
||||
@@ -35,7 +35,6 @@ describe('parseSettings()', () => {
|
||||
});
|
||||
|
||||
it('creates the aux timer names for project files made before the feature existed', () => {
|
||||
// a settings object as found in a project file which predates aux timer naming
|
||||
const oldSettings = {
|
||||
version: '4.5.0',
|
||||
editorKey: null,
|
||||
@@ -47,7 +46,6 @@ describe('parseSettings()', () => {
|
||||
const result = parseSettings({ settings: oldSettings as Settings });
|
||||
|
||||
expect(result.auxTimerNames).toStrictEqual(['', '', '']);
|
||||
// the rest of the settings are untouched
|
||||
expect(result).toMatchObject({ timeFormat: '24', language: 'en' });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { DatabaseModel, Settings } from 'ontime-types';
|
||||
import { normaliseAuxTimerNames } from 'ontime-utils';
|
||||
import { sanitiseAuxTimerNames } from 'ontime-utils';
|
||||
|
||||
import { getPartialProject } from '../../models/dataModel.js';
|
||||
|
||||
@@ -23,7 +23,7 @@ export function parseSettings(data: Partial<DatabaseModel>): Settings {
|
||||
operatorKey: data.settings.operatorKey ?? defaultSettings.operatorKey,
|
||||
timeFormat: data.settings.timeFormat ?? defaultSettings.timeFormat,
|
||||
language: data.settings.language ?? defaultSettings.language,
|
||||
// property added in v4.6.0, older project files will not contain it
|
||||
auxTimerNames: normaliseAuxTimerNames(data.settings.auxTimerNames),
|
||||
// older project files predate this property
|
||||
auxTimerNames: sanitiseAuxTimerNames(data.settings.auxTimerNames),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { body } from 'express-validator';
|
||||
import { normaliseAuxTimerNames } from 'ontime-utils';
|
||||
import { sanitiseAuxTimerNames } from 'ontime-utils';
|
||||
|
||||
import { requestValidationFunction } from '../validation-utils/validationFunction.js';
|
||||
|
||||
@@ -28,11 +28,7 @@ export const validateSettings = [
|
||||
pinValidator('operatorKey'),
|
||||
body('timeFormat').isString().isIn(['12', '24']).withMessage('Time format can only be "12" or "24"'),
|
||||
body('language').isString().trim().notEmpty(),
|
||||
body('auxTimerNames')
|
||||
.isArray()
|
||||
.withMessage('auxTimerNames must be an array')
|
||||
// normalise to a fixed length array of trimmed, length capped strings
|
||||
.customSanitizer(normaliseAuxTimerNames),
|
||||
body('auxTimerNames').isArray().withMessage('auxTimerNames must be an array').customSanitizer(sanitiseAuxTimerNames),
|
||||
|
||||
requestValidationFunction,
|
||||
];
|
||||
|
||||
@@ -5,7 +5,7 @@ import cookieParser from 'cookie-parser';
|
||||
import cors from 'cors';
|
||||
import express from 'express';
|
||||
import { LogOrigin, SimpleDirection, SimplePlayback, runtimeStorePlaceholder } from 'ontime-types';
|
||||
import { normaliseAuxTimerNames } from 'ontime-utils';
|
||||
import { sanitiseAuxTimerNames } from 'ontime-utils';
|
||||
import serverTiming from 'server-timing';
|
||||
|
||||
import { oscServer } from './adapters/OscAdapter.js';
|
||||
@@ -206,7 +206,7 @@ export const startServer = async (): Promise<{ message: string; serverPort: numb
|
||||
* Module initialises the services and provides initial payload for the store
|
||||
*/
|
||||
const state = getState();
|
||||
const auxTimerNames = normaliseAuxTimerNames(getDataProvider().getSettings().auxTimerNames);
|
||||
const [auxName1, auxName2, auxName3] = sanitiseAuxTimerNames(getDataProvider().getSettings().auxTimerNames);
|
||||
eventStore.init({
|
||||
clock: state.clock,
|
||||
timer: state.timer,
|
||||
@@ -222,27 +222,27 @@ export const startServer = async (): Promise<{ message: string; serverPort: numb
|
||||
current: timerConfig.auxTimerDefault,
|
||||
playback: SimplePlayback.Stop,
|
||||
direction: SimpleDirection.CountDown,
|
||||
name: auxTimerNames[0] ?? '',
|
||||
name: auxName1,
|
||||
},
|
||||
auxtimer2: {
|
||||
duration: timerConfig.auxTimerDefault,
|
||||
current: timerConfig.auxTimerDefault,
|
||||
playback: SimplePlayback.Stop,
|
||||
direction: SimpleDirection.CountDown,
|
||||
name: auxTimerNames[1] ?? '',
|
||||
name: auxName2,
|
||||
},
|
||||
auxtimer3: {
|
||||
duration: timerConfig.auxTimerDefault,
|
||||
current: timerConfig.auxTimerDefault,
|
||||
playback: SimplePlayback.Stop,
|
||||
direction: SimpleDirection.CountDown,
|
||||
name: auxTimerNames[2] ?? '',
|
||||
name: auxName3,
|
||||
},
|
||||
ping: 1,
|
||||
});
|
||||
|
||||
// seed the aux timer names onto the running service so they persist across commands
|
||||
auxTimerService.loadNames(auxTimerNames);
|
||||
// AuxTimerService owns its own SimpleTimer instances, so the store above doesn't update them
|
||||
auxTimerService.loadNames(getDataProvider().getSettings().auxTimerNames);
|
||||
|
||||
// initialise message service
|
||||
messageService.init(eventStore.set, eventStore.get);
|
||||
|
||||
@@ -29,9 +29,6 @@ export class SimpleTimer {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets the custom name of the timer
|
||||
*/
|
||||
public setName(name: string): SimpleTimerState {
|
||||
this.state.name = name;
|
||||
return this.state;
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { DatabaseModel, Rundown } from 'ontime-types';
|
||||
import { generateId, normaliseAuxTimerNames } from 'ontime-utils';
|
||||
import { generateId } from 'ontime-utils';
|
||||
|
||||
import { ONTIME_VERSION } from '../ONTIME_VERSION.js';
|
||||
|
||||
@@ -30,7 +30,7 @@ const dbModel: DatabaseModel = {
|
||||
operatorKey: null,
|
||||
timeFormat: '24',
|
||||
language: 'en',
|
||||
auxTimerNames: normaliseAuxTimerNames(),
|
||||
auxTimerNames: ['', '', ''],
|
||||
},
|
||||
viewSettings: {
|
||||
overrideStyles: false,
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import { DatabaseModel, OntimeView } from 'ontime-types';
|
||||
import { normaliseAuxTimerNames } from 'ontime-utils';
|
||||
|
||||
import { backstageRundown, broadcastRundown, stageRundown } from './demoRundowns.js';
|
||||
|
||||
@@ -30,7 +29,7 @@ export const demoDb: DatabaseModel = {
|
||||
operatorKey: null,
|
||||
timeFormat: '24',
|
||||
language: 'en',
|
||||
auxTimerNames: normaliseAuxTimerNames(),
|
||||
auxTimerNames: ['', '', ''],
|
||||
},
|
||||
viewSettings: {
|
||||
dangerColor: '#ff7300',
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { RuntimeStore, SimpleDirection, SimplePlayback } from 'ontime-types';
|
||||
import { normaliseAuxTimerNames } from 'ontime-utils';
|
||||
import { sanitiseAuxTimerNames } from 'ontime-utils';
|
||||
|
||||
import { SimpleTimer } from '../../classes/simple-timer/SimpleTimer.js';
|
||||
import { timerConfig } from '../../setup/config.js';
|
||||
@@ -26,13 +26,11 @@ export class AuxTimerService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Applies custom names to the aux timers and broadcasts the change.
|
||||
* Names are given in aux timer order (index 0 is aux timer 1).
|
||||
* Used to seed the names at bootstrap and to keep them in sync
|
||||
* with the settings of the loaded project.
|
||||
* Called at bootstrap and whenever the loaded project's settings change,
|
||||
* so the running timers reflect the current project's aux timer names.
|
||||
*/
|
||||
loadNames(names?: string[]) {
|
||||
const [name1, name2, name3] = normaliseAuxTimerNames(names);
|
||||
const [name1, name2, name3] = sanitiseAuxTimerNames(names);
|
||||
const patch: AuxTimerStateUpdate = {
|
||||
auxtimer1: this.aux1.setName(name1),
|
||||
auxtimer2: this.aux2.setName(name2),
|
||||
|
||||
@@ -90,7 +90,7 @@ async function loadProject(projectData: DatabaseModel, fileName: string, rundown
|
||||
// stop the runtime service
|
||||
runtimeService.stop();
|
||||
|
||||
// the aux timer names belong to the project, apply the ones from the newly loaded project
|
||||
// AuxTimerService holds its own state, independent of the loaded project, so it needs to be updated explicitly
|
||||
auxTimerService.loadNames(projectData.settings.auxTimerNames);
|
||||
|
||||
// load the rundown given by key otherwise load the first in the project
|
||||
@@ -352,7 +352,7 @@ export async function patchCurrentProject(data: Partial<DatabaseModel>) {
|
||||
// we can pass some stuff straight to the data provider
|
||||
await getDataProvider().mergeIntoData(rest);
|
||||
|
||||
// the settings may contain new aux timer names, apply them and notify the clients
|
||||
// AuxTimerService holds its own state, so a settings patch needs to be applied to it explicitly
|
||||
if (rest.settings) {
|
||||
auxTimerService.loadNames(getDataProvider().getSettings().auxTimerNames);
|
||||
sendRefetch(RefetchKey.Settings);
|
||||
|
||||
Reference in New Issue
Block a user