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

Components: add FilePicker component #3971

Merged
merged 5 commits into from
Mar 23, 2016
Merged

Conversation

TooTallNate
Copy link
Contributor

Opens a native browser file picker dialog when the user clicks the contents of the component. Usage looks something like:

 <FilePicker multiple accept="image/*" onPick={ console.log.bind(console) } >
   <a href="#">Select a few images!</a>
 </FilePicker>

Will be used for Reader's OPML "Import" button, but is generic enough to turn into a reusable component.

This is my first React component!! In general I'm looking for feedback regarding implementation, and if this is a good use of a reusable component at all. I was skeptical at first since it's inherently a "functional component" (that is, has no UI built-in) but the more I thought about it, the more I thought it made sense with respect to a classic <input type="file"> element. But perhaps there's a better React mechanism then I'm doing here that I don't yet know of.

Thanks in advance!

/cc @blowery @jancavan

screen shot 2016-03-10 at 4 49 36 pm

@@ -28,6 +28,7 @@
"commander": "2.3.0",
"component-classes": "1.2.1",
"component-closest": "0.1.4",
"component-file-picker": "0.2.1",
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 do something with npm-shrinkwrap.json about this?

Copy link
Contributor

Choose a reason for hiding this comment

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

yup, install clingwrap and clingwrap component-file-picker.

might need to be using npm@2.14.12 when you do it too? that bit is still unclear to me. @gwwar might be able to shed some light on it.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/Automattic/wp-calypso/blob/master/docs/shrinkwrap.md#modifying-dependencies

And we're currently using node 4.3.0 with npm 2.14.12

Feel free to ping me, if the instructions don't make sense

@TooTallNate TooTallNate changed the title components: add FilePicker component Components: add FilePicker component Mar 11, 2016
@TooTallNate TooTallNate added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Feature] Reader The reader site on Calypso. Components labels Mar 11, 2016
}

onSingle( files ) {
alert( 'Selected file: ' + JSON.stringify( files[0].name ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe switch these to use debug ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You think so? For a doc example I thought it would be good to see the "results" of the user action kind of in-your-face. Gridicons uses prompt() for example, which is similar.

We should have some standard way of displaying results in the docs (a built-in log-dump thing would be cool).

Opens a native browser file picker dialog when the user clicks
the contents of the component. Usage looks something like:

     <FilePicker multiple accept="image/*" onPick={ console.log.bind(console) } >
       <a href="#">Select a few images!</a>
     </FilePicker>

Will be used for Reader's OPML "Import" button, but
is generic enough to turn into a reusable component.
No need for an extracted `renderPickers()` function that
only gets called once, that's an anti-pattern.
@blowery
Copy link
Contributor

blowery commented Mar 22, 2016

looks good! 🚢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Components [Feature] Reader The reader site on Calypso.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants