Conversation
zoran995
left a comment
There was a problem hiding this comment.
I did just a quick pass, but wanted to raise a bigger concern first.
I wonder if it would be simple to move more code to the React side and handle rendering there, rather than having to specify all component props in the model class.
IMO, the model class should control the values, but React should handle its presentation. Reasoning also comes from current workflows, which are pretty hard to debug and understand the data flow. If we start implementing a design system, the model class needs to change rather than just updating the rendering.
From the workflow point of view, it could consist of two layers
- workflow models that control data
- and rendering registry where the rendering part is registered for each workflow model.
| const StandardAppWorkflow: FC<PropsType> = observer(({ appWorkflow }) => { | ||
| const terria = useViewState().terria; | ||
| const [t] = useTranslation(); | ||
| const appWorkflowId = useMemo(() => createGuid(), [appWorkflow]); |
There was a problem hiding this comment.
You can use useId here or generate id on workflow instance
|
|
||
| const CESIUM_OPTIONS_KEY = "cesiumOptions"; | ||
|
|
||
| export interface CesiumOptionsSchema { |
|
Thanks @zoran995 for having a look. Not sure I fully understood the suggestion for moving more things to React - but i feel it would be good to have both levels of abstraction, i.e to be able to define workflows from model and also have an escape hatch to do it in React. If we only have the React approach, eventually the components will start to have more and more styling, hooks, UI look and feel stuff added to it. I think workflows are a way of saying, these are my input values and this is how it should be changed, I don't care how exactly it gets rendered, where it gets rendered, just render it the best way possible.
Maybe, but we can probably improve that. I think the complexity is in some part the current UX. From a implementation perspective, we should probably look at renaming SelectableDimensions to Widgets or something and maybe try to simplify the type definitions further. Otherwise the workflow is just describing a nested list of inputs using JS and you can break it up however you want to improve readability.
Ideally we should expose only a minimal subset of the props from the underlying design system through the SelectableDimensions/Widget API with the goal being reducing breaking changes. Though, I'm not sure how much we can fight the urge to add more props to the Widget API. |
What this PR does
Test me
Checklist
doc/.