Media - Add large and medium image sizes #18997

geriux · 2022-07-06T09:49:37Z

Related PRs:

Gutenberg Mobile PR -> Gallery block improvements gutenberg-mobile#5007
Gutenberg PR -> [Mobile] - Gallery block - Add useGetMedia native variant WordPress/gutenberg#42178
WordPressKit-iOS PR -> MediaService - Add large and medium sizes WordPressKit-iOS#529

This PR adds support for the large and medium sizes for images, this data is already available from both the REST API and XMLRPC but it is not stored until now. Having these images allows using a lower size within the editor which improves the performance once images are uploaded.

Now once an image is uploaded from the editor, it will use the large size if available, if not it will fall back to the medium size and if none are available it will use the full size. This change is done to the Gutenberg processors for the Image and Gallery block.

For test instructions and screencast please check the Gutenberg PR description.

Regression Notes

Potential unintended areas of impact

It should only affect the Gutenberg editor media uploads.

What I did to test those areas of impact (or what existing automated tests I relied on)

Manual testing:

Image uploads using the editor and the Image and Gallery block
Gutenberg processors uploading images and closing the editor

What automated tests I added (or what prevented me from doing so)

None, we do manual testing with media uploads from the editor.

PR submission checklist:

I have completed the Regression Notes.
I have considered adding unit tests for my changes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

peril-wordpress-mobile · 2022-07-06T09:49:42Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

wpmobilebot · 2022-07-06T09:54:39Z

Podfile

  # pod 'WordPressKit', :git => 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', :tag => ''
  # pod 'WordPressKit', :git => 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', :branch => ''
-  # pod 'WordPressKit', :git => 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', :commit => ''
+  pod 'WordPressKit', :git => 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', :commit => '9d9efb43c3f502e98cfbe26fdd8ed11e2a04f02e'


🚫 Use the new Ruby 1.9 hash syntax.

wpmobilebot · 2022-07-06T10:30:38Z

You can test the changes in Jetpack from this Pull Request by:

Clicking here or scanning the QR code below to access App Center
Then installing the build number pr18997-016f67e on your iPhone

If you need access to App Center, please ask a maintainer to add you.

wpmobilebot · 2022-07-06T10:34:00Z

You can test the changes in WordPress from this Pull Request by:

Clicking here or scanning the QR code below to access App Center
Then installing the build number pr18997-016f67e on your iPhone

If you need access to App Center, please ask a maintainer to add you.

# Conflicts: # Podfile # Podfile.lock

derekblank

I was able to test and verify the steps following the Testing Instructions on WordPress/gutenberg#42178. LGTM! 🚀

geriux · 2022-07-14T12:34:31Z

Hey @twstokes 👋 I was wondering if you would have some spare time to check out this PR, we've tested it in different scenarios and it's working as expected so we are looking for someone with native experience to check the code changes this PR is introducing. You've also done a core data migration before in the project, and you've worked with the Gutenberg Mobile team with media upload issues, that's why I'm pinging you first. I just want to make sure I'm following the right steps and not missing anything for these kinds of changes.

Please let me know if you could check it out, if not that's ok, I could look for someone else, thank you in advance!

twstokes · 2022-07-14T13:34:42Z

Hey @twstokes 👋 I was wondering if you would have some spare time to check out this PR, we've tested it in different scenarios and it's working as expected so we are looking for someone with native experience to check the code changes this PR is introducing. You've also done a core data migration before in the project, and you've worked with the Gutenberg Mobile team with media upload issues, that's why I'm pinging you first. I just want to make sure I'm following the right steps and not missing anything for these kinds of changes.

Please let me know if you could check it out, if not that's ok, I could look for someone else, thank you in advance!

👋 Hey @geriux! Will be glad to!

twstokes

👋 @geriux! I've left some comments in the PR and observations below for your consideration. 🙇

I followed the testing steps here:

Simple Site

Test Case 1 with gallery block ✅
Test Case 1 with image block ✅
Test Case 2 with gallery block ✅
Test Case 2 with image block ✅

Self-hosted Site

Test Case 1 with gallery block ✅
Test Case 2 with image block ✅
Test Case 1 with image block ⚠️ - Switching to thumbnail caused the image to disappear (see video below)

Self-hosted.Image.mp4

Test Case 2 with gallery block ⚠️ - After the draft uploaded I went back to the gallery block and didn't see the Image Size option

Observations

The image size for an image inside a Gallery Block can be set, but then overridden when the setting is changed at the gallery level without warning. This is an edge case and I'm assuming it's already been considered. 👍

WordPress/Classes/Services/MediaService+Swift.swift

WordPress/Classes/Services/PostCoordinator.swift

geriux · 2022-07-14T16:38:11Z

Thank you so much for testing and your feedback @twstokes 🙇 I'll work on addressing the issues and your comments.

…RLStr

# Conflicts: # Podfile # Podfile.lock

geriux · 2022-07-18T17:12:33Z

Hey @twstokes 👋

Thank you for the feedback and for testing, I applied some of the suggestions and also fixed the issue related to the image disappearing when changing the size. Let me know if you can give it another review/test. Thanks again!

twstokes

👋 @geriux! I'm going to approve this PR so you can choose whether my comments below should be addressed now or later.

Self-hosted + thumbnail mode

I tested setting images to thumbnail mode on a self-hosted site and they no longer disappeared. ✅

However, I did notice that when changing the quality of a gallery on the self-hosted site sometimes not all of the thumbnails would change. (see attached video) You can see that the first image in the second row still looks sharp.

This also happened after all the images were done processing and the app had been idle for around a minute. The site did not have Jetpack connected, if that makes any difference.

