PR for #1647 - add ui:props to all material-ui widgets #1672

dekelb · 2020-03-23T20:50:47Z

This PR is a fix for issue #1647

Following the comments in the issue - it seems like the best option was to use the "ui:props" inside the uiSchema (of each field), which being pushed into the "options" variable inside the widget (as part of the WidgetProps).

The reason I decided to use the "ui:props" and not just add properties directly to the "options" is that some of those properties are already being used, and this might cause issues there.

Also - those properties are relevant specifically to the Components that we take from the Theme. I noticed some requests to also support Fields/Components from other libs - using this standard we will be able to support other field-specific properties.

dekelb · 2020-03-23T22:30:13Z

packages/material-ui/src/CheckboxWidget/CheckboxWidget.tsx

          />
        }
-        label={label}
+        label={label || schema.title}


Added the "schema.title" to be aligned with the rest of the widgets

dekelb · 2020-03-23T22:30:28Z

packages/material-ui/src/CheckboxesWidget/CheckboxesWidget.tsx

              control={checkbox}
              key={index}
              label={option.label}
-            />
-          ) : (


This code is duplicated with no reason

dekelb · 2020-03-23T22:30:44Z

packages/material-ui/src/RangeWidget/RangeWidget.tsx

@@ -41,13 +42,14 @@ const RangeWidget = ({
        //error={!!rawErrors}
        required={required}
      >
-        <FormLabel id={id}>{label}</FormLabel>
+        <FormLabel id={id}>{label || schema.title}</FormLabel>


Added the "schema.title" to be aligned with the rest of the widgets

dekelb · 2020-03-23T22:30:54Z

packages/material-ui/src/UpDownWidget/UpDownWidget.tsx

@@ -33,7 +36,7 @@ const UpDownWidget = ({
      //error={!!rawErrors}
      required={required}
    >
-      <InputLabel>{label}</InputLabel>
+      <InputLabel>{label || schema.title}</InputLabel>


Added the "schema.title" to be aligned with the rest of the widgets

heath-freenome · 2020-03-25T18:13:15Z

packages/material-ui/src/CheckboxesWidget/CheckboxesWidget.tsx

-          return inline ? (
-            <FormControlLabel
+
+          return <FormControlLabel
              control={checkbox}
              key={index}
              label={option.label}


Is this missing the || schema.title that was added in all the other places on purpose?

No, you can find this in line 62 - it's part of the FormLabel, not as part of the FormControlLabel

dekelb · 2020-04-04T16:57:39Z

@epicfaace any update on this one? I'm not so sure why it didn't get into v2-alpha5

epicfaace

Sorry it took some time to get to this.

Can you add some tests that verify this functionality (both the ui:props and schema.title overrides)? You could just extend the snapshot tests that are already in the material-ui folder.
I'm worried that people may use ui:props to override fields that we don't want the user to override, such as, for example, required, id, or onChange (to override these ones, it really might be best to make a custom widget). Perhaps we should define a list of overridable props for each widget using ui:props and only override those. What do you think?

heath-freenome · 2020-04-27T15:58:07Z

One suggestion is to pull out the required, id or onChange props using something like the Lodash omit() function. Make a helper, like the rangeSpec() utility function, called maybe uiProps(), which extracts all. of the non-override-able variables and returns just the other stuff?

Fedorov113 · 2020-05-14T21:36:55Z

Hello, any update on this?

epicfaace · 2020-05-17T14:32:19Z

@dekelb just thought of another thing -- what if someone inputs dangerouslySetInnerHTML into ui:props? This is essentially a security vulnerability if someone allows users to edit uiSchemas.

I'm thinking it would be best if, for each widget, we could define a whitelist of props that could be overridden using ui:props -- this would be best for security. Do you know how such a list could be assembled? Perhaps from the type definitions for material ui widgets?

kedano · 2020-05-20T09:52:59Z

Hi, I'm trying to figure out how I could use the variant prop and other adjustments to my schema with material-ui.

Could it be an idea like @epicfaace mention to whitelist at least the props that has something to do with the design in the material-ui doc? https://material-ui.com/api/text-field/

howientc · 2020-08-31T18:45:32Z

Any progress on this PR?

epicfaace · 2020-08-31T20:57:00Z

We need to add a whitelist for allowed props. See how it was done with the fluent ui theme.

ronny-rentner · 2020-09-21T07:08:56Z

We need to add a whitelist for allowed props. See how it was done with the fluent ui theme.

Why do you need an allowlist?

Just let all props through, if they do something or not, it doesn't hurt and it will work when upstream adds new props automatically.

ronny-rentner · 2020-09-21T11:37:08Z

@dekelb just thought of another thing -- what if someone inputs dangerouslySetInnerHTML into ui:props? This is essentially a security vulnerability if someone allows users to edit uiSchemas.

Why would that someone not validate and escape user input correctly as you would always do it with any kind of user input?

I'm thinking it would be best if, for each widget, we could define a whitelist of props that could be overridden using ui:props -- this would be best for security. Do you know how such a list could be assembled? Perhaps from the type definitions for material ui widgets?

The list will change constantly. It should just let extra props through that it doesn't know. It's a common scheme to let pass all other props down in the hierarchy.

ronny-rentner · 2020-09-21T12:22:22Z

PS: Check out upstream: https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/TextField/TextField.js
They also have "...other" and pass through all props.

epicfaace · 2020-09-21T12:54:56Z

Why would that someone not validate and escape user input correctly as you would always do it with any kind of user input?

One use case of react-jsonschema-form is allowing users to specify and save their own JSON Schemas (which I use myself in a form builder). It doesn't seem trivial to validate that input; at least, there don't exist libraries that could do that automatically (something like XSS sanitizers) and it would require you to perform the validation yourself.

The current behavior of react-jsonschema-form is that no possible JSON Schema / uiSchema can have a security impact, because there's no way to set the dangerouslyInnerHTML prop. With this change, this guarantee changes, because suddenly uiSchemas become untrusted user input and can cause XSS vulnerabilities. At the very least, this would be a backwards incompatible change.

kedano · 2020-11-26T09:08:25Z

Hi, any update on this? Still waiting on the whitelisting?

wight554 · 2022-04-10T17:01:22Z

While this one is seems to be dead, can you pass rest props to all components like you do here https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/material-ui/src/TextWidget/TextWidget.tsx#L53
It should be safe enough in this case
I'd like to create own minimal wrappers for components with own props whitelist, but currently I need to rewrite all non-textwidget widgets to achieve that
cc @epicfaace

wight554 · 2022-04-10T17:05:35Z

Currently I can do something like this with TextWidget and I'd like to be able to do this for other widgets as well:

export const TextWidget = ({ options, ...props }: WidgetProps) => {
  const allProps: object & WhitelistedProps =
    typeof options?.props === 'object' && options?.props !== null ? options?.props : {};

  const pickedProps = pick(allProps, PROPS_WHITELIST);

  return <Widgets.TextWidget {...props} options={options} {...pickedProps} />;
};

epicfaace · 2022-06-26T15:32:16Z

While this one is seems to be dead, can you pass rest props to all components like you do here https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/material-ui/src/TextWidget/TextWidget.tsx#L53 It should be safe enough in this case I'd like to create own minimal wrappers for components with own props whitelist, but currently I need to rewrite all non-textwidget widgets to achieve that cc @epicfaace

@wight554 sorry I don't understand what you are proposing, could you provide more details?

wight554 · 2022-06-26T16:39:26Z

While this one is seems to be dead, can you pass rest props to all components like you do here https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/material-ui/src/TextWidget/TextWidget.tsx#L53 It should be safe enough in this case I'd like to create own minimal wrappers for components with own props whitelist, but currently I need to rewrite all non-textwidget widgets to achieve that cc @epicfaace

@wight554 sorry I don't understand what you are proposing, could you provide more details?

The idea was to add props spreading to all widget components
E.g. there's no ... props in select widget https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/material-ui/src/SelectWidget/SelectWidget.tsx#L68 like in link I've shared before

epicfaace · 2022-06-26T19:44:49Z

Ok. Why do you think it would be safe enough in this case?

wight554 · 2022-06-26T20:02:39Z

Ok. Why do you think it would be safe enough in this case?

There's no way to pass these, unless you wrap component with some hoc
Doing this for all will allow creating generic hoc for all components that'll check ui:props and pass it per widget

stale · 2023-12-12T02:17:46Z

This issue has been automatically marked as possibly close because it has not had recent activity. It will be closed if no further activity occurs. Please leave a comment if this is still an issue for you. Thank you.

stale · 2024-01-11T15:59:59Z

This issue was closed because of lack of recent activity. Reopen if you still need assistance.

added ui:props to all material-ui widgets

1d1fb6d

dekelb commented Mar 23, 2020

View reviewed changes

dekelb mentioned this pull request Mar 25, 2020

Material UI Range widget does not show the current value #1674

Closed

3 tasks

heath-freenome reviewed Mar 25, 2020

View reviewed changes

epicfaace self-requested a review March 25, 2020 19:27

Merge branch 'master' into feature/material-ui-options

943e847

epicfaace and others added 3 commits April 4, 2020 14:48

Merge branch 'master' into feature/material-ui-options

7fbce91

Merge branch 'master' into feature/material-ui-options

7242fa7

Merge branch 'master' into feature/material-ui-options

57a4784

epicfaace reviewed Apr 25, 2020

View reviewed changes

Merge branch 'master' into feature/material-ui-options

bc680d9

Merge branch 'master' into feature/material-ui-options

971c423

epicfaace added the awaiting changes label May 15, 2020

epicfaace added this to In progress in PRs May 23, 2020

epicfaace moved this from In progress to Review in progress in PRs May 23, 2020

kedano mentioned this pull request May 25, 2020

Support extra material ui options for fields #1647

Open

epicfaace mentioned this pull request Jun 14, 2020

Better consistency with Material UI patterns with usage of <TextField /> #1790

Merged

7 tasks

epicfaace mentioned this pull request Jun 13, 2021

[Material-UI] [Date field] Change the date format from 'mm-dd-yyyy' to 'dd-mm-yyyy' #2425

Closed

3 tasks

stale bot added the possibly close To confirm if this issue can be closed label Dec 12, 2023

stale bot closed this Jan 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR for #1647 - add ui:props to all material-ui widgets #1672

PR for #1647 - add ui:props to all material-ui widgets #1672

dekelb commented Mar 23, 2020

dekelb Mar 23, 2020

dekelb Mar 23, 2020

dekelb Mar 23, 2020

dekelb Mar 23, 2020

heath-freenome Mar 25, 2020

dekelb Mar 25, 2020 •

edited

dekelb commented Apr 4, 2020

epicfaace left a comment

heath-freenome commented Apr 27, 2020

Fedorov113 commented May 14, 2020

epicfaace commented May 17, 2020 •

edited

kedano commented May 20, 2020 •

edited

howientc commented Aug 31, 2020

epicfaace commented Aug 31, 2020

ronny-rentner commented Sep 21, 2020

ronny-rentner commented Sep 21, 2020

ronny-rentner commented Sep 21, 2020

epicfaace commented Sep 21, 2020

kedano commented Nov 26, 2020

wight554 commented Apr 10, 2022 •

edited

wight554 commented Apr 10, 2022

epicfaace commented Jun 26, 2022

wight554 commented Jun 26, 2022

epicfaace commented Jun 26, 2022

wight554 commented Jun 26, 2022

stale bot commented Dec 12, 2023

stale bot commented Jan 11, 2024

PR for #1647 - add ui:props to all material-ui widgets #1672

PR for #1647 - add ui:props to all material-ui widgets #1672

Conversation

dekelb commented Mar 23, 2020

dekelb Mar 23, 2020

Choose a reason for hiding this comment

dekelb Mar 23, 2020

Choose a reason for hiding this comment

dekelb Mar 23, 2020

Choose a reason for hiding this comment

dekelb Mar 23, 2020

Choose a reason for hiding this comment

heath-freenome Mar 25, 2020

Choose a reason for hiding this comment

dekelb Mar 25, 2020 • edited

Choose a reason for hiding this comment

dekelb commented Apr 4, 2020

epicfaace left a comment

Choose a reason for hiding this comment

heath-freenome commented Apr 27, 2020

Fedorov113 commented May 14, 2020

epicfaace commented May 17, 2020 • edited

kedano commented May 20, 2020 • edited

howientc commented Aug 31, 2020

epicfaace commented Aug 31, 2020

ronny-rentner commented Sep 21, 2020

ronny-rentner commented Sep 21, 2020

ronny-rentner commented Sep 21, 2020

epicfaace commented Sep 21, 2020

kedano commented Nov 26, 2020

wight554 commented Apr 10, 2022 • edited

wight554 commented Apr 10, 2022

epicfaace commented Jun 26, 2022

wight554 commented Jun 26, 2022

epicfaace commented Jun 26, 2022

wight554 commented Jun 26, 2022

stale bot commented Dec 12, 2023

stale bot commented Jan 11, 2024

dekelb Mar 25, 2020 •

edited

epicfaace commented May 17, 2020 •

edited

kedano commented May 20, 2020 •

edited

wight554 commented Apr 10, 2022 •

edited