-
Notifications
You must be signed in to change notification settings - Fork 34
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
Fix #3263: Adding base64 file upload component #3431
Conversation
@leplatrem and @nathan-barrett - How does this feel? |
504138f
to
ab54fbb
Compare
Nice! |
c190af7
to
bb05cec
Compare
bb05cec
to
c674e2b
Compare
const hugeTestTxt = `data:text/plain;base64,${"aGkK".repeat(1024 * 512)}`; // "hi" repeatedly | ||
|
||
class fileReaderMock { | ||
error: FileReader["error"] = null; |
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 mock may be worth committing to memory. This was the most difficult thing in the PR to develop.
Awesome getting this in, lgtm |
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.
Nice! Thanks Alex for working on this.
I would recommend adding a test that does an assertion on the string field value that we obtain (eg data:content/type;base64,
)
Important
We should probably add info about this in the records vs attachment section of our RS docs https://remote-settings.readthedocs.io/en/latest/getting-started.html#records-vs-attachments
src/components/rjsf/Base64File.tsx
Outdated
); | ||
} | ||
|
||
async function getBase64Str(evt, changeCallback) { |
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.
nit: it's called onChange
elsewhere
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.
Yeah I think I was just avoiding that as we were getting a prop from rjsf called onChange
. But I can just rename that one to rjsfOnChange
.
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.
then forget it 👌
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.
Nah, already did the thing
Renamed the PR title, because:
|
Co-authored-by: Mathieu Leplatre <mathieu@mozilla.com>
Resolves #3263.
Adding a base64 encoding file upload option. See demo video below for how this prototype works.
This PR:
See the demo video and screenshots below.
simplescreenrecorder-2025-02-12_12.13.19.mp4
Validation error for too large files:
