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.

Sign up for GitHub

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

Jump to bottom

[docs] Improve FormControl duplication warning #22823

Merged
merged 3 commits into from Oct 5, 2020
Merged

[docs] Improve FormControl duplication warning #22823

merged 3 commits into from Oct 5, 2020

Conversation

talgautb
Copy link
Contributor

@talgautb talgautb commented Oct 1, 2020 •
edited by oliviertassinari

Closes #22535

@mui-pr-bot
Copy link

mui-pr-bot commented Oct 1, 2020 •
edited

Details of bundle changes

Generated by 🚫 dangerJS against e166f92

@github-actions github-actions bot mentioned this pull request Oct 1, 2020
Closed
eps1lon
eps1lon previously requested changes Oct 1, 2020
View reviewed changes
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Could you fix CI by committing the changes introduced by yarn docs:api and update the tests failing when running yarn test:unit?

@eps1lon eps1lon added docs Improvements or additions to the documentation package: material-ui Specific to @mui/material labels Oct 1, 2020
@talgautb
Copy link
Contributor Author

talgautb commented Oct 1, 2020

@eps1lon update PR, all test passed

@oliviertassinari oliviertassinari changed the title [docs] update warning in FormControl [docs] Improve FormControl duplication warning Oct 1, 2020
eps1lon
eps1lon reviewed Oct 1, 2020
View reviewed changes
docs/pages/api-docs/form-control.md
@@ -38,7 +38,7 @@ You can find one composition example below and more going to [the demos](/compon
</FormControl>
```

⚠️Only one input can be used within a FormControl.
⚠️ Only one `InputBase` can be used within a FormControl because it create visual inconsistencies.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a concrete example for this? Ideally we could list all so that people searching by problem find the cause.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #22535 (comment), we have:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we describe this pattern in words?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do i need to add this to docs?

Copy link
Member

@eps1lon eps1lon Oct 2, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd be nice. "visual inconsistencies" is not helpful. Either we describe examples or we remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i get it from comment, I think to delete this will be good decision if we have no other variants

@oliviertassinari oliviertassinari dismissed eps1lon’s stale review October 4, 2020 18:36

Improved description proposed

@eps1lon eps1lon merged commit c01ce01 into mui:next Oct 5, 2020
@eps1lon
Copy link
Member

eps1lon commented Oct 5, 2020

@talgautb It's a great first pull request on Material-UI 👌🏻. Thank you for working on it!

@jonathan-stone
Copy link

Thanks for improving the docs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eps1lon eps1lon eps1lon left review comments

@oliviertassinari oliviertassinari oliviertassinari approved these changes

Assignees
No one assigned
Labels
docs Improvements or additions to the documentation package: material-ui Specific to @mui/material
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[TextField] Docs: "only single input." Checkbox: 3 checkboxes in a FormControl
5 participants
@talgautb @mui-pr-bot @eps1lon @jonathan-stone @oliviertassinari