mirror of
https://github.com/cpvalente/ontime.git
synced 2026-08-19 14:14:17 +00:00
refactor(finder): flatten the result type and trim the tests
The three-variant result union differed only in whether an entry had a cue, an event index and a parent, and the UI narrowed it with runtime property checks anyway, so the discrimination bought nothing while costing three near-identical branches to build a result. One flat type removes those branches and the checks around them. Skipping non-searchable entries asked whether an entry was a group or a milestone, then asked again when building the result. It now asks once whether it is a delay, which is what the guard actually meant. The search functions move out of the memo closure to the module, where they are ordinary pure functions. Whether to show the matched field compared against its display label, so a custom field labelled "Title" would have had its match hidden. It now compares the field key. Drops the review document, which had served its purpose, and reduces the finder tests to a single one covering the path a user takes: open from a focused field, find an entry by its note, scope with a badge, and reveal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DzALEq9gGWwFmwTdgAiFcY
This commit is contained in:
Claude
@@ -1,10 +1,10 @@
|
||||
import { EntryId, MaybeString, SupportedEntry } from 'ontime-types';
|
||||
import { EntryId, MaybeString } from 'ontime-types';
|
||||
import { KeyboardEvent, PointerEvent, useDeferredValue, useEffect, useRef, useState } from 'react';
|
||||
|
||||
import Input from '../../../common/components/input/input/Input';
|
||||
import Kbd from '../../../common/components/kbd/Kbd';
|
||||
import Modal from '../../../common/components/modal/Modal';
|
||||
import useFinder, { FilterableEntry } from './useFinder';
|
||||
import useFinder, { FinderResult } from './useFinder';
|
||||
|
||||
import style from './Finder.module.scss';
|
||||
|
||||
@@ -69,7 +69,7 @@ export default function Finder({ isOpen, onClose }: FinderProps) {
|
||||
}
|
||||
};
|
||||
|
||||
const submit = (entry: FilterableEntry | undefined) => {
|
||||
const submit = (entry: FinderResult | undefined) => {
|
||||
if (!entry) {
|
||||
return;
|
||||
}
|
||||
@@ -132,10 +132,8 @@ export default function Finder({ isOpen, onClose }: FinderProps) {
|
||||
{!error && results.length === 0 && <li className={style.empty}>No results</li>}
|
||||
{results.map((entry) => {
|
||||
const isSelected = activeEntry?.id === entry.id;
|
||||
const displayIndex = entry.type === SupportedEntry.Event ? entry.eventIndex : '-';
|
||||
const displayCue = 'cue' in entry ? entry.cue : '';
|
||||
// the title and cue are already on the row, anything else needs showing
|
||||
const showMatch = entry.match !== null && entry.match.label !== 'Title' && entry.match.label !== 'Cue';
|
||||
// the title and cue are already on the row, a match anywhere else needs showing
|
||||
const showMatch = entry.match !== null && entry.match.key !== 'title' && entry.match.key !== 'cue';
|
||||
|
||||
return (
|
||||
<li
|
||||
@@ -149,9 +147,9 @@ export default function Finder({ isOpen, onClose }: FinderProps) {
|
||||
>
|
||||
<div className={style.data}>
|
||||
<div className={style.index} style={{ '--color': entry.colour }}>
|
||||
{displayIndex}
|
||||
{entry.eventIndex ?? '-'}
|
||||
</div>
|
||||
<div className={style.cue}>{displayCue}</div>
|
||||
<div className={style.cue}>{entry.cue}</div>
|
||||
<div className={style.title}>{entry.title}</div>
|
||||
{showMatch && (
|
||||
<div className={style.match} data-testid='finder-result-match'>
|
||||
|
||||
@@ -1,12 +1,14 @@
|
||||
import {
|
||||
CustomFields,
|
||||
EntryId,
|
||||
MaybeNumber,
|
||||
MaybeString,
|
||||
OntimeEntry,
|
||||
SupportedEntry,
|
||||
OntimeEvent,
|
||||
OntimeGroup,
|
||||
OntimeMilestone,
|
||||
isOntimeDelay,
|
||||
isOntimeEvent,
|
||||
isOntimeGroup,
|
||||
isOntimeMilestone,
|
||||
} from 'ontime-types';
|
||||
import { useCallback, useMemo } from 'react';
|
||||
|
||||
@@ -21,69 +23,75 @@ const excerptPadding = 40;
|
||||
|
||||
const indexFilter = 'index';
|
||||
|
||||
/**
|
||||
* A field the user can scope a search to.
|
||||
* Custom fields are appended to these at runtime.
|
||||
*/
|
||||
const staticFilters = [
|
||||
/** Everything except delays, which carry no text to search */
|
||||
type SearchableEntry = OntimeEvent | OntimeGroup | OntimeMilestone;
|
||||
|
||||
type FinderFilter = { key: string; label: string };
|
||||
|
||||
/** The fields common to every project, custom fields are appended at runtime */
|
||||
const staticFilters: FinderFilter[] = [
|
||||
{ key: indexFilter, label: 'Index' },
|
||||
{ key: 'cue', label: 'Cue' },
|
||||
{ key: 'title', label: 'Title' },
|
||||
{ key: 'note', label: 'Note' },
|
||||
] as const;
|
||||
];
|
||||
|
||||
export type FinderFilter = { key: string; label: string };
|
||||
/** Why an entry matched, so the UI can show the user */
|
||||
type FinderMatch = { key: string; label: string; excerpt: string };
|
||||
|
||||
/** Describes why an entry matched, so the UI can show the user */
|
||||
export type FinderMatch = { label: string; excerpt: string };
|
||||
|
||||
type FilterableBase = {
|
||||
export type FinderResult = {
|
||||
id: EntryId;
|
||||
/** position in the flat rundown, which is how the rundown reveals an entry */
|
||||
index: number;
|
||||
/** 1-based position among events, null for groups and milestones */
|
||||
eventIndex: MaybeNumber;
|
||||
title: string;
|
||||
/** groups have no cue */
|
||||
cue: string;
|
||||
colour: string;
|
||||
parent: MaybeString;
|
||||
/** absent when the entry was found by index rather than by matching text */
|
||||
match: FinderMatch | null;
|
||||
};
|
||||
|
||||
type FilterableGroup = FilterableBase & { type: SupportedEntry.Group };
|
||||
type FilterableEvent = FilterableBase & {
|
||||
type: SupportedEntry.Event;
|
||||
eventIndex: number;
|
||||
cue: string;
|
||||
parent: MaybeString;
|
||||
};
|
||||
type FilterableMilestone = FilterableBase & {
|
||||
type: SupportedEntry.Milestone;
|
||||
cue: string;
|
||||
parent: MaybeString;
|
||||
};
|
||||
type SearchOutcome = { results: FinderResult[]; error: MaybeString; total: number };
|
||||
|
||||
export type FilterableEntry = FilterableGroup | FilterableEvent | FilterableMilestone;
|
||||
const noResults: SearchOutcome = { results: [], error: null, total: 0 };
|
||||
|
||||
type SearchableField = { key: string; label: string; value: string };
|
||||
function toResult(entry: SearchableEntry, index: number, eventIndex: MaybeNumber, match: FinderMatch | null) {
|
||||
return {
|
||||
id: entry.id,
|
||||
index,
|
||||
eventIndex,
|
||||
title: entry.title,
|
||||
cue: 'cue' in entry ? entry.cue : '',
|
||||
colour: entry.colour,
|
||||
parent: 'parent' in entry ? entry.parent : null,
|
||||
match,
|
||||
} satisfies FinderResult;
|
||||
}
|
||||
|
||||
type SearchableField = FinderFilter & { value: string };
|
||||
|
||||
/**
|
||||
* Collects the text fields of an entry, in the order we prefer to report a match.
|
||||
* The text fields of an entry, in the order we prefer to report a match.
|
||||
* Image custom fields hold a URL, which nobody searches for.
|
||||
*/
|
||||
function getSearchableFields(entry: OntimeEntry, customFields: CustomFields): SearchableField[] {
|
||||
function getSearchableFields(entry: SearchableEntry, customFields: CustomFields): SearchableField[] {
|
||||
const fields: SearchableField[] = [];
|
||||
|
||||
if ('cue' in entry && entry.cue) {
|
||||
fields.push({ key: 'cue', label: 'Cue', value: entry.cue });
|
||||
}
|
||||
if ('title' in entry && entry.title) {
|
||||
if (entry.title) {
|
||||
fields.push({ key: 'title', label: 'Title', value: entry.title });
|
||||
}
|
||||
if ('note' in entry && entry.note) {
|
||||
if (entry.note) {
|
||||
fields.push({ key: 'note', label: 'Note', value: entry.note });
|
||||
}
|
||||
if ('custom' in entry) {
|
||||
for (const [key, value] of Object.entries(entry.custom)) {
|
||||
const definition = customFields[key];
|
||||
if (!value || definition?.type !== 'text') {
|
||||
continue;
|
||||
}
|
||||
for (const [key, value] of Object.entries(entry.custom)) {
|
||||
const definition = customFields[key];
|
||||
if (value && definition?.type === 'text') {
|
||||
fields.push({ key, label: definition.label || key, value });
|
||||
}
|
||||
}
|
||||
@@ -98,16 +106,34 @@ function makeExcerpt(value: string, matchIndex: number, searchLength: number): s
|
||||
return `${start > 0 ? '…' : ''}${value.slice(start, end)}${end < value.length ? '…' : ''}`;
|
||||
}
|
||||
|
||||
/** The first field of an entry to contain the search string, if any */
|
||||
function findMatch(
|
||||
entry: SearchableEntry,
|
||||
customFields: CustomFields,
|
||||
filterKey: MaybeString,
|
||||
searchString: string,
|
||||
): FinderMatch | null {
|
||||
for (const field of getSearchableFields(entry, customFields)) {
|
||||
if (filterKey !== null && field.key !== filterKey) {
|
||||
continue;
|
||||
}
|
||||
const matchIndex = field.value.toLowerCase().indexOf(searchString);
|
||||
if (matchIndex !== -1) {
|
||||
return { key: field.key, label: field.label, excerpt: makeExcerpt(field.value, matchIndex, searchString.length) };
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Splits the raw search value into an optional field filter and the text to look for.
|
||||
* Both `cue 12` and `cue:12` are accepted so the filter badges and typing agree.
|
||||
* Both `cue 12` and `cue:12` are accepted so that typing agrees with the filter badges.
|
||||
*/
|
||||
function parseQuery(searchValue: string, filters: FinderFilter[]) {
|
||||
for (const filter of filters) {
|
||||
// the value is already lowercased, custom field keys are not
|
||||
// the search value is already lowercased, custom field keys are not
|
||||
const prefix = filter.key.toLowerCase();
|
||||
if (searchValue === prefix) {
|
||||
// the filter is selected but nothing to search for yet
|
||||
return { filterKey: filter.key, searchString: '' };
|
||||
}
|
||||
if (searchValue.startsWith(`${prefix} `) || searchValue.startsWith(`${prefix}:`)) {
|
||||
@@ -117,6 +143,67 @@ function parseQuery(searchValue: string, filters: FinderFilter[]) {
|
||||
return { filterKey: null, searchString: searchValue };
|
||||
}
|
||||
|
||||
/** Finds the single event at a 1-based position in the rundown */
|
||||
function searchByIndex(data: OntimeEntry[], indexString: string): SearchOutcome {
|
||||
const target = Number(indexString);
|
||||
if (isNaN(target) || target < 1) {
|
||||
return { ...noResults, error: 'Invalid index' };
|
||||
}
|
||||
|
||||
let eventIndex = 0;
|
||||
for (let i = 0; i < data.length; i++) {
|
||||
const entry = data[i];
|
||||
if (!isOntimeEvent(entry)) {
|
||||
continue;
|
||||
}
|
||||
eventIndex++;
|
||||
if (eventIndex === target) {
|
||||
return { results: [toResult(entry, i, eventIndex, null)], error: null, total: 1 };
|
||||
}
|
||||
}
|
||||
|
||||
return noResults;
|
||||
}
|
||||
|
||||
/**
|
||||
* Matches entries on a single field when one is selected, otherwise on every text field.
|
||||
* Results keep rundown order, which keeps them predictable during a show.
|
||||
*/
|
||||
function searchByText(
|
||||
data: OntimeEntry[],
|
||||
customFields: CustomFields,
|
||||
filterKey: MaybeString,
|
||||
searchString: string,
|
||||
): SearchOutcome {
|
||||
const results: FinderResult[] = [];
|
||||
let total = 0;
|
||||
// indexes exposed to the UI are 1-based
|
||||
let eventIndex = 0;
|
||||
|
||||
for (let i = 0; i < data.length; i++) {
|
||||
const entry = data[i];
|
||||
if (isOntimeDelay(entry)) {
|
||||
continue;
|
||||
}
|
||||
const isEvent = isOntimeEvent(entry);
|
||||
if (isEvent) {
|
||||
eventIndex++;
|
||||
}
|
||||
|
||||
const match = findMatch(entry, customFields, filterKey, searchString);
|
||||
if (match === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
total++;
|
||||
if (results.length < maxResults) {
|
||||
results.push(toResult(entry, i, isEvent ? eventIndex : null, match));
|
||||
}
|
||||
}
|
||||
|
||||
return { results, error: null, total };
|
||||
}
|
||||
|
||||
/**
|
||||
* @param searchValue - the text the user is looking for
|
||||
* @param activeFilter - a field selected from the filter badges, if any
|
||||
@@ -127,7 +214,7 @@ export default function useFinder(searchValue: string, activeFilter: MaybeString
|
||||
|
||||
const selectAndRevealEntry = useSelectAndRevealEntry(rundownId);
|
||||
|
||||
/** The filters offered to the user: fixed fields plus whatever the project defines */
|
||||
/** The filters offered to the user: the fixed fields plus whatever the project defines */
|
||||
const filters = useMemo<FinderFilter[]>(() => {
|
||||
const customFilters = Object.entries(customFields)
|
||||
.filter(([_key, field]) => field.type === 'text')
|
||||
@@ -136,165 +223,36 @@ export default function useFinder(searchValue: string, activeFilter: MaybeString
|
||||
}, [customFields]);
|
||||
|
||||
const { results, error, total, appliedFilter } = useMemo(() => {
|
||||
const empty = { results: [] as FilterableEntry[], error: null, total: 0, appliedFilter: activeFilter };
|
||||
|
||||
if (!data || data.length === 0) {
|
||||
return { ...empty, error: 'No data' };
|
||||
if (data.length === 0) {
|
||||
return { ...noResults, error: 'No data', appliedFilter: activeFilter };
|
||||
}
|
||||
|
||||
const normalised = searchValue.trim().toLowerCase();
|
||||
if (normalised === '') {
|
||||
return empty;
|
||||
return { ...noResults, appliedFilter: activeFilter };
|
||||
}
|
||||
|
||||
/**
|
||||
* A selected badge wins, but typing a prefix still works for anyone who knows the
|
||||
* keywords, and lights up the matching badge rather than being silently ignored.
|
||||
* A selected badge wins, but typing a keyword still works for anyone who knows them,
|
||||
* and lights up the matching badge rather than being silently ignored.
|
||||
*/
|
||||
const { filterKey, searchString } = activeFilter
|
||||
? { filterKey: activeFilter, searchString: normalised }
|
||||
: parseQuery(normalised, filters);
|
||||
|
||||
if (filterKey === indexFilter) {
|
||||
return { ...searchByIndex(searchString), appliedFilter: filterKey };
|
||||
return { ...searchByIndex(data, searchString), appliedFilter: filterKey };
|
||||
}
|
||||
|
||||
if (searchString === '') {
|
||||
// a filter is selected, but there is nothing to match on yet
|
||||
return empty;
|
||||
}
|
||||
|
||||
return { ...searchByField(filterKey, searchString), appliedFilter: filterKey };
|
||||
|
||||
/** Returns the single event at a given 1-based index */
|
||||
function searchByIndex(indexString: string) {
|
||||
const searchIndex = Number(indexString);
|
||||
if (isNaN(searchIndex) || searchIndex < 1) {
|
||||
return { ...empty, error: 'Invalid index' };
|
||||
}
|
||||
|
||||
let eventIndex = 1;
|
||||
for (let i = 0; i < data.length; i++) {
|
||||
const entry = data[i];
|
||||
if (!isOntimeEvent(entry)) {
|
||||
continue;
|
||||
}
|
||||
if (eventIndex === searchIndex) {
|
||||
return {
|
||||
error: null,
|
||||
total: 1,
|
||||
results: [
|
||||
{
|
||||
type: SupportedEntry.Event,
|
||||
id: entry.id,
|
||||
index: i,
|
||||
eventIndex,
|
||||
title: entry.title,
|
||||
cue: entry.cue,
|
||||
colour: entry.colour,
|
||||
parent: entry.parent,
|
||||
match: null,
|
||||
} satisfies FilterableEvent,
|
||||
],
|
||||
};
|
||||
}
|
||||
eventIndex++;
|
||||
}
|
||||
|
||||
return empty;
|
||||
}
|
||||
|
||||
/**
|
||||
* Matches entries on a single field when one is selected, otherwise on every text field.
|
||||
* Results keep rundown order, which keeps them predictable during a show.
|
||||
*/
|
||||
function searchByField(key: MaybeString, searchString: string) {
|
||||
const results: FilterableEntry[] = [];
|
||||
let total = 0;
|
||||
// indexes exposed to the UI are 1-based
|
||||
let eventIndex = 1;
|
||||
|
||||
for (let i = 0; i < data.length; i++) {
|
||||
const entry = data[i];
|
||||
const isEvent = isOntimeEvent(entry);
|
||||
const currentEventIndex = eventIndex;
|
||||
if (isEvent) {
|
||||
eventIndex++;
|
||||
}
|
||||
|
||||
if (!isEvent && !isOntimeGroup(entry) && !isOntimeMilestone(entry)) {
|
||||
// delays carry no text to search
|
||||
continue;
|
||||
}
|
||||
|
||||
const candidates = getSearchableFields(entry, customFields).filter(
|
||||
(field) => key === null || field.key === key,
|
||||
);
|
||||
|
||||
let match: FinderMatch | null = null;
|
||||
for (const field of candidates) {
|
||||
const matchIndex = field.value.toLowerCase().indexOf(searchString);
|
||||
if (matchIndex !== -1) {
|
||||
match = { label: field.label, excerpt: makeExcerpt(field.value, matchIndex, searchString.length) };
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (match === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
total++;
|
||||
if (results.length >= maxResults) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (isEvent) {
|
||||
results.push({
|
||||
type: SupportedEntry.Event,
|
||||
id: entry.id,
|
||||
index: i,
|
||||
eventIndex: currentEventIndex,
|
||||
title: entry.title,
|
||||
cue: entry.cue,
|
||||
colour: entry.colour,
|
||||
parent: entry.parent,
|
||||
match,
|
||||
} satisfies FilterableEvent);
|
||||
} else if (isOntimeGroup(entry)) {
|
||||
results.push({
|
||||
type: SupportedEntry.Group,
|
||||
id: entry.id,
|
||||
index: i,
|
||||
title: entry.title,
|
||||
colour: entry.colour,
|
||||
match,
|
||||
} satisfies FilterableGroup);
|
||||
} else {
|
||||
results.push({
|
||||
type: SupportedEntry.Milestone,
|
||||
id: entry.id,
|
||||
index: i,
|
||||
title: entry.title,
|
||||
cue: entry.cue,
|
||||
colour: entry.colour,
|
||||
parent: entry.parent,
|
||||
match,
|
||||
} satisfies FilterableMilestone);
|
||||
}
|
||||
}
|
||||
|
||||
return { results, error: null, total };
|
||||
return { ...noResults, appliedFilter: filterKey };
|
||||
}
|
||||
return { ...searchByText(data, customFields, filterKey, searchString), appliedFilter: filterKey };
|
||||
}, [data, customFields, filters, searchValue, activeFilter]);
|
||||
|
||||
const select = useCallback(
|
||||
(selectedEvent: FilterableEntry) => {
|
||||
selectAndRevealEntry({
|
||||
id: selectedEvent.id,
|
||||
index: selectedEvent.index,
|
||||
parent: 'parent' in selectedEvent ? selectedEvent.parent : null,
|
||||
});
|
||||
(result: FinderResult) => {
|
||||
selectAndRevealEntry({ id: result.id, index: result.index, parent: result.parent });
|
||||
},
|
||||
[selectAndRevealEntry],
|
||||
);
|
||||
|
||||
@@ -1,337 +0,0 @@
|
||||
# Finder — review and expansion roadmap
|
||||
|
||||
## Context
|
||||
|
||||
The Finder is a `mod+F` modal that searches the rundown and jumps to an entry. At the time of this
|
||||
review it was four files and ~370 lines, essentially untouched since introduction, with no tests and
|
||||
mounted only in the rundown editor.
|
||||
|
||||
This document reviews the feature and sets out a roadmap. The framing question was a
|
||||
business-development one: is it worth investing in, and can it beat the competition?
|
||||
|
||||
Scope decided for the roadmap: **editor + cuesheet**, **navigation-only** (the Finder reveals
|
||||
entries, it does not act on them), and **results stay in rundown order** — no ranking.
|
||||
|
||||
| File | Role |
|
||||
| ----------------------------------------------------------------- | --------------------------------------- |
|
||||
| `apps/client/src/views/editor/finder/Finder.tsx` | Modal, input, result list, footer hints |
|
||||
| `apps/client/src/views/editor/finder/useFinder.tsx` | Search logic |
|
||||
| `apps/client/src/views/editor/finder/Finder.module.scss` | Styling |
|
||||
| `apps/client/src/features/rundown/placements/FinderPlacement.tsx` | `mod+F` hotkey + mount |
|
||||
|
||||
---
|
||||
|
||||
## 1. Review findings
|
||||
|
||||
_This section records the feature as it stood when reviewed, and is kept as the rationale for the
|
||||
roadmap below. The findings in 1.1, 1.2 and 1.6 have since been addressed — see phases 1 and 2._
|
||||
|
||||
### 1.1 It doesn't search what people search for
|
||||
|
||||
A bare query is **title-only**. The two things an operator has in their head are the **cue** and
|
||||
something written in a **note** or **custom field**.
|
||||
|
||||
- Typing `Q12` finds nothing — you must know to type `cue Q12`.
|
||||
- `note` is never searched, on any entry type.
|
||||
- `custom` fields are never searched. This is the real indictment: Ontime invites teams to model
|
||||
their show in custom fields, then makes that data unfindable.
|
||||
- Milestones carry a `cue` in the type, but only their title is matched.
|
||||
- Prefixes are brittle: `INDEX 4` works (input is lowercased) but `index4` and `cue:Q12` don't, and
|
||||
any title beginning "cue " or "title " can't be found as typed.
|
||||
|
||||
### 1.2 Two live bugs
|
||||
|
||||
- **Stale selection.** `selected` is never reset when `results` changes — not on a new query, not on
|
||||
the background-refetch replay at `useFinder.tsx:228-236`. If the list shrinks, `results[selected]`
|
||||
is `undefined` and `select()` throws on `.id`.
|
||||
- **Click hits the wrong row.** `onClick={submit}` reads `selected` from state, not the clicked
|
||||
element. It works only because a `mousemove` normally precedes the click — touch input navigates to
|
||||
the wrong entry. That's precisely the backstage-tablet case.
|
||||
|
||||
Two smaller ones: `mod+F` doesn't close the modal (Mantine's `useHotkeys` ignores `INPUT`, so the
|
||||
toggle stops firing once focus is in the box), and the `index <n>` bound compares against the flat
|
||||
entry count instead of the event count.
|
||||
|
||||
### 1.3 Nobody can find the Finder
|
||||
|
||||
There is **no visible affordance anywhere**. The only discovery path is the shortcut cheat sheet in
|
||||
`EventEditorEmpty.tsx` — which you see only when nothing is selected. Neither `RundownHeader.tsx` nor
|
||||
the cuesheet toolbar has a search control.
|
||||
|
||||
On touch there is no keyboard, so `RundownHeaderMobile.tsx` having no search button means the Finder
|
||||
is **completely unreachable on mobile and tablet** — the devices where finding an event by scrolling
|
||||
is hardest.
|
||||
|
||||
### 1.4 Reach
|
||||
|
||||
Editor-only. But `apps/client/src/views/cuesheet/cuesheet-table/CuesheetTable.tsx:160` already
|
||||
registers a scroll handler commented _"for explicit jumps (finder/keyboard)"_ — the plumbing was
|
||||
anticipated and built, the Finder was never mounted there. Cheapest high-value win available.
|
||||
|
||||
**Constraint:** the cuesheet is `permission='operator'` and can be exposed via URL presets with
|
||||
per-column read permissions (`cuesheet.policies.ts`, `getCuesheetColumnAccessPolicy().canRead(key)`).
|
||||
Searching notes and custom fields there **must** filter through that policy, or a locked-down preset
|
||||
link leaks columns it was configured to hide. Data exposure, not a nicety.
|
||||
|
||||
### 1.5 Index source — a trap worth naming
|
||||
|
||||
`useFinder` re-implements the 1-based `eventIndex` walk that `common/utils/rundownMetadata.ts`
|
||||
already computes. Tempting to just reuse `useFlatRundownWithMetadata()` — **don't**: its memo depends
|
||||
on `selectedEventId`, so it recomputes on every event load during a show, and it spreads every entry.
|
||||
`useFlatRundown` has the same class of problem (memoised on the react-query object, replaced on every
|
||||
refetch). Build from raw `rundown.entries` + `flatOrder`, memoised on **`rundown.revision`**.
|
||||
|
||||
### 1.6 Tests
|
||||
|
||||
No unit or component tests, and `useSelectAndRevealEntry` is untested too. E2E coverage is a single
|
||||
smoke test — `Find in rundown` in `e2e/tests/features/209-rundown-shortcuts.spec.ts` opens the modal
|
||||
with `mod+F` and closes it with `Escape`. Nothing exercises searching, selecting, or navigating
|
||||
results, so every behaviour described above is unguarded.
|
||||
|
||||
---
|
||||
|
||||
## 2. Business-development assessment
|
||||
|
||||
**A command palette alone is table stakes** — every modern tool has one. Shipping `mod+K` wins nothing.
|
||||
|
||||
**A runtime-aware, multi-surface finder is a real wedge.** [Shoflo](https://shoflo.tv/),
|
||||
[Rundown Studio](https://rundownstudio.app/) and [Cuez](https://cuez.app/script-rundown/) are cloud
|
||||
rundown _editors_ — their search is document search over a document you're authoring. Ontime knows
|
||||
show state: what's loaded, what's past, the offset, what's flagged. A finder that marks the loaded
|
||||
event and shows scheduled time and delay per row is a different category of tool, and that data
|
||||
already exists in `RundownMetadata` and is currently discarded. Add that it works identically on the
|
||||
FOH editor and a backstage tablet, and it's a coherence story the single-web-app competitors can't tell.
|
||||
|
||||
**Honest counterweight: don't oversell this internally.** This is a retention and credibility feature,
|
||||
not an acquisition one. Nobody picks Ontime over Shoflo for a search box. What it does: removes a
|
||||
recurring "I can't find my event" friction that bites hardest at the worst moment, makes the
|
||||
custom-fields investment finally pay off, and makes Ontime feel professional to the power users who
|
||||
become advocates.
|
||||
|
||||
**And the biggest problems here are bugs, absent scope and invisibility — not missing sophistication.**
|
||||
Sequence accordingly.
|
||||
|
||||
---
|
||||
|
||||
## 3. Roadmap
|
||||
|
||||
Estimates assume one developer familiar with the codebase.
|
||||
|
||||
Phases 1 and 2 are **delivered**; the remaining phases are proposals. Items still outstanding inside
|
||||
a delivered phase are called out where they sit.
|
||||
|
||||
### Phase 1 — Correctness _(done)_
|
||||
|
||||
1. **Track `selectedId`, not `selectedIndex`** — derive the active row by id lookup, falling back to
|
||||
the first. Correct across updates, reordering and result changes; better than "reset to 0", which
|
||||
throws away the user's position on every refetch.
|
||||
2. **Pass the result to the click handler**; delete the `data-index` / `dataset.index` mechanism.
|
||||
3. **Ignore pointer-move events where the cursor hasn't moved** — otherwise arrow-key navigation
|
||||
scrolls the list under a stationary cursor, fires a move event, and yanks the selection back. The
|
||||
12-result cap hides this today; a longer list won't.
|
||||
4. **Scroll the active row into view** on arrow navigation.
|
||||
5. **`mod+F` opens and closes from anywhere.** The hotkey hook skips input elements by default, so
|
||||
the shortcut was dead while editing an entry — exactly when a user reaches for it. Opting out of
|
||||
that lets one binding both open and close. The global `Escape` handler is gone; Base UI's
|
||||
`Dialog` already dismisses, and `preventDefault: true` document-wide conflicted with inline
|
||||
field editing.
|
||||
6. **Fix the `index <n>` bound** to use the event count.
|
||||
7. **Search milestones by cue.** The cue search only walked events, so a milestone could never be
|
||||
found by the cue it displays.
|
||||
|
||||
### Phase 2 — Search depth _(done)_
|
||||
|
||||
8. **Search cue + title + note + text custom fields** across events, groups and milestones on a bare
|
||||
query. `image` custom fields are skipped — they hold a URL.
|
||||
9. **Filter badges.** The syntax was previously discoverable only through a line of footer text.
|
||||
A badge row now offers the fixed fields plus every project custom field, scoping the search while
|
||||
keeping what the user already typed. Both `cue x` and `cue:x` parse, so badges and typing agree.
|
||||
10. **Keep rundown order — no ranking.** Deterministic, and it keeps results predictable during a
|
||||
show. _The honest consequence:_ matching more fields while holding position order means a distant
|
||||
cue match can be pushed off by nearer note matches. Mitigated by raising the cap, reporting the
|
||||
true total, and naming the matched field per row.
|
||||
11. **Name the matching field** with an excerpt, so a hit inside a long note is legible.
|
||||
12. **Results are a pure derivation of (data, query)** — the controlled input removed the
|
||||
`useEffect` that replayed the last search on rundown changes, and with it the stale-index crash.
|
||||
|
||||
**Measured cost of widening the scan** — cue + title + note + 3 custom fields versus title alone:
|
||||
|
||||
| Rundown | Title only | All fields | All fields, prebuilt index |
|
||||
| ------- | ---------- | ---------- | -------------------------- |
|
||||
| 200 | 0.061 ms | 0.051 ms | 0.020 ms |
|
||||
| 1,000 | 0.149 ms | 0.112 ms | 0.020 ms |
|
||||
| 5,000 | 0.608 ms | 0.563 ms | 0.116 ms |
|
||||
|
||||
All-fields measures the same as title-only at every size — both are dominated by loop overhead
|
||||
rather than string comparison. Performance was never the reason to search one field.
|
||||
|
||||
**Still outstanding from this phase:** `flag:` and `group:` filters; highlighting the matched
|
||||
substring within the excerpt; unit tests for the query parser (covered by e2e today, but the parser
|
||||
is now a pure function and worth testing directly — the highest-value case is `index <n>` staying
|
||||
aligned with 1-based UI indices when delays, groups and milestones interleave).
|
||||
|
||||
### Phase 3 — Visibility, UX and polish _(~1 day, partly done)_
|
||||
|
||||
This is the phase that changes how many people ever use the feature.
|
||||
|
||||
#### Visibility
|
||||
|
||||
13. **Add a search control to `RundownHeader.tsx`** with the `mod+F` hint visible on it. Single
|
||||
biggest discoverability win.
|
||||
14. **Add one to `RundownHeaderMobile.tsx`** — mandatory, not optional: without a keyboard the
|
||||
feature currently does not exist on touch devices.
|
||||
15. **Add one to the cuesheet toolbar** when the Finder mounts there (Phase 4).
|
||||
|
||||
#### UX
|
||||
|
||||
16. **Fix the empty state** — it shows "No results" before you've typed anything. Show the filter
|
||||
hints, or recent searches, on open.
|
||||
17. **Align with the app's other search box.** `SettingsSearch.tsx` has a leading `IoSearch` icon and
|
||||
a clear button; the Finder has neither. Two search boxes in one app should look like one idea.
|
||||
18. **Show result count** ("showing 12 of 47") instead of a silent cap.
|
||||
19. **Indicate entry type** — events, groups and milestones are visually identical today apart from
|
||||
the index showing `-`.
|
||||
20. **Show group breadcrumbs**, so hits inside collapsed groups are legible.
|
||||
21. **Preserve the last query on reopen**, selected so typing replaces it.
|
||||
22. **`Home` / `End` / `PageUp` / `PageDown`** in the result list, matching `useRundownKeyboard`.
|
||||
|
||||
#### Polish
|
||||
|
||||
23. **Use `getAccessibleColour`** (`common/utils/styleUtils`) for the index badge. It currently sets
|
||||
`background: var(--color)` raw with fixed foreground text, so a light entry colour is unreadable.
|
||||
The cuesheet `EventRow`, `MilestoneRow` and `OperatorEvent` already do this correctly — the
|
||||
Finder is the odd one out.
|
||||
24. **Fix row text handling** — fixed `3rem` rows with no ellipsis on long titles, and `.cue` capped
|
||||
at `max-height: 1em`, which clips descenders.
|
||||
25. **Stop the `Go ⏎` label shifting layout** when it appears only on the selected row.
|
||||
26. **Give the modal a real header** instead of `title=''`, which renders an empty header area.
|
||||
27. **Reconsider the 100 ms debounce** — with a controlled input (item 11) it's unnecessary at these
|
||||
rundown sizes and just reads as lag.
|
||||
|
||||
### Phase 4 — Cuesheet _(~1 day)_
|
||||
|
||||
28. **Move the Finder out of `features/rundown/placements/`** — it stops being an editor view once it
|
||||
has two homes — and mount it in the cuesheet. Pass surface and permission explicitly from the
|
||||
mount site rather than inferring them.
|
||||
29. **Gate searchable fields through `getCuesheetColumnAccessPolicy().canRead(key)`** (§1.4).
|
||||
Non-negotiable.
|
||||
30. **Show runtime state in results** — loaded event badged, past entries dimmed, scheduled time and
|
||||
delay per row. Source at render time, not by rebuilding the index (§1.5). This is the
|
||||
differentiator from §2.
|
||||
31. **E2E spec** alongside `209-rundown-shortcuts.spec.ts`, covering both surfaces, with a regression
|
||||
lock for the click bug: click the third result _without_ hovering the first two, assert the third
|
||||
entry is selected.
|
||||
|
||||
### Phase 5 — Find and replace _(~3–4 days)_
|
||||
|
||||
The strongest expansion, and a better business case than a command palette: renaming a sponsor,
|
||||
speaker or venue across a 200-entry rundown is a real recurring pain that the competitors'
|
||||
spreadsheet-shaped editors handle badly. It also makes custom fields markedly more valuable.
|
||||
|
||||
**It does not live inside the Finder modal.** The Finder is a fast, non-destructive jump box and
|
||||
Enter must stay safe. The codebase already has the right pattern for this:
|
||||
`renumber-cues-dialog/RenumberCuesDialog.tsx` — a bulk mutation with its own dialog, its own
|
||||
endpoint, and `useEventSelection.selectedEvents` as its scope. Find-and-replace should be its
|
||||
sibling, reachable from `RundownMenu.tsx` (which today holds only "Manage Rundowns…" and "Clear
|
||||
all") and optionally `mod+shift+F`.
|
||||
|
||||
**What the Finder actually contributes** is its matcher. That's the real expansion: Phase 2's
|
||||
parser/matcher must be built as a shared module rather than Finder-private code, so find, preview
|
||||
and replace all agree on what "matches" means.
|
||||
|
||||
**32. The server gap — this is the actual work.** `batchEditEntries` (`rundown.service.ts:166`)
|
||||
applies **one patch to many ids**. Find-and-replace needs **per-entry distinct values** — each
|
||||
entry's own title with its own substring swapped. Two options:
|
||||
|
||||
- N × `putEditEntry`: N round trips, N revision bumps, N `notifyChanges` broadcasts. During a show
|
||||
that's N timer notifications for one user action. Not acceptable at 40 entries.
|
||||
- **A new endpoint taking `Array<PatchWithId<OntimeEntry>>` applied inside one
|
||||
`createTransaction`/`commit`** — one revision bump, one broadcast. Sits next to `batchEditEntries`
|
||||
and reuses the same machinery. Perhaps 40 lines, and it's the thing that makes everything below
|
||||
possible.
|
||||
|
||||
**33. Replaceable fields — deliberately narrow.**
|
||||
|
||||
- _Safe:_ `title`, `note`, text `custom` values. Free text, no structural meaning.
|
||||
- _Opt-in and validated:_ `cue`. It's a numbering scheme, not free text — `cueUtils` has
|
||||
`getIncrement`/`getCueCandidate` and the Renumber dialog exists precisely because of that. The
|
||||
server also rejects an empty cue outright (`rundown.service.ts:177`), so a replace that empties one
|
||||
fails the whole batch.
|
||||
- _Never:_ times, booleans, colours, enums, ids, `image`-type custom fields.
|
||||
|
||||
**34. Preview is mandatory, because there is no undo anywhere in this app.** I checked — the client
|
||||
has no undo stack of any kind. Find-and-replace would be the first feature that can silently alter
|
||||
dozens of entries. So: show every affected entry with before → after per field, with a per-match
|
||||
opt-out, before anything is written. Confirmation names the count and the field: _"Replace 'Q' with
|
||||
'CUE' in title on 23 entries?"_
|
||||
|
||||
**35. A single-step revert.** Because the atomic endpoint gives one revision per operation, capturing
|
||||
the prior values client-side makes "Undo replace" a second batch call. Not an undo history — one
|
||||
step, scoped to the operation, discarded on the next mutation. Cheap, honest, and it's the difference
|
||||
between a feature people trust and one they don't touch during a show.
|
||||
|
||||
**36. Scoping controls:** whole rundown vs. current selection (the `RenumberCuesDialog` precedent
|
||||
already reads `selectedEvents`); restrict to one field; case-sensitive toggle — the Finder is
|
||||
case-insensitive by design, so replace needs this as an explicit option; whole-word toggle.
|
||||
|
||||
**37. Guard concurrency.** Capture the rundown `revision` at preview time and reject the apply if it
|
||||
moved. Prevents replacing text the user never saw someone else write. Cheap.
|
||||
|
||||
**38. Editor only for v1.** The cuesheet is `permission='operator'` and its URL presets can be
|
||||
read-only or column-restricted; a correct implementation would have to gate every field through
|
||||
`getCuesheetColumnAccessPolicy().canWrite(key)`. _Finding_ is safe to expose broadly; _replacing_ is
|
||||
not. Warn when playback is running rather than blocking.
|
||||
|
||||
**39. No regex.** Live operators, no undo history, and one bad pattern destroys a rundown.
|
||||
Whole-word + case-sensitive covers the real cases — renaming a speaker, a sponsor, a venue.
|
||||
|
||||
Rough split: server endpoint ~0.5d, matcher extraction ~0.5d, dialog + preview ~1.5d, revert ~0.5d,
|
||||
tests ~1d.
|
||||
|
||||
### Not recommended
|
||||
|
||||
- **Ranking / fuzzy matching.** Rundown order is deterministic and simpler, and Ontime cues are short
|
||||
and numeric-ish (`1`, `1.5`, `12A`) where subsequence matching is pure noise — `12` would match
|
||||
`1`…`2` across half the rundown. Live operators need predictable results more than forgiving ones.
|
||||
- **A worker-side index or a server search endpoint.** The client holds the whole rundown; a linear
|
||||
scan over `flatOrder` is microseconds. Add a hard scan ceiling so a pathological spreadsheet import
|
||||
can't freeze the UI thread, and leave it there.
|
||||
- **Virtualising the result list.** A palette showing 400 rows isn't more useful than one showing 50;
|
||||
scanning is the bottleneck, not rendering. Cap and count instead.
|
||||
- **Implicit cross-rundown search.** Revealing an entry in another rundown means changing the
|
||||
_loaded_ rundown — a destructive runtime action mid-show. Only the cuesheet has a non-destructive
|
||||
"viewed rundown" concept. If ever wanted, an opt-in `rundown:` filter there, never in the editor.
|
||||
- **The operator view and public viewers.** The operator view never calls `setScrollHandler`, so
|
||||
"go to entry" would silently do nothing until one is registered.
|
||||
|
||||
---
|
||||
|
||||
## 4. Verification
|
||||
|
||||
Each phase should land behind:
|
||||
|
||||
- **Unit tests** for the query parser and matcher — the highest-value case being `index <n>` staying
|
||||
aligned with 1-based UI event indices when delays, groups and milestones interleave.
|
||||
- **An e2e spec** opening `mod+F` in both the editor and the cuesheet, including a regression lock
|
||||
for the click bug (click the third result without hovering the first two).
|
||||
- **A manual pass** on a project that leans on custom fields, and one on a touch device — the mobile
|
||||
path is currently unreachable and needs checking by hand.
|
||||
|
||||
Two claims in this document are worth re-confirming before they drive a decision, since both are
|
||||
cheap to check and expensive to be wrong about: the cuesheet permission concern
|
||||
(`cuesheet.policies.ts`, `useTablePermissions.tsx`), and the "relevance not speed" assumption —
|
||||
count `flatOrder` length on the largest real project file available rather than trusting the estimate.
|
||||
|
||||
---
|
||||
|
||||
## Sequencing
|
||||
|
||||
`1 (0.5d)` → `2 (1.5–2d)` → `3 (1.5d)` → `4 (1d)` → `5 (3–4d)`. Each independently shippable.
|
||||
|
||||
**If only one thing gets done: Phase 1.** If two: Phase 1 + Phase 3 — the bugs and the invisibility
|
||||
are what actually cost users today; the search-depth work matters most once people can find the box.
|
||||
|
||||
Phase 5 is the one with a real competitive argument, but it depends on Phase 2 shipping its matcher
|
||||
as a shared module rather than Finder-private code. That's a cheap constraint to honour up front and
|
||||
an expensive one to retrofit — worth deciding before Phase 2 starts, even if find-and-replace is
|
||||
months away.
|
||||
Reference in New Issue
Block a user