[docs] Simplify state management in Text Field demo page #35051

PratikDev · 2022-11-08T21:42:52Z

Text Field demo page had several components that weren't managing the state perfectly. some had synced values and some didn't. Since these demos' main purpose isn't to show the usage of controlled vs. uncontrolled modes, I converted all of them to uncontrolled to reduce confusion

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

managing state of MultilineTextFields component on multiline-flexible. so that every input field has a different state Signed-off-by: PratikDev <87067570+PratikDev@users.noreply.github.com>

managing state of SelectTextFields component. so that every input field has a different state Signed-off-by: PratikDev <87067570+PratikDev@users.noreply.github.com>

managing state of ComposedTextField component. so that every input field has a different state Signed-off-by: PratikDev <87067570+PratikDev@users.noreply.github.com>

Signed-off-by: PratikDev <87067570+PratikDev@users.noreply.github.com>

Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

added state so all fields have synced values. used FormControl here, so that all fields have placeholder's floating animation when one is changed Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

added state so that all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

added state so all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

used the unused state "weightRange" as With Normal TextField's state. Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

added state so that all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

mui-bot · 2022-11-08T21:45:25Z

	Messages
📖	Netlify deploy preview: https://deploy-preview-35051--material-ui.netlify.app/

No bundle size changes

Generated by 🚫 dangerJS against f713eeb

applied eslint and prettier Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

michaldudak · 2022-11-14T13:21:16Z

This PR makes all the components controlled. We've discussed making them uncontrolled instead to simplify the code and make the demos more focused on what their primary purpose is.

PratikDev · 2022-11-14T17:22:30Z

oh boyy. i guess i got some misconception. did you mean stateless by "uncontrolled"?

michaldudak · 2022-11-15T18:47:34Z

https://reactjs.org/docs/uncontrolled-components.html

Basically, components without the value prop set (so they are in charge of controlling their own state changes).

PratikDev · 2022-11-15T18:56:24Z

understood. as this PR has gone so blurry it could be confusing for anyone. so should I just delete this and create a new one with a more perfect description? or just commit the edits in it?

PratikDev · 2022-12-01T14:14:46Z

hey @michaldudak, haven't received any feedback about this PR for a long time

michaldudak

Sorry for the delay. With so many PRs and issues, it's hard to prioritize.
I left two comments, but it looks OK in general.

michaldudak · 2022-12-06T12:04:26Z

docs/data/material/components/text-fields/InputAdornments.js

@@ -50,8 +35,7 @@ export default function InputAdornments() {
        <FormControl sx={{ m: 1, width: '25ch' }} variant="outlined">
          <OutlinedInput
            id="outlined-adornment-weight"
-            value={values.weight}
-            onChange={handleChange('weight')}
+            defaultValue=""


Empty defaultValue is not necessary.

michaldudak · 2022-12-06T12:21:43Z

docs/data/material/components/text-fields/MultilineTextFields.tsx

@@ -24,8 +18,7 @@ export default function MultilineTextFields() {
          label="Multiline"
          multiline
          maxRows={4}
-          value={value}
-          onChange={handleChange}
+          defaultValue="Controlled"


It's not controlled anymore. You can remove the defaultValue at all IMO.

so should remove the defaultValue from all the input fields?

In cases where it's "Controlled" now, yes.

are we again having some confusion about"controlled" and "uncontrolled" by any chance? because I think I made all of the fields "uncontrolled" as per our discussion

Yes, that's correct; all the fields are uncontrolled. That's why having a defaultValue="Controlled" is confusing and I'm asking you to remove the defaultValue prop where its value is "Controlled" (literally).

Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

PratikDev · 2022-12-08T19:01:30Z

sorry for all the confusion we had. but seems we're now on the same page. please check out my recent commit. I've removed all the defaultValue="Controlled" and defaultValue=""

michaldudak · 2022-12-08T20:15:19Z

docs/data/material/components/text-fields/InputAdornments.js

-      showPassword: !values.showPassword,
-    });
-  };
+  const handleClickShowPassword = () => setShowPassword(!showPassword);


