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.

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

[docs] Forms Documentation Improvement #8299

Closed
wants to merge 6 commits into from

Conversation

dashtinejad
Copy link
Contributor

I feel that the forms documentation is missing some important and fundamentals inputs:

  • multi select
  • checkbox
  • radio buttons

So, I've added them to the documentation to clarify how to convert those form inputs to React Controlled Components.

@@ -174,6 +174,132 @@ class FlavorForm extends React.Component {

Overall, this makes it so that `<input type="text">`, `<textarea>`, and `<select>` all work very similarly - they all accept a `value` attribute that you can use to implement a controlled component.

## Multi select
Copy link
Contributor

Choose a reason for hiding this comment

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

"multiselect" isn't really its own thing - it's just a feature of select. How about making this example inline with the previous section, and just saying something like:

"You can also do multiple selection, by setting the 'multiple' parameter and using an array as the value."

...followed by sample code that just changes the example to a multiselect example

<input type="checkbox" />
```

You can change this checkbox to controlled component by setting the `checked` property,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to combine the JSX and javascript code in this example. Look at how textarea works, the format is just:

--
In HTML a textarea is like this:

(example)

In React, you change X and Y, so a full example is this:

(full example)

Note some brief stuff stuff.

How about using that parallel format for checkbox and radio buttons?

@lacker
Copy link
Contributor

lacker commented Nov 29, 2016

Originally I was thinking, meh we don't need these examples, but by now a number of folks have suggested re-adding them so I officially change my mind. So it would be cool to add these examples, I just suggest using a format equivalent to the way "textarea" works where the example is a full one rather than describing it bit by bit - I added comments to that effect inline

@dashtinejad
Copy link
Contributor Author

@lacker Good points, I'll work on it.

@gaearon
Copy link
Collaborator

gaearon commented Jan 4, 2017

@keyanzhang Would you like to help reviewing this? Since you worked on a related section recently in another PR.

@lacker
Copy link
Contributor

lacker commented Feb 2, 2017

AFAICT this is still in progress rather than needing review so I tagged it back. Let me know if that is not the case though

@dashtinejad
Copy link
Contributor Author

@lacker As you suggested, I've added 2 new sections (for checkbox and radio) using a format equivalent to the way "textarea" where the example is a full one. Also I've added a note about multi attribute for select tag. Please let me know if I missed something.

Copy link
Collaborator

@sebmarkbage sebmarkbage left a comment

Choose a reason for hiding this comment

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

Changes requested above.

@dashtinejad
Copy link
Contributor Author

@lacker @gaearon @sebmarkbage I really do believe that handling checkbox and radio elements should be on official documentation. Please let me know If I should improve my suggested documentation.

@bvaughn
Copy link
Contributor

bvaughn commented Oct 6, 2017

Thank you for submitting this PR! 😄

Unfortunately the documentation and source code for reactjs.org has been moved to a different repository: https://github.com/reactjs/reactjs.org

I realize this PR is old and there may no longer be interest in seeing it merged. If there is, please open a new PR there. Sorry for the inconvenience!

(For more backstory on why we moved to a new repo, see issue #11075)

@bvaughn bvaughn closed this Oct 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants