[TextField] How to leverage CSS input validation #16903

jonkelling · 2019-08-06T01:44:37Z

I have followed (at least) the PR section of the contributing guide.

This update applies the CSS class Mui-valueFilled to InputBase and InputLabel based on whether the input has an empty value. This allows for styling filled fields without the need for external logic.

An example is provided in /src/pages/components/text-fields/CustomizedInputs.js.

I did not find an open issue related to this. I will happily add one if it's useful.

Other notes:
There are some discrepancies in the use of filled in some of the existing components. For example, FormLabel has a filled class that reflects the value, but filled is used for the filled variant in other components. That's unrelated to this PR--just an observation.

Thanks!

- InputBase now applies valueFilled when uncontrolled and without FormControlContext. - Added/Updated comments for use of valueFilled class.

mui-pr-bot · 2019-08-06T01:59:09Z

Details of bundle changes.

Comparing: 78db8a0...9db0d30

bundle	parsed diff	gzip diff	prev parsed	current parsed	prev gzip	current gzip
@material-ui/core	0.00%	-0.00%	329,663	329,663	90,121	90,120
@material-ui/core/Paper	0.00%	0.00%	68,684	68,684	20,469	20,469
@material-ui/core/Paper.esm	0.00%	0.00%	62,058	62,058	19,209	19,209
@material-ui/core/Popper	0.00%	0.00%	29,185	29,185	10,434	10,434
@material-ui/core/Textarea	0.00%	0.00%	5,759	5,759	2,368	2,368
@material-ui/core/TrapFocus	0.00%	0.00%	3,834	3,834	1,617	1,617
@material-ui/core/styles/createMuiTheme	0.00%	0.00%	16,389	16,389	5,824	5,824
@material-ui/core/useMediaQuery	0.00%	0.00%	3,213	3,213	1,311	1,311
@material-ui/lab	0.00%	0.00%	152,607	152,607	46,520	46,520
@material-ui/styles	0.00%	0.00%	51,401	51,401	15,285	15,285
@material-ui/system	0.00%	0.00%	15,666	15,666	4,350	4,350
Button	0.00%	0.00%	79,421	79,421	24,279	24,279
Modal	0.00%	0.00%	15,050	15,050	5,233	5,233
Portal	0.00%	0.00%	3,579	3,579	1,568	1,568
Rating	0.00%	0.00%	70,735	70,735	22,079	22,079
Slider	0.00%	0.00%	75,066	75,066	23,271	23,271
colorManipulator	0.00%	0.00%	3,904	3,904	1,543	1,543
docs.landing	0.00%	0.00%	52,124	52,124	13,837	13,837
docs.main	0.00%	0.00%	590,550	590,550	188,602	188,608
packages/material-ui/build/umd/material-ui.production.min.js	0.00%	-0.00%	299,964	299,964	86,137	86,136

Generated by 🚫 dangerJS against 9db0d30

oliviertassinari · 2019-08-07T21:18:59Z

@jonkelling Hey, thank you for looking into it. I wish we discussed it primarily in an issue. I have explored another solution. I don't see much value in exposing the filled state. I think that you are trying to solve a validation issue. In this case, I would encourage one of these: 1. use the native browser feature or 2. use a react form library like formik.
What do you think?

jonkelling · 2019-08-07T22:09:30Z

@oliviertassinari I just took a look at your update. Sorry, it seems I'm doing everything in the wrong order! It appears like that will work, and that I may have jumped the gun. I'll try to go through the right channel next time.

jonkelling · 2019-08-07T22:23:24Z

I literally thought about using :valid and remember saying to myself, "naw.. that won't work, clearly I need to do it this way..." and was all proud of myself. Thanks again for the help!! 👍

jonkelling · 2019-08-07T22:42:22Z

@oliviertassinari It appears that required will need to be added to all fields, then, in order to accomplish this, correct? Since the original concern was a visual one, rather than a validation check, it's not really ideal, but it is a path. Not sure...

oliviertassinari · 2019-08-08T10:30:39Z

Since the original concern was a visual one, rather than a validation check, it's not really ideal, but it is a path. Not sure...

@jonkelling Following the new customization demo, I have assumed that the problem you were trying to solve was related to the validation of the field.

Could you expand on the use case of changing the design for a filled input, that is not related to validation?

Do you control the input? If you do, you could do something like this:

themes.overrides = {
  MuiInputBase: {
    root: props => ({
      color: props.value ? 'green' : null,
    }),
  },
},

jonkelling · 2019-08-08T22:51:53Z

@oliviertassinari I was under the impression that the props => ({...}) method did not work this way, but it appears to work when inside the theme overrides like this. This does make sense now that I think about it.

I originally tried that using withStyles and referencing it in the className of each element. In that case it's not actually getting the props for the element, so I disregarded that approach entirely.

It did not work with the method on root, however, but did when I changed it to this:

themes.overrides = {
  MuiInputBase: {
    root: {
      color: props => props.value ? 'green' : null,
    },
  },
},

I will play with it a bit more, but this certainly is the hook I was looking for in the first place! Since themes can be nested, I would imagine this provides all of the flexibility one would need. In that case it would make my PR totally unnecessary, which would be great. The only difference, I suppose, is that this method generates a lot of extra classes--right?--but I would guess that's not a performance concern at all.

I typically look inside implementations instead of relying totally on the docs, but I wonder if an example in docs would have clued me in earlier. I'll get back to you after I play with it a bit more. Thanks again!!!

jonkelling · 2019-08-09T01:05:37Z

@oliviertassinari It seems like styles generated using the props => () method is a bit inconsistent. e.g.:
works:
padding: props => props.value && 4
doesn't works:
padding: props => props.value && '4px !important'

Perhaps as a result of some sort of optimization, like consolidating style tags or something?

oliviertassinari · 2019-08-09T12:26:39Z

@jonkelling I have tried to without any luck. I was hoping that we could do:

const theme = createMuiTheme({
  overrides: {
    MuiOutlinedInput: {
      root: {
        "& fieldset": props => ({
          borderLeftWidth: props.value ? 4 : 1
        })
      }
    }
  }
});

But it doesn't work. https://codesandbox.io/s/material-demo-74qkt. This will most likely be solved in #6115. I would encourage you to create your own wrapping component. As for the validation change, I think that we can accept them.

jonkelling · 2019-08-09T14:09:47Z

I have not looked into styled-components before. Very interesting. That sounds like it could get pretty involved! I would assume it would be a larger effort to migrate and would be in a v5 release then? If that's the case, perhaps my original PR with a modified .Mui-valueFilledv4Hack could serve as a stopgap since v4 probably will not support this in any other way? 😉 👼

Anyway, I understand if not and will go forward with wrapping the components otherwise.

oliviertassinari · 2019-08-09T14:12:54Z

@jonkelling Here is the concern: I doubt that many people would leverage this new class name. This is particularly useful for SASS users. I would hope that for edge cases, the component wrapping approach is OK. And in the future, the feature should be built-in.

jonkelling · 2019-08-09T14:21:10Z

@oliviertassinari True true. I didn't mean to push, but I thought I'd give it a last shot. Overall, I've been impressed with the progress of this library since it was in beta. Thanks for all your work!

oliviertassinari · 2019-08-09T14:23:30Z

@jonkelling Thank you!

jonkelling added 5 commits August 5, 2019 15:54

Added valueFilled pseudo class to InputBase and InputLabel

1fcbedc

Updates for valueFilled class

01673d0

- InputBase now applies valueFilled when uncontrolled and without FormControlContext. - Added/Updated comments for use of valueFilled class.

Merge remote-tracking branch 'upstream/master'

2a9f30c

Updated FilledInput and docs

d702737

Formatted with prettier

a8e504c

oliviertassinari added out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future) and removed out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future) labels Aug 7, 2019

oliviertassinari changed the title ~~Add valueFilled pseudo-class to InputBase and InputLabel~~ [TextField] How to leverage CSS input validation Aug 7, 2019

oliviertassinari added component: text field This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation labels Aug 7, 2019

alternative proposal

9db0d30

oliviertassinari force-pushed the master branch from 7a9d22b to 9db0d30 Compare August 7, 2019 21:09

This comment has been minimized.

Sign in to view

oliviertassinari added the PR: good for merge label Aug 9, 2019

oliviertassinari merged commit 944a176 into mui:master Aug 9, 2019

snyk-bot mentioned this pull request Sep 4, 2019

[Snyk] Upgrade @material-ui/core from 4.3.1 to 4.3.3 iliutastoica/React-Discord-Clone#3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TextField] How to leverage CSS input validation #16903

[TextField] How to leverage CSS input validation #16903

jonkelling commented Aug 6, 2019 •

edited

Loading

mui-pr-bot commented Aug 6, 2019 •

edited

Loading

oliviertassinari commented Aug 7, 2019

This comment has been minimized.

jonkelling commented Aug 7, 2019

jonkelling commented Aug 7, 2019

jonkelling commented Aug 7, 2019

oliviertassinari commented Aug 8, 2019 •

edited

Loading

jonkelling commented Aug 8, 2019

jonkelling commented Aug 9, 2019 •

edited

Loading

oliviertassinari commented Aug 9, 2019 •

edited by joshwooding

Loading

jonkelling commented Aug 9, 2019 •

edited

Loading

oliviertassinari commented Aug 9, 2019 •

edited

Loading

jonkelling commented Aug 9, 2019

oliviertassinari commented Aug 9, 2019

[TextField] How to leverage CSS input validation #16903

[TextField] How to leverage CSS input validation #16903

Conversation

jonkelling commented Aug 6, 2019 • edited Loading

mui-pr-bot commented Aug 6, 2019 • edited Loading

oliviertassinari commented Aug 7, 2019

This comment has been minimized.

jonkelling commented Aug 7, 2019

jonkelling commented Aug 7, 2019

jonkelling commented Aug 7, 2019

oliviertassinari commented Aug 8, 2019 • edited Loading

jonkelling commented Aug 8, 2019

jonkelling commented Aug 9, 2019 • edited Loading

oliviertassinari commented Aug 9, 2019 • edited by joshwooding Loading

jonkelling commented Aug 9, 2019 • edited Loading

oliviertassinari commented Aug 9, 2019 • edited Loading

jonkelling commented Aug 9, 2019

oliviertassinari commented Aug 9, 2019

jonkelling commented Aug 6, 2019 •

edited

Loading

mui-pr-bot commented Aug 6, 2019 •

edited

Loading

oliviertassinari commented Aug 8, 2019 •

edited

Loading

jonkelling commented Aug 9, 2019 •

edited

Loading

oliviertassinari commented Aug 9, 2019 •

edited by joshwooding

Loading

jonkelling commented Aug 9, 2019 •

edited

Loading

oliviertassinari commented Aug 9, 2019 •

edited

Loading