Changing state based on a previous value should use the callback form:

Suggested change

const handleClickShowPassword = () => setShowPassword(!showPassword);

const handleClickShowPassword = () => setShowPassword((show) => !show);

michaldudak · 2022-12-08T20:16:13Z

docs/data/material/components/text-fields/InputAdornments.tsx

@@ -59,8 +35,7 @@ export default function InputAdornments() {
        <FormControl sx={{ m: 1, width: '25ch' }} variant="outlined">
          <OutlinedInput
            id="outlined-adornment-weight"
-            value={values.weight}
-            onChange={handleChange('weight')}
+            defaultValue=""


You can remove empty defaultValue props here as well

michaldudak · 2022-12-08T20:16:45Z

docs/data/material/components/text-fields/MultilineTextFields.js

@@ -24,8 +18,7 @@ export default function MultilineTextFields() {
          label="Multiline"
          multiline
          maxRows={4}
-          value={value}
-          onChange={handleChange}
+          defaultValue="Controlled"


Remove the defaultValue props

Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

PratikDev · 2022-12-09T07:07:26Z

Hi, I think I removed all the defaultValue="" and defaultValue="Controlled". and also changed the state updating in a callback form. There are no more defaultValue="" and defaultValue="Controlled" in any text fields as far I checked. but if there's anything more to change, please let me know. I'll commit them accordingly

michaldudak

Looks good to me now. Thanks for your work!

PratikDev · 2022-12-09T10:32:05Z

the PR still shows Open. is there anything more to do from my end? or I'm such a noob in open source who doesn't even know how things work here?

michaldudak · 2022-12-09T13:55:26Z

No, no need to do anything :) It should be included in the next release. We leave PRs open for some time after approval in case other MUI team members would like to comment.

PratikDev · 2022-12-09T14:16:15Z

oh okay. got it. thanks

…

On Fri, Dec 9, 2022 at 7:55 PM Michał Dudak ***@***.***> wrote: No, no need to do anything :) It should be included in the next release. We leave PRs open for some time after approval in case other MUI team members would like to comment. — Reply to this email directly, view it on GitHub <#35051 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUYIXMQLMV6E43CT5HTPLGTWMM25RANCNFSM6AAAAAAR2Y3GDE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

PratikDev added 25 commits November 8, 2022 00:34

Update SelectTextFields.tsx

ab11839

managing state of SelectTextFields component. so that every input field has a different state Signed-off-by: PratikDev <87067570+PratikDev@users.noreply.github.com>

Update ComposedTextField.tsx

f4f28a1

managing state of ComposedTextField component. so that every input field has a different state Signed-off-by: PratikDev <87067570+PratikDev@users.noreply.github.com>

Update MultilineTextFields.tsx

3496445

Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update SelectTextFields.tsx

741162e

Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update ComposedTextField.tsx

8e864fc

Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update BasicTextFields.js

cf67a31

added state so all fields have synced values. used FormControl here, so that all fields have placeholder's floating animation when one is changed Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update FormPropsTextFields.js

b698ef0

added state so that all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update BasicTextFields.js

2fdd002

added state so all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update ValidationTextFields.tsx

28ed0e7

added state so all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update MultilineTextFields.js

3c18aba

added state so all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update InputWithIcon.js

7126221

added state so all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update InputAdornments.js

fb5fc98

used the unused state "weightRange" as With Normal TextField's state. Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update TextFieldSizes.js

ff10d56

added state so that all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update LayoutTextFields.js

c1c11bd

added state so that all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update FormPropsTextFields.js

1aebf44

added state so that all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update ColorTextFields.js

31f8579

added state so that all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update HelperTextMisaligned.js

f09327e

added state so that all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update HelperTextAligned.js

2d0c899

added state so that all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update ValidationTextFields.js

