diff --git a/apps/client/src/features/app-settings/panel/feature-panel/__tests__/reportSettings.utils.test.ts b/apps/client/src/features/app-settings/panel/feature-panel/__tests__/reportSettings.utils.test.ts index 895cec2a5..cafedff4f 100644 --- a/apps/client/src/features/app-settings/panel/feature-panel/__tests__/reportSettings.utils.test.ts +++ b/apps/client/src/features/app-settings/panel/feature-panel/__tests__/reportSettings.utils.test.ts @@ -8,7 +8,7 @@ import { TimerType, } from 'ontime-types'; -import { deviationBreakdown, formatOffset, getCombinedReport, makeReportCSV } from '../reportSettings.utils'; +import { formatOffset, getCombinedReport, makeReportCSV } from '../reportSettings.utils'; function makeEvent(patch: Partial): OntimeEvent { return { @@ -191,27 +191,3 @@ describe('makeReportCSV()', () => { expect(rows[1]).toContain('Act 1'); }); }); - -describe('deviationBreakdown()', () => { - it('separates a late start from time the show itself lost', () => { - // finished 4m12s behind, but only 2m12s of that was the show's doing - expect(deviationBreakdown(120000, 132000)).toBe('started +2m, +2m12s during the show'); - }); - - it('credits a show which made up a late start', () => { - expect(deviationBreakdown(300000, -120000)).toBe('started +5m, -2m during the show'); - }); - - it('reads a punctual start as such rather than as an offset of zero', () => { - expect(deviationBreakdown(0, 60000)).toBe('started on time, +1m during the show'); - }); - - it('says the show held schedule when it neither lost nor gained', () => { - expect(deviationBreakdown(120000, 0)).toBe('started +2m, held schedule from there'); - }); - - it('has nothing to say without both halves', () => { - expect(deviationBreakdown(null, 1000)).toBe(''); - expect(deviationBreakdown(1000, null)).toBe(''); - }); -}); diff --git a/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportShowSummary.tsx b/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportShowSummary.tsx index dd7f8b08d..9f9bc3f3c 100644 --- a/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportShowSummary.tsx +++ b/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportShowSummary.tsx @@ -2,10 +2,8 @@ import { MaybeNumber, RunSummary, ShowReport } from 'ontime-types'; import { getShowOffsets } from 'ontime-utils'; import { useMemo } from 'react'; -import Tooltip from '../../../../../common/components/tooltip/Tooltip'; import { enDash } from '../../../../../common/utils/styleUtils'; import { formatDuration, formatTime } from '../../../../../common/utils/time'; -import { deviationBreakdown } from '../reportSettings.utils'; import ReportSummaryCard, { FooterItem, Metric } from './ReportSummaryCard'; import style from './ReportShowSummary.module.scss'; @@ -18,20 +16,21 @@ interface ReportShowSummaryProps { } /** - * Leads the report with how far the show finished from where it was planned to. + * Leads the report with whether the show ran to the length it was planned for. * - * That single figure is what a producer is asked about afterwards, so it is - * the headline. It is then split into the part inherited from a late start and - * the part the show itself moved, which have different causes and different - * remedies. + * Running time is the headline rather than finishing time because it is the + * part the team controls and the part that carries into the next run of the + * same rundown. Finishing time is the other question a report is asked, and + * the two can point opposite ways, so it stays beside it as its own row + * rather than being folded into a single figure. */ export default function ReportShowSummary({ rundownTitle, show, summary, worstOverrunTitle }: ReportShowSummaryProps) { const offsets = useMemo(() => getShowOffsets(show), [show]); /** * A show that stopped early has no meaningful end: its last event is simply - * where it got to. Comparing that against the planned end would report a - * large recovery for a show that never finished. + * where it got to. Measuring that against the plan would report a show which + * never finished as having come in comfortably short. */ const didReachEnd = summary.eventsRun > 0 && summary.eventsRun === summary.eventsPlanned; const hasPlan = offsets.startOffset !== null; @@ -40,19 +39,12 @@ export default function ReportShowSummary({ rundownTitle, show, summary, worstOv
} - > - {deviationBreakdown(offsets.startOffset, offsets.duringShow)} - - ) : ( - 'The show did not reach the end of the rundown, so there is nothing to compare the finish against.' - ) + didReachEnd + ? undefined + : 'The show did not reach the end of the rundown, so there is nothing to measure it against.' } footer={ <> @@ -87,6 +79,15 @@ export default function ReportShowSummary({ rundownTitle, show, summary, worstOv offset={offsets.endOffset} /> )} + {didReachEnd && ( + // no offset: the headline is this row's offset, and repeating it here + // would read as a second, different figure + + )}
); @@ -95,3 +96,7 @@ export default function ReportShowSummary({ rundownTitle, show, summary, worstOv function formatMaybeTime(value: MaybeNumber): string { return value === null ? enDash : formatTime(value); } + +function formatMaybeDuration(value: MaybeNumber): string { + return value === null ? enDash : formatDuration(value, false); +} diff --git a/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportSummaryCard.module.scss b/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportSummaryCard.module.scss index 8e9ec4c10..c8c466f59 100644 --- a/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportSummaryCard.module.scss +++ b/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportSummaryCard.module.scss @@ -18,11 +18,17 @@ // at this weight the conclusion fits beside its label rather than under it, // keeping a group heading to two lines however many of them the table has .headline { - flex-direction: row; + flex-flow: row wrap; align-items: baseline; gap: 0.625rem; } + // the conclusion sits beside its label, but a reason why there is none + // needs the full line rather than pushing the workings off to the right + .note { + flex-basis: 100%; + } + .headlineValue { font-size: 1.25rem; } @@ -75,9 +81,7 @@ } .headlineLabel { - font-size: calc(1rem - 3px); - text-transform: uppercase; - letter-spacing: 0.04em; + font-size: calc(1rem - 2px); color: $gray-300; } diff --git a/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportSummaryCard.tsx b/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportSummaryCard.tsx index d81d4a6ab..70b83b1de 100644 --- a/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportSummaryCard.tsx +++ b/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportSummaryCard.tsx @@ -20,6 +20,7 @@ interface ReportSummaryCardProps { footer?: ReactNode; /** a subordinate tier, eg a group inside the show it belongs to */ compact?: boolean; + className?: string; } /** @@ -38,10 +39,11 @@ export default function ReportSummaryCard({ note, footer, compact, + className, children, }: PropsWithChildren) { return ( -
+
{title} {subtitle && {subtitle}} diff --git a/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportTable.module.scss b/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportTable.module.scss index 3abcfb00b..faf0d82b4 100644 --- a/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportTable.module.scss +++ b/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportTable.module.scss @@ -1,8 +1,15 @@ -th.over { +// the rail the whole group hangs off, falling back to a neutral edge when the +// user gave the group no colour of its own +$rail-width: 3px; +$rail-colour: var(--group-colour, #{$gray-500}); + +// event rows carry their values in td, not th: the panel's th styling is meant +// for column headings and renders whatever it holds small, bold and upper case +td.over { color: $playback-over; } -th.under { +td.under { color: $playback-under; } @@ -12,16 +19,38 @@ th.under { background-color: transparent; td { - padding: 1.25rem 0 0.5rem; + padding: 1.25rem 0 0; } } -.groupColour { - width: 0.25rem; - height: 1rem; - border-radius: 2px; - background-color: var(--group-colour, #{$gray-500}); - flex-shrink: 0; +// a continuous coloured edge down the heading and every event under it, closed +// by the cap below. Mirrors how the rundown editor brackets a group, so the +// report reads with the same grammar as the rundown it reports on +.grouped { + box-shadow: inset $rail-width 0 $rail-colour; +} + +td.grouped { + // clear the rail rather than sitting against it + padding-left: 0.75rem; +} + +.groupEndRow { + background-color: transparent; + + td { + padding: 0 0 1.25rem; + } +} + +.groupEnd { + display: block; + height: 0.5rem; + background-color: $rail-colour; + border-radius: 0 0 $rail-width $rail-width; + // the rail tapering off, not a second element: any wider and the foot reads + // as an underline belonging to the last event rather than to the group + width: $rail-width; } .eventCue, diff --git a/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportTable.tsx b/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportTable.tsx index a93fadce7..ba3321e7b 100644 --- a/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportTable.tsx +++ b/apps/client/src/features/app-settings/panel/feature-panel/composite/ReportTable.tsx @@ -42,8 +42,9 @@ export default function ReportTable({ rows, groups }: ReportTableProps) { {section.group && } {section.rows.map((entry) => ( - + ))} + {section.group && } ))} @@ -62,14 +63,10 @@ function GroupRow({ group }: { group: GroupReport }) { return ( - + - - {group.title || 'Untitled group'} - - } + className={style.grouped} + title={group.title || 'Untitled group'} subtitle={ group.actualStart !== null && group.actualEnd !== null ? `ran ${formatTime(group.actualStart)} to ${formatTime(group.actualEnd)}` @@ -78,13 +75,8 @@ function GroupRow({ group }: { group: GroupReport }) { compact headlineLabel={hasTarget ? 'Against target' : 'Against schedule'} headline={group.variance} - note={ - group.variance !== null - ? undefined - : isComplete - ? 'This group has not run.' - : 'Still to finish, so there is nothing to measure the group against yet.' - } + // terse, because the footer already carries how much of the group ran + note={group.variance !== null ? undefined : isComplete ? 'Did not run' : 'Still running'} footer={ <> @@ -118,23 +110,43 @@ function GroupRow({ group }: { group: GroupReport }) { ); } -function EventRow({ entry }: { entry: CombinedReport }) { +function EventRow({ entry, colour }: { entry: CombinedReport; colour?: string }) { const start = punctuality(entry.actualStart, entry.scheduledStart); const end = punctuality(entry.actualEnd, entry.scheduledEnd); + const grouped = colour !== undefined; return ( - - {entry.index} - {entry.cue} - {entry.title} - {formatTime(entry.scheduledStart)} - {formatTime(entry.actualStart)} - {formatTime(entry.scheduledEnd)} - {formatTime(entry.actualEnd)} + + {entry.index} + {entry.cue} + {entry.title} + {formatTime(entry.scheduledStart)} + {formatTime(entry.actualStart)} + {formatTime(entry.scheduledEnd)} + {formatTime(entry.actualEnd)} ); } +/** + * Closes the group off the way the rundown editor does, so the events between + * the heading and this cap read as belonging to it rather than following it. + */ +function GroupEndRow({ colour }: { colour: string }) { + return ( + + + + + + ); +} + +/** The group's own colour, or a neutral rail when the user set none */ +function groupColour(colour?: string): React.CSSProperties { + return { '--group-colour': colour || undefined } as React.CSSProperties; +} + /** Whether an actual time landed before (under) or after (over) its schedule */ function punctuality(actual: number | null, scheduled: number): 'under' | 'over' | null { if (actual === null) return null; diff --git a/apps/client/src/features/app-settings/panel/feature-panel/reportSettings.utils.ts b/apps/client/src/features/app-settings/panel/feature-panel/reportSettings.utils.ts index e23d8aa84..4fcbd281d 100644 --- a/apps/client/src/features/app-settings/panel/feature-panel/reportSettings.utils.ts +++ b/apps/client/src/features/app-settings/panel/feature-panel/reportSettings.utils.ts @@ -79,26 +79,6 @@ export function formatOffset(value: MaybeNumber): string { return `${value > 0 ? '+' : '-'}${formatDuration(Math.abs(value), false)}`; } -/** - * Reads a show's total plan deviation back as the two parts it is made of, so - * a show which finished late because it began late is not mistaken for one - * which overran. - */ -export function deviationBreakdown(startOffset: MaybeNumber, duringShow: MaybeNumber): string { - if (startOffset === null || duringShow === null) { - return ''; - } - - const fromStart = - Math.abs(startOffset) < MILLIS_PER_SECOND ? 'started on time' : `started ${formatOffset(startOffset)}`; - const inShow = - Math.abs(duringShow) < MILLIS_PER_SECOND - ? 'held schedule from there' - : `${formatOffset(duringShow)} during the show`; - - return `${fromStart}, ${inShow}`; -} - /** Whether an offset is behind, ahead, or neither, for colouring */ export function offsetTone(value: MaybeNumber): 'over' | 'under' | 'none' { if (value === null || Math.abs(value) < MILLIS_PER_SECOND) return 'none'; diff --git a/packages/types/src/definitions/core/Report.type.ts b/packages/types/src/definitions/core/Report.type.ts index 40b954242..877d15517 100644 --- a/packages/types/src/definitions/core/Report.type.ts +++ b/packages/types/src/definitions/core/Report.type.ts @@ -29,19 +29,28 @@ export type ShowReport = { }; /** - * How the show sat against its plan. + * How the show sat against its plan, as the two separate questions it answers. * * Follows Ontime's offset convention: positive means behind schedule. - * `duringShow` separates a late start from a badly run show, which have - * different causes and different remedies. + * + * `endOffset` asks whether the show came off air when it promised to, which is + * what an audience or a venue booking is measured against. `durationOffset` + * asks whether the show itself ran long, which is what the team controls and + * what carries over to the next run of the same rundown. They differ by + * exactly `startOffset`: a show can run over and still finish early if it + * started early, so reporting either one alone is misleading. */ export type ShowOffsets = { /** actual start against planned start */ startOffset: MaybeNumber; /** actual end against planned end */ endOffset: MaybeNumber; - /** time lost (positive) or recovered (negative) between start and end */ - duringShow: MaybeNumber; + /** how long the show was planned to take */ + plannedDuration: MaybeNumber; + /** how long it actually took */ + actualDuration: MaybeNumber; + /** actualDuration against plannedDuration, ie whether the show ran long */ + durationOffset: MaybeNumber; }; /** diff --git a/packages/utils/src/report-utils/reportUtils.test.ts b/packages/utils/src/report-utils/reportUtils.test.ts index 6358624d8..0005347a3 100644 --- a/packages/utils/src/report-utils/reportUtils.test.ts +++ b/packages/utils/src/report-utils/reportUtils.test.ts @@ -78,56 +78,106 @@ describe('getEventVariance()', () => { }); describe('getShowOffsets()', () => { + const HOUR = 60 * MIN; + it('reports a late start carried through to a late end', () => { // started 8 late, ended 8 late: the show itself ran clean const offsets = getShowOffsets({ - plannedStart: 19 * 60 * MIN, - plannedEnd: 21 * 60 * MIN, - actualStart: 19 * 60 * MIN + 8 * MIN, - actualEnd: 21 * 60 * MIN + 8 * MIN, + plannedStart: 19 * HOUR, + plannedEnd: 21 * HOUR, + actualStart: 19 * HOUR + 8 * MIN, + actualEnd: 21 * HOUR + 8 * MIN, }); - expect(offsets).toEqual({ startOffset: 8 * MIN, endOffset: 8 * MIN, duringShow: 0 }); + expect(offsets).toEqual({ + startOffset: 8 * MIN, + endOffset: 8 * MIN, + plannedDuration: 2 * HOUR, + actualDuration: 2 * HOUR, + durationOffset: 0, + }); }); - it('separates time lost during the show from a late start', () => { + it('separates how long the show ran from when it finished', () => { const offsets = getShowOffsets({ - plannedStart: 19 * 60 * MIN, - plannedEnd: 21 * 60 * MIN, - actualStart: 19 * 60 * MIN + 8 * MIN, - actualEnd: 21 * 60 * MIN + 12 * MIN, + plannedStart: 19 * HOUR, + plannedEnd: 21 * HOUR, + actualStart: 19 * HOUR + 8 * MIN, + actualEnd: 21 * HOUR + 12 * MIN, }); - expect(offsets).toMatchObject({ startOffset: 8 * MIN, endOffset: 12 * MIN, duringShow: 4 * MIN }); + expect(offsets).toMatchObject({ startOffset: 8 * MIN, endOffset: 12 * MIN, durationOffset: 4 * MIN }); }); - it('reports time recovered as a negative', () => { + it('reports a show which ran short as a negative', () => { const offsets = getShowOffsets({ - plannedStart: 19 * 60 * MIN, - plannedEnd: 21 * 60 * MIN, - actualStart: 19 * 60 * MIN + 10 * MIN, - actualEnd: 21 * 60 * MIN + 2 * MIN, + plannedStart: 19 * HOUR, + plannedEnd: 21 * HOUR, + actualStart: 19 * HOUR + 10 * MIN, + actualEnd: 21 * HOUR + 2 * MIN, }); - expect(offsets.duringShow).toBe(-8 * MIN); + expect(offsets.durationOffset).toBe(-8 * MIN); + }); + + it('can report a show which ran over and still finished early', () => { + // the case which makes reporting either figure alone misleading: + // started 10 early, ran 4 over, so came off 6 early + const offsets = getShowOffsets({ + plannedStart: 19 * HOUR, + plannedEnd: 21 * HOUR, + actualStart: 19 * HOUR - 10 * MIN, + actualEnd: 21 * HOUR - 6 * MIN, + }); + + expect(offsets.endOffset).toBe(-6 * MIN); + expect(offsets.durationOffset).toBe(4 * MIN); + }); + + it('keeps the two offsets exactly one start offset apart', () => { + const offsets = getShowOffsets({ + plannedStart: 19 * HOUR, + plannedEnd: 21 * HOUR, + actualStart: 19 * HOUR + 3 * MIN, + actualEnd: 21 * HOUR + 11 * MIN, + }); + + expect(offsets.endOffset! - offsets.startOffset!).toBe(offsets.durationOffset); }); it('does not read a show ending after midnight as a day early', () => { const offsets = getShowOffsets({ - plannedStart: 23 * 60 * MIN, + plannedStart: 23 * HOUR, plannedEnd: dayInMs - 10 * MIN, - actualStart: 23 * 60 * MIN, + actualStart: 23 * HOUR, actualEnd: 5 * MIN, // ran past midnight }); expect(offsets.endOffset).toBe(15 * MIN); + // 23:00 to 00:05 is an hour and five minutes, not a negative + expect(offsets.actualDuration).toBe(HOUR + 5 * MIN); + expect(offsets.durationOffset).toBe(15 * MIN); }); - it('has nothing to report without a plan', () => { - expect(getShowOffsets({ plannedStart: null, plannedEnd: null, actualStart: 1, actualEnd: 2 })).toEqual({ + it('still reports how long a show ran when it had no plan to run against', () => { + const offsets = getShowOffsets({ plannedStart: null, plannedEnd: null, actualStart: 0, actualEnd: 5 * MIN }); + + // how long it took is knowable, whether that was long or short is not + expect(offsets).toEqual({ startOffset: null, endOffset: null, - duringShow: null, + plannedDuration: null, + actualDuration: 5 * MIN, + durationOffset: null, + }); + }); + + it('has nothing to report for a show which has not run', () => { + expect(getShowOffsets({ plannedStart: 0, plannedEnd: MIN, actualStart: null, actualEnd: null })).toMatchObject({ + startOffset: null, + endOffset: null, + actualDuration: null, + durationOffset: null, }); }); }); diff --git a/packages/utils/src/report-utils/reportUtils.ts b/packages/utils/src/report-utils/reportUtils.ts index 6fadc666d..11496e2cc 100644 --- a/packages/utils/src/report-utils/reportUtils.ts +++ b/packages/utils/src/report-utils/reportUtils.ts @@ -65,18 +65,38 @@ export function getEventVariance(entry: OntimeEventReport | undefined): EventVar * A sum of event overruns cannot answer this: gaps absorb overrun, skipped * events give time back, and a late start moves the whole show without any * event running long. + * + * Finishing time and running time are reported separately because they answer + * different questions and can point opposite ways: a show which starts early + * and runs over still finishes early. */ export function getShowOffsets(show: ShowReport): ShowOffsets { const startOffset = offsetBetween(show.plannedStart, show.actualStart); const endOffset = offsetBetween(show.plannedEnd, show.actualEnd); + const plannedDuration = durationBetween(show.plannedStart, show.plannedEnd); + const actualDuration = durationBetween(show.actualStart, show.actualEnd); + return { startOffset, endOffset, - duringShow: startOffset === null || endOffset === null ? null : endOffset - startOffset, + plannedDuration, + actualDuration, + durationOffset: plannedDuration === null || actualDuration === null ? null : actualDuration - plannedDuration, }; } +/** + * Time from one point in the day to another, or null if either is missing. + * @private + */ +function durationBetween(from: number | null, to: number | null): number | null { + if (from === null || to === null) { + return null; + } + return elapsedBetween(from, to); +} + /** * Signed distance from a planned time to the time it happened. * Positive means late, matching Ontime's offset convention.