Skip to content

doc: added documentation for input type=file #323

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

Merged

Conversation

vishalvrv9
Copy link
Contributor

Have added some explanation on how to implement the form input type=file in React as suggested in #86. Please review and revert 😄

Have used a ref to the file input and used it to access the file(s) in submit handler instead of saving the file in the state. (Was suggested as a better solution in #218)

@facebook-github-bot
Copy link
Collaborator

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link
Collaborator

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@reactjs-bot
Copy link

reactjs-bot commented Nov 21, 2017

Deploy preview ready!

Built with commit 82cf824

https://deploy-preview-323--reactjs.netlify.com

@vishalvrv9
Copy link
Contributor Author

@bvaughn
Was wondering if this PR works?

}
```

[Try it on CodePen.](https://codepen.io/fetard/pen/bYvaJY?editors=0010)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add this code to the examples directory and point to either Codepen or CodeSandbox using the gatsby-remark-code-repls plugin. 😄 This way it's easier going forward for others to help maintain the demo.

Copy link
Contributor Author

@vishalvrv9 vishalvrv9 Dec 20, 2017

Choose a reason for hiding this comment

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

Have been travelling. Will do that within 48 hours. 😅

@vishalvrv9
Copy link
Contributor Author

Have committed the changes in this branch as suggested in the review @bvaughn

@bvaughn bvaughn mentioned this pull request Dec 21, 2017
Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

Added some thoughts.

@@ -0,0 +1,26 @@
class FileInput extends React.Component {
Copy link
Contributor

Choose a reason for hiding this comment

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

This file should probably be in a different directory. 😄 examples/component-and-props is meant for things that go along with the content/docs/component-and-props.md file. Maybe we should make a new examples/forms folder? This way we can migrate the other snippets out of the markdown content as well?

);
}
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks nice! If you merge master into your PR, we can make this even better using another plug-in I wrote 😄 gatsby-remark-embed-snippet. This way you don't need to duplicate code between the markdown file and the CodePen link. Instead, you can just embed it directly:

embed:components-and-props/input-type-file.js

(That being said, I think we should change where input-type-file.js is located, as I mention below)

this.handleSubmit = this.handleSubmit.bind(this);
}

handleSubmit (event) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming you are interested in going the embed route I mentioned above, you can add line-highlighting into the file by adding a comment above handleSubmit like so:

// highlight-range{1-4}

<form onSubmit={this.handleSubmit}>
<label>
Upload file:
<input type='file' ref={input => {this.fileInput = input}} />
Copy link
Contributor

Choose a reason for hiding this comment

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

And another above <input... /> like so:

{/* highlight-next-line */}

@vishalvrv9
Copy link
Contributor Author

@bvaughn Can't exactly figure out what went wrong. Was wondering if you could help resolve? 😥

@bvaughn
Copy link
Contributor

bvaughn commented Dec 22, 2017

Yup! Looks like CI is failing on the yarn prettier command. Run it locally and you'll see that the example gets slightly reformatted.

Since this reformats the example slightly, you'll want to verify the line number highlights. To make sure Gatsby regenerates the markdown for this example, the safest thing to do is clear cache before spinning up the devserver: rm -rf .cache && yarn dev. (This is a downside of using this plug-in and maybe something I can address with a future update...)

Anyway, the end result after doing both of the following should be something like this:

diff --git a/examples/forms/input-type-file.js b/examples/forms/input-type-file.js
index 5cc084d9f..5eff7c5b8 100644
--- a/examples/forms/input-type-file.js
+++ b/examples/forms/input-type-file.js
@@ -1,27 +1,45 @@
 class FileInput extends React.Component {
   constructor(props) {
     super(props);
-    this.handleSubmit = this.handleSubmit.bind(this);
+    this.handleSubmit = this.handleSubmit.bind(
+      this
+    );
   }
-// highlight-range{1-4}
-  handleSubmit (event) {
+
+  // highlight-range{5}
+  handleSubmit(event) {
     event.preventDefault();
-    alert(`Selected file - ${this.fileInput.files[0].name}`);
+    alert(
+      `Selected file - ${
+        this.fileInput.files[0].name
+      }`
+    );
   }
 
   render() {
     return (
-      <form onSubmit={this.handleSubmit}>
+      <form
+        onSubmit={this.handleSubmit}>
         <label>
           Upload file:
-          {/* highlight-next-line */}
-          <input type='file' ref={input => {this.fileInput = input}} />
+          {/* highlight-range{1-6} */}
+          <input
+            type="file"
+            ref={input => {
+              this.fileInput = input;
+            }}
+          />
         </label>
-        <br/>
-        <button type='submit'>Submit</button>
+        <br />
+        <button type="submit">
+          Submit
+        </button>
       </form>
     );
   }
 }
 
-ReactDOM.render(<FileInput />, document.getElementById('root'));
+ReactDOM.render(
+  <FileInput />,
+  document.getElementById('root')
+);

@vishalvrv9
Copy link
Contributor Author

@bvaughn That fixed it. Thanks. 😄 (Apologies for not being able to figure out the CI problem myself)

vishalvrv9 and others added 2 commits December 24, 2017 22:11
Replaced "a <input>" with "an <input>" in a couple of places.

Simplified wording slightly to (hopefully) be easier for non-native speakers to read.
Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

This looks good to me. I tweaked some wording a little. (Hope that's okay.) Thanks for the contribution!

@bvaughn bvaughn merged commit e177ed1 into reactjs:master Jan 2, 2018
@vishalvrv9 vishalvrv9 deleted the vishalvrv9-docs-inputfile-improvement branch January 2, 2018 18:45
@vishalvrv9
Copy link
Contributor Author

Immensely thankful @bvaughn for your mentorship towards my first contribution. ✌️

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.

4 participants