f69f337

added state so that all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update MultilineTextFields.js

cb43ed3

added state so that all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update InputWithIcon.js

240b33d

added state so that all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update TextFieldSizes.js

02656f4

added state so that all fields have synced values Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

zannager added docs component: text field labels Nov 9, 2022

PratikDev added 2 commits November 9, 2022 19:55

Update BasicTextFields.js

24f09cd

applied eslint and prettier Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Update ColorTextFields.js

7a61629

applied eslint and prettier Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

fixed argos issue

007238f

PratikDev added 2 commits November 17, 2022 00:07

Made all TextFields uncontrolled

76e6361

Made all TextFields uncontrolled

b97422c

Merge branch 'mui:master' into master

34c0b32

michaldudak reviewed Dec 6, 2022

View reviewed changes

michaldudak changed the title ~~[docs] fix state management in Text Field demo page~~ [docs] Simplify state management in Text Field demo page Dec 6, 2022

PratikDev added 6 commits December 7, 2022 15:52

Merge branch 'mui:master' into master

d6d71e2

Merge branch 'mui:master' into master

a86cf3f

Merge branch 'mui:master' into master

2f652d7

Merge branch 'mui:master' into master

8e8a05d

removed defaultValue="Controlled"

1a59337

Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

removed defaultValue=""

21c7626

Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

michaldudak reviewed Dec 8, 2022

View reviewed changes

PratikDev added 3 commits December 9, 2022 12:56

removed defaultValue="Controlled"

4138ecb

Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

Removed defaultValue="" & state change in callback

a808d33

Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

state changing in callback

f713eeb

Signed-off-by: Pratik Dev <87067570+PratikDev@users.noreply.github.com>

michaldudak approved these changes Dec 9, 2022

View reviewed changes

michaldudak merged commit 846bd9b into mui:master Dec 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[docs] Simplify state management in Text Field demo page #35051

[docs] Simplify state management in Text Field demo page #35051

PratikDev commented Nov 8, 2022 •

edited

Loading

mui-bot commented Nov 8, 2022 •

edited

Loading

michaldudak commented Nov 14, 2022

PratikDev commented Nov 14, 2022

michaldudak commented Nov 15, 2022

PratikDev commented Nov 15, 2022

PratikDev commented Dec 1, 2022

michaldudak left a comment

michaldudak Dec 6, 2022

michaldudak Dec 6, 2022

PratikDev Dec 7, 2022

michaldudak Dec 7, 2022

PratikDev Dec 8, 2022

michaldudak Dec 8, 2022

PratikDev commented Dec 8, 2022

michaldudak Dec 8, 2022

michaldudak Dec 8, 2022

michaldudak Dec 8, 2022

PratikDev commented Dec 9, 2022

michaldudak left a comment

PratikDev commented Dec 9, 2022

michaldudak commented Dec 9, 2022

PratikDev commented Dec 9, 2022 via email

	const handleClickShowPassword = () => setShowPassword(!showPassword);
	const handleClickShowPassword = () => setShowPassword((show) => !show);

[docs] Simplify state management in Text Field demo page #35051

[docs] Simplify state management in Text Field demo page #35051

Conversation

PratikDev commented Nov 8, 2022 • edited Loading

mui-bot commented Nov 8, 2022 • edited Loading

michaldudak commented Nov 14, 2022

PratikDev commented Nov 14, 2022

michaldudak commented Nov 15, 2022

PratikDev commented Nov 15, 2022

PratikDev commented Dec 1, 2022

michaldudak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PratikDev commented Dec 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PratikDev commented Dec 9, 2022

michaldudak left a comment

Choose a reason for hiding this comment

PratikDev commented Dec 9, 2022

michaldudak commented Dec 9, 2022

PratikDev commented Dec 9, 2022 via email

PratikDev commented Nov 8, 2022 •

edited

Loading

mui-bot commented Nov 8, 2022 •

edited

Loading