Conversation
|
Size Change: 0 B Total Size: 1.16 MB ℹ️ View Unchanged
|
fb595a8 to
e683619
Compare
61a183b to
3a6bda3
Compare
|
Hey @iamthomasbishop 👋 now it is turn for Gallery and media editing support. Builds are ready for you to test for a design review whenever you get a chance, thank you! |
There was a problem hiding this comment.
Excited to see progress on this Gerardo!
I tested this on Android (Pixel 3a - real device) and I think the styles refactoring may have broken a few things regarding scrollable captions:
The caption is no longer scrollable, and the buttons get covered by the large caption. Also, I encountered some issues after the caption expanded beyond the size of the container, where even after deleting the text, the caption remained large, and in one case, weirdly, I couldn't tap out to unfocus the text input. I'm not sure what could cause that behavior. 🤔
Another issue I encountered was in using the replace option. Because we are in Gallery, naturally the multiple-selection flag is set, but in this case, using replace, the additional images are added as image blocks after the Gallery block. I'd expect either that the replace option only permits selection of 1 image, or that additionally selected images are inserted into the Gallery block in the position of the image they are replacing.
Replace allows multiple, but appends as Image blocks
| Replace allows multiple selection | Multiple images appended as Image blocks |
|---|---|
 |
 |
Other than that, all the other behavior seems to be working correctly 👍
|
@geriux I had a chance to take the new build for a spin and didn't find any big design issues. I agree w/ @mkevins feedback (nice catches, Matt!), so once those things are done, I think we're in good shape. One thing that's semi-related but doesn't necessarily have to be part of this work -- can we utilize the mock "iOS system materials" (using the third-party library @geriux used on Compact Notices recently) on the gallery image caption background? Fine by me if we need to roll this into a separate ticket, but Matt's feedback on the caption made me think of this 😄 |
# Conflicts: # packages/react-native-editor/CHANGELOG.md
… fix caption styles and scrolling
|
Hey @mkevins 👋 Thank you for the review and tests! Nice catches indeed 🙇♂️ I updated the code and made new builds with the caption fixes, I also removed the Can you please give it another review when you can? Thanks a lot! |
|
Hey @iamthomasbishop 👋
That'd be a nice thing to add yes! But I think it'd be better to do it on a separate task. Is there one opened? Thanks! |
Most definitely can be a separate task. I will create a ticket and drop some notes on it sometime early next week. |
|
Hey @lukewalczak 👋 I was wondering if you would be able to review this PR =) It's a small one and most of the testing was done already, let me know! Thanks! |
# Conflicts: # packages/react-native-editor/CHANGELOG.md
|
Hey @lukewalczak thank you for your comments! I've just updated the PR with the changes, let me know! Thanks! |
lukewalczak
left a comment
There was a problem hiding this comment.
LGTM from code perspective!
|
Sure, merging using admin-rights now... |
Gutenberg Mobile PR-> wordpress-mobile/gutenberg-mobile#2496WordPress iOS PR-> wordpress-mobile/WordPress-iOS#14498WordPress Android PR-> wordpress-mobile/WordPress-Android#12497Description
Continuing adding media editing support to blocks this PR adds it to Gallery block.
There aren't any major changes other than adding the, other changes are wrapping some parts of the code within new containers to fix some absolute positioning issues.MediaUploadcomponent ingallery-image.nativeto support replacing images within a galleryIt adds two new props to the
Imagecomponent:heightandresizeMode.Allows disabling the
Replaceoption in the media editing button.Note: The replace option was removed and will be added in a future iteration.
How has this been tested?
Steps to test
---Edit an image
Replace an image
Full screen preview
Remove a picture
Screenshots
Types of changes
New feature
Checklist: