Mobile media files block #25242
Mobile media files block #25242
Conversation
mzorz
added
[Type] Enhancement
|
Size Change: 0 B Total Size: 1.21 MB ℹ️ View Unchanged
|
| export function requestStoryCreatorLoad( mediaFiles, blockClientId ) { | ||
| return RNReactNativeGutenbergBridge.requestStoryCreatorLoad( | ||
| mediaFiles, | ||
| blockClientId | ||
| ); | ||
| } |
There was a problem hiding this comment.
Stories are a WPCom-only feature, right?
If this is the case, it might not be good to reference it directly.
It might be better to have a more generic bridge method that could be used by the Story block or eventually other blocks with similar behavior.
There was a problem hiding this comment.
It might be better to have a more generic bridge method that could be used by the Story block or eventually other blocks with similar behavior.
Excellent idea, here are the PRs tackling that (made them on top of the second part PRs to make the changes clearer)
Gutenberg: #26005
Jetpack: Automattic/jetpack#17456
Gutenberg mobile: wordpress-mobile/gutenberg-mobile#2706
WPAndroid: wordpress-mobile/WordPress-Android#13101
* added mobile StoryUpdateProgress component and bridge code to send/receive save progress * updated WPAndroid bridge DeferredEventEmitter to handle Story save events separately * changed all Save event interface methods to use String ids instead of int, and removed serverMediaId params as these don't apply while saving locally * redefined upload/save state constants * added onStorySaveResult handling to bridge, and renamed STORY_SAVE_STATE_* events to MEDIA_SAVE_STATE_* where appropriate * checking for matches of mediaId in mediaFiles while saving to send save progress updates * added mediaModelCreated() method to the bridge, so a new ID can be assigned to a mediaFile in a Story block * mediaId should always be a string in mediaFiles so, converting to avoid strict comparison to fail * removed commented code * updated documentation * added missing implementation of method storySaveSync() in demo app * fixed prettier warning * Update packages/block-editor/src/components/story-update-progress/README.md Co-authored-by: Joel Dean <[email protected]> * Update packages/block-editor/src/components/story-update-progress/README.md Co-authored-by: Joel Dean <[email protected]> * Mobile Stories block (part3: refactor / rename) (#26005) * renames for generic media files collection block and BlockMediaUpdateProgres * referencing the right props method in finishMediaSaveWithFailure * mistaken renames of parameters in bridge methods * renamed more abstract/generic method names requestMediaFilesEditorLoad * removed extra whtie space * renamed argument type Co-authored-by: Joel Dean <[email protected]>
packages/block-editor/src/components/block-media-update-progress/README.md
Outdated
Show resolved
Hide resolved
…ss/README.md Co-authored-by: Paul Von Schrottky <[email protected]>
packages/block-editor/src/components/block-media-update-progress/index.native.js
Outdated
Show resolved
Hide resolved
…/WordPress/gutenberg into try/jetpack-stories-block-mobile
packages/react-native-editor/android/app/src/main/java/com/gutenberg/MainApplication.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Hi @mzorz! 👋
I was reviewing Automattic/jetpack#17140 and had some questions, mostly about BlockMediaUpdateProgress and related methods, so I thought I'd comment here.
- These seems to two concepts, namely upload and save. Each has very similar methods (state reset, success, failure, progress) and I think the upload refers to the media upload itself while the save is a separate operation after uploading (please correct me if I'm wrong). Is there any room for unifying these two concepts into one, or reducing in some way the number of methods (currently eight main ones)?
- From its name,
requestMediaFilesEditorLoad, it's not too clear to me what this native method does - What does it mean for media to be reset, i.e.
mediaUploadStateReset— is that when the user cancels a media upload? Reset seems to imply (to me) that the upload will start again, but I'm not too clear on this.
Saving happens before uploading. Unifying is something that has come up very recently and the idea is to eventually (in a later PR) replace the Saving and Uploading both need to show progress, and that's what this component is for, with the added feature that the underlying block knows about a mediaFiles collection (which could have just 1 item, which essentially the particular case for Video or Image blocks in the future). Right now it is not our intention to refactor this and possibly break progress updates for very common blocks such as Video or Image blocks 😅 so, will leave them out of the project scope for now.
This originally was more descriptive but also very specific
This comes from MediaUploadProgress and has been here for a while. In that specific scenario (upload reset) it's used to clear the file selection on the media block and allow the user to tap on the block and trigger the media picker to set the media file again. In our particular case it's not used in the Story block component for now, but will be used when / if Let me know if any other doubt arises 👍 |
* added mobile StoryUpdateProgress component and bridge code to send/receive save progress * updated WPAndroid bridge DeferredEventEmitter to handle Story save events separately * changed all Save event interface methods to use String ids instead of int, and removed serverMediaId params as these don't apply while saving locally * redefined upload/save state constants * added onStorySaveResult handling to bridge, and renamed STORY_SAVE_STATE_* events to MEDIA_SAVE_STATE_* where appropriate * checking for matches of mediaId in mediaFiles while saving to send save progress updates * added mediaModelCreated() method to the bridge, so a new ID can be assigned to a mediaFile in a Story block * mediaId should always be a string in mediaFiles so, converting to avoid strict comparison to fail * removed commented code * updated documentation * added missing implementation of method storySaveSync() in demo app * fixed prettier warning * renames for generic media files collection block and BlockMediaUpdateProgres * referencing the right props method in finishMediaSaveWithFailure * mistaken renames of parameters in bridge methods * renamed more abstract/generic method names requestMediaFilesEditorLoad * removed extra whtie space * renamed argument type * renamed event paramter name to easier to understand 'success' boolean, matching the native counterpart * added cancel and retry bridge methods specific for mediaFiles collection based blocks * added requestMediaFilesSaveCancelDialog bridge method * renamed bridge method mediaModelCreatedForFile for more general purpose mediaIdChanged * Add missing iOS bridge declarations * added jsdoc like description for methods * removed unneeded return statement in some bridge methods * Update packages/react-native-bridge/index.js Co-authored-by: Joel Dean <[email protected]> * Update packages/react-native-bridge/index.js Co-authored-by: Joel Dean <[email protected]> * Update packages/react-native-bridge/index.js Co-authored-by: Joel Dean <[email protected]> * fixed typo, added punctuation Co-authored-by: eToledo <[email protected]> Co-authored-by: Joel Dean <[email protected]>
…dle building (#26247) * added mobile StoryUpdateProgress component and bridge code to send/receive save progress * updated WPAndroid bridge DeferredEventEmitter to handle Story save events separately * changed all Save event interface methods to use String ids instead of int, and removed serverMediaId params as these don't apply while saving locally * redefined upload/save state constants * added onStorySaveResult handling to bridge, and renamed STORY_SAVE_STATE_* events to MEDIA_SAVE_STATE_* where appropriate * checking for matches of mediaId in mediaFiles while saving to send save progress updates * added mediaModelCreated() method to the bridge, so a new ID can be assigned to a mediaFile in a Story block * mediaId should always be a string in mediaFiles so, converting to avoid strict comparison to fail * removed commented code * updated documentation * added missing implementation of method storySaveSync() in demo app * fixed prettier warning * renames for generic media files collection block and BlockMediaUpdateProgres * referencing the right props method in finishMediaSaveWithFailure * mistaken renames of parameters in bridge methods * renamed more abstract/generic method names requestMediaFilesEditorLoad * removed extra whtie space * renamed argument type * renamed event paramter name to easier to understand 'success' boolean, matching the native counterpart * added cancel and retry bridge methods specific for mediaFiles collection based blocks * added requestMediaFilesSaveCancelDialog bridge method * renamed bridge method mediaModelCreatedForFile for more general purpose mediaIdChanged * added jetpack node_modules folder to cleanup step in JSbundle building tasks
* adding upload progress tests on BLockMediaUpdateProgress component (mediaFiles collection) * added save event tests for BlockMediaUpdateProgress component * ids in mediaFiles array are always strings, fixed that * updated function name passed by props * fixed mediaFiles const passing for saving, using tempMediaFiles * fixed state change for saveReset signal, was changing upload state instead * fixed typo * fixed using save state var * changed to more descriptive test names * removed commented line * renamed vars to match bridge signal name
* hide Story block in non-DEV builds * prettified * passing enableStories feature flag in GutenbergProps and hiding block from picker if false * added space * renamed bridge GutenbergProps property to agnostic abstraction mediaFilesCollectionBlock * renamed var on Capabilities enum * removed stories specific block behavior
| export function mediaSaveSync() { | ||
| return RNReactNativeGutenbergBridge.mediaSaveSync(); | ||
| } |
There was a problem hiding this comment.
Could we add JDocs descriptions to this function too? 🙏
There was a problem hiding this comment.
👍 good idea, added in 08e77ba
| export function subscribeMediaSave( callback ) { | ||
| return gutenbergBridgeEvents.addListener( 'mediaSave', callback ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Also here, could we add a description of how the MediaSave event should be used? (i.e, what parameters does it expect, etc...
|
Closing this in favor of #26721 |
Description
This PR adds bridge support for the jetpack Story block in mobile gutenberg
Related Jetpack PR: Automattic/jetpack#17140
Related Gutenberg-mobile PR: wordpress-mobile/gutenberg-mobile#2611
Related WPAndroid PR: wordpress-mobile/WordPress-Android#12939
How has this been tested?
Screenshots
Types of changes
Checklist: