JSON Block Definition interface #9402
Conversation
|
@gonfunko bump, will this be considered? 👀 |
|
Yes, sorry for the delay on this! Will try to get you an initial review this week, thank you for the ping on it. |
gonfunko
left a comment
gonfunko
left a comment
There was a problem hiding this comment.
This looks promising; added a couple notes. I'd like to run this by my colleagues before merging, but as you may have seen we're transitioning from Google to the Raspberry Pi Foundation so it may be a couple weeks until things are settled.
| interface CommonArg { | ||
| name?: string; | ||
| } | ||
|
|
||
| /** Input Args */ | ||
| interface InputValueArg extends CommonArg { | ||
| type: 'input_value'; | ||
| check?: string | string[]; | ||
| align?: FieldsAlign | ||
| } | ||
| interface InputStatementArg extends CommonArg { | ||
| type: 'input_statement'; | ||
| check?: string | string[]; | ||
| } | ||
| interface InputDummyArg extends CommonArg { | ||
| type: 'input_dummy'; | ||
| } | ||
|
|
||
| /** Field Args */ | ||
| interface FieldInputArg extends CommonArg{ | ||
| type: 'field_input' | ||
| text: string | ||
| } | ||
|
|
||
| interface FieldNumberArg extends CommonArg { | ||
| type: 'field_number'; | ||
| value?: number; | ||
| min?: number; | ||
| max?: number; | ||
| precision?: number; | ||
| } | ||
|
|
||
| interface FieldDropdownArg extends CommonArg { | ||
| type: 'field_dropdown'; | ||
| options: [string, string][]; | ||
| } | ||
|
|
||
| interface FieldCheckboxArg extends CommonArg { | ||
| type: 'field_checkbox'; | ||
| checked?: boolean | 'TRUE' | 'FALSE'; | ||
| } | ||
|
|
||
| interface FieldImageArg { | ||
| type: 'field_image'; | ||
| src: string; | ||
| width: number; | ||
| height: number; | ||
| alt?: string; | ||
| flipRtl?: boolean | 'TRUE' | 'FALSE'; | ||
| } | ||
|
|
||
| interface FieldVariableArg extends CommonArg { | ||
| type: 'field_variable' | ||
| variable: string | null | ||
| } |
There was a problem hiding this comment.
Each of the Field classes defines and exports a FieldXYZFromJsonConfig interface, which I think should be used in place of these to avoid duplication/skew.
| type?: string; | ||
| style?: string; | ||
| colour?: string | number; | ||
| output: string | string[] | null; |
There was a problem hiding this comment.
What was the motivation for making output mandatory while allowing previousStatement and nextStatement to be omitted entirely? In principle they should all be optional, but if there's code that's relying on output being there I think that would be the better thing to fix.
| enableContextMenu?: boolean; | ||
| suppressPrefixSuffix?: boolean; | ||
|
|
||
| [key: `message${number}`]: string | undefined; |
There was a problem hiding this comment.
This is clever! TIL about this TS mechanism
2 participants
The basics
The details
Resolves
Implements JSON Block Definition interface proposed in #9387
Proposed Changes
Introduces a new
JsonBlockDefinitionincore/interfaces/core/interfaces/i_json_block_definition.tswithJsonBlockDefinitionandBlockArgdefinitionsBlock.jsonInit,jsonInitFactory,defineBlocksWithJsonArrayandcreateBlockDefinitionsFromJsonArrayto use the new interfaceinterpolateto use the newBlockArgtypeblocks/to explicitly set'output': nullto match the default behaviour in the block factoryAnyDuringMigrationusage for JSON block definitionsReason for Changes
Makes it easier to write valid JSON Block definitions without having to always rely on the block factory
Test Coverage
I ran the unit test suite and manually loaded the playground with the updated blocks to ensure they still render and connect correctly
Documentation
Not needed
Additional Information
I found another issue while testing (unrelated to this) in the default Playground, when
Categories: (typed variables)is selected, the colour category errors out because of an invalidcolour_pickerdefinition