Skip to content

feat: expand Properties panel capabilities#1712

Merged
jstarpl merged 6 commits into
Sofie-Automation:mainfrom
nrkno:contrib/feat/UserEditingTypeSTATE
Jun 2, 2026
Merged

feat: expand Properties panel capabilities#1712
jstarpl merged 6 commits into
Sofie-Automation:mainfrom
nrkno:contrib/feat/UserEditingTypeSTATE

Conversation

@jstarpl

@jstarpl jstarpl commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

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 oneOf schema 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 type UserEditingType.STATE is 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

  • I have added one or more unit tests for this PR
  • I have updated the relevant unit tests
  • No unit test changes are needed for this PR

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

  • PR is ready to be reviewed.
  • The functionality has been tested by the author.
  • Relevant unit tests has been added / updated.
  • Relevant documentation (code comments, system documentation) has been added / updated.

@jstarpl jstarpl marked this pull request as draft April 10, 2026 11:03
@coderabbitai

coderabbitai Bot commented Apr 10, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds 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.

Changes

UserEditing STATE + related updates

Layer / File(s) Summary
Type definitions and exports
packages/blueprints-integration/src/userEditing.ts, packages/corelib/src/dataModel/UserEditingDefinitions.ts
Adds UserEditingType.STATE and UserEditingDefinitionState / CoreUserEditingDefinitionState interfaces with id, label, optional icon/iconInactive/isActive, and includes them in the unions.
Blueprint <-> Core translation
packages/job-worker/src/blueprints/context/lib.ts
Handles STATE in both translateUserEditsToBlueprint and translateUserEditsFromBlueprint, mapping id/icon/iconInactive/isActive and wrapping/omitting label namespaces as needed.
Schema UI fields & form components
packages/shared-lib/src/lib/JSONSchemaUtil.ts, packages/webui/src/client/lib/forms/SchemaFormWithOverrides.tsx, packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/*
Adds ui:icon, ui:readOnly, and ui:oneOf:discriminant; introduces OneOfButtonsWithOverrides and styles; wires displayType dispatch and readOnly propagation.
TimeMs input & input readOnly plumbing
packages/webui/src/client/lib/Components/TimeMsInput.tsx, packages/webui/src/client/lib/Components/*Input.tsx
Adds TimeMsInputControl with parse/format/validation/keyboard handling; threads readOnly into FloatInput and other input components and exposes DOM readOnly.
WebUI custom piece icons & timeline renderers
packages/webui/src/client/ui/SegmentTimeline/..., packages/webui/src/client/styles/*
Filter piece.userEditOperations for ACTION/STATE ops with icons, render BlueprintAssetIcon per op, and include these icons in multiple renderer outputs; add CSS for oneOf-buttons and custom icon layout.
Gateway error helper
packages/playout-gateway/src/tsrHandler.ts
Tighten fixError to accept unknown thrown values, validate object shape before extracting message/name/stack, and use stringifyError fallback.

Sequence Diagram

sequenceDiagram
  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)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • Sofie-Automation/sofie-core#1544: touches the same blueprints context translation area (translateUserEditsToBlueprint/FromBlueprint) and related user-edit handling.

Suggested reviewers

  • nytamin
  • Saftret
  • hummelstrand
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: expand Properties panel capabilities' accurately reflects the main change in the PR, which expands the Properties panel with new widgets, input types, and icon rendering capabilities.
Description check ✅ Passed The PR description provides comprehensive context about the contribution, clearly explains current vs. new behavior, identifies affected areas, and links to the relevant RFC issue #1703.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Apr 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 30.76923% with 18 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/job-worker/src/blueprints/context/lib.ts 10.00% 18 Missing ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 | 🟠 Major

Read-only mode leaves editingValue uncleared 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 | 🟠 Major

Read-only blur path leaves stale local state.

On Line 58, returning early in handleBlur skips setEditingValue(null). Since Line 66 still sets editingValue on 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: Missing Delete key in allowed keys list.

The Delete key is not included, which users commonly expect for removing characters alongside Backspace. 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 && partsCount is redundant—partsCount is always ≥ 1 at this point (since split(':') on any string returns at least one element). This can be simplified to just i === 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 option showZeroFrames (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.

showValue is already guaranteed to be a string after line 167, so showValue ?? '' 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 in useCallback.

discProperty and typeValue are 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: Replace as any with satisfies for better type safety.

The as any cast on line 223 loses type safety. Use satisfies 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 to any.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 446e86b and 70cc031.

📒 Files selected for processing (28)
  • packages/blueprints-integration/src/userEditing.ts
  • packages/corelib/src/dataModel/UserEditingDefinitions.ts
  • packages/job-worker/src/blueprints/context/lib.ts
  • packages/playout-gateway/src/tsrHandler.ts
  • packages/shared-lib/src/lib/JSONSchemaUtil.ts
  • packages/webui/src/client/lib/Components/Button.tsx
  • packages/webui/src/client/lib/Components/FloatInput.tsx
  • packages/webui/src/client/lib/Components/IntInput.tsx
  • packages/webui/src/client/lib/Components/MultiLineTextInput.tsx
  • packages/webui/src/client/lib/Components/TextInput.tsx
  • packages/webui/src/client/lib/Components/TimeMsInput.tsx
  • packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.scss
  • packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsx
  • packages/webui/src/client/lib/forms/SchemaFormWithOverrides.tsx
  • packages/webui/src/client/styles/main.scss
  • packages/webui/src/client/styles/propertiesPanel.scss
  • packages/webui/src/client/styles/rundownView.scss
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/CustomLayerItemRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/DefaultLayerItemRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/L3rdSourceRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/LocalLayerItemRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/MicSourceRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/SplitsSourceRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/VTSourceRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/SegmentTimeline.tsx
  • packages/webui/src/client/ui/SegmentTimeline/SourceLayerItem.tsx
  • packages/webui/src/client/ui/UserEditOperations/PropertiesPanel.tsx
  • packages/webui/src/client/ui/UserEditOperations/RenderUserEditOperations.tsx

Comment thread packages/blueprints-integration/src/userEditing.ts
Comment thread packages/playout-gateway/src/tsrHandler.ts
Comment thread packages/webui/src/client/lib/Components/FloatInput.tsx Outdated
Comment thread packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsx Outdated
Comment thread packages/webui/src/client/styles/main.scss Outdated
@jstarpl jstarpl added the Contribution from NRK Contributions sponsored by NRK (nrk.no) label Apr 10, 2026
@jstarpl jstarpl marked this pull request as ready for review April 13, 2026 15:27

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 70cc031 and 0781dfb.

📒 Files selected for processing (10)
  • packages/blueprints-integration/src/userEditing.ts
  • packages/playout-gateway/src/tsrHandler.ts
  • packages/webui/src/client/lib/Components/FloatInput.tsx
  • packages/webui/src/client/lib/Components/TimeMsInput.tsx
  • packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.scss
  • packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsx
  • packages/webui/src/client/styles/main.scss
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/CustomLayerItemRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/DefaultLayerItemRenderer.tsx
  • packages/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,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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).

@jstarpl jstarpl requested a review from nytamin May 14, 2026 12:41
@jstarpl jstarpl force-pushed the contrib/feat/UserEditingTypeSTATE branch 2 times, most recently from 98a177d to db0ae30 Compare June 2, 2026 13:33

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Re-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 when piece.instance.piece.name changes, so toggling piece icons can leave setAnchoredElsWidths() with a stale rightLabelWidth and 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

📥 Commits

Reviewing files that changed from the base of the PR and between 98a177d and db0ae30.

📒 Files selected for processing (18)
  • packages/blueprints-integration/src/userEditing.ts
  • packages/playout-gateway/src/tsrHandler.ts
  • packages/webui/src/client/lib/Components/FloatInput.tsx
  • packages/webui/src/client/lib/Components/TimeMsInput.tsx
  • packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.scss
  • packages/webui/src/client/lib/forms/SchemaFormOneOfButtons/OneOfButtons.tsx
  • packages/webui/src/client/styles/main.scss
  • packages/webui/src/client/styles/propertiesPanel.scss
  • packages/webui/src/client/styles/rundownView.scss
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/CustomLayerItemRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/DefaultLayerItemRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/L3rdSourceRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/LocalLayerItemRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/MicSourceRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/SplitsSourceRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/VTSourceRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/SourceLayerItem.tsx
  • packages/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

@jstarpl jstarpl force-pushed the contrib/feat/UserEditingTypeSTATE branch from db0ae30 to 8915d71 Compare June 2, 2026 13:59
jstarpl added 6 commits June 2, 2026 16:09
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
@jstarpl jstarpl force-pushed the contrib/feat/UserEditingTypeSTATE branch from 8915d71 to 6bf3302 Compare June 2, 2026 14:09
@jstarpl jstarpl merged commit 3826017 into Sofie-Automation:main Jun 2, 2026
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Contribution from NRK Contributions sponsored by NRK (nrk.no)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants