Sub-PR: expression fields fix playground root #1098
Conversation
Related to #1073
bpmn-io-tasks
bot
added
in progress
Skaiir
changed the title
| if (!apiLinkTarget) { | ||
| return; | ||
| } | ||
|
|
||
| apiLinkTarget.api = { | ||
| attachDataContainer: (node) => inputDataRef.current.attachTo(node), | ||
| attachResultContainer: (node) => outputDataRef.current.attachTo(node), | ||
| attachFormContainer: (node) => formViewerRef.current.attachTo(node), | ||
| attachEditorContainer: (node) => formEditorRef.current.attachTo(node), | ||
| attachPaletteContainer: (node) => formEditorRef.current.get('palette').attachTo(node), | ||
| attachPropertiesPanelContainer: (node) => formEditorRef.current.get('propertiesPanel').attachTo(node), | ||
| get: (name, strict) => formEditorRef.current.get(name, strict), | ||
| getDataEditor: () => inputDataRef.current, | ||
| getEditor: () => formEditorRef.current, | ||
| getForm: () => formViewerRef.current, | ||
| getResultView: () => outputDataRef.current, | ||
| getSchema: () => formEditorRef.current.getSchema(), | ||
| saveSchema: () => formEditorRef.current.saveSchema(), | ||
| setSchema: setSchema, | ||
| setData: setData | ||
| }; |
There was a problem hiding this comment.
Is this better than the callback function? If you use the callback pattern then it means that PlaygroundRoot doesn't need to have any knowledge of how the API object works.
There was a problem hiding this comment.
I guess it's mostly a preference thing. I just found the callback harder to read personally.
In any case, this class isn't exported at the moment so it's mostly just an implementation detail.
|
|
||
| }, [ apiLinkTarget ]); | ||
|
|
||
| // separate effect for state to avoid re-creating the api object every time |
There was a problem hiding this comment.
Why is this a problem?
There was a problem hiding this comment.
I mostly did this because the state changes constantly, while the rest of the API really only gets set up once ideally.
| } | ||
|
|
||
| apiLinkTarget.api.getState = () => ({ schema, data }); | ||
| apiLinkTarget.api.load = load; |
There was a problem hiding this comment.
So we have two ways to load data: through the initial props and this api method.
There was a problem hiding this comment.
Yes exactly. I agree this isn't great and not react-like, but I didn't want to make any changes to any of the logic. Initializing with props feels quite hacky.
| useEffect(() => { | ||
| dataEditorRef.current.setValue(toString(initialData)); | ||
| }, [ initialData ]); | ||
| if (!config.initialSchema) { | ||
| return; | ||
| } | ||
|
|
||
| load(config.initialSchema, config.initialData || {}); | ||
| }, [ config.initialSchema, config.initialData, load ]); |
There was a problem hiding this comment.
I would have these be the default values of the useState calls.
const [ schema, setSchema ] = useState(config.initialSchema);
const [ data, setData ] = useState(config.initialData || {});
useEffect(() => {
if (!schema) {
return;
}
formEditorRef.current.importSchema(schema, data);
inputDataRef.current.setValue(toString(data));
if (inputDataContainerRef.current) {
const variables = getSchemaVariables(schema);
inputDataRef.current.setVariables(variables);
}
}, [ data, schema, getSchemaVariables ]);This means you can git rid of the effect on 188-190, 192-197 and all load does it set the states.
Sub-pr of #1074