feat: Top bar redesign and new T-Timers feature#1690
Open
rjmunro wants to merge 125 commits intoSofie-Automation:mainfrom
Open
feat: Top bar redesign and new T-Timers feature#1690rjmunro wants to merge 125 commits intoSofie-Automation:mainfrom
rjmunro wants to merge 125 commits intoSofie-Automation:mainfrom
Conversation
* Simplify timer state storage Makes calculations of timing consistent for all timer types * Separate T-Timer mode and state State contains the dynamic how much time is left information . Mode contains more static information about the timer's properties etc. * Fix t-timer tests
…d the hover state of labels. Show timer labels are still font-size 600 for some weird reason, needs to be fixed later.
… counter and adjusted the close icon placement.
… remaining part time for old uses
Remove unneccesary cast to any
…he new Rehearsal striped could be seen through the labels when not in the active state. Refrained from doing the same thing with the dimmed hours and colons in counters since the text glow used for highlighting made them stand out too much.
…matches the angle of "missing VT" zebras, indicating a problem. This way the new striped Rehearsal Top Bar background is logically separated from the warnings.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a three-slot T‑timer subsystem: data model, migration, playout/timer utilities and jobs, PlayoutModel integration, blueprint context APIs/services, UI/header components and styles, tests, mocks, and updated ingest/sync flows to propagate timer state and projections. Changes
Sequence Diagram(s)sequenceDiagram
actor Blueprint
participant Context as Blueprint Context
participant TTimers as TTimersService
participant Playout as PlayoutModel
participant DB as RundownPlaylist
participant Job as RecalcJob
Note over Blueprint,Context: Blueprint code interacts with timer context
Blueprint->>Context: getTimer(index)
Context->>TTimers: getTimer(index)
TTimers-->>Context: IPlaylistTTimer
Context-->>Blueprint: IPlaylistTTimer
Blueprint->>Context: startCountdown(index,duration,opts)
Context->>TTimers: startCountdown(index,duration,opts)
TTimers->>Playout: updateTTimer(timer)
Playout->>DB: persist tTimers change
Note over Job,Playout: Scheduled recalculation job
Job->>Playout: runWithPlayoutModel(playlist)
Playout->>TTimers: recalculateTTimerProjections()
TTimers->>Playout: updateTTimer(projectedState)
Playout->>DB: persist projected state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
|
…ize rendering of the entire GUI only when the URL parameter 'zoom' is set to '100' or no localStorage variable 'uiZoomLevel' exists, that occurs when the user has a local, non-default font size rendering.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
And fix the yarn lint:fix command
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a redesigned Rundown top bar (header) in WebUI and adds a new “T-Timers” feature (3 playlist-scoped timers controllable from blueprints), including backend support for estimate/anchor calculation and UI rendering on Director/Presenter screens.
Changes:
- Add T-Timer data model + blueprint API (start/pause/resume/restart, anchor-based estimates) and job-worker logic to recalculate estimates.
- Replace the old Rundown header timing display with a new header layout, new clock/timer components, and updated styling.
- Add chrono-node dependency + lockfile updates to support parsing time-of-day targets.
Reviewed changes
Copilot reviewed 93 out of 95 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/yarn.lock | Dependency lock updates (incl. chrono-node). |
| packages/webui/src/client/ui/Shelf/PartTimingPanel.tsx | Update import path for remaining-time component. |
| packages/webui/src/client/ui/SegmentTimeline/SegmentTimeline.tsx | Update import path for remaining-time component. |
| packages/webui/src/client/ui/SegmentStoryboard/StoryboardPart.tsx | Update import path for remaining-time component. |
| packages/webui/src/client/ui/SegmentList/SegmentListHeader.tsx | Adjust class name used when header is hidden. |
| packages/webui/src/client/ui/SegmentList/OnAirLine.tsx | Update import path for remaining-time component. |
| packages/webui/src/client/ui/SegmentList/LinePart.tsx | Update import path for remaining-time component. |
| packages/webui/src/client/ui/RundownView/RundownViewContextProviders.tsx | Add explicit .js extension for import. |
| packages/webui/src/client/ui/RundownView/RundownTiming/TimeOfDay.tsx | Allow extra className + wrap value for new header styling. |
| packages/webui/src/client/ui/RundownView/RundownTiming/CurrentPartOrSegmentRemaining.tsx | Remove old implementation (moved/replaced). |
| packages/webui/src/client/ui/RundownView/RundownRightHandControls.tsx | Adjust class name used when header is hidden. |
| packages/webui/src/client/ui/RundownView/RundownNotifier.tsx | Import path tweak for reload response handler. |
| packages/webui/src/client/ui/RundownView/RundownHeader/useRundownPlaylistOperations.tsx | Add explicit .js extension for import. |
| packages/webui/src/client/ui/RundownView/RundownHeader/_shared.scss | New shared SCSS placeholder to break circular imports. |
| packages/webui/src/client/ui/RundownView/RundownHeader/TimingDisplay.tsx | Remove old timing header component. |
| packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderTimingDisplay.tsx | New “Over/Under” timing diff display for header. |
| packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderTimers.tsx | New header rendering for up to 2 active T-Timers. |
| packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderPlannedStart.tsx | New header planned start/start-in display. |
| packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderExpectedEnd.tsx | New header planned/estimated end display. |
| packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderDurations.tsx | New header planned/remaining duration display. |
| packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.tsx | Major header redesign + new layout + menu/context menu integration. |
| packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss | New header styling. |
| packages/webui/src/client/ui/RundownView/RundownHeader/RundownContextMenu.tsx | New context menu component + hamburger trigger. |
| packages/webui/src/client/ui/RundownView/RundownHeader/HeaderFreezeFrameIcon.tsx | Add freeze-frame indicator in header based on VT duration mismatch. |
| packages/webui/src/client/ui/RundownView/RundownHeader/CurrentPartOrSegmentRemaining.tsx | New shared remaining-time implementation + header variants. |
| packages/webui/src/client/ui/RundownView/RundownHeader/Countdown.tsx | New countdown display component used by header timers/clocks. |
| packages/webui/src/client/ui/RundownView/RundownHeader/Countdown.scss | Styling for the new Countdown component. |
| packages/webui/src/client/ui/RundownView.tsx | Pass rundownCount and adjust “hide header” class naming. |
| packages/webui/src/client/ui/ClockView/TTimerDisplay.tsx | New T-Timer renderer for Presenter/Director screens. |
| packages/webui/src/client/ui/ClockView/PresenterScreen.tsx | Show active/default T-Timer in presenter status bar. |
| packages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreen.tsx | Show active/default T-Timer in director view. |
| packages/webui/src/client/ui/ClockView/CameraScreen/Part.tsx | Update import path for remaining-time component. |
| packages/webui/src/client/styles/rundownView.scss | Rename old header styling selectors to _OLD + some gradient tweaks. |
| packages/webui/src/client/styles/notifications.scss | Update selector to match renamed old header class. |
| packages/webui/src/client/styles/defaultColors.scss | Add CSS variables for new header colors/highlight. |
| packages/webui/src/client/styles/countdown/presenter.scss | Add T-Timer layout/styles in presenter status bar. |
| packages/webui/src/client/styles/countdown/director.scss | Add T-Timer layout/styles in director view. |
| packages/webui/src/client/styles/contextMenu.scss | Context menu spacing/typography updates + divider styling. |
| packages/webui/src/client/styles/bootstrap-customize.scss | Set --bs-body-font-size. |
| packages/webui/src/client/styles/_colorScheme.scss | Expose new header CSS variables as SCSS vars. |
| packages/webui/src/client/lib/tTimerUtils.ts | Client utilities for T-Timer diff/over-under + default selection helper. |
| packages/webui/src/client/lib/rundownTiming.ts | Only compute remaining segment budget when budgetDuration > 0. |
| packages/webui/src/client/lib/tests/rundownTiming.test.ts | Update playlist test mock to include tTimers. |
| packages/webui/src/mocks/defaultCollectionObjects.ts | Add default tTimers to mocked playlists. |
| packages/openapi/run_server_tests.mjs | Remove eslint-disable comment near import. |
| packages/live-status-gateway/src/topics/tests/utils.ts | Update playlist test mock to include tTimers. |
| packages/job-worker/src/workers/studio/jobs.ts | Register new studio job handler for T-Timer estimate recalculation. |
| packages/job-worker/src/workers/studio/child.ts | Queue initial T-Timer estimate recalculation at startup. |
| packages/job-worker/src/rundownPlaylists.ts | Ensure default playlists include 3 tTimers. |
| packages/job-worker/src/playout/tTimersJobs.ts | New job implementation to recalc T-Timer estimates for active playlists. |
| packages/job-worker/src/playout/tTimers.ts | Core T-Timer operations + estimate recalculation algorithm + time-of-day parsing. |
| packages/job-worker/src/playout/setNext.ts | Recalculate T-Timer estimates after setNext / queued-segment changes. |
| packages/job-worker/src/playout/model/implementation/PlayoutModelImpl.ts | Add updateTTimer method to playout model. |
| packages/job-worker/src/playout/model/PlayoutModel.ts | Add updateTTimer to PlayoutModel interface. |
| packages/job-worker/src/playout/lookahead/util.ts | Add ignoreQuickLoop option for ordered-part lookup. |
| packages/job-worker/src/playout/tests/tTimersJobs.test.ts | New tests for the recalc job behavior. |
| packages/job-worker/src/playout/tests/snapshots/playout.test.ts.snap | Snapshot update to include tTimers in playlists. |
| packages/job-worker/src/ingest/syncChangesToPartInstance.ts | Provide additional blueprint sync ingest context + persist changed tTimers + add part-index helper. |
| packages/job-worker/src/ingest/commit.ts | Recalculate T-Timer estimates after ingest/playlist updates. |
| packages/job-worker/src/ingest/tests/updateNext.test.ts | Update playlist test mock to include tTimers. |
| packages/job-worker/src/ingest/tests/syncChangesToPartInstance.test.ts | Update mocks to include playlist tTimers and ingest-model helpers. |
| packages/job-worker/src/ingest/tests/syncChangesToPartInstance-computeCurrentPartIndex.test.ts | New tests for computeCurrentPartIndex helper. |
| packages/job-worker/src/events/tests/externalMessageQueue.test.ts | Update playlist test mock to include tTimers. |
| packages/job-worker/src/blueprints/context/services/TTimersService.ts | New blueprint-facing service for controlling T-Timers. |
| packages/job-worker/src/blueprints/context/services/PartAndPieceInstanceActionService.ts | Update blueprint segment type import/return type. |
| packages/job-worker/src/blueprints/context/adlibActions.ts | Expose T-Timers to action execution context. |
| packages/job-worker/src/blueprints/context/SyncIngestUpdateToPartInstanceContext.ts | Expose T-Timers to sync-ingest context and collect changed timers. |
| packages/job-worker/src/blueprints/context/RundownActivationContext.ts | Expose T-Timers to activation context. |
| packages/job-worker/src/blueprints/context/OnTakeContext.ts | Expose T-Timers to onTake context. |
| packages/job-worker/src/blueprints/context/OnSetAsNextContext.ts | Expose T-Timers to onSetAsNext context. |
| packages/job-worker/src/blueprints/tests/context-adlibActions.test.ts | Update mocks to include playlist tTimers. |
| packages/job-worker/src/blueprints/tests/context-OnTakeContext.test.ts | Update mocks to include playlist tTimers. |
| packages/job-worker/src/blueprints/tests/context-OnSetAsNextContext.test.ts | Update mocks to include playlist tTimers. |
| packages/job-worker/src/mocks/defaultCollectionObjects.ts | Add default tTimers to mocked playlists. |
| packages/job-worker/package.json | Add chrono-node dependency. |
| packages/corelib/src/worker/studio.ts | Add new StudioJobs entry for T-Timer estimate recalculation. |
| packages/corelib/src/dataModel/RundownPlaylist.ts | Add T-Timer types, TimerState helper, and tTimers to playlist model. |
| packages/blueprints-integration/src/context/tTimersContext.ts | New blueprint API surface for T-Timer access/control. |
| packages/blueprints-integration/src/context/syncIngestChangesContext.ts | Add T-Timers context to sync-ingest interface. |
| packages/blueprints-integration/src/context/rundownContext.ts | Add T-Timers context to activation context interface. |
| packages/blueprints-integration/src/context/partsAndPieceActionContext.ts | Update blueprint segment type name. |
| packages/blueprints-integration/src/context/onTakeContext.ts | Add T-Timers context to onTake interface. |
| packages/blueprints-integration/src/context/onSetAsNextContext.ts | Add T-Timers context to onSetAsNext interface + segment type rename. |
| packages/blueprints-integration/src/context/adlibActionContext.ts | Add T-Timers context to action execution interface. |
| packages/blueprints-integration/src/api/showStyle.ts | Extend sync ingest new data payload (allParts + currentPartIndex). |
| package.json | Adjust lint:fix invocation for packages lint script. |
| meteor/yarn.lock | Meteor lock updates (incl. chrono-node + other resolutions). |
| meteor/server/migration/X_X_X.ts | Add migration step to backfill tTimers on existing playlists. |
| meteor/server/api/tests/peripheralDevice.test.ts | Update playlist test mock to include tTimers. |
| meteor/server/api/tests/externalMessageQueue.test.ts | Update playlist test mock to include tTimers. |
| meteor/server/tests/cronjobs.test.ts | Update playlist test mock to include tTimers. |
| meteor/mocks/defaultCollectionObjects.ts | Add default tTimers to mocked playlists. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+49
to
+81
| // TODO: remove this mock | ||
| let mockTimer: RundownTTimer | undefined | ||
|
|
||
| export function getDefaultTTimer(_tTimers: [RundownTTimer, RundownTTimer, RundownTTimer]): RundownTTimer | undefined { | ||
| // FORCE MOCK: | ||
| /* | ||
| const active = tTimers.find((t) => t.mode) | ||
| if (active) return active | ||
| */ | ||
|
|
||
| if (!mockTimer) { | ||
| const now = Date.now() | ||
| mockTimer = { | ||
| index: 0, | ||
| label: 'MOCK TIMER', | ||
| mode: { | ||
| type: 'countdown', | ||
| }, | ||
| state: { | ||
| zeroTime: now + 60 * 60 * 1000, // 1 hour | ||
| duration: 0, | ||
| paused: false, | ||
| }, | ||
| estimateState: { | ||
| zeroTime: now + 65 * 60 * 1000, // 65 mins -> 5 mins over | ||
| duration: 0, | ||
| paused: false, | ||
| }, | ||
| } as any | ||
| } | ||
|
|
||
| return mockTimer | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/webui/src/client/styles/countdown/presenter.scss (1)
166-176:⚠️ Potential issue | 🟠 MajorPrevent status-bar overflow in narrow layouts
Line 166 adds another fixed-content column, but the first
autotrack can still over-constrain truncation. Use a shrinkable first track and setmin-width: 0on the rundown-name item so ellipsis keeps working reliably.Suggested fix
.presenter-screen__rundown-status-bar { display: grid; - grid-template-columns: auto fit-content(20em) fit-content(5em); + grid-template-columns: minmax(0, 1fr) fit-content(20em) fit-content(5em); grid-template-rows: fit-content(1em); font-size: 6em; color: `#888`; padding: 0 0.2em; .presenter-screen__rundown-status-bar__rundown-name { + min-width: 0; overflow: hidden; white-space: nowrap; text-overflow: ellipsis; line-height: 1.44em; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/styles/countdown/presenter.scss` around lines 166 - 176, The status bar grid uses an unshrinkable first track which prevents the .presenter-screen__rundown-status-bar__rundown-name from truncating; change the grid definition to make the first column shrinkable (e.g., replace the leading "auto" with a flexible track such as minmax(0, 1fr) or 1fr) and add min-width: 0 to .presenter-screen__rundown-status-bar__rundown-name so white-space: nowrap and text-overflow: ellipsis can take effect reliably in narrow layouts.meteor/server/api/__tests__/peripheralDevice.test.ts (1)
66-82:⚠️ Potential issue | 🟡 MinorKeep this fixture on the fixed-size
tTimersschema.
tTimers: [] as anyno longer matches the runtime contract. If timer-aware code starts touching this playlist, the test will either hide a regression or fail for the wrong reason. Seed the three default entries here instead.🧱 Suggested fixture update
- tTimers: [] as any, + tTimers: [ + { index: 1, label: '', mode: null, state: null }, + { index: 2, label: '', mode: null, state: null }, + { index: 3, label: '', mode: null, state: null }, + ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@meteor/server/api/__tests__/peripheralDevice.test.ts` around lines 66 - 82, The test fixture uses RundownPlaylists.mutableCollection.insertAsync with tTimers: [] as any which violates the fixed-size tTimers schema; replace the empty-cast with the three default timer entries expected by the schema (seed the three default tTimers objects for the playlist created by RundownPlaylists.mutableCollection.insertAsync for rundownPlaylistID, keeping PlaylistTimingType.None and other fields unchanged) so the inserted document matches the runtime contract and avoids masking timer-related regressions.packages/job-worker/src/blueprints/context/OnSetAsNextContext.ts (1)
67-69:⚠️ Potential issue | 🔴 CriticalBug:
currentPartStatereturnsnextPartState.The getter for
currentPartStateincorrectly returnsthis.partAndPieceInstanceService.nextPartStateinstead ofcurrentPartState. This causes blueprints to receive incorrect state information. Other similar context classes (OnTakeContext,adlibActions) correctly returncurrentPartState.🐛 Fix the getter
public get currentPartState(): ActionPartChange { - return this.partAndPieceInstanceService.nextPartState + return this.partAndPieceInstanceService.currentPartState }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/job-worker/src/blueprints/context/OnSetAsNextContext.ts` around lines 67 - 69, The getter currentPartState in OnSetAsNextContext is returning the wrong value (this.partAndPieceInstanceService.nextPartState); change it to return the actual current part state by using this.partAndPieceInstanceService.currentPartState so blueprints receive correct state; update the getter in class OnSetAsNextContext (currentPartState) to reference the currentPartState property on partAndPieceInstanceService instead of nextPartState.
🟡 Minor comments (15)
packages/webui/src/client/styles/_colorScheme.scss-40-45 (1)
40-45:⚠️ Potential issue | 🟡 MinorRemove unexpected empty lines to fix Stylelint errors.
Static analysis flagged
scss/dollar-variable-empty-line-beforeerrors on lines 41 and 43. Remove the empty lines before the variable declarations to comply with the project's SCSS formatting rules.🧹 Proposed fix to remove empty lines
$ui-dark-color: var(--ui-dark-color); $ui-dark-color-brighter: var(--ui-dark-color-brighter); - $color-interactive-highlight: var(--color-interactive-highlight); - $color-header-inactive: var(--color-header-inactive); $color-header-rehearsal: var(--color-header-rehearsal); $color-header-on-air: var(--color-header-on-air);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/styles/_colorScheme.scss` around lines 40 - 45, Remove the unexpected blank lines before the SCSS dollar-variable declarations to satisfy the scss/dollar-variable-empty-line-before rule: delete the empty line above $color-interactive-highlight and the empty line above $color-header-inactive so the declarations $color-interactive-highlight, $color-header-inactive, $color-header-rehearsal, and $color-header-on-air appear without preceding empty lines; keep the variable names and values unchanged.packages/webui/src/client/styles/rundownView.scss-212-221 (1)
212-221:⚠️ Potential issue | 🟡 MinorFix the new stylelint violations before landing this stylesheet.
These changed blocks currently trip the configured rules (
declaration-empty-line-before,scss/double-slash-comment-empty-line-before, andfont-family-name-quotes), so lint will stay red until the spacing and font stack are normalized. In particular, the new font family should be written asRoboto, 'Helvetica Neue', Arial, sans-serif.Also applies to: 491-502, 3615-3619
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/styles/rundownView.scss` around lines 212 - 221, The new gradient background blocks (the background property with four linear-gradient entries) and the font-family declarations are causing stylelint failures; fix them by normalizing spacing and line breaks to satisfy declaration-empty-line-before and scss/double-slash-comment-empty-line-before (ensure there is an empty line before any // comment and consistent spacing around commas and properties), and update the font stack to the normalized quoted form: Roboto, 'Helvetica Neue', Arial, sans-serif; apply the same fixes to the other occurrences referenced (the other gradient blocks and font-family uses noted in the comment).packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderTimingDisplay.tsx-26-37 (1)
26-37:⚠️ Potential issue | 🟡 MinorHandle the zero-diff state separately.
When
overUnderClock === 0, this renders asUnder −00:00. Exact-on-time should be neutral or hidden, not classified as under.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderTimingDisplay.tsx` around lines 26 - 37, The component currently treats overUnderClock === 0 as "Under"; update RundownHeaderTimingDisplay so zero is handled as an exact/neutral state: add an isExact check (overUnderClock === 0) alongside the existing isUnder (overUnderClock < 0), and use it to render a neutral label (e.g., 'On time' or hide the label) and neutral/hidden chip (no plus/minus sign and a distinct class instead of --under/--over). Replace usages of isUnder ternaries for the label, chip className and sign with logic that first checks isExact, then isUnder, then over to ensure exact-on-time is not classified as under. Ensure class names like rundown-header__clocks-diff, rundown-header__clocks-diff--under/--over and rundown-header__clocks-diff__chip--under/--over are updated to include an exact/neutral variant if you need distinct styling.packages/blueprints-integration/src/context/tTimersContext.ts-100-118 (1)
100-118:⚠️ Potential issue | 🟡 MinorAlign the exported JSDoc with the actual API.
Lines 103 and 113 refer to
setAnchorPart, but the interface exposessetEstimateAnchorPart. Line 157 also callstargetRawa string even though the type allowsnumber.📝 Suggested doc fix
- * Clears any anchor part set via setAnchorPart. + * Clears any anchor part set via setEstimateAnchorPart. ... - * Clears any anchor part set via setAnchorPart. + * Clears any anchor part set via setEstimateAnchorPart. ... - * The raw target string of the timer, as provided when setting the timer + * The raw target value of the timer, as provided when setting the timerAlso applies to: 156-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blueprints-integration/src/context/tTimersContext.ts` around lines 100 - 118, The JSDoc for setEstimateTime and setEstimateDuration incorrectly refers to setAnchorPart and misstates targetRaw's type; update the comments to reference the actual exported API name setEstimateAnchorPart (not setAnchorPart) and fix any mention of targetRaw to reflect its real type (allowing number as well as string per the interface). Locate the JSDoc blocks around setEstimateTime and setEstimateDuration and replace the incorrect function name and type text so the docs match the actual interface symbols setEstimateAnchorPart and the union type of targetRaw.packages/blueprints-integration/src/context/tTimersContext.ts-1-164 (1)
1-164:⚠️ Potential issue | 🟡 MinorAdd the new timer context to the context barrel exports.
The file
packages/blueprints-integration/src/context/tTimersContext.tsis not re-exported fromcontext/index.ts, which means external blueprint code cannot import the new timer types (ITTimersContext,IPlaylistTTimer, etc.) from the package entrypoint. Addexport * from './tTimersContext.js'topackages/blueprints-integration/src/context/index.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/blueprints-integration/src/context/tTimersContext.ts` around lines 1 - 164, The context barrel is missing exports for the new T-timer types (ITTimersContext, IPlaylistTTimer, IPlaylistTTimerState, etc.), so external code can't import them; fix by adding a re-export for tTimersContext (e.g. add "export * from './tTimersContext.js'") to the context index barrel so ITTimersContext, IPlaylistTTimer and related types are exposed from the package entrypoint; ensure the export path matches the module resolution used by other context exports.packages/webui/src/client/ui/ClockView/TTimerDisplay.tsx-42-50 (1)
42-50:⚠️ Potential issue | 🟡 MinorTreat
0over/under as neutral.Lines 45-49 route
overUnder === 0through the--understyle and render an en dash, so an exactly on-schedule timer looks like an underrun. Either hide the badge at zero or give it a neutral class/sign.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/ClockView/TTimerDisplay.tsx` around lines 42 - 50, The badge treats 0 as "under" — change the visibility and class logic so exact zero is neutral: only render the over/under span when overUnder !== undefined && overUnder !== 0, and update the class conditions to use overUnder > 0 for 't-timer-display__over-under--over' and overUnder < 0 for 't-timer-display__over-under--under'; keep the sign logic in the content to render '+' for >0 and '\u2013' for <0 (or, if you prefer a neutral visible state instead of hiding, add a 't-timer-display__over-under--neutral' class for overUnder === 0 and render a neutral sign). Ensure these changes are made where overUnder is used in the TTimerDisplay render (classNames call and the conditional that wraps the span) and continue to pass Math.abs(overUnder) to RundownUtils.formatDiffToTimecode.packages/corelib/src/dataModel/RundownPlaylist.ts-175-185 (1)
175-185:⚠️ Potential issue | 🟡 MinorUse a nullish check for
pauseTime.
pauseTimeis defined asnumber | nulland0is a valid timestamp. The current truthy guard skips the auto-pause branch whenpauseTime: 0, causingtimerStateToDuration()to return the running value instead of the paused value.🛠️ Suggested fix
- } else if (state.pauseTime && now >= state.pauseTime) { + } else if (state.pauseTime != null && now >= state.pauseTime) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/corelib/src/dataModel/RundownPlaylist.ts` around lines 175 - 185, The guard in timerStateToDuration incorrectly treats pauseTime==0 as falsy; change the check to a nullish check (e.g. state.pauseTime != null or state.pauseTime !== null) so the auto-pause branch runs when pauseTime is 0 and the subsequent comparison now >= state.pauseTime is valid; update the conditional in function timerStateToDuration (and any related TimerState usage) to use a null-check rather than a truthy check.meteor/server/api/__tests__/externalMessageQueue.test.ts-44-44 (1)
44-44:⚠️ Potential issue | 🟡 MinorReplace
[] as anywith a properly typed 3-slottTimersfixture.The
tTimersfield at line 44 uses[] as anywhich bypasses type safety. Although this field is not accessed in the current test, the fixture should match the RundownPlaylist schema and the pattern used in other test files throughout the codebase.Suggested fixture update
- tTimers: [] as any, + tTimers: [ + { index: 1, label: '', mode: null, state: null }, + { index: 2, label: '', mode: null, state: null }, + { index: 3, label: '', mode: null, state: null }, + ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@meteor/server/api/__tests__/externalMessageQueue.test.ts` at line 44, Replace the untyped placeholder tTimers: [] as any with a properly typed 3-entry fixture that conforms to the RundownPlaylist.tTimers shape used across tests: create an array of three timer objects (populate required fields with minimal valid values) and type it using the RundownPlaylist (or the specific Timer interface) instead of any; update the tTimers field in the test fixture so it uses that typed array (refer to tTimers and RundownPlaylist/timer interface for field names).meteor/server/migration/X_X_X.ts-65-68 (1)
65-68:⚠️ Potential issue | 🟡 MinorOff-by-one error in validation condition.
The condition
playlistCount > 1should beplaylistCount > 0. Currently, if exactly one playlist is missingtTimers, the validation returnsfalse(indicating no migration needed), even though migration is required.🐛 Proposed fix
validate: async () => { const playlistCount = await RundownPlaylists.countDocuments({ tTimers: { $exists: false } }) - if (playlistCount > 1) return `There are ${playlistCount} RundownPlaylists without T-timers` + if (playlistCount > 0) return `There are ${playlistCount} RundownPlaylists without T-timers` return false },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@meteor/server/migration/X_X_X.ts` around lines 65 - 68, The validation in the migration's validate function incorrectly checks if playlistCount > 1, causing a single RundownPlaylists document missing tTimers to be treated as no-op; change the condition to playlistCount > 0 so that any count above zero returns the diagnostic string. Locate validate in this migration file (the async validate function calling RundownPlaylists.countDocuments with { tTimers: { $exists: false } }) and update the boolean check to use > 0.packages/job-worker/src/playout/__tests__/tTimersJobs.test.ts-205-209 (1)
205-209:⚠️ Potential issue | 🟡 MinorTest doesn't match its description.
This test claims to verify handling of "playlist deleted between query and lock" but doesn't set up that scenario at all—it just calls the function with no playlists. The comment acknowledges this ("harder to set up properly") but the current implementation is misleading.
Either implement the actual race condition test (perhaps using mocks to simulate deletion mid-operation) or rename this test to reflect what it actually tests (e.g., "should handle empty studio").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/job-worker/src/playout/__tests__/tTimersJobs.test.ts` around lines 205 - 209, The test titled 'should handle playlist deleted between query and lock' in tTimersJobs.test.ts is misleading because it never sets up that race; either implement the race or rename the test. To implement: mock the playlist-fetching and locking functions used by handleRecalculateTTimerEstimates so the first call returns a playlist and a subsequent call (after acquiring the lock) returns null/throws to simulate deletion, then assert the function resolves undefined; alternatively, simply rename the test to reflect the actual behavior (e.g., 'should handle empty studio') and keep the current assertion against handleRecalculateTTimerEstimates.packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss-14-14 (1)
14-14:⚠️ Potential issue | 🟡 MinorThis stylesheet is still lint-red.
Stylelint is already flagging these added lines for
scss/at-extend-no-missing-placeholder,font-family-name-quotes,selector-not-notation,scss/double-slash-comment-*, anddeclaration-empty-line-before. This file will not land cleanly until those are fixed.Also applies to: 191-214, 309-310, 394-401, 442-443, 507-508, 559-559, 577-577, 630-630
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss` at line 14, RundownHeader.scss is failing multiple stylelint rules; fix by (1) replacing any `@extend` usages that target non-placeholder selectors with extends of a %placeholder (create a %placeholder selector and use `@extend` %placeholder) to satisfy scss/at-extend-no-missing-placeholder, (2) normalize font-family entries like the current font-family declaration to use double-quoted family names (e.g., "Roboto Flex", "Roboto", sans-serif) to satisfy font-family-name-quotes, (3) convert any double-slash comments (// ...) to block comments (/* ... */) to satisfy scss/double-slash-comment-*, (4) change any non standard :not notation to the functional :not(...) form to satisfy selector-not-notation, and (5) add or remove blank lines before declarations to obey declaration-empty-line-before; scan the file RundownHeader.scss for the affected selectors and blocks (including the shown font-family line and the other ranges called out) and apply these patterns consistently.packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss-190-194 (1)
190-194:⚠️ Potential issue | 🟡 MinorThis selector no longer matches the new
Countdownmarkup.
Countdown.tsxnow renders.countdown__counter/.countdown__timeofdaydirectly, so the.countdown__valueoverride here never applies. The “center it since there's no label” adjustment needs to target the class that is actually rendered.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss` around lines 190 - 194, The SCSS rule in RundownHeader.scss targets .rundown-header__clocks-time-now .countdown__value but Countdown.tsx now outputs .countdown__counter / .countdown__timeofday, so the margin-left override never applies; update the selector inside the .rundown-header__clocks-time-now block to target the actual rendered element (e.g. .countdown__timeofday or .countdown__counter) and set margin-left: 0 there so the time is centered when there is no label, keeping the existing `@extend` .countdown--timeofday.packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss-336-339 (1)
336-339:⚠️ Potential issue | 🟡 MinorRemove the negative
line-heightvalue—it's invalid CSS and will be ignored by browsers.Negative
line-heightvalues are not permitted by the CSS specification. Use a positive unitless value (e.g.,line-height: 1) or adjust vertical alignment viapaddingor flexbox instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss` around lines 336 - 339, The CSS rule for .rundown-header__clocks-timers__timer__over-under contains an invalid negative line-height; change it to a valid positive unitless value (e.g., line-height: 1) or remove it and adjust vertical alignment using padding, margin, or flexbox on .rundown-header__clocks-timers__timer__over-under (or its parent) to achieve the intended positioning.packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.tsx-141-145 (1)
141-145:⚠️ Potential issue | 🟡 MinorKeep a fallback title in the single-rundown branch.
currentRundownandfirstRundownare both optional, so this can render a blank header during initial data load. Falling back toplaylist.nameavoids the empty state.🔧 Suggested fix
{rundownCount > 1 ? ( <span className="playlist-name">{playlist.name}</span> ) : ( - <span className="rundown-name">{(currentRundown ?? firstRundown)?.name}</span> + <span className="rundown-name"> + {(currentRundown ?? firstRundown)?.name ?? playlist.name} + </span> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.tsx` around lines 141 - 145, The single-rundown branch can render an empty title because currentRundown and firstRundown are optional; update the JSX that renders the <span className="rundown-name"> to fall back to playlist.name when (currentRundown ?? firstRundown)?.name is undefined or empty. Locate the conditional that uses rundownCount and change the expression for the single-rundown branch to use (currentRundown ?? firstRundown)?.name ?? playlist.name (or equivalent) so the header always shows playlist.name as a fallback.packages/job-worker/src/playout/tTimers.ts-209-213 (1)
209-213:⚠️ Potential issue | 🟡 MinorRemove
undefinedfrom return statement in void function.The function is typed
voidand should use a barereturnstatement, consistent with line 40 of the same function.🔧 Suggested fix
if (timerAnchors.size === 0) { if (span) span.end() - return undefined + return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/job-worker/src/playout/tTimers.ts` around lines 209 - 213, The early-exit branch checks timerAnchors.size and calls span.end(), but returns "undefined" in a function declared void; replace "return undefined" with a bare "return" to match the function's void signature and the earlier return usage (see the other early return that uses just "return"), keeping the span.end() call and referencing the same timerAnchors and span variables.
🧹 Nitpick comments (13)
packages/webui/src/client/styles/countdown/director.scss (1)
498-549: Consider tokenizing newly added literal colors for easier theming.This block introduces several hardcoded colors. Defining local SCSS variables (or reusing colorScheme tokens) would make future theme changes safer and more consistent.
♻️ Suggested refactor
+ $t-timer-bg-color: `#333`; + $t-timer-text-color: `#fff`; + $t-timer-dimmed-color: `#aaa`; + $t-timer-under-bg-color: `#ffe900`; + $t-timer-under-text-color: `#000`; + .director-screen__body__t-timer { ... .t-timer-display { - background: `#333`; + background: $t-timer-bg-color; ... &__label { - color: `#fff`; + color: $t-timer-text-color; } &__value { - color: `#fff`; + color: $t-timer-text-color; .t-timer-display__part { &--dimmed { - color: `#aaa`; + color: $t-timer-dimmed-color; } } } &__over-under { &--over { background-color: $general-late-color; - color: `#fff`; + color: $t-timer-text-color; } &--under { - background-color: `#ffe900`; - color: `#000`; + background-color: $t-timer-under-bg-color; + color: $t-timer-under-text-color; } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/styles/countdown/director.scss` around lines 498 - 549, This SCSS block hardcodes multiple colors; replace literal color values with SCSS variables or existing theme tokens to enable theming. Update the selectors (&__label, &__value, &__over-under and modifiers &--over, &--under, and the .t-timer-display__part--dimmed color) to use new or existing variables (e.g., $countdown-bg, $countdown-text, $countdown-dim, $countdown-over-bg, $countdown-under-bg) or the shared colorScheme tokens, and ensure variables are imported/defined at the top so the styles compile and match current color semantics.packages/webui/src/client/styles/contextMenu.scss (2)
67-74: Consider using Bootstrap CSS variable for border color consistency.The divider uses hardcoded
#dddfor its border, but line 19 usesvar(--bs-border-color)for the<hr>element. Using the CSS variable would maintain consistency with Bootstrap theming.♻️ Suggested change
&.react-contextmenu-item--divider { cursor: default; padding: 0; margin: 0 15px; width: auto; - border-bottom: 1px solid `#ddd`; + border-bottom: 1px solid var(--bs-border-color); height: 0; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/styles/contextMenu.scss` around lines 67 - 74, The divider rule .react-contextmenu-item--divider in contextMenu.scss currently uses a hardcoded border color `#ddd`; update that rule to use the Bootstrap CSS variable var(--bs-border-color) instead of `#ddd` so the context menu divider matches the project's Bootstrap theming (change border-bottom: 1px solid `#ddd` to border-bottom: 1px solid var(--bs-border-color)).
44-48: Consider consolidating redundant padding declarations.The
paddingshorthand on line 44 sets left/right to13px, but lines 47-48 immediately override these values. The horizontal padding from the shorthand is never applied.♻️ Suggested consolidation
.react-contextmenu-item { - padding: 2px 13px 4px 13px; + padding: 2px 30px 4px 18px; color: `#494949`; font-weight: 300; - padding-left: 18px; - padding-right: 30px; cursor: pointer;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/styles/contextMenu.scss` around lines 44 - 48, The rule that currently contains "padding: 2px 13px 4px 13px" is redundant because its horizontal values are immediately overridden by "padding-left: 18px" and "padding-right: 30px"; replace these four declarations with a single consolidated shorthand "padding: 2px 18px 4px 30px" (remove the separate padding-left and padding-right lines) so the block that contains "padding: 2px 13px 4px 13px" becomes the single authoritative padding definition.packages/webui/src/client/styles/countdown/presenter.scss (1)
179-244: Consider extracting repeated hard-coded timer colors into SCSS tokensThe new timer block is clear, but repeated literals (
#333,#fff,#aaa,#ffe900,#000) make theme updates harder. Centralizing these as local SCSS vars (or existing palette vars) will improve maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/styles/countdown/presenter.scss` around lines 179 - 244, The CSS uses repeated hard-coded colors (`#333`, `#fff`, `#aaa`, `#ffe900`, `#000`) inside .presenter-screen__rundown-status-bar__t-timer -> .t-timer-display and its children (&__label, &__value, &__over-under and modifiers) which makes theming hard; replace these literals with SCSS variables (either existing palette tokens or new local variables at the top of this file like $timer-bg, $timer-fore, $timer-dim, $timer-under, $timer-under-fore) and use $general-late-color for the over state as already done, updating .t-timer-display, .t-timer-display__label, .t-timer-display__value, .t-timer-display__part--dimmed, and .t-timer-display__over-under--under/--over to reference those variables.packages/webui/src/client/ui/RundownView/RundownHeader/HeaderFreezeFrameIcon.tsx (1)
18-26: Consider avoidingas anytype casts.The casts at lines 19 and 25 suggest a type mismatch between
PartInstanceIdand the timing map key types. If these are semantically the same, consider fixing the type definitions to maintain type safety. If truly incompatible, a type guard or explicit string conversion would be clearer thanas any.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/HeaderFreezeFrameIcon.tsx` around lines 18 - 26, The code is using `as any` when indexing timing maps with `partInstanceId`; remove the casts by making the key types compatible or converting the id explicitly: update the timing map types (e.g., change timingDurations.partDisplayDurations and timingDurations.partDurations to use the same PartInstanceId key type or Record<PartInstanceId, ...>), or if the maps are keyed by string, coerce `partInstanceId` to string with a clear conversion before indexing. Locate the expressions that compute `partDisplayDuration` and `partDuration` (references: partDisplayDuration, partInstanceId, timingDurations.partDisplayDurations, timingDurations.partDurations) and replace the `as any` usages with either corrected types or an explicit, typed conversion/guard so TypeScript no longer requires `as any`.packages/webui/src/client/styles/bootstrap-customize.scss (1)
9-9: Prefer1remfor Bootstrap body font size token.Line 9 hardcodes
16px, but Bootstrap 5.3+ defaults to1remfor--bs-body-font-sizeand explicitly recommends this for accessibility. Using1remrespects user text-size preferences while maintaining the same visual default.Suggested change
- --bs-body-font-size: 16px; + --bs-body-font-size: 1rem;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/styles/bootstrap-customize.scss` at line 9, Update the Bootstrap body font-size token by replacing the hardcoded pixel value for the CSS custom property --bs-body-font-size with the rem-based token "1rem"; locate the declaration of --bs-body-font-size in bootstrap-customize.scss and change its value to 1rem, and confirm there are no other conflicting overrides of --bs-body-font-size elsewhere in the file or project that would negate this change.packages/job-worker/src/blueprints/context/services/TTimersService.ts (2)
31-41: Consider adding defensive validation for timer array length.The constructor assumes the
timersarray has exactly 3 elements. While the data model should enforce this, a defensive check could prevent runtime errors if the invariant is violated.💡 Suggested defensive check
constructor( timers: ReadonlyDeep<RundownTTimer[]>, emitChange: (updatedTimer: ReadonlyDeep<RundownTTimer>) => void, playoutModel: PlayoutModel, jobContext: JobContext ) { + if (timers.length !== 3) { + throw new Error(`Expected exactly 3 timers, got ${timers.length}`) + } this.timers = [ new PlaylistTTimerImpl(timers[0], emitChange, playoutModel, jobContext), new PlaylistTTimerImpl(timers[1], emitChange, playoutModel, jobContext), new PlaylistTTimerImpl(timers[2], emitChange, playoutModel, jobContext), ] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/job-worker/src/blueprints/context/services/TTimersService.ts` around lines 31 - 41, The constructor for TTimersService assumes timers has three elements and directly indexes timers[0..2], which can throw if the array is shorter; add a defensive validation in the constructor (before creating PlaylistTTimerImpl instances) to check timers.length === 3 (or at least >= 3 depending on intended behavior) and handle violations by either throwing a clear error or filling missing entries with sensible defaults, then use the validated/normalized array to instantiate PlaylistTTimerImpl for each slot (referencing the constructor, this.timers, timers, and PlaylistTTimerImpl).
219-224: Consider logging or returning a result when part is not found.
setEstimateAnchorPartByExternalIdsilently returns without any indication if the part with the givenexternalIdis not found. This could mask issues if blueprints pass incorrect external IDs.Consider returning a boolean to indicate success/failure, or logging a warning when the part is not found.
💡 Suggested improvement
setEstimateAnchorPartByExternalId(externalId: string): void { const part = this.#playoutModel.getAllOrderedParts().find((p) => p.externalId === externalId) - if (!part) return + if (!part) { + // Part not found - could optionally log a warning here + return + } this.setEstimateAnchorPart(unprotectString(part._id)) }Alternatively, change the return type to
booleanto indicate success:- setEstimateAnchorPartByExternalId(externalId: string): void { + setEstimateAnchorPartByExternalId(externalId: string): boolean { const part = this.#playoutModel.getAllOrderedParts().find((p) => p.externalId === externalId) - if (!part) return + if (!part) return false this.setEstimateAnchorPart(unprotectString(part._id)) + return true }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/job-worker/src/blueprints/context/services/TTimersService.ts` around lines 219 - 224, The method setEstimateAnchorPartByExternalId currently silently returns if no part matches the externalId, which can hide bugs; update this function to either return a boolean indicating success/failure or emit a warning when no part is found: locate setEstimateAnchorPartByExternalId (which uses `#playoutModel.getAllOrderedParts`().find(...) and calls setEstimateAnchorPart(unprotectString(part._id))) and change its signature to return boolean (true after calling setEstimateAnchorPart, false if part is not found) or, if keeping void, add a process/logger warning (or debug) that includes the externalId when the part lookup fails so callers can detect/more easily diagnose bad external IDs.packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderPlannedStart.tsx (2)
11-14: Consider usingreadonlyfor props consistency.The sibling component
RundownHeaderExpectedEnd.tsxmarks its props asreadonly, but this component does not. For consistency across the codebase, consider applying the same pattern here.Suggested change
}: { - playlist: DBRundownPlaylist - simplified?: boolean + readonly playlist: DBRundownPlaylist + readonly simplified?: boolean }): JSX.Element | null {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderPlannedStart.tsx` around lines 11 - 14, The props object for the RundownHeaderPlannedStart component should be marked readonly to match sibling RundownHeaderExpectedEnd; update the component signature/type for RundownHeaderPlannedStart to use a readonly props shape (e.g., change the inline object type for playlist and simplified to readonly) so the parameter becomes readonly { playlist: DBRundownPlaylist; simplified?: boolean }, and ensure the JSX.Element | null return signature remains unchanged.
26-28: Redundant condition check.After the early return on line 19 (
if (expectedStart == null) return null),expectedStartis guaranteed to be defined. The checkexpectedStart !== undefinedon line 26 is always true at this point.Simplified condition
- {!simplified && expectedStart !== undefined ? ( + {!simplified ? ( <Countdown label={t('Plan. Start')} time={expectedStart} className="rundown-header__show-timers-countdown" /> ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderPlannedStart.tsx` around lines 26 - 28, Remove the redundant undefined check on expectedStart in the JSX: since RundownHeaderPlannedStart returns early when expectedStart == null (line with the early return), expectedStart is guaranteed non-null when rendering; change the conditional rendering to only check simplified (i.e., render Countdown when !simplified) and keep the existing Countdown props (label, time={expectedStart}, className) and component reference (Countdown) intact.packages/job-worker/src/playout/__tests__/tTimersJobs.test.ts (1)
16-63: Tests only verify no errors, not correct behavior.The test "should handle studio with active playlists" sets up a playlist with tTimers but only asserts that the function resolves to
undefined. There's no verification that:
- Timer estimates were actually recalculated
- The playlist document was updated with new estimates
- The correct timers were processed
Consider adding assertions that verify the function's side effects, such as checking that
context.directCollections.RundownPlaylists.findOne()returns updated timer data after the call.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/job-worker/src/playout/__tests__/tTimersJobs.test.ts` around lines 16 - 63, The test only asserts no error from handleRecalculateTTimerEstimates but doesn't verify side effects; after calling handleRecalculateTTimerEstimates(context) fetch the playlist via context.directCollections.RundownPlaylists.findOne({_id: playlistId}) and assert the tTimers array was updated with the expected estimate/state/mode fields (or specific properties your implementation writes), and add assertions that the correct timer entries (e.g., index 1/2/3) were processed (non-null estimates or changed mode/state) to confirm recalculation occurred rather than merely resolving.packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderDurations.tsx (1)
27-31: Zero duration would be hidden.The condition
!simplified && expectedDurationwould evaluate to false ifexpectedDurationis0, hiding the planned duration display. If a zero duration is a valid edge case that should be displayed, consider using explicit comparison:Handle zero duration explicitly
- {!simplified && expectedDuration ? ( + {!simplified && expectedDuration !== undefined ? (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderDurations.tsx` around lines 27 - 31, The planned duration is hidden when expectedDuration === 0 because the conditional uses truthiness; update the rendering condition in RundownHeaderDurations (the JSX using Countdown and expectedDuration) to explicitly check for a non-null/undefined value (e.g., expectedDuration !== undefined && expectedDuration !== null) while keeping the !simplified guard so that zero durations are rendered; leave the Countdown and RundownUtils.formatDiffToTimecode usage unchanged.packages/webui/src/client/ui/RundownView/RundownHeader/Countdown.scss (1)
21-41: Consider using color scheme variables for hardcoded colors.Multiple hardcoded color values (
#ffffff,#40b8fa) are used throughout the file while_colorSchemeis imported. For consistency and easier theme management, consider using color variables from the scheme.Also applies to: 43-63, 76-79, 81-86, 88-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/Countdown.scss` around lines 21 - 41, Replace hardcoded colors like `#ffffff` and `#40b8fa` in Countdown.scss with the corresponding variables from the imported _colorScheme to ensure theme consistency; update the &__counter rule and the other color usages around the noted ranges (approx. lines 43-63, 76-79, 81-86, 88-94) to reference the scheme variables (e.g., the scheme's primary/foreground/interactive color tokens) instead of literal hex values, and ensure any variable names used match the exported names from _colorScheme so the stylesheet compiles and theming works correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 572bcdaf-320f-463f-bf64-f3e8020a124e
⛔ Files ignored due to path filters (4)
meteor/yarn.lockis excluded by!**/yarn.lock,!**/*.lockpackages/job-worker/src/ingest/mosDevice/__tests__/__snapshots__/mosIngest.test.ts.snapis excluded by!**/*.snappackages/job-worker/src/playout/__tests__/__snapshots__/playout.test.ts.snapis excluded by!**/*.snappackages/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (91)
meteor/__mocks__/defaultCollectionObjects.tsmeteor/server/__tests__/cronjobs.test.tsmeteor/server/api/__tests__/externalMessageQueue.test.tsmeteor/server/api/__tests__/peripheralDevice.test.tsmeteor/server/migration/X_X_X.tspackage.jsonpackages/blueprints-integration/src/api/showStyle.tspackages/blueprints-integration/src/context/adlibActionContext.tspackages/blueprints-integration/src/context/onSetAsNextContext.tspackages/blueprints-integration/src/context/onTakeContext.tspackages/blueprints-integration/src/context/partsAndPieceActionContext.tspackages/blueprints-integration/src/context/rundownContext.tspackages/blueprints-integration/src/context/syncIngestChangesContext.tspackages/blueprints-integration/src/context/tTimersContext.tspackages/corelib/src/dataModel/RundownPlaylist.tspackages/corelib/src/worker/studio.tspackages/job-worker/package.jsonpackages/job-worker/src/__mocks__/defaultCollectionObjects.tspackages/job-worker/src/blueprints/__tests__/context-OnSetAsNextContext.test.tspackages/job-worker/src/blueprints/__tests__/context-OnTakeContext.test.tspackages/job-worker/src/blueprints/__tests__/context-adlibActions.test.tspackages/job-worker/src/blueprints/context/OnSetAsNextContext.tspackages/job-worker/src/blueprints/context/OnTakeContext.tspackages/job-worker/src/blueprints/context/RundownActivationContext.tspackages/job-worker/src/blueprints/context/SyncIngestUpdateToPartInstanceContext.tspackages/job-worker/src/blueprints/context/adlibActions.tspackages/job-worker/src/blueprints/context/services/PartAndPieceInstanceActionService.tspackages/job-worker/src/blueprints/context/services/TTimersService.tspackages/job-worker/src/blueprints/context/services/__tests__/TTimersService.test.tspackages/job-worker/src/events/__tests__/externalMessageQueue.test.tspackages/job-worker/src/ingest/__tests__/syncChangesToPartInstance-computeCurrentPartIndex.test.tspackages/job-worker/src/ingest/__tests__/syncChangesToPartInstance.test.tspackages/job-worker/src/ingest/__tests__/updateNext.test.tspackages/job-worker/src/ingest/commit.tspackages/job-worker/src/ingest/syncChangesToPartInstance.tspackages/job-worker/src/playout/__tests__/tTimers.test.tspackages/job-worker/src/playout/__tests__/tTimersJobs.test.tspackages/job-worker/src/playout/lookahead/util.tspackages/job-worker/src/playout/model/PlayoutModel.tspackages/job-worker/src/playout/model/implementation/PlayoutModelImpl.tspackages/job-worker/src/playout/setNext.tspackages/job-worker/src/playout/tTimers.tspackages/job-worker/src/playout/tTimersJobs.tspackages/job-worker/src/rundownPlaylists.tspackages/job-worker/src/workers/studio/child.tspackages/job-worker/src/workers/studio/jobs.tspackages/live-status-gateway/src/topics/__tests__/utils.tspackages/openapi/run_server_tests.mjspackages/webui/src/__mocks__/defaultCollectionObjects.tspackages/webui/src/client/lib/__tests__/rundownTiming.test.tspackages/webui/src/client/lib/rundownTiming.tspackages/webui/src/client/lib/tTimerUtils.tspackages/webui/src/client/styles/_colorScheme.scsspackages/webui/src/client/styles/bootstrap-customize.scsspackages/webui/src/client/styles/contextMenu.scsspackages/webui/src/client/styles/countdown/director.scsspackages/webui/src/client/styles/countdown/presenter.scsspackages/webui/src/client/styles/defaultColors.scsspackages/webui/src/client/styles/notifications.scsspackages/webui/src/client/styles/rundownView.scsspackages/webui/src/client/ui/ClockView/CameraScreen/Part.tsxpackages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreen.tsxpackages/webui/src/client/ui/ClockView/PresenterScreen.tsxpackages/webui/src/client/ui/ClockView/TTimerDisplay.tsxpackages/webui/src/client/ui/RundownView.tsxpackages/webui/src/client/ui/RundownView/RundownHeader/Countdown.scsspackages/webui/src/client/ui/RundownView/RundownHeader/Countdown.tsxpackages/webui/src/client/ui/RundownView/RundownHeader/CurrentPartOrSegmentRemaining.tsxpackages/webui/src/client/ui/RundownView/RundownHeader/HeaderFreezeFrameIcon.tsxpackages/webui/src/client/ui/RundownView/RundownHeader/RundownContextMenu.tsxpackages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scsspackages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.tsxpackages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderDurations.tsxpackages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderExpectedEnd.tsxpackages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderPlannedStart.tsxpackages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderTimers.tsxpackages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderTimingDisplay.tsxpackages/webui/src/client/ui/RundownView/RundownHeader/TimingDisplay.tsxpackages/webui/src/client/ui/RundownView/RundownHeader/_shared.scsspackages/webui/src/client/ui/RundownView/RundownHeader/useRundownPlaylistOperations.tsxpackages/webui/src/client/ui/RundownView/RundownNotifier.tsxpackages/webui/src/client/ui/RundownView/RundownRightHandControls.tsxpackages/webui/src/client/ui/RundownView/RundownTiming/CurrentPartOrSegmentRemaining.tsxpackages/webui/src/client/ui/RundownView/RundownTiming/TimeOfDay.tsxpackages/webui/src/client/ui/RundownView/RundownViewContextProviders.tsxpackages/webui/src/client/ui/SegmentList/LinePart.tsxpackages/webui/src/client/ui/SegmentList/OnAirLine.tsxpackages/webui/src/client/ui/SegmentList/SegmentListHeader.tsxpackages/webui/src/client/ui/SegmentStoryboard/StoryboardPart.tsxpackages/webui/src/client/ui/SegmentTimeline/SegmentTimeline.tsxpackages/webui/src/client/ui/Shelf/PartTimingPanel.tsx
💤 Files with no reviewable changes (2)
- packages/webui/src/client/ui/RundownView/RundownTiming/CurrentPartOrSegmentRemaining.tsx
- packages/webui/src/client/ui/RundownView/RundownHeader/TimingDisplay.tsx
Comment on lines
+197
to
+227
| it('should return running freeRun state', () => { | ||
| const tTimers = createEmptyTTimers() | ||
| tTimers[0].mode = { type: 'freeRun' } | ||
| tTimers[0].state = { paused: false, zeroTime: 15000 } | ||
| const updateFn = jest.fn() | ||
| const mockPlayoutModel = createMockPlayoutModel(tTimers) | ||
| const mockJobContext = createMockJobContext() | ||
| const timer = new PlaylistTTimerImpl(tTimers[0], updateFn, mockPlayoutModel, mockJobContext) | ||
|
|
||
| expect(timer.state).toEqual({ | ||
| mode: 'freeRun', | ||
| currentTime: 5000, // 10000 - 5000 | ||
| paused: false, // pauseTime is null = running | ||
| }) | ||
| }) | ||
|
|
||
| it('should return paused freeRun state', () => { | ||
| const tTimers = createEmptyTTimers() | ||
| tTimers[0].mode = { type: 'freeRun' } | ||
| tTimers[0].state = { paused: true, duration: 3000 } | ||
| const updateFn = jest.fn() | ||
| const mockPlayoutModel = createMockPlayoutModel(tTimers) | ||
| const mockJobContext = createMockJobContext() | ||
| const timer = new PlaylistTTimerImpl(tTimers[0], updateFn, mockPlayoutModel, mockJobContext) | ||
|
|
||
| expect(timer.state).toEqual({ | ||
| mode: 'freeRun', | ||
| currentTime: 3000, // 8000 - 5000 | ||
| paused: true, // pauseTime is set = paused | ||
| }) | ||
| }) |
There was a problem hiding this comment.
The free-run getter fixtures are inverted.
Lines 200 and 216 do not match the persisted freeRun convention exercised later in this file (see Lines 581 and 646). With zeroTime: 15000 and duration: 3000, these tests are seeding a future start time / positive paused duration, so they can validate the wrong sign contract.
🧪 Suggested fixture update
- tTimers[0].state = { paused: false, zeroTime: 15000 }
+ tTimers[0].state = { paused: false, zeroTime: 5000 }
- tTimers[0].state = { paused: true, duration: 3000 }
+ tTimers[0].state = { paused: true, duration: -3000 }📝 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.
Suggested change
| it('should return running freeRun state', () => { | |
| const tTimers = createEmptyTTimers() | |
| tTimers[0].mode = { type: 'freeRun' } | |
| tTimers[0].state = { paused: false, zeroTime: 15000 } | |
| const updateFn = jest.fn() | |
| const mockPlayoutModel = createMockPlayoutModel(tTimers) | |
| const mockJobContext = createMockJobContext() | |
| const timer = new PlaylistTTimerImpl(tTimers[0], updateFn, mockPlayoutModel, mockJobContext) | |
| expect(timer.state).toEqual({ | |
| mode: 'freeRun', | |
| currentTime: 5000, // 10000 - 5000 | |
| paused: false, // pauseTime is null = running | |
| }) | |
| }) | |
| it('should return paused freeRun state', () => { | |
| const tTimers = createEmptyTTimers() | |
| tTimers[0].mode = { type: 'freeRun' } | |
| tTimers[0].state = { paused: true, duration: 3000 } | |
| const updateFn = jest.fn() | |
| const mockPlayoutModel = createMockPlayoutModel(tTimers) | |
| const mockJobContext = createMockJobContext() | |
| const timer = new PlaylistTTimerImpl(tTimers[0], updateFn, mockPlayoutModel, mockJobContext) | |
| expect(timer.state).toEqual({ | |
| mode: 'freeRun', | |
| currentTime: 3000, // 8000 - 5000 | |
| paused: true, // pauseTime is set = paused | |
| }) | |
| }) | |
| it('should return running freeRun state', () => { | |
| const tTimers = createEmptyTTimers() | |
| tTimers[0].mode = { type: 'freeRun' } | |
| tTimers[0].state = { paused: false, zeroTime: 5000 } | |
| const updateFn = jest.fn() | |
| const mockPlayoutModel = createMockPlayoutModel(tTimers) | |
| const mockJobContext = createMockJobContext() | |
| const timer = new PlaylistTTimerImpl(tTimers[0], updateFn, mockPlayoutModel, mockJobContext) | |
| expect(timer.state).toEqual({ | |
| mode: 'freeRun', | |
| currentTime: 5000, // 10000 - 5000 | |
| paused: false, // pauseTime is null = running | |
| }) | |
| }) | |
| it('should return paused freeRun state', () => { | |
| const tTimers = createEmptyTTimers() | |
| tTimers[0].mode = { type: 'freeRun' } | |
| tTimers[0].state = { paused: true, duration: -3000 } | |
| const updateFn = jest.fn() | |
| const mockPlayoutModel = createMockPlayoutModel(tTimers) | |
| const mockJobContext = createMockJobContext() | |
| const timer = new PlaylistTTimerImpl(tTimers[0], updateFn, mockPlayoutModel, mockJobContext) | |
| expect(timer.state).toEqual({ | |
| mode: 'freeRun', | |
| currentTime: 3000, // 8000 - 5000 | |
| paused: true, // pauseTime is set = paused | |
| }) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/job-worker/src/blueprints/context/services/__tests__/TTimersService.test.ts`
around lines 197 - 227, The tests for PlaylistTTimerImpl use inverted freeRun
fixtures: tTimers[0].state.zeroTime and tTimers[0].state.duration are seeded
with positive values but the persisted convention expects negative offsets,
causing the assertions to validate the wrong sign; fix both tests by negating
those seeded values (set zeroTime to -15000 in the "running freeRun" case and
duration to -3000 in the "paused freeRun" case) so the computed
timer.state.currentTime and paused semantics match the expected comments.
Comment on lines
+159
to
+162
| // Persist t-timer changes | ||
| for (const timer of syncContext.changedTTimers) { | ||
| this.#playoutModel.updateTTimer(timer) | ||
| } |
There was a problem hiding this comment.
Rollback coverage does not include partially applied timer updates.
If a blueprint queues multiple timer changes and updateTTimer() throws partway through, earlier timer writes stay on the playlist because the catch only restores existingPartInstance. Snapshot/restore the touched timer slots, or validate the whole batch before mutating the playlist.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/job-worker/src/ingest/syncChangesToPartInstance.ts` around lines 159
- 162, The loop in syncChangesToPartInstance applies each timer via
this.#playoutModel.updateTTimer(timer) without atomicity, so if updateTTimer
throws mid-loop earlier timer writes remain; fix by snapshotting the timer slots
touched (from syncContext.changedTTimers) before any mutation and on error
restore those slots from the snapshot (or alternatively validate all timers in
changedTTimers up-front and abort if any validation fails) so that updateTTimer
calls are effectively all-or-nothing; reference the syncChangesToPartInstance
function, syncContext.changedTTimers, and the this.#playoutModel.updateTTimer
method when implementing the snapshot/restore or pre-validation logic.
Comment on lines
+261
to
+277
| } else { | ||
| // Segment budget timing - we're already inside a budgeted segment | ||
| const segmentStartedPlayback = | ||
| playlist.segmentsStartedPlayback?.[currentPartInstance.segmentId as unknown as string] | ||
| if (segmentStartedPlayback) { | ||
| const segmentElapsed = now - segmentStartedPlayback | ||
| const remaining = currentSegmentBudget - segmentElapsed | ||
| isPushing = remaining < 0 | ||
| totalAccumulator = Math.max(0, remaining) | ||
| } else { | ||
| totalAccumulator = currentSegmentBudget | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Save remaining current part time for pauseTime calculation | ||
| const currentPartRemainingTime = totalAccumulator |
There was a problem hiding this comment.
Don’t seed totalAccumulator with the whole remaining segment budget.
In the budgeted-current-segment path, totalAccumulator becomes “time until segment end”, but Line 277 then reuses it as currentPartRemainingTime, and Line 298 adds the full segment budget again on the next boundary flush. That makes anchors inside the current segment resolve to the segment end, pushes pauseTime out to the segment end, and overstates anchors after the segment by another full budget.
Keep the remaining segment budget separate from the current-part remaining time, and only fold the segment budget into the total when the iteration actually crosses that segment boundary.
Also applies to: 296-301, 317-327
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/job-worker/src/playout/tTimers.ts` around lines 261 - 277, The bug
seeds totalAccumulator with the entire remaining segment budget when inside a
budgeted segment (using segmentStartedPlayback/currentSegmentBudget), causing
currentPartRemainingTime to incorrectly equal time-to-segment-end and
double-counting the budget on boundary flushes; fix by introducing a separate
segmentRemaining (or similar) variable to hold currentSegmentBudget -
segmentElapsed and keep currentPartRemainingTime derived only from the
part-specific timing, then only add segmentRemaining into totalAccumulator when
the iteration actually crosses that segment boundary (i.e., at the point where
you advance past the segment), updating any uses around totalAccumulator,
currentPartRemainingTime, and pauseTime computation accordingly so anchors
inside the segment use part remaining time and budgets are folded in only when
crossed.
Comment on lines
+493
to
495
| > .rundown-header_OLD .rundown-overview { | ||
| padding-right: calc(25vw + 1.5em) !important; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm the changed selector in SCSS
rg -n -C2 '\.rundown-header(_OLD)?\s+\.rundown-overview' packages/webui/src/client/styles/notifications.scss
# 2) Locate runtime class usage in UI code
rg -n -C2 'rundown-header_OLD|rundown-header\b|rundown-overview' packages/webui/src/client
# 3) Verify where notification-center-open state is applied
rg -n -C2 'notification-center-open|rundown-view' packages/webui/src/clientRepository: Sofie-Automation/sofie-core
Length of output: 44480
Support both .rundown-header and .rundown-header_OLD in this selector to prevent responsive regression.
On Line 493, the rule targets .rundown-header_OLD only, but RundownHeader.tsx renders the active header with class .rundown-header (without the _OLD suffix). This selector mismatch means the padding adjustment won't apply when the notification center is open at widths below 1600px, allowing the overview to overlap the notification panel.
Recommended fix
- > .rundown-header_OLD .rundown-overview {
+ > .rundown-header_OLD .rundown-overview,
+ > .rundown-header .rundown-overview {
padding-right: calc(25vw + 1.5em) !important;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/webui/src/client/styles/notifications.scss` around lines 493 - 495,
Update the CSS selector so the rule applies to both active header classes by
targeting .rundown-header and .rundown-header_OLD together for the
.rundown-overview rule; modify the selector that currently references
.rundown-header_OLD .rundown-overview to include .rundown-header
.rundown-overview as well (so the padding-right calc(25vw + 1.5em) !important;
is applied for both header variants).
Comment on lines
+24
to
+25
| // global variable for remembering last uttered displayTime | ||
| let prevDisplayTime: number | undefined = undefined |
There was a problem hiding this comment.
Don't let passive timer instances suppress countdown speech/vibration.
prevDisplayTime is module-global and every hook instance updates it, even when that instance is only rendering. If the same countdown is mounted in more than one place, a passive instance can advance the cursor and prevent the speaking/vibrating instance from firing.
🔧 Keep the notification cursor instance-local
-// global variable for remembering last uttered displayTime
-let prevDisplayTime: number | undefined = undefined
-
function usePartRemaining(props: IPartRemainingProps) {
const timingDurations = useTiming(TimingTickResolution.Synced, TimingDataResolution.Synced)
const prevPartInstanceId = useRef<PartInstanceId | null>(null)
+ const prevDisplayTimeRef = useRef<number | undefined>(undefined)
useEffect(() => {
if (props.currentPartInstanceId !== prevPartInstanceId.current) {
- prevDisplayTime = undefined
+ prevDisplayTimeRef.current = undefined
prevPartInstanceId.current = props.currentPartInstanceId
}
@@
- if (prevDisplayTime !== displayTime) {
+ if ((props.speaking || props.vibrating) && prevDisplayTimeRef.current !== displayTime) {
if (props.speaking) {
speak(displayTime)
}
if (props.vibrating) {
vibrate(displayTime)
}
- prevDisplayTime = displayTime
+ prevDisplayTimeRef.current = displayTime
}
}, [Also applies to: 83-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/webui/src/client/ui/RundownView/RundownHeader/CurrentPartOrSegmentRemaining.tsx`
around lines 24 - 25, The module-global prevDisplayTime should be made
instance-local so passive mounts don't advance the shared cursor; replace the
top-level let prevDisplayTime with a per-instance ref (e.g., const
prevDisplayTimeRef = useRef<number|undefined>(undefined)) inside the
component/hook (CurrentPartOrSegmentRemaining) and change all uses of
prevDisplayTime to read/write prevDisplayTimeRef.current (also update the
identical logic referenced around lines 83-113), ensuring each hook instance
tracks its own last-uttered displayTime so only the active instance triggers
speech/vibration.
Comment on lines
+181
to
+216
| const handleToggle = useCallback( | ||
| (e: React.MouseEvent) => { | ||
| e.preventDefault() | ||
| e.stopPropagation() | ||
|
|
||
| if (disabled) return | ||
|
|
||
| if (isOpen) { | ||
| hideMenu({ id: RUNDOWN_CONTEXT_MENU_ID }) | ||
| onClose() | ||
| return | ||
| } | ||
|
|
||
| if (buttonRef.current) { | ||
| const rect = buttonRef.current.getBoundingClientRect() | ||
| showMenu({ | ||
| position: { x: rect.left, y: rect.bottom + 5 }, | ||
| id: RUNDOWN_CONTEXT_MENU_ID, | ||
| }) | ||
| if (onOpen) onOpen() | ||
| } | ||
| }, | ||
| [isOpen, disabled, onClose, onOpen] | ||
| ) | ||
|
|
||
| return ( | ||
| <button | ||
| ref={buttonRef} | ||
| className={`rundown-header__menu-btn ${isOpen ? 'active' : ''} ${disabled ? 'disabled' : ''}`} | ||
| disabled={disabled} | ||
| onMouseDown={handleToggle} | ||
| onClick={(e) => { | ||
| // Prevent double trigger if browser emits both mousedown and click | ||
| e.preventDefault() | ||
| e.stopPropagation() | ||
| }} |
There was a problem hiding this comment.
Move the hamburger toggle to onClick so the control stays keyboard-accessible.
The menu only opens/closes from onMouseDown, while onClick is cancelled. Enter/Space on a button fire click, not mousedown, so keyboard users can't operate this menu.
♿ Minimal fix
return (
<button
ref={buttonRef}
+ type="button"
className={`rundown-header__menu-btn ${isOpen ? 'active' : ''} ${disabled ? 'disabled' : ''}`}
disabled={disabled}
- onMouseDown={handleToggle}
- onClick={(e) => {
- // Prevent double trigger if browser emits both mousedown and click
- e.preventDefault()
- e.stopPropagation()
- }}
+ onClick={handleToggle}
title={t('Menu')}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/webui/src/client/ui/RundownView/RundownHeader/RundownContextMenu.tsx`
around lines 181 - 216, The toggle handler is wired to onMouseDown which
prevents keyboard activation; move the handler to the button's onClick so
Enter/Space can operate it: attach handleToggle to the button's onClick prop
(replace the current onMouseDown={handleToggle} and remove the inline onClick
that calls preventDefault/stopPropagation), keeping the same logic in
handleToggle (including disabled check, buttonRef usage and showMenu/hideMenu
calls) and the active/disabled class handling; this preserves mouse behavior
while restoring keyboard accessibility for the RundownContextMenu toggle.
Comment on lines
+44
to
+45
| const isCountingDown = mode.type === 'countdown' && diff < 0 && isRunning | ||
|
|
There was a problem hiding this comment.
Use the same sign convention for completion that you use for countdown.
Line 44 treats diff < 0 as “still counting down”, but Lines 58-59 mark the timer complete on diff <= 0. That applies the complete state for the whole countdown and drops it once the timer overruns. This check needs to flip to diff >= 0.
🛠️ Suggested fix
'rundown-header__clocks-timers__timer__isCountingUp': mode.type === 'countdown' && !isCountingDown,
'rundown-header__clocks-timers__timer__isComplete':
- mode.type === 'countdown' && timer.state !== null && diff <= 0,
+ mode.type === 'countdown' && timer.state !== null && diff >= 0,
})}Based on learnings, in packages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreenTop.tsx, the TimeToPlannedEndComponent is intentionally passed now - expectedEnd (a negative value when counting down), not expectedEnd - now.
Also applies to: 58-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeaderTimers.tsx`
around lines 44 - 45, The countdown sign convention is inconsistent:
isCountingDown uses diff < 0, but the completion check marks the timer complete
with diff <= 0; change the completion condition to diff >= 0 so it matches the
countdown sign (negative while counting down). Update the conditional(s) in
RundownHeaderTimers.tsx that determine "complete" (the code near isCountingDown
and the completion check around diff <= 0) to use diff >= 0 for completion,
keeping the isCountingDown expression (mode.type === 'countdown' && diff < 0 &&
isRunning) unchanged.
Julusian
added
the
Contribution from BBC
Contributor
|
Adding myself as a reviewer only for the functional parts of it, not the code side. Will sign off when all my checks are done. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/webui/src/client/lib/tTimerUtils.ts`:
- Line 22: Narrow the union on timer.mode before accessing stopAtZero: instead
of directly using timer.mode?.stopAtZero, first check the discriminator or
property existence (e.g., assert timer.mode?.type === 'countdown' or use
`'stopAtZero' in timer.mode`) and only then read timer.mode.stopAtZero when true
and currentDuration < 0; update the condition around
timer.mode/stopAtZero/currentDuration to perform this runtime/type guard so
TypeScript strict checking succeeds for the timer.mode union.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e87ef4d-6250-41a8-844b-8026a4448528
📒 Files selected for processing (1)
packages/webui/src/client/lib/tTimerUtils.ts
| } | ||
|
|
||
| // Apply stopAtZero if configured | ||
| if (timer.mode?.stopAtZero && currentDuration < 0) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify RundownTTimerMode members and local stopAtZero access pattern
set -euo pipefail
echo "== RundownTTimerMode declarations =="
fd RundownPlaylist.ts | while read -r f; do
rg -n -C3 'interface RundownTTimerModeFreeRun|interface RundownTTimerModeCountdown|interface RundownTTimerModeTimeOfDay|stopAtZero' "$f" || true
done
echo
echo "== stopAtZero usage in reviewed file =="
rg -n -C2 'stopAtZero|mode\?\.type|mode\.' packages/webui/src/client/lib/tTimerUtils.tsRepository: Sofie-Automation/sofie-core
Length of output: 1206
🏁 Script executed:
#!/bin/bash
# Check TypeScript configuration and similar discriminated union patterns
echo "== tsconfig.json (strict mode settings) =="
fd tsconfig.json | head -5 | xargs -I {} cat {} | grep -E '"strict"|"strictNullChecks"|"noImplicitAny"' || echo "No strict settings found"
echo
echo "== Other timer.mode access patterns in codebase =="
rg -n 'timer\.mode\?' packages/webui/src/client --type ts --type tsx -C2 | head -50
echo
echo "== Similar discriminated union property access patterns =="
rg -n '\?\..*&&' packages/webui/src/client/lib --type ts -C1 | head -40Repository: Sofie-Automation/sofie-core
Length of output: 3825
🏁 Script executed:
#!/bin/bash
# Check tsconfig inheritance and linting config
echo "== Full tsconfig.json content (packages/webui) =="
find packages/webui -name tsconfig.json -exec cat {} \; | head -100
echo
echo "== ESLint / TypeScript lint config =="
find packages/webui -name '.eslintrc*' -o -name 'eslint.config.*' | xargs cat 2>/dev/null || echo "No ESLint config found"
echo
echo "== Full tTimerUtils.ts to verify type narrowing context =="
cat packages/webui/src/client/lib/tTimerUtils.tsRepository: Sofie-Automation/sofie-core
Length of output: 1950
🏁 Script executed:
#!/bin/bash
# Check tsconfig.app.json for strict mode and full inheritance
echo "== tsconfig.app.json (strict mode check) =="
cat packages/webui/tsconfig.app.json
echo
echo "== Full TypeScript compiler options =="
cat packages/webui/tsconfig.app.json | jq '.compilerOptions | keys' 2>/dev/null || cat packages/webui/tsconfig.app.json | grep -A 50 '"compilerOptions"'Repository: Sofie-Automation/sofie-core
Length of output: 1992
🏁 Script executed:
#!/bin/bash
# Verify if this actually causes a TypeScript compilation error
echo "== Check if build/typecheck runs without errors =="
find . -name "package.json" -path "*/packages/webui/*" -exec grep -l "typecheck\|tsc" {} \;
echo
echo "== Look for CI/build config =="
find . -name ".github" -o -name ".gitlab-ci.yml" -o -name "Makefile" | head -5
echo
echo "== Attempt to understand type narrowing with optional chaining =="
# Check if there are other discriminated union checks using non-optional syntax
rg -n 'mode\.type.*===' packages/webui/src/client --type ts -C1 | head -30
</script>Repository: Sofie-Automation/sofie-core
Length of output: 2679
🏁 Script executed:
#!/bin/bash
# Check if the code actually compiles
echo "== Check webui package.json for type-check script =="
cat packages/webui/package.json | jq '.scripts | keys[]' 2>/dev/null | grep -i type || cat packages/webui/package.json | grep -A 5 '"scripts"'
echo
echo "== Try to understand the type at line 22 context =="
# Check: is timer.mode guaranteed to exist after line 17 return?
# Or is it still optional<RundownTTimerMode>?
ast-grep --pattern 'if (timer.mode?.type === $_) { $$$; }' 2>/dev/null || echo "No match found"
echo
echo "== Check RundownTTimer type definition =="
rg -n 'export interface RundownTTimer|readonly mode' packages/ --type ts | grep -A 2 'RundownTTimer'Repository: Sofie-Automation/sofie-core
Length of output: 818
🏁 Script executed:
#!/bin/bash
# Get full RundownTTimer interface definition
echo "== Full RundownTTimer interface =="
rg -n -A 20 'export interface RundownTTimer' packages/corelib/src/dataModel/RundownPlaylist.ts
echo
echo "== Verify: is .mode optional in RundownTTimer? =="
rg -n 'readonly mode' packages/corelib/src/dataModel/RundownPlaylist.ts | grep -E 'RundownTTimer[^a-zA-Z]|^\s*readonly mode'Repository: Sofie-Automation/sofie-core
Length of output: 2108
🏁 Script executed:
#!/bin/bash
# Try to run type-check on the webui package to see if this is a real error
echo "== Check if there's a build/CI that would catch this =="
find . -name "*.yml" -path "*/.github/*" | head -3 | xargs grep -l "check-types\|tsc" 2>/dev/null || echo "Searching for CI workflow..."
echo
echo "== Search for GitHub Actions workflow =="
find .github/workflows -name "*.yml" -o -name "*.yaml" 2>/dev/null | head -5
echo
echo "== Check for actual type errors in build logs =="
find . -name ".tsbuildinfo" -o -name "build-errors.log" 2>/dev/null | head -3Repository: Sofie-Automation/sofie-core
Length of output: 416
🏁 Script executed:
#!/bin/bash
# Check if CI runs type checking
echo "== node.yaml workflow (type-check step) =="
cat .github/workflows/node.yaml | grep -A 5 -B 5 "check-types\|tsc" || echo "No explicit type-check in workflow"
echo
echo "== Full node.yaml to see all build steps =="
cat .github/workflows/node.yaml | head -100Repository: Sofie-Automation/sofie-core
Length of output: 3988
🏁 Script executed:
#!/bin/bash
# Check if the webui build includes type checking and what build:single does
echo "== Look at what build:single script does =="
rg -n 'build:single' packages/webui/package.json -A 2 -B 2 2>/dev/null || find packages -name "package.json" -exec grep -l "build:single" {} \; | head -1 | xargs cat | jq '.scripts["build:single"]'
echo
echo "== Check if build:single includes tsc/type-check =="
find . -name "*.ts" -path "*/scripts/*" | xargs grep -l "build:single" 2>/dev/null | head -3
echo
echo "== Look at root package.json for build:single =="
cat package.json | jq '.scripts["build:single"]' 2>/dev/null || grep -A 1 '"build:single"' package.json
echo
echo "== Check if there's a turbo.json or build config =="
cat turbo.json 2>/dev/null | head -50 || echo "No turbo.json"Repository: Sofie-Automation/sofie-core
Length of output: 310
Narrow mode before reading stopAtZero to avoid union-type breakage.
At Line 22, timer.mode?.stopAtZero is accessed without narrowing mode.type. stopAtZero is not present on freeRun, so this fails TS strict-mode type-checking during tsc --build.
💡 Suggested fix
- // Apply stopAtZero if configured
- if (timer.mode?.stopAtZero && currentDuration < 0) {
+ // Apply stopAtZero for countdown/timeOfDay modes
+ if (
+ (timer.mode?.type === 'countdown' || timer.mode?.type === 'timeOfDay') &&
+ timer.mode.stopAtZero &&
+ currentDuration < 0
+ ) {
return 0
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/webui/src/client/lib/tTimerUtils.ts` at line 22, Narrow the union on
timer.mode before accessing stopAtZero: instead of directly using
timer.mode?.stopAtZero, first check the discriminator or property existence
(e.g., assert timer.mode?.type === 'countdown' or use `'stopAtZero' in
timer.mode`) and only then read timer.mode.stopAtZero when true and
currentDuration < 0; update the condition around
timer.mode/stopAtZero/currentDuration to perform this runtime/type guard so
TypeScript strict checking succeeds for the timer.mode union.
Contributor
Author
There was a problem hiding this comment.
No. New modes may be added. They may or may not have stopAtZero. If they do we should respect it, if they don't the ? will handle it fine. The current check is simpler and more future proof.
There was a problem hiding this comment.
@rjmunro, understood — the optional chaining is intentional so that any future mode with a stopAtZero field is automatically respected without requiring a code change here. That's a valid design choice.
✏️ Learnings added
Learnt from: rjmunro
Repo: Sofie-Automation/sofie-core PR: 1690
File: packages/webui/src/client/lib/tTimerUtils.ts:22-22
Timestamp: 2026-03-17T12:12:27.582Z
Learning: In `packages/webui/src/client/lib/tTimerUtils.ts`, the `timer.mode?.stopAtZero` access at the `stopAtZero` guard (line ~22) is intentionally written without narrowing `mode.type`. New `RundownTTimerMode` variants may be added in the future that may or may not include `stopAtZero`; the optional chaining (`?.`) is relied upon to handle both cases, making the check mode-agnostic and future-proof. Do not suggest narrowing to specific mode types (e.g. `'countdown' | 'timeOfDay'`) here.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: anteeek
Repo: Sofie-Automation/sofie-core PR: 1600
File: packages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreenTop.tsx:53-61
Timestamp: 2026-01-29T16:14:58.232Z
Learning: In `packages/webui/src/client/ui/ClockView/DirectorScreen/DirectorScreenTop.tsx`, the `TimeToPlannedEndComponent` is intentionally passed `now - expectedEnd` (a negative value when counting down), not `expectedEnd - now`. This is the expected behavior for the countdown display.
Learnt from: Julusian
Repo: Sofie-Automation/sofie-core PR: 1644
File: packages/job-worker/src/blueprints/context/services/PersistantStateStore.ts:23-24
Timestamp: 2026-02-25T14:04:03.101Z
Learning: In `packages/job-worker/src/blueprints/context/services/PersistantStateStore.ts`, the persistent state fields (`#privateState` and `#publicState`) are typed as TimelinePersistentState | undefined and are expected to always be objects (or undefined), never primitive values. Using `|| {}` instead of `?? {}` is acceptable in getAll() and getAllPublic() methods because the type system prevents falsy primitives from being stored.
Contributor
|
Also related: #1601 |
…undown/Playlist label styling.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/job-worker/src/playout/tTimers.ts (1)
261-301:⚠️ Potential issue | 🟠 MajorBudgeted current-segment accumulation still overcounts anchor time.
The current-segment budget remainder is seeded into
totalAccumulator(Line 269), then a full budget is added again at boundary flush (Line 298). This inflates anchor projections and pushespauseTime(Line 326) too far out for anchors in/after the current segment.Suggested direction for fix
+ let currentSegmentRemainingBudget: number | undefined + if (currentPartInstance) { currentSegmentId = currentPartInstance.segmentId const currentSegment = playoutModel.findSegment(currentPartInstance.segmentId) const currentSegmentBudget = currentSegment?.segment.segmentTiming?.budgetDuration @@ } else { // Segment budget timing - we're already inside a budgeted segment const segmentStartedPlayback = playlist.segmentsStartedPlayback?.[currentPartInstance.segmentId as unknown as string] if (segmentStartedPlayback) { const segmentElapsed = now - segmentStartedPlayback const remaining = currentSegmentBudget - segmentElapsed isPushing = remaining < 0 - totalAccumulator = Math.max(0, remaining) + currentSegmentRemainingBudget = Math.max(0, remaining) } else { - totalAccumulator = currentSegmentBudget + currentSegmentRemainingBudget = currentSegmentBudget } + // Keep current-part timing separate from whole-segment budget + const currentPartDuration = + currentPartInstance.part.expectedDurationWithTransition ?? currentPartInstance.part.expectedDuration + if (currentPartDuration) { + const startedPlayback = + currentPartInstance.timings?.plannedStartedPlayback && + currentPartInstance.timings?.plannedStartedPlayback <= now + ? currentPartInstance.timings.plannedStartedPlayback + : now + const playOffset = currentPartInstance.timings?.playOffset || 0 + const remainingPart = currentPartDuration - (now - startedPlayback - playOffset) + totalAccumulator = Math.max(0, remainingPart) + } } } @@ if (segmentBudget !== undefined) { - totalAccumulator += segmentBudget + if (currentSegmentRemainingBudget !== undefined) { + totalAccumulator += currentSegmentRemainingBudget + currentSegmentRemainingBudget = undefined + } else { + totalAccumulator += segmentBudget + } } else { totalAccumulator += segmentAccumulator }Also applies to: 326-327
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/job-worker/src/playout/tTimers.ts` around lines 261 - 301, The code double-counts the current segment budget because totalAccumulator is seeded with currentSegmentBudget/currentPartRemainingTime then when encountering the segment boundary the full segmentBudget is added again in the loop; update the flush logic (where playoutModel.findSegment(...) and segmentBudget are read) to detect the case where the flushed segment is the same as the currently-seeded segment (compare against currentSegmentId/currentPartRemainingTime/currentSegmentBudget) and in that case add only the remaining amount (or skip adding segmentBudget entirely), so totalAccumulator reflects the true remainder and anchors (pauseTime calculation) are not inflated; ensure references are to totalAccumulator, currentSegmentBudget, currentPartRemainingTime, currentSegmentId, playablePartsSlice, and playoutModel.findSegment in the adjusted logic.
🧹 Nitpick comments (2)
packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss (2)
275-280: Address the inline TODO about using a variable.The comment on line 277 notes that
#ff0"should probably be changed to$general-fast-color". If this is correct, consider making the change now for consistency with other color usages in the codebase. Hardcoded colors can lead to maintenance issues when theming or adjusting the color scheme.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss` around lines 275 - 280, Replace the hardcoded color `#ff0` in the .rundown-header__clocks-diff__chip--under block with the SASS variable $general-fast-color (used elsewhere) to maintain theme consistency; update the selector .rundown-header__clocks-diff--under -> .rundown-header__clocks-diff__chip--under to reference $general-fast-color, and ensure the SASS file imports or has access to the file that defines $general-fast-color so the variable resolves (add the import if missing) or provide a sensible fallback variable.
428-430: Consider using the placeholder selector directly in@extendstatements.The
.rundown-header__hoverable-labelclass extends%hoverable-label, but then other selectors (lines 212, 309, 442, 507) extend.rundown-header__hoverable-labelinstead of directly using the placeholder. This creates an extra indirection. For cleaner output, consider having those selectors extend%hoverable-labeldirectly, or consolidate the approach.Similarly, line 191 uses
@extend .countdown--timeofday;which extends a class selector rather than a placeholder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss` around lines 428 - 430, The SCSS introduces an unnecessary indirection by having .rundown-header__hoverable-label extend the placeholder %hoverable-label while other selectors (the ones referenced around lines 212, 309, 442, 507) extend .rundown-header__hoverable-label; update those selectors to extend %hoverable-label directly (or consolidate all to extend the class, but prefer direct placeholder use) and similarly replace the use of `@extend` .countdown--timeofday with extending a placeholder (e.g., create/use %countdown--timeofday) so all extensions target placeholders instead of class selectors; update references to .rundown-header__hoverable-label and .countdown--timeofday accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/corelib/src/dataModel/RundownPlaylist.ts`:
- Around line 179-181: The conditional uses truthiness for state.pauseTime which
incorrectly treats 0 as falsy; change the check in the RundownPlaylist logic to
explicitly test for non-null/undefined (e.g., use state.pauseTime != null or
typeof state.pauseTime !== 'undefined') before comparing with now so that a
numeric 0 is handled correctly in the auto-pause branch that returns
state.zeroTime - state.pauseTime.
In `@packages/job-worker/src/playout/__tests__/tTimersJobs.test.ts`:
- Around line 205-209: The test "should handle playlist deleted between query
and lock" is a placeholder; update it to create an active playlist fixture and
then simulate a deletion race by mocking the repository/DB calls used in
handleRecalculateTTimerProjections so the first query (e.g., getActivePlaylist
or findPlaylistForRecalc) returns the playlist but the subsequent check after
acquiring the lock (the same method or a follow-up existsById/findById) returns
null; use Jest mocks or spies on the specific repository functions used in
handleRecalculateTTimerProjections to sequence these return values, then call
handleRecalculateTTimerProjections and assert it resolves to undefined and does
not throw. Ensure you reference the actual functions/methods used by
handleRecalculateTTimerProjections (e.g., getActivePlaylist, acquireLock,
findById) when setting up the mock sequencing.
In `@packages/job-worker/src/playout/tTimers.ts`:
- Around line 210-213: The early-exit in the block that checks timerAnchors.size
=== 0 currently returns undefined which violates the function's void signature;
change the `return undefined` to a plain `return` (or remove the return
entirely) inside the same block so the function exits without returning a value,
and keep the existing `if (span) span.end()` call intact; this adjustment should
be made in the function that accesses `timerAnchors` and `span` in tTimers.ts.
In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss`:
- Around line 336-346: The CSS rule for
.rundown-header__clocks-timers__timer__over-under contains an invalid
declaration line-height: -1em; — replace it with a valid line-height (e.g., 1em
or 1.1em) or remove the line-height property entirely so browsers don't ignore
it; update the declaration inside the
.rundown-header__clocks-timers__timer__over-under block to a positive/valid
value or delete it.
---
Duplicate comments:
In `@packages/job-worker/src/playout/tTimers.ts`:
- Around line 261-301: The code double-counts the current segment budget because
totalAccumulator is seeded with currentSegmentBudget/currentPartRemainingTime
then when encountering the segment boundary the full segmentBudget is added
again in the loop; update the flush logic (where playoutModel.findSegment(...)
and segmentBudget are read) to detect the case where the flushed segment is the
same as the currently-seeded segment (compare against
currentSegmentId/currentPartRemainingTime/currentSegmentBudget) and in that case
add only the remaining amount (or skip adding segmentBudget entirely), so
totalAccumulator reflects the true remainder and anchors (pauseTime calculation)
are not inflated; ensure references are to totalAccumulator,
currentSegmentBudget, currentPartRemainingTime, currentSegmentId,
playablePartsSlice, and playoutModel.findSegment in the adjusted logic.
---
Nitpick comments:
In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss`:
- Around line 275-280: Replace the hardcoded color `#ff0` in the
.rundown-header__clocks-diff__chip--under block with the SASS variable
$general-fast-color (used elsewhere) to maintain theme consistency; update the
selector .rundown-header__clocks-diff--under ->
.rundown-header__clocks-diff__chip--under to reference $general-fast-color, and
ensure the SASS file imports or has access to the file that defines
$general-fast-color so the variable resolves (add the import if missing) or
provide a sensible fallback variable.
- Around line 428-430: The SCSS introduces an unnecessary indirection by having
.rundown-header__hoverable-label extend the placeholder %hoverable-label while
other selectors (the ones referenced around lines 212, 309, 442, 507) extend
.rundown-header__hoverable-label; update those selectors to extend
%hoverable-label directly (or consolidate all to extend the class, but prefer
direct placeholder use) and similarly replace the use of `@extend`
.countdown--timeofday with extending a placeholder (e.g., create/use
%countdown--timeofday) so all extensions target placeholders instead of class
selectors; update references to .rundown-header__hoverable-label and
.countdown--timeofday accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a5b8c02d-63bd-440a-b00a-99dadf746927
📒 Files selected for processing (17)
meteor/server/__tests__/cronjobs.test.tspackages/blueprints-integration/src/context/tTimersContext.tspackages/corelib/src/dataModel/RundownPlaylist.tspackages/corelib/src/worker/studio.tspackages/job-worker/src/blueprints/context/services/TTimersService.tspackages/job-worker/src/blueprints/context/services/__tests__/TTimersService.test.tspackages/job-worker/src/ingest/commit.tspackages/job-worker/src/playout/__tests__/tTimers.test.tspackages/job-worker/src/playout/__tests__/tTimersJobs.test.tspackages/job-worker/src/playout/setNext.tspackages/job-worker/src/playout/tTimers.tspackages/job-worker/src/playout/tTimersJobs.tspackages/job-worker/src/workers/studio/child.tspackages/job-worker/src/workers/studio/jobs.tspackages/webui/src/client/lib/tTimerUtils.tspackages/webui/src/client/ui/RundownView/RundownHeader/Countdown.scsspackages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss
✅ Files skipped from review due to trivial changes (1)
- packages/job-worker/src/blueprints/context/services/tests/TTimersService.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/job-worker/src/workers/studio/child.ts
- packages/job-worker/src/playout/setNext.ts
- meteor/server/tests/cronjobs.test.ts
- packages/corelib/src/worker/studio.ts
- packages/webui/src/client/lib/tTimerUtils.ts
- packages/job-worker/src/workers/studio/jobs.ts
Comment on lines
+179
to
+181
| } else if (state.pauseTime && now >= state.pauseTime) { | ||
| // Auto-pause at overrun (current part ended) | ||
| return state.zeroTime - state.pauseTime |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how pauseTime is populated and whether any sentinel semantics are used.
rg -nP --type=ts -C3 '\bpauseTime\s*:'
rg -nP --type=ts -C3 '\btimerStateToDuration\s*\('Repository: Sofie-Automation/sofie-core
Length of output: 2847
🏁 Script executed:
sed -n '175,190p' packages/corelib/src/dataModel/RundownPlaylist.tsRepository: Sofie-Automation/sofie-core
Length of output: 542
🏁 Script executed:
rg -nP --type=ts 'type TimerState|interface TimerState' -A 10Repository: Sofie-Automation/sofie-core
Length of output: 1002
Use a nullish check for pauseTime instead of truthiness.
The type pauseTime?: number | null allows numeric values including 0. The current truthiness check treats 0 as falsy and would incorrectly skip the auto-pause branch if pauseTime were ever set to 0. Use an explicit null/undefined check instead.
Proposed fix
- } else if (state.pauseTime && now >= state.pauseTime) {
+ } else if (state.pauseTime != null && now >= state.pauseTime) {📝 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.
Suggested change
| } else if (state.pauseTime && now >= state.pauseTime) { | |
| // Auto-pause at overrun (current part ended) | |
| return state.zeroTime - state.pauseTime | |
| } else if (state.pauseTime != null && now >= state.pauseTime) { | |
| // Auto-pause at overrun (current part ended) | |
| return state.zeroTime - state.pauseTime |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/corelib/src/dataModel/RundownPlaylist.ts` around lines 179 - 181,
The conditional uses truthiness for state.pauseTime which incorrectly treats 0
as falsy; change the check in the RundownPlaylist logic to explicitly test for
non-null/undefined (e.g., use state.pauseTime != null or typeof state.pauseTime
!== 'undefined') before comparing with now so that a numeric 0 is handled
correctly in the auto-pause branch that returns state.zeroTime -
state.pauseTime.
Comment on lines
+205
to
+209
| it('should handle playlist deleted between query and lock', async () => { | ||
| // This test is harder to set up properly, but the function should handle it | ||
| // by checking if playlist exists after acquiring lock | ||
| await expect(handleRecalculateTTimerProjections(context)).resolves.toBeUndefined() | ||
| }) |
There was a problem hiding this comment.
The deletion-race test is currently a placeholder, not a real race test.
This case does not create an active playlist or simulate deletion between the initial query and lock, so it doesn’t validate the intended behavior.
Suggested test direction
it('should handle playlist deleted between query and lock', async () => {
- // This test is harder to set up properly, but the function should handle it
- // by checking if playlist exists after acquiring lock
- await expect(handleRecalculateTTimerProjections(context)).resolves.toBeUndefined()
+ const playlistId = protectString<RundownPlaylistId>('playlist-race')
+ await context.directCollections.RundownPlaylists.insertOne(/* active playlist */)
+
+ const findOneSpy = jest
+ .spyOn(context.directCollections.RundownPlaylists, 'findOne')
+ .mockImplementation(async (_id) => undefined) // simulate deletion after query
+
+ await expect(handleRecalculateTTimerProjections(context)).resolves.toBeUndefined()
+ findOneSpy.mockRestore()
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/job-worker/src/playout/__tests__/tTimersJobs.test.ts` around lines
205 - 209, The test "should handle playlist deleted between query and lock" is a
placeholder; update it to create an active playlist fixture and then simulate a
deletion race by mocking the repository/DB calls used in
handleRecalculateTTimerProjections so the first query (e.g., getActivePlaylist
or findPlaylistForRecalc) returns the playlist but the subsequent check after
acquiring the lock (the same method or a follow-up existsById/findById) returns
null; use Jest mocks or spies on the specific repository functions used in
handleRecalculateTTimerProjections to sequence these return values, then call
handleRecalculateTTimerProjections and assert it resolves to undefined and does
not throw. Ensure you reference the actual functions/methods used by
handleRecalculateTTimerProjections (e.g., getActivePlaylist, acquireLock,
findById) when setting up the mock sequencing.
Comment on lines
+210
to
+213
| if (timerAnchors.size === 0) { | ||
| if (span) span.end() | ||
| return undefined | ||
| } |
There was a problem hiding this comment.
Avoid returning undefined from a void function.
Line 212 should be return (or removed) to satisfy void return expectations and avoid lint failure.
Suggested fix
if (timerAnchors.size === 0) {
if (span) span.end()
- return undefined
+ return
}📝 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.
Suggested change
| if (timerAnchors.size === 0) { | |
| if (span) span.end() | |
| return undefined | |
| } | |
| if (timerAnchors.size === 0) { | |
| if (span) span.end() | |
| return | |
| } |
🧰 Tools
🪛 Biome (2.4.7)
[error] 212-212: The function should not return a value because its return type is void.
(lint/correctness/noVoidTypeReturn)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/job-worker/src/playout/tTimers.ts` around lines 210 - 213, The
early-exit in the block that checks timerAnchors.size === 0 currently returns
undefined which violates the function's void signature; change the `return
undefined` to a plain `return` (or remove the return entirely) inside the same
block so the function exits without returning a value, and keep the existing `if
(span) span.end()` call intact; this adjustment should be made in the function
that accesses `timerAnchors` and `span` in tTimers.ts.
Comment on lines
+336
to
+346
| .rundown-header__clocks-timers__timer__over-under { | ||
| display: inline-block; | ||
| line-height: -1em; | ||
| font-size: 0.75em; | ||
| padding: 0.05em 0.25em; | ||
| border-radius: 999px; | ||
| white-space: nowrap; | ||
| letter-spacing: -0.02em; | ||
| margin-left: 0.25em; | ||
| margin-top: 0em; | ||
| font-variant-numeric: tabular-nums; |
There was a problem hiding this comment.
Invalid CSS: line-height: -1em; is not valid.
Line 338 has line-height: -1em; which is invalid CSS. Line-height values cannot be negative—browsers will ignore this declaration. This appears to be a typo; consider what the intended value should be (e.g., 1em, 1.1em, or simply remove the line).
🐛 Proposed fix
.rundown-header__clocks-timers__timer__over-under {
display: inline-block;
- line-height: -1em;
+ line-height: 1em;
font-size: 0.75em;📝 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.
Suggested change
| .rundown-header__clocks-timers__timer__over-under { | |
| display: inline-block; | |
| line-height: -1em; | |
| font-size: 0.75em; | |
| padding: 0.05em 0.25em; | |
| border-radius: 999px; | |
| white-space: nowrap; | |
| letter-spacing: -0.02em; | |
| margin-left: 0.25em; | |
| margin-top: 0em; | |
| font-variant-numeric: tabular-nums; | |
| .rundown-header__clocks-timers__timer__over-under { | |
| display: inline-block; | |
| line-height: 1em; | |
| font-size: 0.75em; | |
| padding: 0.05em 0.25em; | |
| border-radius: 999px; | |
| white-space: nowrap; | |
| letter-spacing: -0.02em; | |
| margin-left: 0.25em; | |
| margin-top: 0em; | |
| font-variant-numeric: tabular-nums; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss`
around lines 336 - 346, The CSS rule for
.rundown-header__clocks-timers__timer__over-under contains an invalid
declaration line-height: -1em; — replace it with a valid line-height (e.g., 1em
or 1.1em) or remove the line-height property entirely so browsers don't ignore
it; update the declaration inside the
.rundown-header__clocks-timers__timer__over-under block to a positive/valid
value or delete it.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss (1)
336-339:⚠️ Potential issue | 🟡 MinorInvalid negative line-height on over/under chip (
Line 338).
line-height: -1em;is invalid CSS and gets ignored, so chip vertical alignment becomes browser-dependent.🐛 Proposed fix
.rundown-header__clocks-timers__timer__over-under { display: inline-block; - line-height: -1em; + line-height: 1em; font-size: 0.75em;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss` around lines 336 - 339, The CSS rule .rundown-header__clocks-timers__timer__over-under is using an invalid negative line-height (line-height: -1em) which is ignored; change this to a valid value and ensure vertical centering—either remove the line-height declaration or replace it with a positive numeric value (e.g., line-height: 1) or use display: inline-flex; align-items: center; with an explicit height to vertically center the chip; update the .rundown-header__clocks-timers__timer__over-under selector accordingly.
🧹 Nitpick comments (1)
packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss (1)
277-279: Use design token instead of hardcoded#ff0for “under” states.Both under-time chips use a literal yellow while a tokenized approach is already used elsewhere (
$general-late-color). This will be easier to theme consistently.Also applies to: 368-369
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss` around lines 277 - 279, Replace the hardcoded yellow "#ff0" used for the "under" time-chip styles with the design token variable ($general-fast-color) so theming stays consistent; update both occurrences (the block around the first occurrence and the second occurrence noted at the later lines) to use $general-fast-color and keep the existing color: `#000` unchanged. Ensure you modify the selectors that style the under-state chips in RundownHeader.scss (the under-time chip rules adjacent to the late-color usage) so they match the tokenized approach used for $general-late-color.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss`:
- Around line 591-624: The close button (.rundown-header__close-btn) is set to
opacity: 0 but the :focus-visible state doesn’t restore visibility; update the
&:focus-visible block inside .rundown-header__close-btn to set opacity: 1 (and
keep the existing color and drop-shadow), so keyboard focus makes the button
visible and matches the hover behavior for accessibility.
- Line 191: Replace class-based `@extend` usages with placeholder selectors:
change any occurrence of "@extend .hoverable-label;" (and similar ".classname"
extends) to "@extend %hoverable-label;" using the existing %hoverable-label
placeholder from _shared.scss; for the specific ".countdown--timeofday" usage,
create a new placeholder (e.g. "%countdown--timeofday") or a mixin that contains
the same styles currently expected from that class, add it to the shared SCSS,
and update the file to "@extend %countdown--timeofday;" (or include the mixin)
instead of "@extend .countdown--timeofday;". Ensure the placeholder names match
exactly (e.g. %hoverable-label, %countdown--timeofday) and remove the
class-based `@extend` usages so the scss/at-extend-no-missing-placeholder lint
rule is satisfied.
- Line 394: The chained :not selectors in RundownHeader.scss (specifically the
selector instances "&:hover:not(:disabled):not(.disabled)" and the similar
selector near line 403) violate the stylelint rule; replace each chained
:not(...) sequence with a single :not(...) using comma-separated arguments
(e.g., :not(:disabled, .disabled)) so the selectors read "&:hover:not(:disabled,
.disabled)" (and the analogous one at the other occurrence) to comply with
selector-not-notation: complex.
---
Duplicate comments:
In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss`:
- Around line 336-339: The CSS rule
.rundown-header__clocks-timers__timer__over-under is using an invalid negative
line-height (line-height: -1em) which is ignored; change this to a valid value
and ensure vertical centering—either remove the line-height declaration or
replace it with a positive numeric value (e.g., line-height: 1) or use display:
inline-flex; align-items: center; with an explicit height to vertically center
the chip; update the .rundown-header__clocks-timers__timer__over-under selector
accordingly.
---
Nitpick comments:
In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss`:
- Around line 277-279: Replace the hardcoded yellow "#ff0" used for the "under"
time-chip styles with the design token variable ($general-fast-color) so theming
stays consistent; update both occurrences (the block around the first occurrence
and the second occurrence noted at the later lines) to use $general-fast-color
and keep the existing color: `#000` unchanged. Ensure you modify the selectors
that style the under-state chips in RundownHeader.scss (the under-time chip
rules adjacent to the late-color usage) so they match the tokenized approach
used for $general-late-color.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67195b00-1c1c-4ba2-97ca-96f84f097b05
📒 Files selected for processing (1)
packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss
| } | ||
|
|
||
| .rundown-header__clocks-time-now { | ||
| @extend .countdown--timeofday; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all class-based `@extend` usages in this file
rg -n '@extend\s+\.' packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss
# Check whether equivalent placeholders exist in related shared files
rg -n '%[a-zA-Z0-9_-]+' \
packages/webui/src/client/ui/RundownView/RundownHeader/_shared.scss \
packages/webui/src/client/ui/RundownView/RundownHeader/Countdown.scssRepository: Sofie-Automation/sofie-core
Length of output: 495
🏁 Script executed:
#!/bin/bash
# Find stylelint configuration
find . -maxdepth 3 -type f \( -name "stylelintrc*" -o -name ".stylelintrc*" -o -name "package.json" \) | head -20
# Search for countdown--timeofday definition and placeholders
rg -n 'countdown--timeofday|%countdown--timeofday' packages/webui/src/client/ui/RundownView/RundownHeader/
# Check what's in the Countdown.scss file around line 12
cat -n packages/webui/src/client/ui/RundownView/RundownHeader/Countdown.scss | head -20
# Check what's in _shared.scss
cat -n packages/webui/src/client/ui/RundownView/RundownHeader/_shared.scssRepository: Sofie-Automation/sofie-core
Length of output: 2043
🏁 Script executed:
#!/bin/bash
# Check stylelint configuration for the rule
cat .stylelintrc.json | grep -A 5 -B 5 "at-extend"
# Search more broadly for countdown--timeofday definition or placeholder
rg -n 'countdown--timeofday|%countdown' packages/webui/src/client/ui/RundownView/
# Read RundownHeader.scss context around line 191
cat -n packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss | sed -n '185,220p'Repository: Sofie-Automation/sofie-core
Length of output: 1270
🏁 Script executed:
#!/bin/bash
# Read stylelint configuration
cat .stylelintrc.json
# Search for .rundown-header__hoverable-label definition
rg -n '\.rundown-header__hoverable-label' packages/webui/src/client/ui/RundownView/RundownHeader/
# Search for where .countdown--timeofday is defined as a SCSS class
rg -n '\.countdown--timeofday\s*{' packages/webui/src/client/ui/RundownView/Repository: Sofie-Automation/sofie-core
Length of output: 1930
🏁 Script executed:
#!/bin/bash
# Search for .countdown--timeofday class definition (without regex)
rg -n 'countdown--timeofday' packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss
# Read around line 432 where .rundown-header__hoverable-label is defined
cat -n packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss | sed -n '428,440p'
# Check if stylelint-config-standard-scss has at-extend-no-missing-placeholder enabled by default
web_search stylelint-config-standard-scss at-extend-no-missing-placeholder defaultRepository: Sofie-Automation/sofie-core
Length of output: 575
🌐 Web query:
stylelint-config-standard-scss at-extend-no-missing-placeholder rule default enabled
💡 Result:
Yes. stylelint-config-standard-scss enables scss/at-extend-no-missing-placeholder by default (it’s true), because it extends stylelint-config-recommended-scss, which turns that rule on. [1] [2]
Convert class-based @extend usages to placeholder selectors—this violates the configured stylelint rule scss/at-extend-no-missing-placeholder and may block CI.
Five instances of @extend .class found on lines 191, 212, 309, 446, and 511. Replace with %placeholder selectors. For example, lines 212, 309, 446, and 511 should extend %hoverable-label directly (available in _shared.scss) instead of the class selector. Line 191's @extend .countdown--timeofday; has no corresponding SCSS class definition and must be refactored to use a placeholder or mixin approach.
🧰 Tools
🪛 Stylelint (17.4.0)
[error] 191-191: Expected a placeholder selector (e.g. %placeholder) to be used in @extend (scss/at-extend-no-missing-placeholder)
(scss/at-extend-no-missing-placeholder)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss` at
line 191, Replace class-based `@extend` usages with placeholder selectors: change
any occurrence of "@extend .hoverable-label;" (and similar ".classname" extends)
to "@extend %hoverable-label;" using the existing %hoverable-label placeholder
from _shared.scss; for the specific ".countdown--timeofday" usage, create a new
placeholder (e.g. "%countdown--timeofday") or a mixin that contains the same
styles currently expected from that class, add it to the shared SCSS, and update
the file to "@extend %countdown--timeofday;" (or include the mixin) instead of
"@extend .countdown--timeofday;". Ensure the placeholder names match exactly
(e.g. %hoverable-label, %countdown--timeofday) and remove the class-based
`@extend` usages so the scss/at-extend-no-missing-placeholder lint rule is
satisfied.
| filter: drop-shadow(0 0 0 rgba(255, 255, 255, 0)); | ||
| } | ||
|
|
||
| &:hover:not(:disabled):not(.disabled) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "RundownHeader.scss" packages/webuiRepository: Sofie-Automation/sofie-core
Length of output: 144
🏁 Script executed:
cat -n packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss | sed -n '210,220p;390,410p;440,450p;560,570p;635,645p'Repository: Sofie-Automation/sofie-core
Length of output: 1832
🏁 Script executed:
rg "&:.*:not\(" packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scssRepository: Sofie-Automation/sofie-core
Length of output: 162
🏁 Script executed:
rg -n "&.*:not\(" packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss -A 1 -B 1Repository: Sofie-Automation/sofie-core
Length of output: 229
🏁 Script executed:
fd ".stylelintrc*" packages/webuiRepository: Sofie-Automation/sofie-core
Length of output: 53
🏁 Script executed:
fd ".stylelintrc*" --max-depth 3Repository: Sofie-Automation/sofie-core
Length of output: 53
🏁 Script executed:
cat packages/webui/package.json | grep -A 20 -B 2 "stylelint"Repository: Sofie-Automation/sofie-core
Length of output: 53
🏁 Script executed:
find . -name "stylelint*" -o -name ".stylelintrc*" 2>/dev/null | head -20Repository: Sofie-Automation/sofie-core
Length of output: 90
🏁 Script executed:
cat -n .stylelintrc.jsonRepository: Sofie-Automation/sofie-core
Length of output: 1287
🏁 Script executed:
web_search
stylelint-config-standard-scss :not() selector rulesRepository: Sofie-Automation/sofie-core
Length of output: 259
🏁 Script executed:
grep -r "selector-pseudo-class" .stylelintrc.json packages/webuiRepository: Sofie-Automation/sofie-core
Length of output: 53
🏁 Script executed:
cat -n packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss | grep -n ":not("Repository: Sofie-Automation/sofie-core
Length of output: 240
🏁 Script executed:
cat -n packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss | sed -n '212,216p;445,449p;561,565p;636,640p'Repository: Sofie-Automation/sofie-core
Length of output: 723
🌐 Web query:
stylelint-config-standard-scss :not() selector notation rules CSS Level 4
💡 Result:
CSS Selectors Level 4 expanded :not() to accept a selector list (comma‑separated selectors) and more complex selectors, e.g. a:not(.foo, .bar) instead of chaining a:not(.foo):not(.bar). [1][2]
In stylelint-config-standard-scss, the :not() notation is governed by Stylelint’s selector-not-notation rule, inherited from stylelint-config-standard, which sets it to "complex"—i.e., it prefers the Selectors Level 4 comma-list form and will flag chained :not() as the “wrong” notation. [3][1]
If you want the older CSS Level 3 / legacy-browser style instead, override it:
{
"extends": ["stylelint-config-standard-scss"],
"rules": {
"selector-not-notation": "simple"
}
}
``` [1]
**Note:** the two notations can have different **specificity**, which is one reason Stylelint makes it configurable. [1]
**Sources**
[1] https://stylelint.io/user-guide/rules/selector-not-notation/
[2] https://www.w3.org/TR/selectors-4/
[3] https://github.com/stylelint/stylelint-config-standard/blob/main/index.js
---
🏁 Script executed:
```shell
cat -n packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss | sed -n '393,404p'Repository: Sofie-Automation/sofie-core
Length of output: 371
Update :not() selectors to modern comma-separated syntax to comply with stylelint's selector-not-notation: complex rule.
The chained :not(:disabled):not(.disabled) notation violates the active stylelint configuration, which defaults to CSS Selectors Level 4 syntax. Use the comma-separated form instead at lines 394 and 403:
Fix for selector notation
- &:hover:not(:disabled):not(.disabled) {
+ &:hover:not(:disabled, .disabled) {
color: `#ffffff`;
...
}
- &:focus-visible:not(:disabled):not(.disabled) {
+ &:focus-visible:not(:disabled, .disabled) {
color: `#ffffff`;
...
}🧰 Tools
🪛 Stylelint (17.4.0)
[error] 394-394: Expected complex :not() pseudo-class notation (selector-not-notation)
(selector-not-notation)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss` at
line 394, The chained :not selectors in RundownHeader.scss (specifically the
selector instances "&:hover:not(:disabled):not(.disabled)" and the similar
selector near line 403) violate the stylelint rule; replace each chained
:not(...) sequence with a single :not(...) using comma-separated arguments
(e.g., :not(:disabled, .disabled)) so the selectors read "&:hover:not(:disabled,
.disabled)" (and the analogous one at the other occurrence) to comply with
selector-not-notation: complex.
Comment on lines
+591
to
+624
| .rundown-header__close-btn { | ||
| display: flex; | ||
| align-items: center; | ||
| margin-right: 0.75em; | ||
| cursor: pointer; | ||
| color: #40b8fa; | ||
| opacity: 0; | ||
| flex-shrink: 0; | ||
| transition: | ||
| opacity 0.2s; | ||
|
|
||
| svg, | ||
| i { | ||
| filter: drop-shadow(0 0 0 rgba(255, 255, 255, 0)); | ||
| } | ||
|
|
||
| &:hover { | ||
| color: #ffffff; | ||
|
|
||
| svg, | ||
| i { | ||
| filter: drop-shadow(0 0 2px rgba(255, 255, 255, 0.65)); | ||
| } | ||
| } | ||
|
|
||
| &:focus-visible { | ||
| color: #ffffff; | ||
|
|
||
| svg, | ||
| i { | ||
| filter: drop-shadow(0 0 2.5px rgba(255, 255, 255, 0.75)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Close button can remain invisible when focused via keyboard.
Line 597 sets opacity: 0, but the :focus-visible block (Lines 616-623) does not restore opacity. This makes focus state hard/impossible to perceive for keyboard users.
♿ Proposed fix
.rundown-header__close-btn {
...
&:focus-visible {
+ opacity: 1;
color: `#ffffff`;
svg,
i {
filter: drop-shadow(0 0 2.5px rgba(255, 255, 255, 0.75));
}
}
}📝 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.
Suggested change
| .rundown-header__close-btn { | |
| display: flex; | |
| align-items: center; | |
| margin-right: 0.75em; | |
| cursor: pointer; | |
| color: #40b8fa; | |
| opacity: 0; | |
| flex-shrink: 0; | |
| transition: | |
| opacity 0.2s; | |
| svg, | |
| i { | |
| filter: drop-shadow(0 0 0 rgba(255, 255, 255, 0)); | |
| } | |
| &:hover { | |
| color: #ffffff; | |
| svg, | |
| i { | |
| filter: drop-shadow(0 0 2px rgba(255, 255, 255, 0.65)); | |
| } | |
| } | |
| &:focus-visible { | |
| color: #ffffff; | |
| svg, | |
| i { | |
| filter: drop-shadow(0 0 2.5px rgba(255, 255, 255, 0.75)); | |
| } | |
| } | |
| } | |
| .rundown-header__close-btn { | |
| display: flex; | |
| align-items: center; | |
| margin-right: 0.75em; | |
| cursor: pointer; | |
| color: `#40b8fa`; | |
| opacity: 0; | |
| flex-shrink: 0; | |
| transition: | |
| opacity 0.2s; | |
| svg, | |
| i { | |
| filter: drop-shadow(0 0 0 rgba(255, 255, 255, 0)); | |
| } | |
| &:hover { | |
| color: `#ffffff`; | |
| svg, | |
| i { | |
| filter: drop-shadow(0 0 2px rgba(255, 255, 255, 0.65)); | |
| } | |
| } | |
| &:focus-visible { | |
| opacity: 1; | |
| color: `#ffffff`; | |
| svg, | |
| i { | |
| filter: drop-shadow(0 0 2.5px rgba(255, 255, 255, 0.75)); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/webui/src/client/ui/RundownView/RundownHeader/RundownHeader.scss`
around lines 591 - 624, The close button (.rundown-header__close-btn) is set to
opacity: 0 but the :focus-visible state doesn’t restore visibility; update the
&:focus-visible block inside .rundown-header__close-btn to set opacity: 1 (and
keep the existing color and drop-shadow), so keyboard focus makes the button
visible and matches the hover behavior for accessibility.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
About the Contributor
This pull request is posted on behalf of the BBC.
Type of Contribution
This is a Feature
Current Behavior
The options for timers are limited.
Top bar of directors screen needs a refresh.
New Behavior
Testing
Affected areas
Blueprints, Directors screen, Other screens.
Time Frame
Other Information
This PR is a rebase of various branches and from the BBC fork.
Status