[Mobile] - Refactor gallery - cherry pick v2 - step 1 #31413
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
This is only temporary, for testing purposes. This will be replaced with an actual implementation (which will need to access the flag remotely).
This will need to be addressed (since we can't have unwrapped text on mobile). For now, I'm commenting this out to proceed with cherry-picking the changes from the original mobile refactor branch.
mkevins
added
Mobile App - i.e. Android or iOS
|
Size Change: -59 B (0%) Total Size: 1.43 MB
ℹ️ View Unchanged
|
antonis
approved these changes
May 3, 2021
There was a problem hiding this comment.
Great work @mkevins 👍
I tested this and the new gallery behaves pretty well considering the stage of this project:
- Created a gallery with some images
- Images behave as inner blocks
- Reordering works
- Removal works
- Attribute overwriting works as described
- The columns setting seems to work similarly to the old v1 gallery block: Maximum number of columns is set to 3 and only two columns can be rendered in portrait mode (3 columns work only in landscape).
I also noticed the following:
- When an upload is cancelled, it will leave a placeholder image (as discussed). I don't think this is bad especially if we can add a way to resume the upload.
- I noticed a breakage in the columns block where the internal blocks are not outlined (screenshot) and the user is not able to add a block. I think we can tackle this in a separate PR.
Columns block
mkevins
deleted the
rnmobile/refactor/gallery-as-nested-image-blocks-cherry-pick-v2-wip
branch
|
Thanks for reviewing and testing Antonis! Good catch about the columns. I will gather all of these issues as we progress further with the refactor PRs, and hopefully they'll get sorted out in further cherry-picking. It'll be great as a reference that you noted the issues here, since if the further cherry-picking from the former PR does not resolve some issues, the comments on these PRs will serve as a good starting point to determine the root cause. In this case, I suspect it is to do with intermittent style changes to the block-list items, but I haven't confirmed that. |
That makes sense. I also tested the main refactor PR #25940 and the columns work there. Thus it must be some cherrypicked change on this one. |
7 tasks
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
Description
This PR is the first step at adapting the former refactor implementation to the new approach used in the main PR. It mostly cherry-picks changes to the rebased branch, and omits certain changes that were later reverted. After the inclusion of these changes, it should be possible to create new v2 gallery blocks (as inner image blocks), but the UI will not be in it's final state.
Also, a temporary change was included in this PR (acf18ca) to enable the
__experimentalGalleryRefactorflag for testing purposes. This will be removed once a more appropriate implementation is in place.The relevant commits from the former mobile refactor PR are included or omitted as follows:
How has this been tested?
In the block editor, create a gallery and add some images. They should behave as inner image blocks, and should allow reordering and removal. It should also be possible to add captions to both the image blocks, and the outer gallery block. When an inner image block has a setting (attribute) that is also present on the gallery block, a change to the setting on the gallery level should "overwrite" the setting for the individual image block. E.g. if you add a link to an image, and subsequently change the gallery "link to" setting to "media file", the individual image link should no longer be set. However, if the same settings were applied first at the gallery level, then at the image level, the image block should retain its unique setting(s).
Checklist:
*.native.jsfiles for terms that need renaming or removal).