Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for multiple prop on Select #2347

Closed
jchamberlain opened this issue Jun 21, 2024 · 3 comments · Fixed by #2356
Closed

Support for multiple prop on Select #2347

jchamberlain opened this issue Jun 21, 2024 · 3 comments · Fixed by #2356
Milestone

Comments

@jchamberlain
Copy link
Contributor

jchamberlain commented Jun 21, 2024

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
Copy link
Contributor

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?

@lucas-koehler lucas-koehler added this to the Backlog milestone Jul 8, 2024
@TheOneTheOnlyJJ
Copy link

+1 for adding this in the library

@jchamberlain
Copy link
Contributor Author

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

lucas-koehler pushed a commit to jchamberlain/jsonforms that referenced this issue Jul 24, 2024
jchamberlain added a commit to jchamberlain/jsonforms that referenced this issue Aug 2, 2024
@lucas-koehler lucas-koehler linked a pull request Aug 6, 2024 that will close this issue
@lucas-koehler lucas-koehler modified the milestones: Backlog, 3.4 Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants