diff --git a/apps/client/src/common/components/input/text-input/__tests__/useReactiveTextInput.test.ts b/apps/client/src/common/components/input/text-input/__tests__/useReactiveTextInput.test.ts new file mode 100644 index 000000000..31ce728b7 --- /dev/null +++ b/apps/client/src/common/components/input/text-input/__tests__/useReactiveTextInput.test.ts @@ -0,0 +1,31 @@ +import { shouldSubmitValue } from '../useReactiveTextInput'; + +describe('shouldSubmitValue()', () => { + it('submits a value which differs from the initial value', () => { + expect(shouldSubmitValue('new', 'old')).toBe(true); + }); + + it('does not submit a value which has not changed', () => { + expect(shouldSubmitValue('same', 'same')).toBe(false); + }); + + it('submits an empty value when the initial value is known', () => { + // clearing a field which has a value is a valid edit + expect(shouldSubmitValue('', 'old')).toBe(true); + }); + + it('does not submit an empty value when the initial value is unknown', () => { + // an unknown value renders as an empty field, submitting it on blur + // would overwrite every entry being edited with an empty value + expect(shouldSubmitValue('', undefined)).toBe(false); + }); + + it('submits a typed value when the initial value is unknown', () => { + expect(shouldSubmitValue('typed', undefined)).toBe(true); + }); + + it('submits an unchanged value when the caller opts in', () => { + expect(shouldSubmitValue('same', 'same', true)).toBe(true); + expect(shouldSubmitValue('', undefined, true)).toBe(true); + }); +}); diff --git a/apps/client/src/common/components/input/text-input/useReactiveTextInput.tsx b/apps/client/src/common/components/input/text-input/useReactiveTextInput.tsx index f0cc54e39..7df9fcd27 100644 --- a/apps/client/src/common/components/input/text-input/useReactiveTextInput.tsx +++ b/apps/client/src/common/components/input/text-input/useReactiveTextInput.tsx @@ -1,6 +1,23 @@ import { HotkeyItem, getHotkeyHandler } from '@mantine/hooks'; import { ChangeEvent, KeyboardEvent, RefObject, useCallback, useEffect, useMemo, useRef, useState } from 'react'; +/** + * Whether a value should be sent to the server + * An undefined initial value means that the value is unknown, for example when several + * entries are being edited and they do not agree on a value + * It is shown as an empty field, so submitting it would overwrite data the user has not edited + */ +export function shouldSubmitValue( + valueToSubmit: string, + initialText: string | undefined, + allowSubmitSameValue?: boolean, +): boolean { + if (allowSubmitSameValue) { + return true; + } + return valueToSubmit !== (initialText ?? ''); +} + interface UseReactiveTextInputReturn { value: string; onChange: (event: ChangeEvent) => void; @@ -58,9 +75,7 @@ export default function useReactiveTextInput( */ const handleSubmit = useCallback( (valueToSubmit: string) => { - // No need to update if it hasn't changed - // an undefined initial value is shown as an empty field, submitting it would overwrite the underlying data - if (valueToSubmit === (initialText ?? '') && !options?.allowSubmitSameValue) { + if (!shouldSubmitValue(valueToSubmit, initialText, options?.allowSubmitSameValue)) { options?.onCancelUpdate?.(); } else { const cleanVal = valueToSubmit.trim(); diff --git a/apps/client/src/common/hooks/__tests__/entryAction.utils.test.ts b/apps/client/src/common/hooks/__tests__/entryAction.utils.test.ts new file mode 100644 index 000000000..95da33ac5 --- /dev/null +++ b/apps/client/src/common/hooks/__tests__/entryAction.utils.test.ts @@ -0,0 +1,76 @@ +import { OntimeEvent, OntimeGroup, RundownEntries, SupportedEntry } from 'ontime-types'; + +import { applyPatchToEvents, canPredictBatchResult } from '../entryAction.utils'; + +function makeEntries(): RundownEntries { + return { + '1': { type: SupportedEntry.Event, id: '1', title: 'one', custom: { lx: 'a', sound: 'x' } } as OntimeEvent, + '2': { type: SupportedEntry.Event, id: '2', title: 'two', custom: { lx: 'b' } } as OntimeEvent, + '3': { type: SupportedEntry.Event, id: '3', title: 'three', custom: {} } as OntimeEvent, + group: { type: SupportedEntry.Group, id: 'group', title: 'a group', custom: {} } as OntimeGroup, + }; +} + +describe('applyPatchToEvents()', () => { + it('applies the patch to the given events only', () => { + const patched = applyPatchToEvents(makeEntries(), ['1', '2'], { title: 'patched' }); + + expect((patched['1'] as OntimeEvent).title).toBe('patched'); + expect((patched['2'] as OntimeEvent).title).toBe('patched'); + expect((patched['3'] as OntimeEvent).title).toBe('three'); + }); + + it('patches custom fields instead of replacing them', () => { + const patched = applyPatchToEvents(makeEntries(), ['1', '2'], { custom: { lx: 'new' } }); + + // the field which was not part of the patch must survive + expect((patched['1'] as OntimeEvent).custom).toStrictEqual({ lx: 'new', sound: 'x' }); + expect((patched['2'] as OntimeEvent).custom).toStrictEqual({ lx: 'new' }); + }); + + it('keeps the existing custom fields when the patch has none', () => { + const patched = applyPatchToEvents(makeEntries(), ['1'], { title: 'patched' }); + + expect((patched['1'] as OntimeEvent).custom).toStrictEqual({ lx: 'a', sound: 'x' }); + }); + + it('ignores ids which are not in the rundown', () => { + const patched = applyPatchToEvents(makeEntries(), ['1', 'does-not-exist'], { title: 'patched' }); + + expect(Object.keys(patched)).toStrictEqual(['1', '2', '3', 'group']); + expect((patched['1'] as OntimeEvent).title).toBe('patched'); + }); + + it('ignores entries which are not events', () => { + const patched = applyPatchToEvents(makeEntries(), ['group'], { title: 'patched' }); + + expect((patched.group as OntimeGroup).title).toBe('a group'); + }); + + it('does not mutate the given entries', () => { + const entries = makeEntries(); + applyPatchToEvents(entries, ['1'], { title: 'patched', custom: { lx: 'new' } }); + + expect((entries['1'] as OntimeEvent).title).toBe('one'); + expect((entries['1'] as OntimeEvent).custom).toStrictEqual({ lx: 'a', sound: 'x' }); + }); + + it('handles repeated ids', () => { + const patched = applyPatchToEvents(makeEntries(), ['1', '1'], { title: 'patched' }); + + expect((patched['1'] as OntimeEvent).title).toBe('patched'); + }); +}); + +describe('canPredictBatchResult()', () => { + it('allows resolving patches which do not affect the schedule', () => { + expect(canPredictBatchResult({ title: 'a title' })).toBe(true); + expect(canPredictBatchResult({ colour: 'red', flag: true })).toBe(true); + }); + + it('defers to the server when the duration changes', () => { + // a duration cascades into the start and end of every linked event + expect(canPredictBatchResult({ duration: 1000 })).toBe(false); + expect(canPredictBatchResult({ title: 'a title', duration: 1000 })).toBe(false); + }); +}); diff --git a/apps/client/src/common/hooks/entryAction.utils.ts b/apps/client/src/common/hooks/entryAction.utils.ts new file mode 100644 index 000000000..126bb788d --- /dev/null +++ b/apps/client/src/common/hooks/entryAction.utils.ts @@ -0,0 +1,39 @@ +import { EntryId, OntimeEvent, RundownEntries, isOntimeEvent } from 'ontime-types'; + +/** + * Applies a patch to a set of events, used to optimistically resolve a batch edit + * Entries which are missing or are not events are left untouched, mirroring the server + * @returns a new entries object, the given entries are not mutated + */ +export function applyPatchToEvents( + entries: RundownEntries, + ids: EntryId[], + patch: Partial, +): RundownEntries { + const patchedEntries = { ...entries }; + + for (const id of new Set(ids)) { + const entry = patchedEntries[id]; + if (!isOntimeEvent(entry)) { + continue; + } + + patchedEntries[id] = { + ...entry, + ...patch, + // custom fields are patched, not replaced + custom: patch.custom ? { ...entry.custom, ...patch.custom } : entry.custom, + }; + } + + return patchedEntries; +} + +/** + * Whether the result of a batch edit can be resolved without the server + * Changing the duration cascades through the rundown, so the resulting + * schedule is only known once the server has recalculated it + */ +export function canPredictBatchResult(patch: Partial): boolean { + return !('duration' in patch); +} diff --git a/apps/client/src/common/hooks/useEntryAction.ts b/apps/client/src/common/hooks/useEntryAction.ts index 26274449e..f500e136b 100644 --- a/apps/client/src/common/hooks/useEntryAction.ts +++ b/apps/client/src/common/hooks/useEntryAction.ts @@ -52,6 +52,7 @@ import { } from '../api/rundown'; import { logAxiosError } from '../api/utils'; import { useEditorSettings } from '../stores/editorSettings'; +import { applyPatchToEvents, canPredictBatchResult } from './entryAction.utils'; export type EventOptions = Partial<{ // options of any new entries (event / delay / group) @@ -483,28 +484,9 @@ function useEntryActionsForRundown(scopedRundownId: string | undefined) { // Snapshot the previous value const previousRundown = queryClient.getQueryData(queryKey); - // changing the duration cascades through the rundown, we cannot resolve the new schedule here - // we skip the optimistic update and wait for the recalculated rundown from the server - const canPredictResult = !('duration' in data.data); - - if (previousRundown && canPredictResult) { - const { data: patch, ids } = data; - const eventIds = new Set(ids); - const newRundown = { ...previousRundown.entries }; - - eventIds.forEach((eventId) => { - if (Object.hasOwn(newRundown, eventId)) { - const event = newRundown[eventId]; - if (isOntimeEvent(event)) { - newRundown[eventId] = { - ...event, - ...patch, - // custom fields are patched, not replaced - custom: patch.custom ? { ...event.custom, ...patch.custom } : event.custom, - }; - } - } - }); + // when the result cannot be resolved here we wait for the recalculated rundown from the server + if (previousRundown && canPredictBatchResult(data.data)) { + const newRundown = applyPatchToEvents(previousRundown.entries, data.ids, data.data); queryClient.setQueryData(queryKey, { id: previousRundown.id, diff --git a/apps/client/src/features/rundown/entry-editor/EntryEditor.module.scss b/apps/client/src/features/rundown/entry-editor/EntryEditor.module.scss index 1cf8d1328..47b582f52 100644 --- a/apps/client/src/features/rundown/entry-editor/EntryEditor.module.scss +++ b/apps/client/src/features/rundown/entry-editor/EntryEditor.module.scss @@ -75,6 +75,26 @@ gap: 1rem; } +// a label with an action aligned to the end of the row +.labelRow { + display: flex; + align-items: baseline; + justify-content: space-between; + gap: 0.5rem; +} + +.clearAction { + font-size: $aux-text-size; + color: $blue-400; + margin-bottom: $element-inner-spacing; // match the label + text-decoration: underline; + text-underline-offset: 2px; + + &:hover { + color: $blue-500; + } +} + .splitTwo { display: grid; grid-template-columns: 1fr 1fr; diff --git a/apps/client/src/features/rundown/entry-editor/composite/EventTextArea.tsx b/apps/client/src/features/rundown/entry-editor/composite/EventTextArea.tsx index 81cf2d76e..8b90b7a36 100644 --- a/apps/client/src/features/rundown/entry-editor/composite/EventTextArea.tsx +++ b/apps/client/src/features/rundown/entry-editor/composite/EventTextArea.tsx @@ -5,6 +5,8 @@ import { AutoTextarea } from '../../../../common/components/input/auto-textarea/ import useReactiveTextInput from '../../../../common/components/input/text-input/useReactiveTextInput'; import { EventEditorUpdateFields } from '../EventEditor'; +import style from '../EntryEditor.module.scss'; + interface CountedTextAreaProps { className?: string; field: EventEditorUpdateFields; @@ -32,11 +34,21 @@ export default function EventTextArea({ submitOnCtrlEnter: true, }); + // an unknown value cannot be cleared by emptying the field, we offer an explicit action + const canClear = initialValue === undefined; + return (
- - {label} - +
+ + {label} + + {canClear && ( + + )} +
- - {label} - +
+ + {label} + + {canClear && ( + + )} +
{ await expect(page.getByTestId('entry-1').getByTestId('entry__title')).toHaveValue('first'); await expect(page.getByTestId('entry-2').getByTestId('entry__title')).toHaveValue('second'); + // a mixed field cannot be cleared by emptying it, so an explicit action is offered + const clearTitle = editor.getByRole('button', { name: 'Clear', exact: true }).first(); + await expect(clearTitle).toBeVisible(); + await clearTitle.click(); + await expect(page.getByTestId('entry-1').getByTestId('entry__title')).toHaveValue(''); + await expect(page.getByTestId('entry-2').getByTestId('entry__title')).toHaveValue(''); + + // the events now agree, so the field can be cleared by hand and the action is withdrawn + await expect(editor.getByLabel('Title', { exact: true })).not.toHaveAttribute('placeholder', 'Mixed'); + // editing a field applies it to the whole selection await title.fill('shared title'); await title.press('Enter');