We’re writing a custom renderer set for jsonforms using the material-renderers as a backup and will eventually remove material all-together once all classes are replaced.
One major difference between Material and @ui5/webcomponents-react is that a component like <Input> will be passed a valueState and valueStateMessage such that when you’re focused on it you will see the error message as a kind of tooltip in a red message strip; whereas the Material renderer will be rendering the messages outside the component itself, as a FormHelperText.
One thing I’ve noticed is that Cells, such as those which render in an ArrayControl in a Table, the error is not rendered by the Cell itself, but by the layout that renders the Cell; however, for our corresponding TextCell ours will need the error message to put on the <Input> (which is itself just a wrapper around a <ui5-input> web component).
This works fine when I want to take the default error message - such as must have the required property 'name'; however, we also need to provide our own translations, so that won’t work for us.
I’ve constructed the <JsonForms> outer component so that we pass a JsonFormsI18nState with our own translate and translateError functions, these work fine for things like a TextControl where the error prop passed from the mapStateToControlProps is calling the ErrorTranslator; however, if the control is wrapped with a <CellDispatch> the mapStateToCellProps function will not call the errorTranslator at all.
This seems like a bug to me, one which doesn’t raise any red flags in a normal renderer set which doesn’t need to place error messages on the lowest level control tags. There just doesn’t seem like a clean solution there outside of having the Cell to throw away the errors prop and generate its own by calling mapStateToControlProps after a few state hooks.
But my guess is that most of the time this particular errors prop returned by the cellProps is typically dereferenced immediately when no actual Cell Renderer reads it - am I right?
Nice find! Yes, we overlooked the error messages for cells, likely because we don’t use them anymore in our off the shelf renderer sets.
I agree, for now we should definitely also invoke the i18n mechanism on this prop. Do you want to contribute this feature?
In the long run we should probably remove the errors determination here (e.g. with a major revision in the far future), but indicate how an developer can get them back.
I can certainly contribute the feature, please let me know what steps I need to take - is it as simple as creating a Pull Request from a fork, I can do that quickly.
For this particular issue I think the fix is very straight forward, and I’ve already created a work around in my own renderer set by wrapping the wrapper functions, but I need to do the same operation that mapStateToCellProps is doing.
I can certainly contribute the feature, please let me know what steps I need to take - is it as simple as creating a Pull Request from a fork, I can do that quickly.
Yes exactly. Just make sure that all test are running etc. They are also executed in our CI in case you have trouble running them locally.