jsonforms

jsonforms copied to clipboard

Is your feature request related to a problem? Please describe.

When using the material renderers, it would be nice to have the option to use the different material-ui input variants (standard, filled, outlined).

Currently the material renderers are implemented using the Input component with no option to change the variant. That means you have to implement custom renderers if you want to use other variants.

Describe the solution you'd like

The ideal solution would be updating the material renderers to take into consideration variants declared in a custom material theme provided to the JsonForms component, that way you can use material-ui's built-in theme support to build a custom theme and material renderers would choose the appropriate variant (or the standard variant if no custom theme is defined).

Describe alternatives you've considered

There's two alternatives by implementing custom renderers:

1. Copy/pasting the material renderers source and changing the Input component to the variant you want. This means you now how to keep track of changes to source for this renderer.
2. Implementing your own custom renderer from scratch, meaning you have to re-invent the wheel and take care of possible edge cases that the material renderers library already covers.

Framework

React

RendererSet

Material

Additional context

If the team agrees this would be a good feature to implement, I'm willing to work on it and submit a PR.

Hi @CarlosGomez-dev! Thanks for the feature request.

In general this definitely makes sense, so if we could add this feature in a non-disruptive way then I'm definitely for it.

Can you give a small example of what you're suggesting? If I understand correctly you would like to suggest using Input, FilledInput and OutlinedInput depending on whether the user customized the props of the corresponding Material UI Form Input? So for example when a user set the variant of TextField to filled we need to check all textual inputs and render FilledInput then?

It's a little bit misleading in a way that we are not actually using TextFields so the maybe this maps sometimes in unexpected ways. Or are you suggesting that we introduce custom JSON Forms options? So for example jsonforms.input.variant could then just set the variant for all inputs. This would be similar to the existing custom theming support.

Did you already implement an approach for your own renderer set?

Hello @sdirix,

Can you give a small example of what you're suggesting? If I understand correctly you would like to suggest using Input, FilledInput and OutlinedInput depending on whether the user customized the props of the corresponding Material UI Form Input? So for example when a user set the variant of TextField to filled we need to check all textual inputs and render FilledInput then? It's a little bit misleading in a way that we are not actually using TextFields so the maybe this maps sometimes in unexpected ways.

That is what I had in mind, default props is material-ui calls them.

I took a deeper look and can see that MaterialInputControl is rendering textual inputs inside a FormControl, so you would need to target MuiFormControl instead of MuiTextField in a custom theme, but users would need to dive into the source code to learn that which is less than ideal.

Or are you suggesting that we introduce custom JSON Forms options? So for example jsonforms.input.variant could then just set the variant for all inputs. This would be similar to the existing custom theming support.

Seeing the discoverability issue above, this approach seems more user-friendly. This means MaterialInputControl would need to be aware of this property, as the label variant must match the input variant, and then every material-renderer input also needs to be aware of this property to render the correct variant.

I would implement this similarly to how the official TextInput handles it.

Did you already implement an approach for your own renderer set?

I haven't. I created a custom renderer set with the specific variant and style I needed, but a feature like this would be very useful as I could simply define a variant via jsonforms.input.variant and if I wanted to define a custom theme I would know exactly which input to target. (MuiInput/MuiFilledInput/MuiOutlinedInput)

Hi @CarlosGomez-dev, sounds good to me! I'll discuss with the team and come back to you next week.

Hi @CarlosGomez-dev, we very much would like to see this feature. So if you have time a PR is very welcomed!

Regarding the implementation we're wondering whether it should be implemented by determining the appropriate Input variant component in MaterialInputControl and then hand it over the to InnerComponent. This way the determination code does not have to be duplicated in all MuiInputXYZ components. What do you think?

I would love this ❤️ . Are you still working on this @CarlosGomez-dev ?

I just bumped in this issue as well. Naively I assumed simply adjusting the default properties in MUI theme would do the trick.

I saw however that JSONForms uses lower level components, like Input instead of TextField. I just quickly ran trough the code, changed all variants to outline and changed Input to TextField and that resolved all my issues.

I however do not have any insight why the lower level components were used.

@sdirix is there some specific reason for that?

I have three options:

• contribute refactoring to properly apply mui theming
• fork and maintain my own material controls
• use alternative library or custom implementation

I would prefer contributing but only if that is in alignment with the project.

Hi! The use of lower level inputs has historical reasons. Originally the cells renderers were mandatory and not only used in Tables but reused in all control renderers. At some point this was refactored, so now we have dedicated self-contained renderers, however internally they still use the cells primitives. Just no longer by dispatching to them but by reusing an extracted component.

We are definitely willing to accept changes here. Feel free to contribute using the approach outlined above

In case you want to go the full way and replace all invocations of MaterialInputControl with the fully integrated variant of Material UI, e.g. TextField, then I would recommend first doing an analysis of the consequences so we can internally discuss whether that is the way we want to go. For this you could for example replace only the MaterialTextControl and MaterialNumberControl and highlight the required code changes.

Note that we require test cases for all contributions, especially ones which change a lot of the render output ;)

I see in the approach outlined above you guys discussed a custom config to the JSONForms directly, right? But why would that be better than relying on the MUI theming configuration?

Effectively a custom TextField is created in JSONForms library, so I guess using the MuiTextField in theming config would be completely fine and expected.

---

So what I could do is reverse engineer the TextField without introducing breaking changes to JSONForms and switch between different variants of inputs. I would do most of the work in MaterialInputControl to avoid duplications and pass down the correct variant.

Something like:

const { ..., variant = 'standard' } = useThemeProps({
    props: props as ControlProps & WithInput & TextFieldProps,
    name: 'MuiTextField',
});

const variantComponent = {
  standard: Input,
  filled: FilledInput,
  outlined: OutlinedInput,
};
const VariantComponent = variantComponent[variant];

...

return (
  <FormControl
    ...
    variant={variant}
  >
    <InnerComponent
      {...props}
      id={id + '-input'}
      isValid={isValid}
      visible={visible}
      VariantComponent={VariantComponent}
    />
  </FormControl>
)

@klemenoslaj I started on this today but would really like more insight into what you were thinking. I'm pretty new to JSONForms and I'm not quite sure how you would use VariantComponent as a property to the InnerComponent. Any elaboration would be greatly appreciated.

Agreed, this tool looks wonderful, and I don't think we can use it without the ability to configure MUI a little bit more

@sdirix Is someone working on this issue? If not, I am willing to provide a PR which would leverage MUI's theme provider setting for variant.

Hi @sebastianfrey, so you would do the work in MaterialInputControl as suggested above? Is yes, then that's fine!

@sdirix, yes, as suggested in the linked comment I would leverage useThemeProps() from MUI to implement the desired behavior.