-
Notifications
You must be signed in to change notification settings - Fork 4.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
[WIP] Block editor: Add MediaUploadProvider
#66380
base: add/61447-upload-media-pkg
Are you sure you want to change the base?
[WIP] Block editor: Add MediaUploadProvider
#66380
Conversation
@youknowriad Is this roughly what you had in mind? |
Size Change: +417 B (+0.02%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
a810b80
to
fe31fbf
Compare
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 looks great overall.
It's clear that's a bit too much added complexity but unfortunately, it's kind of necessary evil.
Correct me if I'm wrong but for me the setting passed to the block editor, shouldn't use any store. It's a setting that will be used internally by the new package to actually perform the upload right? |
Yes. What I mean by this is: The function passed to the block editor will be passed on unmodified to the upload store for eventually uploading the file to the server. Now, if someone, for example the image block, calls I just need to figure out how to best do that :) |
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.
So this is looking good to me overall. Let me know if you want to merge this into the other PR or if we should review the other PR separate from this to help some people looking at the diff maybe. No preference for me.
Awesome, thanks! Yeah let's review the other PR first to make things easier :) Appreciate any eyes there. |
7d3f04d
to
52288c5
Compare
packages/upload-media/src/components/provider/with-registry-provider.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Jon Surrell <[email protected]>
Warning
This is still work in progress and still incomplete!
What?
Adds a new
MediaUploadProvider
component toblock-editor
to leverage the new upload data store from the experimentalupload-media
package added in #66290.To-do:
getSettings().mediaUpload
in theblock-editor
package to use this new upload store (it should look like https://github.com/swissspidy/media-experiments/blob/8b64ed6150aca612cc649f32861c8049e79cd985/packages/editor/src/init/editor/media-upload.ts#L21-L109)Why?
Every block editor instance should have its own upload store instance
How?
Follows the recommendation from #66290 (comment) to implement
MediaUploadProvider
withuseSubRegistry
.Testing Instructions
Tests should still pass
Everything should still work with the experiment being disabled
Testing Instructions for Keyboard
N/A
Screenshots or screencast
N/A