-
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
[RNMobile] Fix broken "Link To" settings and add "Image Size" settings #40947
Conversation
Requests using "per_page=-1" (unbounded queries) are not supported on mobile, as per: wordpress-mobile/gutenberg-mobile#2661 As an unbounded query was being used to fetch inner images in the gallery, the "link to" settings were not working as expected on native. With this commit, the unbounded query is replaced with "imageIds.length" to fix the issue on native.
Size Change: -117 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
@Mamaduka, thanks so much for taking a look! I hadn't been aware that unbounded queries were unsupported on native until diving into this bug either.
Appreciate the heads-up, I've gone ahead to resolve the conflict now. 🙇♀️ Let me know if anything looks off or if you spot any regressions on the web-side that I may have missed. Aside: Looking at the changes in #40945, I'm wondering if the introduction of |
It might be a good idea to update |
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.
Thanks, @SiobhyB. The code looks good and tests well for me on the desktop.
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.
Code looks good and is working well in my tests. Nice job @SiobhyB ! 👍
Fixes: #23576
Related PRs
gutenberg-mobile
: [RNMobile] Fix broken "Link To" settings and add "Image Size" settings wordpress-mobile/gutenberg-mobile#4841What?
This PR fixes the native version of the Gallery block's broken Link To settings and also adds Image Size settings.
Why?
The reason that the settings are broken in the native version of the block is that
per_page=-1
(an unbounded query) is used when fetching the gallery's images. Unbounded queries aren't currently supported on the native side, as per the open issue at wordpress-mobile/gutenberg-mobile#2661.Updating the query, not only fixes the broken Link To settings, but also enables support for the Image Size settings, which become visible in the native block's settings.
How?
With this PR, the unbounded query is replaced with
imageIds.length
to fix the issue on native. This approach was chosen as it was initially suggested here: #34389 (comment)Testing Instructions
1. Link To Settings
<a href="URL">
tags are surrounding the gallery's images.2. Image Size Settings
size-large
if Large was selected) has been added to the HTML for the gallery's images.3. Regression Testing
We should confirm that the Gallery block still works the same as it did before on the web, by experimenting with the its settings with this branch checked out on the web.
Screenshots or screencast
You'll see that the
<a href="URL">
tags are correctly added when using the Link To settings within the app:Screen.Recording.2022-05-09.at.20.38.14.mov