Feat: Director screen improvements #1600
Conversation
WalkthroughThe changes introduce a new timing reference calculation feature for the director screen. A utility function computes Changes
Sequence DiagramsequenceDiagram
participant Util as Utility Layer
participant DS as DirectorScreen
participant DST as DirectorScreenTop
participant UI as UI Display
Util->>Util: getSelectedPartInstances()
Util->>Util: Compute partInstanceToCountTimeFrom<br/>based on filtering & sorting
Util->>DS: Return with partInstanceToCountTimeFrom
DS->>DS: getDirectorScreenReactive()
DS->>DS: Extend return with<br/>partInstanceToCountTimeFrom
DS->>DST: Pass playlist &<br/>partInstanceToCountTimeFrom
DST->>DST: useTiming()
DST->>DST: Calculate expected start,<br/>duration, end
DST->>DST: getPlaylistTimingDiff()
DST->>DST: Compute now, over/under clock
DST->>UI: Render Planned End,<br/>Time To/Since,<br/>Over/Under display
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/webui/src/client/lib/rundownPlaylistUtil.ts`:
- Around line 120-133: Rename the misleading boolean variable areAllPartsTimed
to hasAnyTimedPart to reflect that it checks for the existence of any timed
part; update the declaration and every usage (including the ['part.untimed']
check in the second UIPartInstances.findOne that assigns
partInstanceToCountTimeFrom) so the logic remains identical but the name
accurately conveys intent.
In `@packages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreen.tsx`:
- Line 491: Replace the hard-coded label "Time to planned start" in the
DirectorScreen component with the i18n translation call used elsewhere; locate
the element with className "director-screen__body__part__timeto-name" inside the
DirectorScreen JSX and change the string to use the t(...) function (consistent
with other labels in this file) so the label is translated at runtime.
In `@packages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreenTop.tsx`:
- Around line 53-61: The countdown is inverted because TimeToPlannedEndComponent
is passed now - expectedEnd (negative when expectedEnd > now); update the prop
in DirectorScreenTop's JSX so TimeToPlannedEndComponent receives expectedEnd -
now instead, ensuring a positive remaining-time value while keeping the
surrounding conditional (expectedEnd && expectedEnd > now) and labels unchanged.
🧹 Nitpick comments (2)
packages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreenTop.tsx (1)
65-70: Inconsistent time source usage.Line 69 uses
getCurrentTime()while other calculations in this component usenow(line 37, 53, 56, 68). This inconsistency could cause subtle timing discrepancies whentimingDurations.currentTimediffers fromgetCurrentTime().♻️ Suggested fix for consistency
<TimeSincePlannedEndComponent value={ rehearsalInProgress ? (partInstanceToCountTimeFrom?.timings?.take || 0) + expectedDuration - now - : getCurrentTime() - (expectedStart + expectedDuration) + : now - (expectedStart + expectedDuration) } />packages/webui/src/client/ui/ClockView/ClockView.tsx (1)
8-8: Consider adding.jsextension for consistency.Other imports in this file use
.jsextensions (e.g., lines 2, 4, 6, 7, 9-16), but this import omits it. While TypeScript/bundler may resolve it correctly, maintaining consistency improves clarity.♻️ Optional fix for consistency
-import { DirectorScreen } from './DirectorScreen/DirectorScreen' +import { DirectorScreen } from './DirectorScreen/DirectorScreen.js'
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/webui/src/client/lib/rundownPlaylistUtil.tspackages/webui/src/client/styles/countdown/director.scsspackages/webui/src/client/styles/counterComponents.scsspackages/webui/src/client/ui/ClockView/ClockView.tsxpackages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreen.tsxpackages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreenTop.tsx
💤 Files with no reviewable changes (1)
- packages/webui/src/client/styles/counterComponents.scss
🧰 Additional context used
🧬 Code graph analysis (1)
packages/webui/src/client/lib/rundownPlaylistUtil.ts (2)
packages/webui/src/client/ui/Collections.tsx (1)
UIPartInstances(53-53)packages/job-worker/src/playout/model/implementation/PlayoutModelImpl.ts (1)
playlist(73-75)
🔇 Additional comments (7)
packages/webui/src/client/lib/rundownPlaylistUtil.ts (1)
92-100: LGTM on the API extension.The return type extension to include
partInstanceToCountTimeFromis well-structured and aligns with the PR objectives for rehearsal timing calculations.packages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreenTop.tsx (1)
20-41: LGTM on component structure.The component is well-structured with clear separation of timing calculations and conditional rendering logic. The use of
useTiminghook anduseTranslationfollows React best practices.packages/webui/src/client/styles/countdown/director.scss (2)
205-267: LGTM on the new timeto-content styling.The styling for the countdown display is comprehensive with proper font-variation-settings for Roboto Flex. The
.overtimemodifier correctly uses$general-late-colorfor visual distinction.
42-48: LGTM on text alignment changes.The centered alignment for
.director-screen__top__time-toand.director-screen__top__planned-toaligns with the UI improvements for the countdown display.packages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreen.tsx (3)
353-357: LGTM on the subscription addition.Adding the subscription for
firstTakenPartInstanceensures reactive updates for the new timing display feature.
387-395: LGTM on the timing display integration.The logic for determining
isFirstPieceAndNoDurationand the integration ofDirectorScreenTopcorrectly implements the PR objective to show countdown to planned start when initial parts are untimed.
382-382: Remove or capture return value fromuseTiming()call.The
useTiming()hook returnsRundownTimingContext(timing data) that is discarded at line 382. If timing updates are needed, capture and use the return value. If not, this call should be removed as it adds unnecessary event listener overhead.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const areAllPartsTimed = !!UIPartInstances.findOne({ | ||
| rundownId: { $in: unorderedRundownIds }, | ||
| ['part.untimed']: { $ne: true }, | ||
| }) | ||
|
|
||
| const partInstanceToCountTimeFrom = UIPartInstances.findOne( | ||
| { | ||
| rundownId: { $in: unorderedRundownIds }, | ||
| reset: { $ne: true }, | ||
| takeCount: { $exists: true }, | ||
| ['part.untimed']: { $ne: areAllPartsTimed }, | ||
| }, | ||
| { sort: { takeCount: 1 } } | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the full function context to understand the complete logic
cat -n packages/webui/src/client/lib/rundownPlaylistUtil.ts | head -180 | tail -100Repository: Sofie-Automation/sofie-core
Length of output: 3858
🏁 Script executed:
# Search for usages of partInstanceToCountTimeFrom to understand how it's used
rg -n "partInstanceToCountTimeFrom" --type=ts -C 5Repository: Sofie-Automation/sofie-core
Length of output: 18889
🏁 Script executed:
# Look for related comments or documentation about the timing logic
rg -n "rehearsal|timing|untimed|timed" packages/webui/src/client/lib/rundownPlaylistUtil.ts -C 2 | head -100Repository: Sofie-Automation/sofie-core
Length of output: 626
Rename variable for clarity: areAllPartsTimed is misleading.
The variable areAllPartsTimed checks if any timed part exists (via findOne), not if all parts are timed. Rename to hasAnyTimedPart or similar for accuracy.
The fallback logic on line 130 is intentional: when timed parts exist, select the first taken timed part; when no timed parts exist, select the first taken untimed part as a timing reference. This is reasonable behavior for rehearsal timing calculations.
🤖 Prompt for AI Agents
In `@packages/webui/src/client/lib/rundownPlaylistUtil.ts` around lines 120 - 133,
Rename the misleading boolean variable areAllPartsTimed to hasAnyTimedPart to
reflect that it checks for the existence of any timed part; update the
declaration and every usage (including the ['part.untimed'] check in the second
UIPartInstances.findOne that assigns partInstanceToCountTimeFrom) so the logic
remains identical but the name accurately conveys intent.
| <div className="director-screen__body__part__timeto-countdown"> | ||
| <Timediff time={expectedStart - getCurrentTime()} /> | ||
| </div> | ||
| <div className="director-screen__body__part__timeto-name">Time to planned start</div> |
There was a problem hiding this comment.
Missing translation for user-facing string.
The string "Time to planned start" should use the t() translation function for i18n consistency, matching how other labels in this component are translated (e.g., lines 528, 537).
🌐 Suggested fix
-<div className="director-screen__body__part__timeto-name">Time to planned start</div>
+<div className="director-screen__body__part__timeto-name">{t('Time to planned start')}</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="director-screen__body__part__timeto-name">Time to planned start</div> | |
| <div className="director-screen__body__part__timeto-name">{t('Time to planned start')}</div> |
🤖 Prompt for AI Agents
In `@packages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreen.tsx` at
line 491, Replace the hard-coded label "Time to planned start" in the
DirectorScreen component with the i18n translation call used elsewhere; locate
the element with className "director-screen__body__part__timeto-name" inside the
DirectorScreen JSX and change the string to use the t(...) function (consistent
with other labels in this file) so the label is translated at runtime.
| {expectedEnd && expectedEnd > now ? ( | ||
| <div className="director-screen__top__time-to director-screen__top__planned-container"> | ||
| <div> | ||
| <TimeToPlannedEndComponent value={now - expectedEnd} /> | ||
| </div> | ||
| <span className="director-screen__top__planned-to"> | ||
| {rehearsalInProgress ? t('Time to rehearsal end') : t('Time to planned end')} | ||
| </span> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -type f -name "*.tsx" -o -name "*.ts" | xargs rg "TimeToPlannedEndComponent" -A 10 -B 2Repository: Sofie-Automation/sofie-core
Length of output: 4658
🏁 Script executed:
fd -type f \( -name "*TimeToPlannedEndComponent*" -o -name "*TimePlanned*" \)Repository: Sofie-Automation/sofie-core
Length of output: 241
🏁 Script executed:
rg "export.*TimeToPlannedEndComponent|function TimeToPlannedEndComponent|const TimeToPlannedEndComponent" -A 15Repository: Sofie-Automation/sofie-core
Length of output: 1570
🏁 Script executed:
rg "formatDiffToTimecode" -B 5 -A 20Repository: Sofie-Automation/sofie-core
Length of output: 50383
🏁 Script executed:
sed -n '45,75p' packages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreenTop.tsxRepository: Sofie-Automation/sofie-core
Length of output: 1158
🏁 Script executed:
rg "formatDiffToTimecode.*{" -A 30 | head -80Repository: Sofie-Automation/sofie-core
Length of output: 180
🏁 Script executed:
fd -name "rundown.js" -o -name "RundownUtils*"Repository: Sofie-Automation/sofie-core
Length of output: 301
🏁 Script executed:
rg "formatDiffToTimecode" -A 35 | grep -A 35 "function formatDiffToTimecode"Repository: Sofie-Automation/sofie-core
Length of output: 2504
🏁 Script executed:
fd "rundown" -type f | grep -i utilsRepository: Sofie-Automation/sofie-core
Length of output: 241
Fix inverted time value sign for countdown display.
TimeToPlannedEndComponent receives now - expectedEnd, which is negative when expectedEnd > now (before the planned end). The function formatDiffToTimecode converts negative values to positive and adds a minus prefix, resulting in semantically incorrect display for a countdown.
Change to expectedEnd - now to provide a positive countdown value that represents time remaining until the planned end, consistent with the component's semantic purpose and matching the pattern used in the else branch for TimeSincePlannedEndComponent.
Code context
{expectedEnd && expectedEnd > now ? (
<div className="director-screen__top__time-to director-screen__top__planned-container">
<div>
<TimeToPlannedEndComponent value={now - expectedEnd} />
</div>
Should be expectedEnd - now.
🤖 Prompt for AI Agents
In `@packages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreenTop.tsx`
around lines 53 - 61, The countdown is inverted because
TimeToPlannedEndComponent is passed now - expectedEnd (negative when expectedEnd
> now); update the prop in DirectorScreenTop's JSX so TimeToPlannedEndComponent
receives expectedEnd - now instead, ensuring a positive remaining-time value
while keeping the surrounding conditional (expectedEnd && expectedEnd > now) and
labels unchanged.
3 participants
About the Contributor
This pull request is posted on behalf of the BBC.
Type of Contribution
This is a: Feature
Current Behavior
Countdown to start
The counter in the Director Screen counts to the planned start, and stops counting when the first part is taken, regardless if it's untimed or not.
Rehearsal timing
The counter counts to the planned end which is unhelpful if you're rehearsing an hour before the planned start (or practicing with an old running order)
New Behavior
Countdown to start
When the countdown to planned start is shown in the On-Air part area (before the first take command has occurred) a label has been added to show "Time to planned start"
If the first part(s) are untimed then it retains the countdown to start rather than showing the on-air part.
As the first part(s) are untimed the start time is still very relevant for knowing when to take the first timed part.
Rehearsal timing
After the first timed part is taken, the Time to Planned End countdown in the top portion of the Director Screen shows a Time to Rehearsal End countdown.
It counts towards the end of the program duration (I.E. End = Time of first timed take + program duration)
Testing
Affected areas
Director Screen
Time Frame
Not urgent, but we would like to get this merged into the in-development release.
Other Information
Status