-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Support file deletion for data-url #4030
Conversation
Is it okay to add it in 5.15.3 section?
@yuki-js Can you explain how Jest was not working? I don't feel comfortable not having tests for this new feature |
Tests became successful (idk why the previous tests failed). I added test cases. Please have a look. |
The descriptions for two tests in StringField.test.jsx were reversed.
CHANGELOG.md
Outdated
@@ -15,6 +15,7 @@ it according to semantic versioning. For example, if your PR adds a breaking cha | |||
should change the heading of the (upcoming) version to include a major version bump. | |||
|
|||
--> | |||
|
|||
# 5.15.3 |
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.
I just switched this to 5.16.0
in my recently merged PR, can you resolve conflicts. Also, it seems much of this file was reformatted. Please revert all of the changes below 5.15.2
. Thanks!
@yuki-js can you resolve conflicts and revert most of the unnecessary formatting changes to |
I apologize for the oversight. Fixed a changelog, also merged #4033 into this PR. |
@yuki-js For some reason the build is now failing. Can you please investigate? Thanks |
It's odd that an error happens in v16, and a seemingly unrelated module has failed. I'll investigate but could you rerun the failed job? It might have a timing issue due to Nx's concurrency. Just a heads up. |
@yuki-js I reran it already. I'll try one more time. In the meantime can you run the build locally and see if it works? The error is in |
Ok, now it works... hmmm |
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.
LGTM
Reasons for making this change
Fix #3957
Fix #4033
Before this PR, uploaded files cannot be deleted.
This PR adds file deletion button, also changes the data flow into the
value
centric one.Also corrected test descriptions for file widget attributes. The descriptions for two tests in StringField.test.jsx were reversed.
Checklist
npm run test:update
to update snapshots, if needed.