[docs] Move "TextField" section higher in the "Selects" page by croraf · Pull Request #17643 · mui/material-ui

croraf · 2019-09-30T20:46:49Z

Motivation: Using MUI a lot, but didn't even know TextField can be used for Selects. Always used the set of components (InputLabel, Select, helpercomponent). The small section in the bottom is not noticeable.

I moved to top along with "Simple Select" examples
reworded it a bit
added a comparison example (JS only)

mui-pr-bot · 2019-09-30T20:56:19Z

No bundle size changes comparing 05c533b...7be2f56

Generated by 🚫 dangerJS against 7be2f56

oliviertassinari · 2019-10-01T14:46:09Z

What do you think of positioning the demo after the customization one? I strongly think that native should be before, promoting the customization before might be interesting too, we tend to keep these customization demos high in the order, they are frequently used and strategical for us.

oliviertassinari · 2019-10-01T14:46:30Z

+  },
+}));
+
+export default function ControlledOpenSelect() {


Component name doesn't match the file name.

oliviertassinari · 2019-10-01T14:46:50Z

+        </Select>
+        <FormHelperText>Select implementation</FormHelperText>
+      </FormControl>
+


I ran prettier and lint and got no errors. This blank line improves readability, although can be removed.

oliviertassinari · 2019-10-01T14:47:05Z

+        id="age-text-field"
+        label="Age"
+        helperText="TextField implementation"
+        SelectProps={


I would propose the removal of all the blank props from the example. A mention of SelectProps in the demo description could help.

I think they are very nice and useful to have in the demo.

But prettier made it look like a mess. They were one per line as in the original commit and it looked so pretty :). Perhaps we can improve prettier as in codesandbox, where it contracts into single line where possible.

croraf · 2019-10-01T15:17:48Z

What do you think of positioning the demo after the customization one? I strongly think that native should be before, promoting the customization before might be interesting too, we tend to keep these customization demos high in the order, they are frequently used and strategical for us.

I would keep the TextField stuff in the "Simple Select" chapter. If "Simple Select" is of lower priority than "Native" or "Custom" then move it bottom, but keep TextField in it. To me personally "TextField" api is the nicest, and I will use it the most. Native select element behaves strange in some cases.

oliviertassinari · 2019-10-01T18:45:27Z

I believe we document the select mode in the dedicated text-field documentation page. Maybe we should avoid the duplication and only link a demo from select to text-field? cc @joshwooding

joshwooding · 2019-10-01T20:14:35Z

@oliviertassinari I agree - it doesn't seem like we need two demos for the same thing. We should probably keep them in the TextField section and make them more prominent like we are doing with the other demos under #17483

croraf · 2019-10-02T08:38:27Z

It is a bit messy that select is part of the TextField at all (especially because it says all over the TextField that it is for accepting user text input), and I think @joshwooding agrees from what I read in comment #17483 (comment).

I think the confusion comes from the material design specs, because in "Components" they don't have Select but only TextField https://material.io/components/text-fields which includes some select showcases.

But in develop they do have the Select component: https://material.io/develop/web/components/input-controls/select-menus/, https://material-components.github.io/material-components-web-catalog/#/component/select.

oliviertassinari · 2019-10-13T16:34:14Z

Given the direction that #17483 is taking, I think that we will be able to update this pull request to something like this:

diff --git a/docs/src/pages/components/selects/selects.md b/docs/src/pages/components/selects/selects.md
index 59dc2b8ab..f7a636001 100644
--- a/docs/src/pages/components/selects/selects.md
+++ b/docs/src/pages/components/selects/selects.md
@@ -30,6 +30,10 @@ Once it's styled, you can either use it directly as a text field or provide it t

 {{"demo": "pages/components/selects/CustomizedSelects.js"}}

+## Text Fields
+
+The `TextField` wrapper component is a complete form control including a label, input and help text. You can find an example with the select mode [in this section](/components/text-fields/#select).
+
 ## Multiple Select

 The `Select` component can handle multiple selections.
@@ -48,7 +52,3 @@ Like with the single selection, you can pull out the new value by accessing `eve
 While it's discouraged by the Material Design specification, you can use a select inside a dialog.

 {{"demo": "pages/components/selects/DialogSelect.js"}}
-
-## Text Fields
-
-The `TextField` wrapper component is a complete form control including a label, input and help text. You can find an example with the select mode [in this section](/components/text-fields/#textfield).

Would it work for you guys?

oliviertassinari · 2019-10-13T16:36:16Z

@croraf I also have mixed feelings about the TextField importing the Select. It could have been better to have a SelectField component that bundles the other field components, à la TextField for the input type text.

croraf · 2019-10-13T17:32:38Z

@oliviertassinari If you are talking about removing <TextFied select/> and use new component SelectField instead, this was suggested in the original Issue #8270. This is a breaking change. Unfortunate is also that material-design TextField includes a select https://material.io/components/text-fields/#anatomy section Icons. Although they also have this section https://material.io/develop/web/components/input-controls/select-menus/

oliviertassinari · 2019-10-13T17:56:47Z

Yes, it would be a breaking change. If the idea get tractions, it could make sense to explore it in the future, during the preparation of v5.

oliviertassinari · 2019-10-29T09:43:18Z

@croraf I have merged #17483, we could move forward with the changes. The key design decisions are 1. the text field support needs to be more visible; 2. we can link the text field demo, the fewer demos we have to maintain, the better. Thank you for raising our attention to this problem!

joshwooding mentioned this pull request Sep 30, 2019

[docs] Break up TextField demos #17483

Merged

1 task

croraf changed the title ~~[docs] Move TextField section in Selects and add comparison example~~ [docs] Move "TextField" section within "Selects" page and add comparison example Oct 1, 2019

oliviertassinari reviewed Oct 1, 2019

View reviewed changes

oliviertassinari added the docs Improvements or additions to the documentation. label Oct 1, 2019

oliviertassinari requested a review from joshwooding October 1, 2019 14:48

croraf added 2 commits October 29, 2019 10:34

Update selects docs

5ed2dcc

prettier

5f0d59a

oliviertassinari force-pushed the docs-selects branch from 7be2f56 to d2a249e Compare October 29, 2019 09:40

oliviertassinari changed the title ~~[docs] Move "TextField" section within "Selects" page and add comparison example~~ [docs] Move "TextField" section higher in the "Selects" page Oct 29, 2019

move the text fields section higher in the page

6693f67

oliviertassinari force-pushed the docs-selects branch from d2a249e to 6693f67 Compare October 29, 2019 09:41

oliviertassinari merged commit 7d5d913 into mui:master Oct 29, 2019

snyk-bot mentioned this pull request Jan 7, 2020

[Snyk] Upgrade @material-ui/core from 4.4.0 to 4.8.2 srdjan-19/rent-a-car-service#3

Open

Uh oh!

Uh oh!

Conversation

croraf commented Sep 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mui-pr-bot commented Sep 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oliviertassinari commented Oct 1, 2019

Uh oh!

oliviertassinari Oct 1, 2019

Choose a reason for hiding this comment

Uh oh!

oliviertassinari Oct 1, 2019

Choose a reason for hiding this comment

Uh oh!

croraf Oct 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oliviertassinari Oct 1, 2019

Choose a reason for hiding this comment

Uh oh!

croraf Oct 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

croraf commented Oct 1, 2019

Uh oh!

oliviertassinari commented Oct 1, 2019

Uh oh!

joshwooding commented Oct 1, 2019

Uh oh!

croraf commented Oct 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oliviertassinari commented Oct 13, 2019

Uh oh!

oliviertassinari commented Oct 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

croraf commented Oct 13, 2019

Uh oh!

oliviertassinari commented Oct 13, 2019

Uh oh!

oliviertassinari commented Oct 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

croraf commented Sep 30, 2019 •

edited

Loading

mui-pr-bot commented Sep 30, 2019 •

edited

Loading

croraf Oct 1, 2019 •

edited

Loading

croraf Oct 1, 2019 •

edited

Loading

croraf commented Oct 2, 2019 •

edited

Loading

oliviertassinari commented Oct 13, 2019 •

edited

Loading

oliviertassinari commented Oct 29, 2019 •

edited

Loading