[RNMobile] Refactor: gallery to nested image blocks #26823
Closed
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
…ocks at the point that the block validation is run
… innerBlocks at the point that the block validation is run" This reverts commit 4d95e95.
… available at point of block validation
…ocks at the point that the block validation is run
… innerBlocks at the point that the block validation is run" This reverts commit 4d95e95.
… available at point of block validation
… gallery - with the idea that we will improve the innerblocks drag and drop in a later PR
This reverts commit 9b0abcc.
…existing gallery - with the idea that we will improve the innerblocks drag and drop in a later PR" This reverts commit cc05d60.
gziolo
added
[Block] Gallery
…transform when dragging multiple images onto the gallery
dratwas
force-pushed
the
rnmobile/refactor/gallery-to-nested-image-blocks
branch
from
f6fc8a5 to
c2fbfa8
Compare
…refactor/gallery-to-nested-image-blocks
|
Size Change: +122 B (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
glendaviesnz
force-pushed
the
refactor/gallery-to-nested-image-blocks
branch
2 times, most recently
from
9c384a8 to
bc485f6
Compare
glendaviesnz
force-pushed
the
refactor/gallery-to-nested-image-blocks
branch
2 times, most recently
from
8ecf39d to
0c3dce0
Compare
26 tasks
glendaviesnz
force-pushed
the
refactor/gallery-to-nested-image-blocks
branch
5 times, most recently
from
c4dd32b to
c72e442
Compare
glendaviesnz
force-pushed
the
refactor/gallery-to-nested-image-blocks
branch
3 times, most recently
from
f4d0ea6 to
9415d57
Compare
glendaviesnz
force-pushed
the
refactor/gallery-to-nested-image-blocks
branch
from
66bdda6 to
4e57fb3
Compare
glendaviesnz
force-pushed
the
refactor/gallery-to-nested-image-blocks
branch
from
3870131 to
d71011b
Compare
This was referenced Apr 29, 2021
7 tasks
This was referenced May 26, 2021
|
Closing this PR in favor of the updated mobile PR #31306 |
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.
Description
Refactors the Gallery block to be a wrapper around Image blocks using InnerBlocks. This is a mobile implementation of #25940
What is done:
In order to make it works, I had to do a lot of changes. The main change is related to the "Tile" component. Till now the Gallery block used the Tile component to set the correct layout of the gallery, however, now we use inner blocks for rendering and there is no possibility to use it in the same way since the InnerBlock component is responsible for the layout. I moved the code from gallery/tile to the block-list/grid-item and use it there. In order to make it works, I also had to add the
gridPropertiesprop to set the correct number of columns (there could be more properties in the future).In the base PR the author moved some code from media-placeholder to the gallery/edit and I had also done the same to be compatible with that.
Another important thing is that the image block is not prepared to be used in the galley. I mean that images in the gallery have the same height etc so I had also make some changes in the image block. I added a prop
isGalleryandimageCropto determine how the image should look like.I couldn't use the
useParentAttributesto get inherited attributes from the gallery because the image/edit.native.js is a class. I implemented it in the class way (using the componentDidUpdate) however, it will be probably changed by the web team and should be implemented here as well - I describe it below.I also had to add some checks in the
link-settings/index.native.jsto prevent setting the URL when it's not necessary because it was setting the wrong URL when using the nested URL from the gallery.I implemented the possibility to use the
blockType.usesContextas it is done on the web. It was used to pass down some attributes from the gallery to the image.The thing that is still under development and I believe we should wait for the final implementation is how the inner blocks should behave when we change some attributes in the gallery. There are 3 attributes that are in the gallery and also in the image block:
The discussion about that #25940 (comment)
At this moment they are passed down through the context (that's why I had to add the possibility to use the BlockContext), however, there is still discussion on how it should be done and how the inner blocks should act when we change one of the listed attributes.
I really suggest waiting for the final decision since I had to change the implementation a few times in the meantime because of the new review.
What should be done:
First of all, as I mentioned we should wait till the web team decides about all implementation details because I don't think that it makes sense to implement it parallelly since they change his approach quite frequently.
Then this PR should be rebased to the latest
refactor/gallery-to-nested-image-blocksThen depending on the web team's decision about how to handle the inherited attributes that are in the gallery block and the image block as well, made changes in the image/edit.native.js.
The important thing is also that the web team wants to keep the old implementation and new implementation together:
#25940 (comment)
How has this been tested?
All sanity tests for gallery
Gallery-1
Gallery-2
Gallery-3
Gallery-4
Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Refactors the Gallery block to be a wrapper around Image blocks using InnerBlocks.
Checklist: