-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[Mobile] - Gallery block - Add useGetMedia native variant #42178
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Image block: Don't call for media data if it already has it
geriux
added
[Status] In Progress
Tracking issues with work in progress
Mobile App - i.e. Android or iOS
Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
labels
Jul 6, 2022
Size Change: +17 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
…mage size was specified or if it doesn't contain a query param
derekblank
approved these changes
Jul 13, 2022
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, nice work! 🚀
# Conflicts: # packages/react-native-editor/CHANGELOG.md
4 tasks
I found this article to be useful while executing test case 2: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Mobile App - i.e. Android or iOS
Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related PRs:
Gutenberg Mobile PR
-> Gallery block improvements wordpress-mobile/gutenberg-mobile#5007WordPress iOS PR
-> Media - Add large and medium image sizes wordpress-mobile/WordPress-iOS#18997WordPress Android PR
-> Media - Add large and medium image sizes wordpress-mobile/WordPress-Android#16875What?
This PR improves the performance of the Gallery and Image block.
Why?
Currently, the experience with the Gallery block is not great when uploading images on mobile. One of the issues is that the parent apps are sending the full size of the image to the editor, which affects how the block itself performs due to memory usage.
Apart from that, the Gallery block is making a lot of network requests with
useGetMedia
and this causes the following issues:mediaId
when making the request, on mobile we use amediaId
for local uploads which are different from the finalmediaId
, so it might occur that your image would change to a different one that matches the localmediaId
with one of your uploaded images from your media library. Then it would be updated back again to the original one once the image finished uploading. This is confusing for the users and it is an unexpected behavior we don't want, there are reports of this happening too.The Image block also makes a network request to get the media data so it would get even worse as the Gallery block uses Image blocks as inner blocks.
How?
First, we introduced React Native FastImage to improve image rendering within the editor in #42009.
We added a
useGetMedia
native variant that will only callgetMediaItems
once all of the images have finished uploading, as well as filtering out images that havefile:
as URL. This will prevent bringing images with the localmediaId
while the images are being uploaded and making a lot of network requests.Main apps will send the
large
size ormedium
to the editor instead of thefull-size
. With this and React Native FastImage only one image will be cached, and memory usage will be very low.The Image block will only set a new URL if it really changed, for example, if the
sizeSlug
is set or different.Testing Instructions
NOTE: Test for both Simple sites and self-hosted, repeat both test cases for the Gallery and Image block.
Test case 1 - Gallery block image uploads
Image size
optionImage size
Thumbnail
,Medium
,Large
, andFull Size
···
on the top rightfile://
as the protocol and that they’re using the large size, you can see they have a query param?w=1024
, and for self-hosted, instead of a query param the size it¡s in the URL file name e.g.image-1024
Test case 2 - Gallery block image uploads with the editor closed
Note: You should limit your network speed so it's easier to test this use case
Image size
optionImage size
Thumbnail
,Medium
,Large
, andFull Size
···
on the top rightfile://
as the protocol and that they’re using the large size, you can see they have a query param?w=1024
, and for self-hosted, instead of a query param the size it¡s in the URL file name e.g.image-1024
Screenshots or screencast
Test case 1
GalleryTestCase1_Final.mov
Test case 2
GalleryBlockTestCase2_Final.mov