fix: explicitly list allowed image types for upload #7819
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
No changes to documentation |
|
Component Testing Report Updated Nov 19, 2024 2:49 PM (UTC) ❌ Failed Tests (3) -- expand for details
|
⚡️ Editor Performance ReportUpdated Fri, 15 Nov 2024 00:07:53 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
There was a problem hiding this comment.
Would devs still be able to add an unsupported type to options.accept to their schema? It only makes sense to customize options.accept with a subset of what the backend support, so would it be sensible for us to filter out unsupported types, and/or give a warning/error if you do this? (probably an edge case, and not a big deal, this PR is still a nice improvement, I'm just curious what you think).
Also noticed that we don't surface why the upload fails if you try to upload an unsupported image (I tried avif) – it just says "The upload could not be completed at this time". We're missing an small education opportunity here as it could instead say "currently only x,y and z are supported"). We should look into fixing this separately.
TL;DR: LGTM – thank you for the additional cleanups too <3
| const buttonText = t(value ? 'search.filter-asset-change' : 'search.filter-asset-select', { | ||
| context: type, | ||
| }) | ||
|
|
||
| const accept = get(type, 'options.accept', type === 'image' ? 'image/*' : '') | ||
| const accept = get( |
There was a problem hiding this comment.
oh wow, TIL you can search by uploading an image
Yes they would, and theoretically yes it would be nice to give a warning if specifying unsupported formats, but not sure it is worth the amount of work/edge cases it would take - since the accept attribute allows for wildcards, mime types and file extensions - it can quickly become a tedious list/function to maintain. |
Yes, I did spot that - got derailed because the backend didn't respond with a structured error code and started working on some improvements on that front. Because of i18n, we can't/shouldn't just throw the server side error message, so will need to check an error code and give localized messages. Will continue improving on this front 👍 |
Description
We're currently allowing
image/*in the image input, but only a certain types are allowed by our backend. In particular, we recently added support for AVIF images to be output, but we don't yet support them to be uploaded. Instead of letting users upload the entire image before they get a message saying they are not supported, we should flag our support in the image selection dialog.Also fixed a race condition issue with the
clearUploadStatuscallback - when the image fails to be uploaded, eg rejected because of an unsupported format, the value doesn't seem to have a value - and thus does not get unset. Instead it is stuck in the pending state (100% done) until someone resets it. I don't quite see the need to check before unsetting - but let me know if I am overlooking something.Did some minor typing/lint cleanups along the way.
What to review
Testing
Ideally there should be a test to ensure that you can't select eg an AVIF, but I couldn't find a good way of doing so at the moment :/
Notes for release