feat: expand Properties panel capabilities#1712
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a UserEditing STATE variant (core + blueprints), wired translation, schema UI fields (icon/readOnly/oneOf discriminant), a TimeMs input and oneOf-buttons form with styles, readOnly plumbing for inputs, WebUI custom piece icon rendering across timeline components, and a tightened TSR error normalization. ChangesUserEditing STATE + related updates
Sequence DiagramsequenceDiagram
participant Core
participant JobWorker
participant Blueprints
participant WebUI
Core->>JobWorker: Provide CoreUserEditingDefinitionState (id,label,icon,...)
JobWorker->>Blueprints: translateUserEditsToBlueprint (map STATE -> UserEditingDefinitionState)
Blueprints-->>JobWorker: blueprint-side STATE definitions
JobWorker->>Core: translateUserEditsFromBlueprint (wrap labels)
WebUI->>WebUI: render piece.userEditOperations -> filter ACTION/STATE -> BlueprintAssetIcon(icon/iconInactive)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/webui/src/client/lib/Components/TextInput.tsx (1)
54-70:⚠️ Potential issue | 🟠 MajorRead-only mode leaves
editingValueuncleared on blur.Line 56 returns before
setEditingValue(null), while Line 69 still sets editing state on focus. This can keep stale local text/state in read-only mode.Proposed fix
const handleBlur = useCallback( (event: React.FocusEvent<HTMLInputElement>) => { - if (readOnly) return + if (readOnly) { + setEditingValue(null) + return + } let value: string = event.target.value if (value) { value = value.trim() @@ -const handleFocus = useCallback((event: React.FocusEvent<HTMLInputElement>) => { - setEditingValue(event.currentTarget.value) -}, []) +const handleFocus = useCallback( + (event: React.FocusEvent<HTMLInputElement>) => { + if (readOnly) return + setEditingValue(event.currentTarget.value) + }, + [readOnly] +)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/lib/Components/TextInput.tsx` around lines 54 - 70, The handlers leave stale local state in read-only mode: modify handleBlur (function handleBlur) so setEditingValue(null) is always called even when readOnly is true (e.g., clear before returning or in a finally-style flow) and update handleFocus (function handleFocus) to no-op when readOnly is true (check the readOnly prop before calling setEditingValue). Ensure handleUpdate behavior remains unchanged when not readOnly.packages/webui/src/client/lib/Components/MultiLineTextInput.tsx (1)
56-67:⚠️ Potential issue | 🟠 MajorRead-only blur path leaves stale local state.
On Line 58, returning early in
handleBlurskipssetEditingValue(null). Since Line 66 still setseditingValueon focus, read-only fields can get stuck in editing mode and show stale local value.Proposed fix
const handleBlur = useCallback( (event: React.FocusEvent<HTMLTextAreaElement>) => { - if (readOnly) return + if (readOnly) { + setEditingValue(null) + return + } handleUpdate(splitValueIntoLines(event.target.value)) setEditingValue(null) }, [handleUpdate, readOnly] ) -const handleFocus = useCallback((event: React.FocusEvent<HTMLTextAreaElement>) => { - setEditingValue(event.currentTarget.value) -}, []) +const handleFocus = useCallback( + (event: React.FocusEvent<HTMLTextAreaElement>) => { + if (readOnly) return + setEditingValue(event.currentTarget.value) + }, + [readOnly] +)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/lib/Components/MultiLineTextInput.tsx` around lines 56 - 67, The handleBlur callback currently returns early when readOnly is true and therefore never calls setEditingValue(null), which allows editingValue (set by handleFocus) to remain stale; update handleBlur (or its control flow) so that setEditingValue(null) is invoked unconditionally on blur (even when readOnly), while still avoiding handleUpdate when readOnly — locate the handleBlur and handleFocus callbacks and ensure editingValue is cleared on blur for all paths to prevent read-only fields from staying in edit mode.
🧹 Nitpick comments (7)
packages/webui/src/client/lib/Components/TimeMsInput.tsx (4)
23-45: MissingDeletekey in allowed keys list.The
Deletekey is not included, which users commonly expect for removing characters alongsideBackspace. Consider adding it for better usability.Suggested fix
const ALLOWED_KEYS = [ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '.', ':', ',', 'Backspace', + 'Delete', 'Tab', 'Enter', 'Escape', 'ArrowLeft', 'ArrowRight', 'Home', 'End', ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/lib/Components/TimeMsInput.tsx` around lines 23 - 45, The ALLOWED_KEYS array in TimeMsInput.tsx is missing the 'Delete' key which prevents users from using Delete to remove characters; update the ALLOWED_KEYS constant (the array named ALLOWED_KEYS) to include 'Delete' alongside 'Backspace' so both deletion keys are accepted when filtering key input in the TimeMsInput component.
66-86: Redundant condition check on line 80.The condition
i === 0 && partsCountis redundant—partsCountis always ≥ 1 at this point (sincesplit(':')on any string returns at least one element). This can be simplified to justi === 0.Suggested simplification
- else if (i === 0 && partsCount) ms += number * 1000 + else if (i === 0) ms += number * 1000🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/lib/Components/TimeMsInput.tsx` around lines 66 - 86, In parseTime, the check "else if (i === 0 && partsCount)" is redundant because partsCount is always >= 1; replace that branch with "else if (i === 0)" so the milliseconds for the seconds-only integer case are handled correctly. Locate the function parseTime and update the conditional that uses partsCount (and ensure no other logic is changed).
47-64: Misleading variable and option naming.The variable
frames(line 48) and optionshowZeroFrames(line 55) store/control milliseconds, not video frames. This could cause confusion, especially in a broadcast context where frames have a specific meaning (e.g., 25fps).Suggested naming improvement
-function formatTime(time: number, opts?: { showZeroHours?: boolean; showZeroFrames?: boolean }): string { - const frames = time % 1000 - const ms = String(frames).padStart(3, '0') +function formatTime(time: number, opts?: { showZeroHours?: boolean; showZeroMs?: boolean }): string { + const milliseconds = time % 1000 + const ms = String(milliseconds).padStart(3, '0') const ss = String(Math.floor(time / 1000) % 60).padStart(2, '0') const mm = String(Math.floor(time / 60000) % 60).padStart(2, '0') const hours = Math.floor(time / 3600000) let result = `${mm}:${ss}` - if (frames > 0 || opts?.showZeroFrames) { + if (milliseconds > 0 || opts?.showZeroMs) { result += `.${ms}` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/lib/Components/TimeMsInput.tsx` around lines 47 - 64, The variable and option in formatTime are misleadingly named as "frames" and "showZeroFrames" while they represent milliseconds; rename the local variable frames to millis (or ms) and the option showZeroFrames to showZeroMillis (or showMillisAlways) inside the formatTime function to reflect actual units, update string/formatting logic to use the new names (e.g., const millis = time % 1000 and pad as before), and search/update any call sites that pass or rely on showZeroFrames to use the new option name to keep API consistent (preserve behavior, only rename for clarity).
163-174: Minor: Redundant null coalescing on line 174.
showValueis already guaranteed to be a string after line 167, soshowValue ?? ''is redundant.Suggested simplification
let showValue: string | number | undefined = editingValue ?? undefined if (showValue === undefined && value !== undefined) { showValue = formatTime(value) } if (showValue === undefined) showValue = '' return ( <Form.Control type="text" className={ClassNames('form-control', classNames, editingValue !== null && modifiedClassName)} placeholder={placeholder} - value={showValue ?? ''} + value={showValue}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/lib/Components/TimeMsInput.tsx` around lines 163 - 174, The value prop passed to Form.Control in TimeMsInput.tsx uses a redundant nullish coalescing (value={showValue ?? ''}) because showValue is already normalized to a string by the preceding logic (editingValue branch, formatTime(value), or ''). Remove the "?? ''" and set the prop to value={showValue} to simplify; locate the showValue computation and the Form.Control usage to update the value prop accordingly (references: variable showValue, function formatTime, component Form.Control, and class modifiedClassName).packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsx (3)
198-202: Unused dependencies inuseCallback.
discPropertyandtypeValueare in the dependency array but are not used in the callback body.Suggested fix
const handleSelect = useCallback(() => { if (!selected) { handleUpdate?.(editingValue) } - }, [editingValue, discProperty, typeValue, selected, handleUpdate]) + }, [editingValue, selected, handleUpdate])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsx` around lines 198 - 202, The useCallback for handleSelect in OneOfButtons.tsx lists unused dependencies (discProperty, typeValue); update the dependency array to only include variables actually referenced inside the callback (editingValue, selected, and handleUpdate) so React's dependency tracking is correct and avoid unnecessary re-creations of handleSelect.
100-133: Redundant validation checks inside render callback.The checks at lines 103 and 105-117 are redundant since the validation at lines 83-98 already ensures all variants have the required discriminant property before this code is reached. This defensive code adds noise without adding safety.
Suggested simplification
return ( <LabelAndOverridesForOneOfButtons {...childProps.commonAttrs}> {(value, handleUpdate) => { - return props.schema.oneOf && - props.schema.oneOf.map((variant, index) => { - const type = variant.properties?.[discProperty]?.const - if (type === undefined) - return ( - <p> - {t( - 'Discriminant property "{{ discProperty }}" used, but is undefined for variant at index {{ index }}', - { - discProperty, - index, - } - )} - </p> - ) - - return ( - <OneOfVariantButtonComplex - value={value} - key={`${index}_${type}`} - discProperty={discProperty} - selected={value?.[discProperty] === type} - schema={variant} - translationNamespaces={props.translationNamespaces} - handleUpdate={handleUpdate} - /> - ) - }) + return props.schema.oneOf!.map((variant, index) => ( + <OneOfVariantButtonComplex + value={value} + key={`${index}_${variant.properties![discProperty]!.const}`} + discProperty={discProperty} + selected={value?.[discProperty] === variant.properties![discProperty]!.const} + schema={variant} + translationNamespaces={props.translationNamespaces} + handleUpdate={handleUpdate} + /> + )) }} </LabelAndOverridesForOneOfButtons> )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsx` around lines 100 - 133, The render callback inside LabelAndOverridesForOneOfButtons currently contains redundant discriminant checks; remove the defensive if (type === undefined) branch and the surrounding error <p> rendering, and simplify the mapping to directly map props.schema.oneOf to OneOfVariantButtonComplex using the computed const value (type) for key, selected, discProperty and schema props; keep using childProps.commonAttrs, value and handleUpdate as before and rely on the earlier validation that ensured every variant has the required discriminant property.
216-229: Replaceas anywithsatisfiesfor better type safety.The
as anycast on line 223 loses type safety. Usesatisfies JSONSchema<any, JSONSchema.TypeValue>instead—this type is already used elsewhere in the file and will verify the modified schema conforms to the expected type without widening toany.Suggested change
<SchemaFormWithState object={editingValue} schema={ { ...schema, // we don't need to display a title for the variant, it's already titled by the button [SchemaFormUIField.Title]: undefined, - } as any + } satisfies JSONSchema<any, JSONSchema.TypeValue> } onUpdate={onUpdateLocal} translationNamespaces={translationNamespaces} allowTables={false} showClearButtonForNonRequiredFields={true} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsx` around lines 216 - 229, The inline cast using "as any" on the modified schema passed to SchemaFormWithState reduces type safety; replace it by asserting the object satisfies JSONSchema<any, JSONSchema.TypeValue> (the same type used elsewhere) so the spread of schema with [SchemaFormUIField.Title]: undefined is checked at compile time. Update the schema expression passed into SchemaFormWithState (the object built from schema and [SchemaFormUIField.Title]) to use the satisfies JSONSchema<any, JSONSchema.TypeValue> clause instead of as any, leaving editingValue, onUpdateLocal, translationNamespaces and other props untouched.
🤖 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/blueprints-integration/src/userEditing.ts`:
- Around line 92-94: Update the JSDoc for the enum value UserEditingType.STATE
to correctly describe the STATE type instead of "Action"; locate the
UserEditingType enum and change the comment for the STATE member to a brief
accurate description such as "State" or "Represents a state change" so the JSDoc
matches the enum semantics.
In `@packages/playout-gateway/src/tsrHandler.ts`:
- Around line 266-269: In the error fallback branch inside the TSR error
handling (the block that currently returns { message: name + ': ' + 'Unknown
error: ' + JSON.stringify(e) }), replace JSON.stringify(e) with the already
imported stringifyError(e) to avoid throwing on circular structures or BigInt;
update the return to build the message using stringifyError(e) so unknown errors
are safely serialized (refer to the existing stringifyError symbol).
In `@packages/webui/src/client/lib/Components/FloatInput.tsx`:
- Around line 53-67: The blur handler (handleBlur) currently returns early when
readOnly and never calls setEditingValue(null), which leaves stale editingValue
after handleFocus; update handleBlur so it always calls setEditingValue(null)
even if readOnly (call setEditingValue(null) before the early return or remove
the early return and guard only the handleUpdate branch), referencing
handleBlur, handleFocus, setEditingValue, readOnly, handleUpdate and zeroBased
to locate the fix.
In
`@packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.scss`:
- Around line 1-8: The CSS rule for the selector ".field-one-of-buttons
.field-content" uses "justify-items", which doesn't apply to flex containers;
replace "justify-items: flex-start;" with "justify-content: flex-start;" in the
OneOfButtons.scss file so horizontal alignment works correctly with the flexbox
layout.
In `@packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsx`:
- Around line 172-180: The effect in OneOfButtons.tsx can create a feedback loop
because it depends on editingValue and handleUpdate while calling
handleUpdate(editingValue); fix by avoiding mutable state in the deps: either
require/memoize the parent handler (ensure handleUpdate is stable via
useCallback) or stop including editingValue in the dependency array and use a
ref (e.g., editingValueRef.current) to pass the current value into handleUpdate;
update the effect to read from oldValue.current and editingValueRef.current,
call handleUpdate(editingValueRef.current) and remove editingValue from the
dependency list (keep value, discProperty, typeValue, selected, handleUpdate or
only the stable handleUpdate if you chose memoization).
In `@packages/webui/src/client/styles/main.scss`:
- Line 88: The SCSS import in main.scss references the file with a .scss
extension (the OneOfButtons.scss import), which violates the
scss/load-partial-extension rule; remove the trailing “.scss” from that import
so it imports the OneOfButtons module without the extension to satisfy
Stylelint.
In
`@packages/webui/src/client/ui/SegmentTimeline/Renderers/DefaultLayerItemRenderer.tsx`:
- Line 76: When custom icons rendered by renderCustomPieceIcons() toggle, the
anchored/right-label widths must be recomputed; update the component so that any
state change that activates/deactivates those icons invokes the same width
recalculation used when the name changes (the existing recalculation
method—e.g., recalculateAnchoredWidths/recalcAnchoredWidths invoked on name
change). Add a call to that recalculation function inside the icon-toggle
handler or componentDidUpdate/useEffect that watches the icon-active state so
offsets are updated whenever renderCustomPieceIcons() output changes.
---
Outside diff comments:
In `@packages/webui/src/client/lib/Components/MultiLineTextInput.tsx`:
- Around line 56-67: The handleBlur callback currently returns early when
readOnly is true and therefore never calls setEditingValue(null), which allows
editingValue (set by handleFocus) to remain stale; update handleBlur (or its
control flow) so that setEditingValue(null) is invoked unconditionally on blur
(even when readOnly), while still avoiding handleUpdate when readOnly — locate
the handleBlur and handleFocus callbacks and ensure editingValue is cleared on
blur for all paths to prevent read-only fields from staying in edit mode.
In `@packages/webui/src/client/lib/Components/TextInput.tsx`:
- Around line 54-70: The handlers leave stale local state in read-only mode:
modify handleBlur (function handleBlur) so setEditingValue(null) is always
called even when readOnly is true (e.g., clear before returning or in a
finally-style flow) and update handleFocus (function handleFocus) to no-op when
readOnly is true (check the readOnly prop before calling setEditingValue).
Ensure handleUpdate behavior remains unchanged when not readOnly.
---
Nitpick comments:
In `@packages/webui/src/client/lib/Components/TimeMsInput.tsx`:
- Around line 23-45: The ALLOWED_KEYS array in TimeMsInput.tsx is missing the
'Delete' key which prevents users from using Delete to remove characters; update
the ALLOWED_KEYS constant (the array named ALLOWED_KEYS) to include 'Delete'
alongside 'Backspace' so both deletion keys are accepted when filtering key
input in the TimeMsInput component.
- Around line 66-86: In parseTime, the check "else if (i === 0 && partsCount)"
is redundant because partsCount is always >= 1; replace that branch with "else
if (i === 0)" so the milliseconds for the seconds-only integer case are handled
correctly. Locate the function parseTime and update the conditional that uses
partsCount (and ensure no other logic is changed).
- Around line 47-64: The variable and option in formatTime are misleadingly
named as "frames" and "showZeroFrames" while they represent milliseconds; rename
the local variable frames to millis (or ms) and the option showZeroFrames to
showZeroMillis (or showMillisAlways) inside the formatTime function to reflect
actual units, update string/formatting logic to use the new names (e.g., const
millis = time % 1000 and pad as before), and search/update any call sites that
pass or rely on showZeroFrames to use the new option name to keep API consistent
(preserve behavior, only rename for clarity).
- Around line 163-174: The value prop passed to Form.Control in TimeMsInput.tsx
uses a redundant nullish coalescing (value={showValue ?? ''}) because showValue
is already normalized to a string by the preceding logic (editingValue branch,
formatTime(value), or ''). Remove the "?? ''" and set the prop to
value={showValue} to simplify; locate the showValue computation and the
Form.Control usage to update the value prop accordingly (references: variable
showValue, function formatTime, component Form.Control, and class
modifiedClassName).
In `@packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsx`:
- Around line 198-202: The useCallback for handleSelect in OneOfButtons.tsx
lists unused dependencies (discProperty, typeValue); update the dependency array
to only include variables actually referenced inside the callback (editingValue,
selected, and handleUpdate) so React's dependency tracking is correct and avoid
unnecessary re-creations of handleSelect.
- Around line 100-133: The render callback inside
LabelAndOverridesForOneOfButtons currently contains redundant discriminant
checks; remove the defensive if (type === undefined) branch and the surrounding
error <p> rendering, and simplify the mapping to directly map props.schema.oneOf
to OneOfVariantButtonComplex using the computed const value (type) for key,
selected, discProperty and schema props; keep using childProps.commonAttrs,
value and handleUpdate as before and rely on the earlier validation that ensured
every variant has the required discriminant property.
- Around line 216-229: The inline cast using "as any" on the modified schema
passed to SchemaFormWithState reduces type safety; replace it by asserting the
object satisfies JSONSchema<any, JSONSchema.TypeValue> (the same type used
elsewhere) so the spread of schema with [SchemaFormUIField.Title]: undefined is
checked at compile time. Update the schema expression passed into
SchemaFormWithState (the object built from schema and [SchemaFormUIField.Title])
to use the satisfies JSONSchema<any, JSONSchema.TypeValue> clause instead of as
any, leaving editingValue, onUpdateLocal, translationNamespaces and other props
untouched.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 55fd0024-eff6-4962-9db8-de635c628bca
📒 Files selected for processing (28)
packages/blueprints-integration/src/userEditing.tspackages/corelib/src/dataModel/UserEditingDefinitions.tspackages/job-worker/src/blueprints/context/lib.tspackages/playout-gateway/src/tsrHandler.tspackages/shared-lib/src/lib/JSONSchemaUtil.tspackages/webui/src/client/lib/Components/Button.tsxpackages/webui/src/client/lib/Components/FloatInput.tsxpackages/webui/src/client/lib/Components/IntInput.tsxpackages/webui/src/client/lib/Components/MultiLineTextInput.tsxpackages/webui/src/client/lib/Components/TextInput.tsxpackages/webui/src/client/lib/Components/TimeMsInput.tsxpackages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.scsspackages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsxpackages/webui/src/client/lib/forms/SchemaFormWithOverrides.tsxpackages/webui/src/client/styles/main.scsspackages/webui/src/client/styles/propertiesPanel.scsspackages/webui/src/client/styles/rundownView.scsspackages/webui/src/client/ui/SegmentTimeline/Renderers/CustomLayerItemRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/DefaultLayerItemRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/L3rdSourceRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/LocalLayerItemRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/MicSourceRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/SplitsSourceRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/VTSourceRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/SegmentTimeline.tsxpackages/webui/src/client/ui/SegmentTimeline/SourceLayerItem.tsxpackages/webui/src/client/ui/UserEditOperations/PropertiesPanel.tsxpackages/webui/src/client/ui/UserEditOperations/RenderUserEditOperations.tsx
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/playout-gateway/src/tsrHandler.ts`:
- Line 275: The stack suffix is concatenated without a separator, causing
outputs like "At deviceDevice"; update the concatenation in tsrHandler.ts where
the stack is built (the expression using e.stack and name) to include a
separator (e.g., a space or colon) before the device name so it reads "At device
<name>" (ensure the updated concatenation still only runs when e.stack is
defined).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 43f2dfae-a83c-442a-a96f-f6a208a60492
📒 Files selected for processing (10)
packages/blueprints-integration/src/userEditing.tspackages/playout-gateway/src/tsrHandler.tspackages/webui/src/client/lib/Components/FloatInput.tsxpackages/webui/src/client/lib/Components/TimeMsInput.tsxpackages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.scsspackages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsxpackages/webui/src/client/styles/main.scsspackages/webui/src/client/ui/SegmentTimeline/Renderers/CustomLayerItemRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/DefaultLayerItemRenderer.tsxpackages/webui/src/client/ui/UserEditOperations/PropertiesPanel.tsx
✅ Files skipped from review due to trivial changes (2)
- packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.scss
- packages/webui/src/client/ui/UserEditOperations/PropertiesPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/webui/src/client/styles/main.scss
- packages/blueprints-integration/src/userEditing.ts
- packages/webui/src/client/ui/SegmentTimeline/Renderers/CustomLayerItemRenderer.tsx
- packages/webui/src/client/ui/SegmentTimeline/Renderers/DefaultLayerItemRenderer.tsx
| stack: e.stack && e.stack + '\nAt device' + name, | ||
| message: e.message !== undefined ? name + ': ' + e.message : undefined, | ||
| name: e.name !== undefined ? name + ': ' + e.name : undefined, | ||
| stack: e.stack !== undefined ? e.stack + '\nAt device' + name : undefined, |
There was a problem hiding this comment.
Fix stack suffix formatting in device context.
Line 275 currently builds ...'\nAt device' + name, which produces At deviceDevice ... without a separator.
🔧 Proposed fix
- stack: e.stack !== undefined ? e.stack + '\nAt device' + name : undefined,
+ stack: e.stack !== undefined ? `${e.stack}\nAt device ${name}` : undefined,📝 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.
| stack: e.stack !== undefined ? e.stack + '\nAt device' + name : undefined, | |
| stack: e.stack !== undefined ? `${e.stack}\nAt device ${name}` : undefined, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/playout-gateway/src/tsrHandler.ts` at line 275, The stack suffix is
concatenated without a separator, causing outputs like "At deviceDevice"; update
the concatenation in tsrHandler.ts where the stack is built (the expression
using e.stack and name) to include a separator (e.g., a space or colon) before
the device name so it reads "At device <name>" (ensure the updated concatenation
still only runs when e.stack is defined).
98a177d to
db0ae30
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/webui/src/client/ui/SegmentTimeline/Renderers/LocalLayerItemRenderer.tsx (1)
31-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRe-measure the anchored labels when the custom icon set changes.
After Line 70, the right label width is no longer driven only by the piece name.
componentDidUpdate()still recalculates widths only whenpiece.instance.piece.namechanges, so toggling piece icons can leavesetAnchoredElsWidths()with a stalerightLabelWidthand misplace the overflow label.Suggested fix
componentDidUpdate(prevProps: Readonly<IProps>, prevState: Readonly<IState>): void { if (super.componentDidUpdate && typeof super.componentDidUpdate === 'function') { super.componentDidUpdate(prevProps, prevState) } - if (this.props.piece.instance.piece.name !== prevProps.piece.instance.piece.name) { + const prevPiece = prevProps.piece.instance.piece + const nextPiece = this.props.piece.instance.piece + const prevIcons = JSON.stringify(prevPiece.userEditOperations ?? []) + const nextIcons = JSON.stringify(nextPiece.userEditOperations ?? []) + + if (nextPiece.name !== prevPiece.name || nextIcons !== prevIcons) { this.updateAnchoredElsWidths() } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/webui/src/client/ui/SegmentTimeline/Renderers/LocalLayerItemRenderer.tsx` around lines 31 - 38, componentDidUpdate currently only recalculates anchored widths when this.props.piece.instance.piece.name changes; extend that check to also detect changes to the piece's custom icon set (or any prop that affects the right label width) and call updateAnchoredElsWidths() in that case — e.g. add a condition like this.props.piece.instance.piece.iconSet !== prevProps.piece.instance.piece.iconSet (or compare the specific prop you use to toggle icons) so setAnchoredElsWidths()/updateAnchoredElsWidths() runs when icons toggle; keep the existing name check and ensure componentDidUpdate still calls super.componentDidUpdate and then triggers updateAnchoredElsWidths when either the name or the icon-set-related prop changed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@packages/webui/src/client/ui/SegmentTimeline/Renderers/LocalLayerItemRenderer.tsx`:
- Around line 31-38: componentDidUpdate currently only recalculates anchored
widths when this.props.piece.instance.piece.name changes; extend that check to
also detect changes to the piece's custom icon set (or any prop that affects the
right label width) and call updateAnchoredElsWidths() in that case — e.g. add a
condition like this.props.piece.instance.piece.iconSet !==
prevProps.piece.instance.piece.iconSet (or compare the specific prop you use to
toggle icons) so setAnchoredElsWidths()/updateAnchoredElsWidths() runs when
icons toggle; keep the existing name check and ensure componentDidUpdate still
calls super.componentDidUpdate and then triggers updateAnchoredElsWidths when
either the name or the icon-set-related prop changed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 56a873bc-c4f9-47f6-a2f6-b2ccfe74b503
📒 Files selected for processing (18)
packages/blueprints-integration/src/userEditing.tspackages/playout-gateway/src/tsrHandler.tspackages/webui/src/client/lib/Components/FloatInput.tsxpackages/webui/src/client/lib/Components/TimeMsInput.tsxpackages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.scsspackages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsxpackages/webui/src/client/styles/main.scsspackages/webui/src/client/styles/propertiesPanel.scsspackages/webui/src/client/styles/rundownView.scsspackages/webui/src/client/ui/SegmentTimeline/Renderers/CustomLayerItemRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/DefaultLayerItemRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/L3rdSourceRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/LocalLayerItemRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/MicSourceRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/SplitsSourceRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/VTSourceRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/SourceLayerItem.tsxpackages/webui/src/client/ui/UserEditOperations/PropertiesPanel.tsx
✅ Files skipped from review due to trivial changes (2)
- packages/webui/src/client/ui/SegmentTimeline/Renderers/VTSourceRenderer.tsx
- packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.scss
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/webui/src/client/ui/SegmentTimeline/Renderers/L3rdSourceRenderer.tsx
- packages/webui/src/client/ui/SegmentTimeline/Renderers/SplitsSourceRenderer.tsx
- packages/webui/src/client/ui/SegmentTimeline/Renderers/MicSourceRenderer.tsx
- packages/webui/src/client/ui/SegmentTimeline/Renderers/DefaultLayerItemRenderer.tsx
- packages/webui/src/client/ui/SegmentTimeline/SourceLayerItem.tsx
- packages/webui/src/client/styles/main.scss
- packages/blueprints-integration/src/userEditing.ts
- packages/webui/src/client/lib/Components/FloatInput.tsx
- packages/webui/src/client/styles/rundownView.scss
- packages/playout-gateway/src/tsrHandler.ts
- packages/webui/src/client/ui/SegmentTimeline/Renderers/CustomLayerItemRenderer.tsx
- packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsx
- packages/webui/src/client/lib/Components/TimeMsInput.tsx
db0ae30 to
8915d71
Compare
feat: wip feat: implement oneOfButtons form widget feat: enhance OneOfButtons with improved styling and add TimeMsInputControl for time input handling fix: improve OneOfButtons, add ui:icon to variants fix(OneOfButtons): switch label and icons around
chore: lint
chore: lint chore: lint
8915d71 to
6bf3302
Compare
About the Contributor
This pull request is posted on behalf of the NRK.
Type of Contribution
This is a:
Feature
Current Behavior
The user available library of input widgets for Properties Panel is limited. The from schema can't handle mutually exclusive structures/alternatives.
New Behavior
A new widget/pattern is added for using a
oneOfschema operator allowing for a typical TypeScript pattern of a discriminated union. A new widget for time value inputs is added, automatically converting a "mm:ss"-formatted input into Sofie-standard milliseconds. A new typeUserEditingType.STATEis added to surface states that are not toggleable. Much like with icons declared on a Segment level, icons declared on a Piece level are now shown.Testing
Affected areas
This PR affects the properties panel.
Time Frame
We intend to finish the development on this feature in two weeks time.
Other Information
This PR resolves RFC #1703
Status