-
Notifications
You must be signed in to change notification settings - Fork 844
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
[EuiFilePicker] Fix can still be cleared when disabled #5603
Conversation
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
💚 CLA has been signed |
jenkins test this |
Hey Greg! Thanks for starting up the tests. I actually realized just now though that when using |
Hey @ayoung19! |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5603/ |
That's an awesome suggestion and makes perfect sense! And it actually matches the behavior of |
Functionality has been added, I think tests need to be reran. |
Jenkins test this |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5603/ |
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.
Perfect! Verified the fix with the preview deployment.
Thanks, @ayoung19!
(not urgent and fix is fine, mentioning for completeness) I'm kinda surprised we didn't have to update any test snapshots for this - do we not have a Jest unit test for when |
Yeah I saw that |
Summary
I simply passed down the disabled prop in EuiFilePicker into it's child button components to disallow removing the files when disabled. This doesn't fix the use case of programmatically removing the file but that shouldn't matter at all since that's the expected behavior where users should be implementing that themselves.Above approach introduced some inconsistent UI behavior so instead the clearButton is actually removed from the UI entirely when
EuiFilePicker
is disabled.Checklist
Added documentationChecked Code Sandbox works for any docs examplesAdded or updated jest and cypress testsChecked for breaking changes and labeled appropriately