-
Notifications
You must be signed in to change notification settings - Fork 81
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
feat!: Expose FileInput component ref with a clearFiles method #1165
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a look - lets screenshare if it seems useful
disabled={disabled} | ||
onChange={handleChange} | ||
multiple={multiple} | ||
accept={accept} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting a type error here for enterKeyHint
being a missing prop as soon I added the new FileInput to the application.
Adding that prop with the value of enter
(default) resolved the issue. It sounded similar to this issue. 🤔 Not sure what to make of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops this was an issue in our code. SMH excuse me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works in mc-review. ⭐ The key thing missing in our code was changing everything to use ref.current?.input?.files
instead of ref.current.files
.
…-input-forward-ref-958
I updated the title to include a "!" for the breaking change so that the squash will automatically preserve that. Feel free also when merging to add a helpful blurb about how to incorporate this change in the body of the squash commit message. |
@haworku Maybe a question more for my understanding, but the FileInputRef currently looks like this: export type FileInputRef = {
clearFiles: () => void
input: HTMLInputElement | null
files: File[]
} shouldn't |
005f324
to
4f902e5
Compare
We may want to adjust the storybook story to make sure that the ref's files prop we are exposing is what is being used. This can be a future item since the functionality of the Component will not need to change. |
Summary
This PR modifies the
FileInput
component to useReact.forwardRef
and theuseImperativeHandle
hook to expose the underlying<input type="file" />
element, a method to clear the files state, and the value of thefiles
state.Related Issues or PRs
Fixes #958
How To Test
In an application, try passing a
ref
into the FileInput component, and then useref.current.files
andref.current.clearFiles()
to interact with the file input.Screenshots (optional)