feat: File Input component #900
Conversation
…-input-component-341
| const imageId = makeSafeForID(files[i].name) | ||
| const key = `filePreview_${imageId}` | ||
| filePreviews.push( | ||
| <FilePreview key={key} imageId={imageId} file={files[i]} /> |
There was a problem hiding this comment.
I think we should surface an optional prop for FilePreview (defaulting to this component) so people could build their own.
There was a problem hiding this comment.
this feels like an enhancement that goes beyond the USWDS implementation, mind adding as a new feature request?
| className={imageClasses} | ||
| onError={onImageError} | ||
| /> | ||
| {name} |
There was a problem hiding this comment.
Got into this weird state when I had a file (screenshot) which had a long name.
Heres USWDS version
There was a problem hiding this comment.
Ah, I believe this is a result of needing to update to USWDS 2.8.1 (this update included some changes to the File Input CSS classes). I get the same result if I was on another branch with 2.8.0, and view the FileInput without running yarn install
There was a problem hiding this comment.
ha! looks like github deployments is just on time for me over here 😄
| const expectedSrc = 'data:text/plain;base64,VGVzdCBGaWxlIENvbnRlbnRz' | ||
|
|
||
| const imageEl = getByTestId('file-input-preview-image') | ||
| await waitFor(() => expect(imageEl).not.toHaveClass('is-loading')) |
There was a problem hiding this comment.
nice. I haven't used waitFor before TIL.
suzubara
requested review from
ahobson,
brandonlenz,
gidjin,
sarboc,
SirenaBorracha and
tinyels
as code owners
|
@haworku I believe for your use case, we'll want to make it so you can programmatically clear/reset the value of the FileInput, which I think requires the same This might be a good component to try this out first on, since it will be tested in a real world app right away, and then we'll have a better idea of how to roll out this change to other JS input components as needed. How does that sound? |
There was a problem hiding this comment.
Just approved the Happo diffs. There were a lot of very subtle changes to form labels (whitespace and font size), beyond what is mentioned specifically in the 2.8.1 patch notes. I suspect these changes were simply not mentioned, but its perfectly reasonable to presume that's where the diffs came from.
@suzubara Yep. Sound good. This is a good idea to use to experiment with the pattern here. agree we probably need to clear input pretty soon after each drop while the async upload happens and is displayed through our other list component. |
| name: string | ||
| disabled?: boolean | ||
| multiple?: boolean | ||
| accept?: string |
There was a problem hiding this comment.
It would be nice for the type of accept? to be string | string[] to make it more obvious how to pass multiple options and avoid needing to split on a string inside the component. I can imagine these use cases:
<FileInput accepts="pdf" ... />
<FileInput accepts={["pdf", "docx", "png"]} ... />The second one needs a little more syntax than the current accept="pdf,docx,png", but I think having the type system help a developer provide the options correctly outweighs that.
There was a problem hiding this comment.
hmm yeah, I think the default string syntax is due to this attribute being passed to the <input /> (and that's the syntax for the native accept attr) but I could see allowing an array of strings being a nice enhancement. In that case we'd also need to join them into a single string to pass into the <input /> so there's some extra handling either way.
There was a problem hiding this comment.
I didn't realize that input had an accept attribute natively. In that case it's probably fine as-is, at least in terms of being consistent with how we handle other things in the library. Thanks for the thoughtful response!
Summary
Implements the USWDS File Input component.
There are some tests that aren't 100% accurate, because simulating a drop event doesn't seem to accurately trigger the onChange handler for a file input. I left outstanding comments in those cases and maybe they can be improved upon in the future.
Related Issues or PRs
Closes #341
How To Test
Note: The FileInput requires updating to USWDS 2.8.1, which is included in this PR. Please make sure you run
yarn installbefore testing after you check out this branch.Test that you can use the input in a multipart form to upload file(s), and the file type validation works as expected.
If you want to handle uploading async on the change event, there is an
onChangeprop exposed you can use to pass in a custom handler. Note that the FileInput component will still display dropped files, and you may want to force a re-render to clear the value after uploading.Screenshots (optional)