eclipsesource / jsonforms

Customizable JSON Schema-based forms with React, Angular and Vue support out of the box.
http://jsonforms.io
Other
2.1k stars 358 forks source link

Support for `multiple` prop on `Select` #2347

Closed jchamberlain closed 3 weeks ago

jchamberlain commented 2 months ago

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

I would like to use MUI's Select component, but with the multiple flag enabled. See https://mui.com/material-ui/react-select/#multiple-select. Unfortunately, the MuiSelect component in the material renderers does not accept the multiple prop nor provide any way to pass it to the underlying Select.

Describe the solution you'd like

Locally I've added a new interface to material-renderers/src/util/theme.ts:

export interface WithSelectProps {
  multiple?: boolean;
}

Then added it to the MuiSelect component:

-import { i18nDefaults, WithInputProps } from '../util';
+import { i18nDefaults, WithInputProps, WithSelectProps } from '../util';

 export const MuiSelect = React.memo(function MuiSelect(
-  props: EnumCellProps & WithClassname & TranslateProps & WithInputProps
+  props: EnumCellProps & WithClassname & TranslateProps & WithInputProps & WithSelectProps
 ) {
   const {
     data,
@@ -46,6 +46,7 @@ export const MuiSelect = React.memo(function MuiSelect(
     config,
     label,
     t,
+    multiple,
   } = props;
   const appliedUiSchemaOptions = merge({}, config, uischema.options);
   const noneOptionLabel = useMemo(
@@ -63,6 +64,7 @@ export const MuiSelect = React.memo(function MuiSelect(
       value={data !== undefined ? data : ''}
       onChange={(ev) => handleChange(path, ev.target.value || undefined)}
       fullWidth={true}
+      multiple={multiple || false}
     >

This worked perfectly for me. I'm not sure if this is the right approach, but could something like this be considered for inclusion?

Describe alternatives you've considered

MUI's Autocomplete can do some of this, but it's overly complicated for most of my use cases and it can be confusing to users who are expecting normal <select>-like behavior.

Framework

React

RendererSet

Material

Additional context

No response

lucas-koehler commented 1 month ago

Hi @jchamberlain , thank you for the suggestion. I don't think there is a problem with your approach :) While we could also allow all SelectProps and forward them, there could be collisions between Material's SelectProps and JsonForms's props. Thus, I think your approach makes sense. Would you like to contribute it?

TheOneTheOnlyJJ commented 1 month ago

+1 for adding this in the library

jchamberlain commented 1 month ago

Thank you for the feedback, @lucas-koehler! I've just opened a PR with the changes.