[RNMobile] Enable Third-Party Blocks to Set Aspect Ratio of Image via Inner Blocks #35089
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 selector will be used to make images in the editor look 'square' within the Tiled Gallery block. The CSS will be iterated on in future commits.
2 tasks
SiobhyB
changed the base branch from
rnmobile/add/tiled-gallery-block
to
trunk
SiobhyB
changed the base branch from
trunk
to
rnmobile/add/tiled-gallery-block
SiobhyB
changed the base branch from
rnmobile/add/tiled-gallery-block
to
trunk
SiobhyB
changed the base branch from
trunk
to
rnmobile/add/tiled-gallery-block
SiobhyB
changed the base branch from
rnmobile/add/tiled-gallery-block
to
trunk
SiobhyB
changed the base branch from
trunk
to
rnmobile/add/tiled-gallery-block
SiobhyB
changed the base branch from
rnmobile/add/tiled-gallery-block
to
trunk
SiobhyB
changed the base branch from
trunk
to
rnmobile/add/tiled-gallery-block
SiobhyB
changed the base branch from
rnmobile/add/tiled-gallery-block
to
trunk
As the image block's caption component isn't part of the Tiled Gallery's design, this commit sets up the necessary code to hide it from view. This is done through the use of a "hideImageCaption" context, which lives under a new "blockStyles" object. It should be possible to further utilise this object for future styling of inner blocks.
|
Size Change: +2.31 kB (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
added 7 commits
September 27, 2021 11:07
With this new commit, the purpose of the new object is clarifyied by tweaking its name from "blockStyles" to "imageBlockStyles".
…m/WordPress/gutenberg into rnmobile/add/squared-aspect-ratio
At the moment, a fixed height is applied to the image block if context is detected. This not only effects the standard Gallery block (intended) but also the Tiled Gallery Block (not intended). With this commit, a new "fixedHeight" context is added to only set a fixed height if it's explicitly set. This is necessary as the Tiled Gallery block will need varying heights as part of its design.
In order to change the image's shape via a third-party block (such as Jetpack's Tiled Gallery), we need to be able to pass a style to the "shapeStyle" prop. With this commit, the "shapeStyle" prop is changed to accept both a reference to the image block's classes and a third-party block's styles. This change means that it's no longer necessary for third-party blocks, like the Tiled Gallery, to set styles directly in the image block's stylesheet. So, this commit also removes the ".is-style-squared" class.
The 'hideImageCaption' and 'fixedHeight' context objects are enabled for the image block in this commit. These objects will be used for the purpose of tweaking the image block's default styles in blocks where it's used i.e. currently, the Gallery block and Jetpack's Tiled Gallery block.
SiobhyB
added
Mobile App - i.e. Android or iOS
SiobhyB
commented
Sep 27, 2021
| }; | ||
|
|
||
| const imageContainerStyles = [ hasImageContext && styles.fixedHeight ]; | ||
| const imageContainerStyles = [ |
There was a problem hiding this comment.
👋 @geriux, I wanted to check in with you, as this PR makes some changes to the ones you introduced with your recent PR in #34957.
The reason for the changes is that hasImageContext was evaluating to true for the Tiled Gallery block, and the block needs to have more flexible heights. You'll see from this PR that I've worked around this by adding a new fixedHeight context object to the Gallery block, so it's only used with that block.
Does that seem like an okay approach to you? Interested to know whether you have other thoughts.
There was a problem hiding this comment.
Since the only block that was using context was the Gallery block I used a generic fixedHeight name to avoid limiting the height of the component.
Seeing that now the Tiled block would need this as well, what if we use a name like isGallery instead of fixedHeight for the context?
Do we also need to pass fixedHeight from the Image block?
There was a problem hiding this comment.
@geriux, thank you for looking into this! 🙇♀️
Seeing that now the Tiled block would need this as well, what if we use a name like
isGalleryinstead offixedHeightfor the context?
Ah, the Tiled Gallery block doesn't need this context. I created it as hasImageContext was evaluating as true for the Tiled Gallery block and applying the fixed height that was needed only for the core Gallery block. This was preventing images from being styled as squares within the Tiled Gallery block. That's what led me to create a context that would only apply to the core Gallery block, not other blocks that are using contexts.
I'm happy to rename it if you want it be clearer that this is for use with the core gallery, maybe something like galleryHasFixedHeight or isCoreGallery?
Do we also need to pass
fixedHeightfrom the Image block?
That's a good point! It doesn't really need to be passed as an additionalImageProp. Do you think a new imageBlockStyles object would make sense here?
There was a problem hiding this comment.
I've gone ahead to create an imageBlockStyles object and moved the fixedHeight context to it in ce9ec06.
SiobhyB
changed the title
SiobhyB
changed the base branch from
trunk
to
rnmobile/add/tiled-gallery-block
…ared-aspect-ratio
SiobhyB
changed the title
added 2 commits
October 4, 2021 10:40
In order to limit this PR's scope, this commit removes the 'hideImageCaption' context. Instead, the focus will be on changes needed for a third-party block to change the image's aspect ratio. The proposal for contexts to hide certain settings in the image block will be moved to #35315.
As mentioned in #35089 (comment), it's not necessary to pass the 'fixedHeight' as an 'additionalImageProps'. This commit moves that context to a new 'imageBlockStyles' object. This object has the potential to be added to in the future.
SiobhyB
changed the title
SiobhyB
changed the base branch from
rnmobile/add/tiled-gallery-block
to
trunk
SiobhyB
changed the base branch from
trunk
to
rnmobile/add/tiled-gallery-block
SiobhyB
changed the title
|
Noting that the original scope of this PR has been refined. The PR originally included the introduction of a new context, Now, this PR's scope is limited to changes that are necessary for third-party blocks to change the aspect ratio of images pulled in via inner blocks. It involves a tweak to the image block's |
|
I'm closing this PR as, to make things as clear and easy to follow along with as possible, it's necessary to split it up even further into the following:
|
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.
Description
The React Native version of Jetpack's Tiled Gallery block will be using the Inner Block API to display images in a gallery. As part of this project, we need to change the aspect ratio of the images that are displayed.
It is likely that other third-party blocks will face similar use cases in the future if they're pulling in the image block as an inner block. With that in mind, and to help achieve our end goal, this PR introduces some changes that will enable any third-party block to define the aspect ratio of an image when used as an inner block.
Note, the goal behind this PR isn't to introduce Jetpack-specific code, but rather to introduce generic changes that will hopefully be useful or re-usable for other third-party blocks. We have tried to be mindful against introducing anything that is specific only to Jetpack to the Gutenberg codebase.
Types of changes
Outlined below are specifics around the two main changes that this PR includes. They are the introduction of a new context,
fixedHeight, to prevent an overlap with the core Gallery block's set height and a tweak to the image block'sshapeStyleprop.Address Conflict with Core Gallery Block's Style
allowResizeandimageCrop.imageCropcontext translates toresizeMode='cover', which is what we need to use in order to change the inner image's aspect ratios.imageCrop, meant the image block'shasImageContextvariable began to evaluate astruefor the Tiled Gallery block. This then meant thefixedHeightstyle was being applied to it, which prevented the aspect ratio of images from being changed.fixedHeightcontext to the Gallery block. As explained here, this change prevents styles that are specific to the core Gallery block from impacting any block that uses theimageCropcontext.Allow Third-Party Styles to be Passed
shapeStyleprop was updated to accept not only a string, but also full styles. This is to allow third-party blocks, the Tiled Gallery block in this case, to pass styles without needing to make edits directly to the image block's core files.How has this been tested?
As this PR is part of a wider effort to port the Jetpack Tiled Gallery block to Gutenberg Mobile, steps to specifically test the Jetpack block can be found in the Jetpack repository at Automattic/jetpack#21166, with the central Gutenberg Mobile PR at wordpress-mobile/gutenberg-mobile#4017.
For this specific PR, on the Gutenberg side, the following has been tested in the iOS and Android apps, with this branch checked out from view:
Screenshots
These screenshots show our specific use cases for using contexts, which is to remove and style parts of the image block when used as an inner block. You'll see we were able to use contexts to make the images appear as squares:
Checklist:
*.native.jsfiles for terms that need renaming or removal).