Thumbnail.MP4

Self-hosted + image size option on a draft post

I had the same results here as last time.

Test Case 2 with gallery block ⚠️ - After the draft uploaded I went back to the gallery block and didn't see the Image Size option

geriux · 2022-07-19T10:01:21Z

Hey @twstokes thank you so much for testing again!

I see why you're getting that issue, since you're using a self-hosted site without probably the Gutenberg plugin activated, it's loading the older version of the Gallery block, which has a lower usage. I've tested these scenarios with the Gutenberg plugin and the newer version of the Gallery block (that has nested blocks) and its working correctly.

I'll merge this since these changes are based on the newest Gallery block.

Thanks again!

geriux added Media Gutenberg Editing and display of Gutenberg blocks. labels Jul 6, 2022

This was referenced Jul 6, 2022

Gallery block improvements wordpress-mobile/gutenberg-mobile#5007

Merged

MediaService - Add large and medium sizes wordpress-mobile/WordPressKit-iOS#529

Merged

wpmobilebot reviewed Jul 6, 2022

View reviewed changes

geriux mentioned this pull request Jul 8, 2022

[Mobile] - Gallery block - Add useGetMedia native variant WordPress/gutenberg#42178

Merged

Gerardo added 4 commits July 8, 2022 12:24

Media - Add support for large and medium image sizes

f0364ae

Add remoteLargeURL and remoteMediumURL to Media

0890583

Update MIGRATIONS

288635d

Update Gutenberg ref and WordPressKit

4fefef4

geriux force-pushed the add/media-large-medium-sizes branch from 4ca8ab9 to 4fefef4 Compare July 8, 2022 10:32

Gerardo added 2 commits July 8, 2022 15:56

Update Gutenberg ref

b7227a0

Merge branch 'trunk' into add/media-large-medium-sizes

60ffb3a

# Conflicts: # Podfile # Podfile.lock

geriux changed the title ~~Media - Add large and medium sizes~~ Media - Add large and medium image sizes Jul 12, 2022

geriux marked this pull request as ready for review July 12, 2022 13:13

geriux added this to the 20.4 milestone Jul 12, 2022

geriux requested a review from derekblank July 12, 2022 13:14

derekblank approved these changes Jul 14, 2022

View reviewed changes

derekblank mentioned this pull request Jul 14, 2022

Is RNFastImage required for iOS apps? wordpress-mobile/gutenberg-mobile#5025

Closed

twstokes self-requested a review July 14, 2022 13:34

twstokes reviewed Jul 14, 2022

View reviewed changes

WordPress/Classes/Services/MediaService+Swift.swift Show resolved Hide resolved

WordPress/Classes/Services/PostCoordinator.swift Outdated Show resolved Hide resolved

WordPress/Classes/Services/PostCoordinator.swift Outdated Show resolved Hide resolved

Gerardo added 2 commits July 18, 2022 16:10

PostCoordinator - Use if let instead for the imageURL and use remoteU…

07103af

…RLStr

Merge branch 'trunk' into add/media-large-medium-sizes

0b1ab1f

# Conflicts: # Podfile # Podfile.lock

twstokes approved these changes Jul 18, 2022

View reviewed changes

Gerardo added 2 commits July 20, 2022 14:49

Merge branch 'trunk' into add/media-large-medium-sizes

96eb10b

Update WordPressKit and Gutenberg references

016f67e

geriux merged commit 7b8c3b9 into trunk Jul 20, 2022

geriux deleted the add/media-large-medium-sizes branch July 20, 2022 17:10

twstokes mentioned this pull request Aug 17, 2022

Fix crash that can occur when accessing media over XML-RPC #19198

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Media - Add large and medium image sizes #18997

Media - Add large and medium image sizes #18997

geriux commented Jul 6, 2022 •

edited

Loading

peril-wordpress-mobile bot commented Jul 6, 2022 •

edited

Loading

wpmobilebot Jul 6, 2022 •

edited

Loading

wpmobilebot commented Jul 6, 2022 •

edited

Loading

wpmobilebot commented Jul 6, 2022 •

edited

Loading

derekblank left a comment

geriux commented Jul 14, 2022

twstokes commented Jul 14, 2022

twstokes left a comment

geriux commented Jul 14, 2022

geriux commented Jul 18, 2022

twstokes left a comment

geriux commented Jul 19, 2022

Media - Add large and medium image sizes #18997

Media - Add large and medium image sizes #18997

Conversation

geriux commented Jul 6, 2022 • edited Loading

Regression Notes

peril-wordpress-mobile bot commented Jul 6, 2022 • edited Loading

wpmobilebot Jul 6, 2022 • edited Loading

Choose a reason for hiding this comment

wpmobilebot commented Jul 6, 2022 • edited Loading

wpmobilebot commented Jul 6, 2022 • edited Loading

derekblank left a comment

Choose a reason for hiding this comment

geriux commented Jul 14, 2022

twstokes commented Jul 14, 2022

twstokes left a comment

Choose a reason for hiding this comment

Simple Site

Self-hosted Site

Observations

geriux commented Jul 14, 2022

geriux commented Jul 18, 2022

twstokes left a comment

Choose a reason for hiding this comment

Self-hosted + thumbnail mode

Self-hosted + image size option on a draft post

geriux commented Jul 19, 2022

geriux commented Jul 6, 2022 •

edited

Loading

peril-wordpress-mobile bot commented Jul 6, 2022 •

edited

Loading

wpmobilebot Jul 6, 2022 •

edited

Loading

wpmobilebot commented Jul 6, 2022 •

edited

Loading

wpmobilebot commented Jul 6, 2022 •

edited

Loading