feat: Show AB Resolver Channels on directors screen#1566
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
rjmunro
force-pushed
the
rjmunro/directors-view-show-ab
branch
5 times, most recently
from
e3778a9 to
94bda04
Compare
rjmunro
force-pushed
the
rjmunro/directors-view-show-ab
branch
from
3e56076 to
94bda04
Compare
Saftret
added
the
Contribution from BBC
rjmunro
force-pushed
the
rjmunro/directors-view-show-ab
branch
3 times, most recently
from
8a370d3 to
f46cd9d
Compare
|
rjmunro
force-pushed
the
rjmunro/directors-view-show-ab
branch
from
f46cd9d to
4a00709
Compare
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe PR introduces AB Channel Display configuration support, allowing blueprints to define which source and output layers display AB resolver channel assignments. Changes span data models, blueprint APIs, mock helpers, UI publications, and UI components including a new Director screen integration. Changes
Sequence DiagramsequenceDiagram
participant Blueprint as Blueprint
participant ShowStyle as ShowStyleBase
participant Director as DirectorScreen
participant Session as ABSession Pool
Blueprint->>ShowStyle: Provides abChannelDisplay config<br/>(sourceLayerIds, outputLayerIds, types)
ShowStyle->>ShowStyle: Store abChannelDisplay &<br/>blueprintAbChannelDisplay
Director->>ShowStyle: Fetch currentShowStyleBase<br/>with abChannelDisplay config
Director->>Director: For each PieceInstance:<br/>evaluate shouldDisplayAbChannel()
Director->>Session: Query AB session assignments<br/>for assigned piece
Session-->>Director: Return ABSession ID if matched
alt AB Channel Display Enabled
Director->>Director: Render AB Channel ID<br/>(single alphanumeric)
else AB Channel Display Disabled
Director->>Director: Render Server Name
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
rjmunro
changed the title
rjmunro
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @packages/webui/src/client/ui/ClockView/DirectorScreen.tsx:
- Around line 446-505: The two useTracker hooks (for currentClipPlayer and
nextClipPlayer) are being invoked conditionally inside the
playlist/playlistId/segments guard which violates React's Rules of Hooks; move
the useTracker(...) calls out of that conditional so they run unconditionally on
every render (keep the existing early-return checks inside each hook callback
that return undefined when currentPartInstance/nextPartInstance, showStyle data,
or playlist?.assignedAbSessions are missing), ensuring the hooks remain in the
same order and then use the resulting currentClipPlayer and nextClipPlayer
values within the original conditional block.
- Around line 104-107: Remove the debug console.log in the conditional inside
DirectorScreen (the block checking hasSourceLayerIdFilter and
hasSourceLayerTypeFilter): delete the console.log(`[AB Channel] ✓ No source
layer filters specified, showing all`) line and leave the return true; intact so
the behavior is unchanged but no debug output goes to production logs.
In @packages/webui/src/client/ui/Settings/ShowStyle/AbChannelDisplay.tsx:
- Around line 59-69: The useMemo in AbChannelDisplay mutates props/state by
calling sort() directly on arrays from showStyleBase.abChannelDisplay and
blueprintDefault; fix it by comparing sorted copies instead (e.g., spread the
arrays into new arrays before sorting) for each of sourceLayerIds,
sourceLayerTypes and outputLayerIds, and guard with defaults like empty arrays
or optional chaining to avoid calling sort on undefined; keep the same boolean
comparisons and the existing dependency array for useMemo.
🧹 Nitpick comments (8)
packages/webui/src/client/ui/Settings/components/ColumnPackedGrid.scss (2)
8-29: Potential conflict between auto-fit grid and explicit media query overrides.Line 10 defines
grid-template-columns: repeat(auto-fit, minmax(250px, 1fr))which provides automatic column fitting. However, the media queries at lines 14-28 explicitly override this with fixed column counts. The auto-fit rule will never apply because the media queries cover all viewport widths.Consider either removing the auto-fit base rule (since media queries fully override it) or removing the media queries to let auto-fit handle responsiveness naturally.
98-104: Consider refactoring to avoid!importantdeclarations.The
!importantdeclarations may cause specificity issues as the codebase grows. Since the selected state should override hover, consider increasing specificity naturally:♻️ Suggested refactor
-.column-packed-grid__item--selected { - background-color: rgba(76, 175, 80, 0.3) !important; - - &:hover { - background-color: rgba(76, 175, 80, 0.4) !important; - } -} +.column-packed-grid__item.column-packed-grid__item--selected { + background-color: rgba(76, 175, 80, 0.3); + + &:hover { + background-color: rgba(76, 175, 80, 0.4); + } +}packages/webui/src/client/ui/Settings/components/ColumnPackedGrid.tsx (2)
153-156: Redundant dependencies in useMemo.
hasGroupsandhasItemsare derived fromgroupsanditemsrespectively. Including them as dependencies alongsidegroupsanditemsis redundant and can cause unnecessary recalculations when these derived booleans are recalculated.♻️ Suggested fix
const columns = useMemo(() => { - if (!hasGroups && !hasItems) return [] + if ((!groups || groups.length === 0) && (!items || items.length === 0)) return [] return packIntoColumns(groups, items, getItemHeight, targetColumns) -}, [groups, items, getItemHeight, targetColumns, hasGroups, hasItems]) +}, [groups, items, getItemHeight, targetColumns])
71-73: Consider extracting magic numbers as configurable props or constants.The height constants (
GROUP_HEADER_HEIGHT,ITEM_HEIGHT_BASE,GROUP_SPACING) are hardcoded inside the function. If these need to match CSS values elsewhere, consider exporting them as constants or making them configurable via props for better maintainability.packages/webui/src/client/ui/Settings/SettingsMenu.tsx (1)
354-370: Note:tfunction missing from useMemo dependencies (pre-existing pattern).The
tfunction is used inside the useMemo but not listed in dependencies. This follows the existing pattern in this file (e.g., line 285) and typically works becausetis stable, but could theoretically cause stale translations if the language changes at runtime.packages/corelib/src/dataModel/ShowStyleBase.ts (1)
51-70: Consider extracting a shared type for AB Channel Display configuration.Both
abChannelDisplayandblueprintAbChannelDisplayhave identical structures. Extracting a shared type would improve maintainability and reduce the risk of the two definitions drifting apart.♻️ Suggested refactor
+/** Configuration for AB resolver channel display across screens */ +export interface AbChannelDisplayConfig { + /** Source layer IDs that should show AB channel info */ + sourceLayerIds: string[] + /** Configure by source layer type */ + sourceLayerTypes: SourceLayerType[] + /** Only show for specific output layers (e.g., only PGM) */ + outputLayerIds: string[] + /** Enable display on Director screen */ + showOnDirectorScreen: boolean + // Future: showOnPresenterScreen, showOnCameraScreen when those views are implemented +} + export interface DBShowStyleBase { // ... existing fields ... /** Configuration for displaying AB resolver channel assignments across different screens */ - abChannelDisplay?: { - /** Source layer IDs that should show AB channel info */ - sourceLayerIds: string[] - /** Configure by source layer type */ - sourceLayerTypes: SourceLayerType[] - /** Only show for specific output layers (e.g., only PGM) */ - outputLayerIds: string[] - /** Enable display on Director screen */ - showOnDirectorScreen: boolean - // Future: showOnPresenterScreen, showOnCameraScreen when those views are implemented - } + abChannelDisplay?: AbChannelDisplayConfig /** Blueprint default for abChannelDisplay (saved during blueprint upgrade) */ - blueprintAbChannelDisplay?: { - sourceLayerIds: string[] - sourceLayerTypes: SourceLayerType[] - outputLayerIds: string[] - showOnDirectorScreen: boolean - } + blueprintAbChannelDisplay?: AbChannelDisplayConfigpackages/webui/src/client/ui/Settings/ShowStyle/AbChannelDisplay.tsx (1)
30-47: Consider internationalizing source layer type labels.The
sourceLayerTypeLabelsobject uses hardcoded English strings. For consistency with the rest of the UI (which usest()for translations), these labels should be internationalized.Suggested approach
const sourceLayerTypeLabels = useMemo<Partial<Record<SourceLayerType, string>>>( () => ({ - [SourceLayerType.UNKNOWN]: 'Unknown', - [SourceLayerType.CAMERA]: 'Camera', - [SourceLayerType.VT]: 'VT', - [SourceLayerType.REMOTE]: 'Remote', - [SourceLayerType.SCRIPT]: 'Script', - [SourceLayerType.GRAPHICS]: 'Graphics', - [SourceLayerType.SPLITS]: 'Splits', - [SourceLayerType.AUDIO]: 'Audio', - [SourceLayerType.LOWER_THIRD]: 'Lower Third', - [SourceLayerType.LIVE_SPEAK]: 'Live Speak', - [SourceLayerType.TRANSITION]: 'Transition', - [SourceLayerType.LIGHTS]: 'Lights', - [SourceLayerType.LOCAL]: 'Local', + [SourceLayerType.UNKNOWN]: t('Unknown'), + [SourceLayerType.CAMERA]: t('Camera'), + [SourceLayerType.VT]: t('VT'), + [SourceLayerType.REMOTE]: t('Remote'), + [SourceLayerType.SCRIPT]: t('Script'), + [SourceLayerType.GRAPHICS]: t('Graphics'), + [SourceLayerType.SPLITS]: t('Splits'), + [SourceLayerType.AUDIO]: t('Audio'), + [SourceLayerType.LOWER_THIRD]: t('Lower Third'), + [SourceLayerType.LIVE_SPEAK]: t('Live Speak'), + [SourceLayerType.TRANSITION]: t('Transition'), + [SourceLayerType.LIGHTS]: t('Lights'), + [SourceLayerType.LOCAL]: t('Local'), }), - [] + [t] )meteor/server/migration/upgrades/showStyleBase.ts (1)
132-154: Consider adding type safety toupdateSet.The
Record<string, any>type loses the type safety that was previously present when the object was inline. While this works, a more type-safe approach could prevent runtime errors.Optional improvement
-const updateSet: Record<string, any> = { +const updateSet: Partial<{ + 'sourceLayersWithOverrides.defaults': ReturnType<typeof normalizeArray> + 'outputLayersWithOverrides.defaults': ReturnType<typeof normalizeArray> + lastBlueprintConfig: DBShowStyleBase['lastBlueprintConfig'] + blueprintAbChannelDisplay: DBShowStyleBase['blueprintAbChannelDisplay'] + abChannelDisplay: DBShowStyleBase['abChannelDisplay'] +}> = {Alternatively, keep the current approach since Mongo update operations typically use dynamic keys.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (36)
packages/webui/public/icons/channels/0.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/1.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/2.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/3.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/4.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/5.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/6.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/7.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/8.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/9.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/A.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/B.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/C.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/D.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/E.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/F.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/G.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/H.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/I.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/J.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/K.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/L.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/M.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/N.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/O.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/P.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/Q.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/R.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/S.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/T.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/U.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/V.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/W.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/X.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/Y.svgis excluded by!**/*.svgpackages/webui/public/icons/channels/Z.svgis excluded by!**/*.svg
📒 Files selected for processing (16)
meteor/__mocks__/helpers/database.tsmeteor/server/migration/upgrades/showStyleBase.tsmeteor/server/publications/showStyleUI.tspackages/blueprints-integration/src/api/showStyle.tspackages/blueprints-integration/src/documents/piece.tspackages/corelib/src/dataModel/ShowStyleBase.tspackages/job-worker/src/blueprints/context/lib.tspackages/meteor-lib/src/api/showStyles.tspackages/webui/src/__mocks__/helpers/database.tspackages/webui/src/client/ui/ClockView/DirectorScreen.tsxpackages/webui/src/client/ui/Settings/SettingsMenu.tsxpackages/webui/src/client/ui/Settings/ShowStyle/AbChannelDisplay.scsspackages/webui/src/client/ui/Settings/ShowStyle/AbChannelDisplay.tsxpackages/webui/src/client/ui/Settings/ShowStyleBaseSettings.tsxpackages/webui/src/client/ui/Settings/components/ColumnPackedGrid.scsspackages/webui/src/client/ui/Settings/components/ColumnPackedGrid.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
packages/blueprints-integration/src/api/showStyle.ts (1)
packages/blueprints-integration/src/content.ts (1)
SourceLayerType(212-212)
packages/webui/src/client/ui/Settings/ShowStyleBaseSettings.tsx (1)
packages/webui/src/client/ui/Settings/ShowStyle/AbChannelDisplay.tsx (1)
AbChannelDisplaySettings(17-251)
meteor/server/publications/showStyleUI.ts (3)
packages/corelib/src/lib.ts (1)
literal(27-27)packages/corelib/src/mongo.ts (1)
MongoFieldSpecifierOnesStrict(29-35)packages/corelib/src/dataModel/ShowStyleBase.ts (1)
DBShowStyleBase(28-78)
packages/meteor-lib/src/api/showStyles.ts (1)
packages/corelib/src/dataModel/ShowStyleBase.ts (1)
DBShowStyleBase(28-78)
🪛 Biome (2.1.2)
packages/webui/src/client/ui/ClockView/DirectorScreen.tsx
[error] 446-446: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 475-475: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (19)
packages/blueprints-integration/src/documents/piece.ts (1)
48-53: LGTM!The new
displayAbChanneloptional property is well-documented and appropriately typed as an optional boolean for overriding show style configuration at the piece level.packages/webui/src/client/ui/Settings/ShowStyle/AbChannelDisplay.scss (1)
1-21: LGTM!Clean SCSS implementation following BEM conventions. The flexbox layout for the header and modifier pattern for section gaps are well-structured.
packages/webui/src/__mocks__/helpers/database.ts (2)
214-214: LGTM!Correctly initializes the new
abChannelDisplayfield asundefinedin the default mock, maintaining consistency with theDBShowStyleBasetype.
558-558: LGTM!Properly propagates
abChannelDisplayfromDBShowStyleBasetoUIShowStyleBasein the conversion function, consistent with the mapping pattern used for other fields.packages/blueprints-integration/src/api/showStyle.ts (2)
40-40: LGTM!Import correctly added for
SourceLayerTypeused in the new configuration structure.
326-338: LGTM!Well-documented configuration structure for AB channel display. The design requiring all fields when the object is present ensures blueprints explicitly configure all filtering dimensions.
The forward-looking comment about future screen types (Line 337) is helpful for maintainability.
Empty array semantics are handled correctly: empty
sourceLayerIdsandoutputLayerIdsmean "no filtering" / "show all", as explicitly documented in the consumption logic at line 100 of DirectorScreen.tsx. The filtering is implemented as inclusive filters (OR logic for source layers when multiple filters exist), with clear comments explaining the behavior.packages/job-worker/src/blueprints/context/lib.ts (2)
107-107: LGTM!Correctly adds
displayAbChannelto the allowlist of blueprint piece keys, following the established pattern for property registration.
256-256: LGTM!Properly propagates
displayAbChannelfrom the piece to the blueprint representation inconvertPieceToBlueprints, consistent with the mapping pattern used for other piece properties.packages/webui/src/client/ui/Settings/components/ColumnPackedGrid.tsx (1)
1-237: Well-structured reusable component with good TypeScript generics.The component is well-designed with:
- Clear interface definitions with JSDoc comments
- Generic type support for flexibility
- Proper separation into sub-components
- Accessible checkbox patterns using
<label>wrappingpackages/meteor-lib/src/api/showStyles.ts (1)
52-54: Good use of indexed access type to maintain type consistency.Using
DBShowStyleBase['abChannelDisplay']ensures thatUIShowStyleBase.abChannelDisplaystays in sync with the source of truth in the data model. This is a maintainable approach.meteor/__mocks__/helpers/database.ts (1)
895-903: Mock correctly mirrors production mapping.The addition of
abChannelDisplayin the mock'sconvertToUIShowStyleBasefunction matches the production publication logic inmeteor/server/publications/showStyleUI.ts. The use ofComplete<UIShowStyleBase>ensures TypeScript will flag if any required fields are missing.packages/webui/src/client/ui/Settings/SettingsMenu.tsx (1)
354-370: Navigation item correctly integrated.The new "AB Channel Display" menu item follows the established pattern for show style settings navigation. The
subPathmatches the route definition inShowStyleBaseSettings.tsx.packages/webui/src/client/ui/Settings/ShowStyleBaseSettings.tsx (1)
13-13: Route correctly wired following established patterns.The import and route for
AbChannelDisplaySettingsfollow the same pattern as other settings panels (e.g.,HotkeyLegendSettings). The route path/ab-channel-displaymatches the menu'ssubPathinSettingsMenu.tsx.Also applies to: 135-137
packages/corelib/src/dataModel/ShowStyleBase.ts (1)
51-62: Good documentation and extensibility consideration.The inline comment on line 61 noting future fields (
showOnPresenterScreen,showOnCameraScreen) is helpful for maintainers understanding the planned evolution of this feature.meteor/server/publications/showStyleUI.ts (1)
29-43: LGTM! Field additions are consistent with the data model.The
abChannelDisplayfield is correctly added toShowStyleBaseFields, the MongoDB projection (fieldSpecifier), and properly propagated to the publishedUIShowStyleBaseobject on line 88. This aligns with theDBShowStyleBaseinterface changes.packages/webui/src/client/ui/Settings/ShowStyle/AbChannelDisplay.tsx (1)
186-250: Well-structured settings UI component.The component cleanly separates concerns with memoized computations, proper callback handling, and a clear JSX structure. The reset-to-blueprint functionality and toggle controls are implemented appropriately.
meteor/server/migration/upgrades/showStyleBase.ts (1)
143-150: Good blueprint-vs-user override handling.The logic correctly stores the blueprint default in
blueprintAbChannelDisplay(enabling reset functionality) while only initializingabChannelDisplayif the user hasn't already customized it. This preserves user overrides across blueprint upgrades.packages/webui/src/client/ui/ClockView/DirectorScreen.tsx (2)
507-552: Well-structured AB channel rendering logic.The precomputation of
currentPlayerElandnextPlayerElbefore the JSX return improves readability. The fallback from SVG icons to text display for non-alphanumeric player IDs is a sensible approach.
70-74: Remove unnecessaryas anycast on line 71.The
displayAbChannelproperty is properly defined inIBlueprintPieceand correctly included in thePieceandPieceInstancePiecetypes. Theas anycast is unnecessary and bypasses type checking. SincepieceInstance.pieceis typed asPieceInstancePiece, the property should be directly accessible without casting:const piece = pieceInstance.piece if (piece.displayAbChannel !== undefined) { return piece.displayAbChannel }
| const currentClipPlayer: string | undefined = useTracker(() => { | ||
| if (!currentPartInstance || !currentShowStyleBase || !playlist?.assignedAbSessions) return undefined | ||
| const config = currentShowStyleBase.abChannelDisplay | ||
| const instances = PieceInstances.find({ | ||
| partInstanceId: currentPartInstance.instance._id, | ||
| reset: { $ne: true }, | ||
| }).fetch() | ||
| for (const pi of instances) { | ||
| // Use configuration to determine if this piece should display AB channel | ||
| if (!shouldDisplayAbChannel(pi, currentShowStyleBase, config)) continue | ||
| const ab = pi.piece.abSessions | ||
| if (!ab || ab.length === 0) continue | ||
| for (const s of ab) { | ||
| const pool = playlist.assignedAbSessions?.[s.poolName] | ||
| if (!pool) continue | ||
| const matches: ABSessionAssignment[] = [] | ||
| for (const key in pool) { | ||
| const a = pool[key] | ||
| if (a && a.sessionName === s.sessionName) matches.push(a) | ||
| } | ||
| const live = matches.find((m) => !m.lookahead) | ||
| const la = matches.find((m) => m.lookahead) | ||
| if (live) return String(live.playerId) | ||
| if (la) return String(la.playerId) | ||
| } | ||
| } | ||
| return undefined | ||
| }, [currentPartInstance?.instance._id, currentShowStyleBase?._id, playlist?.assignedAbSessions]) | ||
|
|
||
| const nextClipPlayer: string | undefined = useTracker(() => { | ||
| if (!nextPartInstance || !nextShowStyleBaseId || !playlist?.assignedAbSessions) return undefined | ||
| // We need the ShowStyleBase to resolve sourceLayer types | ||
| const ssb = UIShowStyleBases.findOne(nextShowStyleBaseId) | ||
| if (!ssb) return undefined | ||
| const config = ssb.abChannelDisplay | ||
| const instances = PieceInstances.find({ | ||
| partInstanceId: nextPartInstance.instance._id, | ||
| reset: { $ne: true }, | ||
| }).fetch() | ||
| for (const pi of instances) { | ||
| // Use configuration to determine if this piece should display AB channel | ||
| if (!shouldDisplayAbChannel(pi, ssb, config)) continue | ||
| const ab = pi.piece.abSessions | ||
| if (!ab || ab.length === 0) continue | ||
| for (const s of ab) { | ||
| const pool = playlist.assignedAbSessions?.[s.poolName] | ||
| if (!pool) continue | ||
| const matches: ABSessionAssignment[] = [] | ||
| for (const key in pool) { | ||
| const a = pool[key] | ||
| if (a && a.sessionName === s.sessionName) matches.push(a) | ||
| } | ||
| const live = matches.find((m) => !m.lookahead) | ||
| const la = matches.find((m) => m.lookahead) | ||
| if (live) return String(live.playerId) | ||
| if (la) return String(la.playerId) | ||
| } | ||
| } | ||
| return undefined | ||
| }, [nextPartInstance?.instance._id, nextShowStyleBaseId, playlist?.assignedAbSessions]) |
There was a problem hiding this comment.
Critical: useTracker hooks called conditionally violate React's Rules of Hooks.
The useTracker hooks for currentClipPlayer and nextClipPlayer are called inside the conditional block if (playlist && playlistId && segments). React requires hooks to be called unconditionally and in the same order on every render. This will cause state corruption, crashes, or undefined behavior.
Suggested fix: Move hooks outside the conditional
Move the useTracker calls before the conditional block and handle the undefined cases inside the hook callbacks:
function DirectorScreenRender({
playlist,
segments,
currentShowStyleBaseId,
currentShowStyleBase,
nextShowStyleBaseId,
playlistId,
currentPartInstance,
currentSegment,
nextPartInstance,
nextSegment,
rundownIds,
}: Readonly<DirectorScreenProps & DirectorScreenTrackedProps>) {
useSetDocumentClass('dark', 'xdark')
const { t } = useTranslation()
const timingDurations = useTiming()
+ // Compute current and next clip player ids (for pieces with AB sessions)
+ // Note: Hooks must be called unconditionally per React rules
+ const currentClipPlayer: string | undefined = useTracker(() => {
+ if (!currentPartInstance || !currentShowStyleBase || !playlist?.assignedAbSessions) return undefined
+ const config = currentShowStyleBase.abChannelDisplay
+ const instances = PieceInstances.find({
+ partInstanceId: currentPartInstance.instance._id,
+ reset: { $ne: true },
+ }).fetch()
+ for (const pi of instances) {
+ if (!shouldDisplayAbChannel(pi, currentShowStyleBase, config)) continue
+ const ab = pi.piece.abSessions
+ if (!ab || ab.length === 0) continue
+ for (const s of ab) {
+ const pool = playlist.assignedAbSessions?.[s.poolName]
+ if (!pool) continue
+ const matches: ABSessionAssignment[] = []
+ for (const key in pool) {
+ const a = pool[key]
+ if (a && a.sessionName === s.sessionName) matches.push(a)
+ }
+ const live = matches.find((m) => !m.lookahead)
+ const la = matches.find((m) => m.lookahead)
+ if (live) return String(live.playerId)
+ if (la) return String(la.playerId)
+ }
+ }
+ return undefined
+ }, [currentPartInstance?.instance._id, currentShowStyleBase?._id, playlist?.assignedAbSessions])
+
+ const nextClipPlayer: string | undefined = useTracker(() => {
+ if (!nextPartInstance || !nextShowStyleBaseId || !playlist?.assignedAbSessions) return undefined
+ const ssb = UIShowStyleBases.findOne(nextShowStyleBaseId)
+ if (!ssb) return undefined
+ const config = ssb.abChannelDisplay
+ const instances = PieceInstances.find({
+ partInstanceId: nextPartInstance.instance._id,
+ reset: { $ne: true },
+ }).fetch()
+ for (const pi of instances) {
+ if (!shouldDisplayAbChannel(pi, ssb, config)) continue
+ const ab = pi.piece.abSessions
+ if (!ab || ab.length === 0) continue
+ for (const s of ab) {
+ const pool = playlist.assignedAbSessions?.[s.poolName]
+ if (!pool) continue
+ const matches: ABSessionAssignment[] = []
+ for (const key in pool) {
+ const a = pool[key]
+ if (a && a.sessionName === s.sessionName) matches.push(a)
+ }
+ const live = matches.find((m) => !m.lookahead)
+ const la = matches.find((m) => m.lookahead)
+ if (live) return String(live.playerId)
+ if (la) return String(la.playerId)
+ }
+ }
+ return undefined
+ }, [nextPartInstance?.instance._id, nextShowStyleBaseId, playlist?.assignedAbSessions])
+
if (playlist && playlistId && segments) {
const expectedStart = PlaylistTiming.getExpectedStart(playlist.timing) || 0
// ... rest of the conditional logic
- // Move the useTracker calls from here to above the if statementThe hooks already handle the case when dependencies are undefined by returning undefined early, so moving them outside the conditional is safe.
🧰 Tools
🪛 Biome (2.1.2)
[error] 446-446: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 475-475: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In @packages/webui/src/client/ui/ClockView/DirectorScreen.tsx around lines 446 -
505, The two useTracker hooks (for currentClipPlayer and nextClipPlayer) are
being invoked conditionally inside the playlist/playlistId/segments guard which
violates React's Rules of Hooks; move the useTracker(...) calls out of that
conditional so they run unconditionally on every render (keep the existing
early-return checks inside each hook callback that return undefined when
currentPartInstance/nextPartInstance, showStyle data, or
playlist?.assignedAbSessions are missing), ensuring the hooks remain in the same
order and then use the resulting currentClipPlayer and nextClipPlayer values
within the original conditional block.
| [SourceLayerType.UNKNOWN]: 'Unknown', | ||
| [SourceLayerType.CAMERA]: 'Camera', | ||
| [SourceLayerType.VT]: 'VT', | ||
| [SourceLayerType.REMOTE]: 'Remote', | ||
| [SourceLayerType.SCRIPT]: 'Script', | ||
| [SourceLayerType.GRAPHICS]: 'Graphics', | ||
| [SourceLayerType.SPLITS]: 'Splits', | ||
| [SourceLayerType.AUDIO]: 'Audio', | ||
| [SourceLayerType.LOWER_THIRD]: 'Lower Third', | ||
| [SourceLayerType.LIVE_SPEAK]: 'Live Speak', | ||
| [SourceLayerType.TRANSITION]: 'Transition', | ||
| [SourceLayerType.LIGHTS]: 'Lights', | ||
| [SourceLayerType.LOCAL]: 'Local', |
There was a problem hiding this comment.
These strings aren't exposed to i18n and there's possibility of duplication/misalignment if these are changed/new SourceLayerTypes are added.
It seems that there already exists a sourceLayerString function inside of client/ui/Settings/ShowStyle/SourceLayer.tsx that does the same thing, but ensures that the strings are translatable. It's not exported, but could be moved to a library function and reused here.
| const GROUP_HEADER_HEIGHT = 40 // Height for group checkbox + label | ||
| const ITEM_HEIGHT_BASE = 30 // Base height for checkbox + label | ||
| const GROUP_SPACING = 10 // Extra spacing after group |
There was a problem hiding this comment.
Considering that these are component-global assumptions I would put these at the top of the file, explaining what they are.
|
|
||
| const timingDurations = useTiming() | ||
|
|
||
| if (playlist && playlistId && segments) { |
There was a problem hiding this comment.
Haven't noticed this before, but this breaks React's rule of hooks for later uses of useTracker.
| if (live) return String(live.playerId) | ||
| if (la) return String(la.playerId) |
There was a problem hiding this comment.
I think these should be unprotectString not a String cast.
And add a reset button in the UI to return them to the blueprints default state,
Fix from PR notes
rjmunro
force-pushed
the
rjmunro/directors-view-show-ab
branch
from
4a00709 to
e07e230
Compare
By converting one to a set and iterating the other.
rjmunro
force-pushed
the
rjmunro/directors-view-show-ab
branch
from
e07e230 to
16ca0ca
Compare
|
|
||
| const timingDurations = useTiming() | ||
|
|
||
| if (playlist && playlistId && segments) { |
|
4 participants
About the Contributor
This pull request is posted on behalf of the BBC.
Type of Contribution
This is a Feature
Current Behavior
If you are using Sofie's AB playback features to allocate clips to players, it is hard to know which player will play the next clip. This can be important, e.g. if the sound is being mixed manually.
New Behavior
Icons are now available on the directors screen indicating which player will be used for a clip. What kind of clips and when can be set in the configuration.
Testing
Affected areas
This PR affects directors view and adds a settings screen. It also makes extra controls available to blueprints.
Time Frame
Other Information
Status
Summary
This feature PR adds AB Resolver channel indicators to the Directors screen, enabling users to see which playback player (channel) will be used for a clip. The display is fully configurable through both ShowStyle settings UI and blueprint definitions.
Changes by Category
Data Model & Core Types
DBShowStyleBasewith two new optional configuration objects:abChannelDisplay: user-configurable AB display settings (persisted in database)blueprintAbChannelDisplay: blueprint default values (stored during blueprint upgrade)showOnDirectorScreenflagdisplayAbChanneloptional boolean property toIBlueprintPieceto allow per-piece display overrideUIShowStyleBaseand related interfaces to include the newabChannelDisplayfieldBlueprint Integration & Configuration
BlueprintResultApplyShowStyleConfigwith optionalabChannelDisplayconfiguration objectconvertPieceToBlueprintsto propagate thedisplayAbChannelfield from piece instances to blueprint representationServer & Data Migration
showStyleBase.tsmigration to:updateSetobject for cleaner update payload constructionblueprintAbChannelDisplayfrom blueprint resultsabChannelDisplayfrom blueprint defaults if not already setabChannelDisplayin relevant projectionsshowStyleUI.tspublication to:abChannelDisplayto theShowStyleBaseFieldstypeabChannelDisplayin Mongo projection and returned UI objectabChannelDisplayfield initializationDirectors Screen Display
DirectorScreen.tsx:shouldDisplayAbChannel()helper function to determine display eligibility based on piece override, show style config, and source/output layer filtersassignedAbSessions/trackedAbSessionsin playlist data fetchescurrentShowStyleBaseprop toDirectorScreenRenderfor AB channel resolution contextSettings UI
/ab-channel-displayin show style settings menuAbChannelDisplaySettingscomponent providing:ShowStyleBases.updatewith$setand$unsetoperationsReusable Components
ColumnPackedGridcomponent for flexible grid layout:ColumnPackedGridGroup,ColumnPackedGridItem<TItem>, andColumnPackedGridProps<TItem>interfacesCoverage Notes
Codecov reports 55.88% patch coverage with 15 lines missing coverage in two files:
meteor/server/publications/showStyleUI.ts(0% coverage)meteor/server/migration/upgrades/showStyleBase.ts(73.91% coverage)Status
Feature implementation is complete with local testing reported. Unit tests and documentation updates remain pending.