Refactor Segment/Part properties to be a property instead of an operation #38
Conversation
| * Properties that are user editable from the properties panel in the Sofie UI, if the user saves changes to these | ||
| * it will trigger a user edit operation of type DefaultUserOperationEditProperties | ||
| */ | ||
| userEditProperties?: Readonly<UserEditingProperties> |
There was a problem hiding this comment.
This is ReadOnly<...> here, but the plain type in Segment. They should be consistent
| @@ -66,3 +66,45 @@ export interface CoreUserEditingDefinitionSourceLayerForm { | |||
| value: Record<string, any> | |||
| } | |||
| } | |||
|
|
|||
| export interface UserEditingProperties { | |||
There was a problem hiding this comment.
I think this is missing any translationNamespaces properties that would be good to do in some way.
Maybe that could be done as a single property at the root of this interface, that sofie injects when processing the blueprint version of it?
I would also recommend making anywhere we convert from the bluerpint to core versions of this interface be very explicit conversions. Because while they 99% match, the core version should also be adding translationNamespaces in various places, such as any uses of ITranslatableMessage. If the conversion misses adding any of these then that is some bugs waiting to be discovered by the first person to try translating this ui section.
| @@ -29,6 +29,7 @@ export enum SchemaFormUIField { | |||
| * Currently only valid for: | |||
| * - object properties. Valid values are 'json'. | |||
| * - string properties. Valid values are 'base64-image'. | |||
| * - boolean properties. Valid values are 'switch'. | |||
There was a problem hiding this comment.
I think there is also a page in the documentation which describes the valid values of this
| ...change, | ||
| pieceTypeProperties: { | ||
| type: key, | ||
| value: {}, // todo - take defaults |
|
|
||
| const selectedGroupId = change.pieceTypeProperties.type | ||
| const selectedGroupSchema = properties.schema[selectedGroupId]?.schema | ||
| const parsedSchema = selectedGroupSchema ? JSONBlobParse(selectedGroupSchema) : undefined |
There was a problem hiding this comment.
It might be beneficial to useMemo this line, to avoid doing the JSONBlobParse on every render of this component
| schema={parsedSchema} | ||
| object={value} | ||
| onUpdate={onUpdate} | ||
| translationNamespaces={[]} |
There was a problem hiding this comment.
it would be good to have this set to a proper value if possible
| /** | ||
| * @todo - retrieve translationNamespaces for correct blueprint translations? | ||
| */ | ||
| function GlobalPropertiesEditor({ |
There was a problem hiding this comment.
Same comments for this as PropertiesEditor
| <div> | ||
| {actions.map((action) => ( | ||
| <button | ||
| title={'User Action : ' + action.label.key} |
There was a problem hiding this comment.
This is skipping any translation
| * A list of id's of operations to be exposed on the properties panel as buttons. These operations | ||
| * must be available on the element | ||
| * | ||
| * note - perhaps these should have their own full definitions? |
There was a problem hiding this comment.
should this note be handled in this PR?
As the blueprint version of this interface has the full definition for these, it would make sense to keep that here too. It looks like the ui is expecting them to be the full definitions too
There was a problem hiding this comment.
Do we still want/need the CoreUserEditingDefinitionSourceLayerForm and similar types?
I don't mind if they stay, just want to ask the question
| @@ -74,7 +74,7 @@ export enum UserEditingType { | |||
| } | |||
|
|
|||
| export interface UserEditingSourceLayer { | |||
| sourceLayerLabel: string | |||
| sourceLayerLabel: string // translate? take from type? | |||
No description provided.