[Mobile] - Gallery block - Fix getMediaItems #63426
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
|
Size Change: +2.13 kB (+0.12%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
2 tasks
2 tasks
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
This was referenced Jul 11, 2024
@geriux The error message is now gone, but I still experience the inability to add the same image twice and the occasional unexpected position changes when following the steps shared in wordpress-mobile/gutenberg-mobile#6973 (review). This occurs in both the iOS and Android builds. Do you experience this as well? |
|
Hey @dcalhoun thanks for testing this change!
Yes, I just reproduced it. I was always adding photos from my device and not using the WordPress Media Library. 😅 So, this is also happening in the web editor. The only difference is that in the native WordPress media library, we are not highlighting the selected media, so you are able to select it, but then the code filters it out. In the web UI, you see already added images as selected:
I believe we had this functionality before the recent media library updates 🤔 As for the images changing their position, this issue is also occurring in the web editor. This happens by adding an image from the Image block appender, where you are able to insert an image that’s already added to the Gallery. When a duplicated image is added, they will change their position to be next to each other because there's a sorting part in the code. We should improve the WordPress Media Library in the host apps to avoid adding duplicate images by highlighting the ones that are already added. |
Ah, that makes sense. Thank you for investigating.
I am unable to reproduce the unexpected gallery item re-ordering in the web editor. However, I suppose the issue within the native editor might only be significantly disruptive when modifying a large gallery. The user can recover from this bug by manually re-ordering the gallery items after uploading. |
Yeah, I agree they could just move the position of the image. This is how I was able to reproduce it: GalleryWebBug.mov |
dcalhoun
approved these changes
Jul 12, 2024
There was a problem hiding this comment.
I didn’t find the culprit of this regression, but after some investigation, I confirmed that the data was being fetched correctly, yet it wasn’t being stored in the state.
Very interesting. Thank you for investigating and sharing your findings.
The test plan succeeded for me on an iPhone SE and Galaxy S20. 🚀
scruffian
pushed a commit
that referenced
this pull request
Jul 15, 2024
* Mobile - Fix getMediaItems * Rearrange conditions for parseResponse * Fix response conditions Co-authored-by: geriux <geriux@git.wordpress.org> Co-authored-by: dcalhoun <dpcalhoun@git.wordpress.org>
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jul 18, 2024
* Mobile - Fix getMediaItems * Rearrange conditions for parseResponse * Fix response conditions Co-authored-by: geriux <geriux@git.wordpress.org> Co-authored-by: dcalhoun <dpcalhoun@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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:
What?
This PR addresses a regression in the Gallery block that was causing error notices to appear in certain scenarios. Additionally, it prevents the images from randomly changing their position when new images are added.
Why?
To provide a good user experience when adding new images by preventing random errors and unexpected behavior.
How?
I didn’t find the culprit of this regression, but after some investigation, I confirmed that the data was being fetched correctly, yet it wasn’t being stored in the state.
This was causing the useGetMedia hook to not return the expected data of the images.
By looking more into the code, I realized the fetch promise was being intercepted by a catch, so no data was being stored.
The error was happening in fetchAllMiddleware, where parseResponse is called and expects to have a Response parameter in JSON format.
After all this, I decided to update the native implementation of fetchHandler and update how it parses the response.
Now it will return a
Responseobject if theparseparameter is set tofalse.Testing Instructions
Note
Please use the following builds to test:
Steps provided from another PR review:
Testing Instructions for Keyboard
No testing is needed.
Screenshots or screencast
GalleryBugBefore.mov
GalleryBugAfter.mov