[RNMobile] Tiled Gallery Block: Aspect Ratio for Square Layout #21166

SiobhyB · 2021-09-23T13:31:55Z

Gutenberg Mobile: wordpress-mobile/gutenberg-mobile#4017

This PR is one part of a wider effort to port the Jetpack Tiled Gallery block to Mobile Gutenberg. It will be merged into a feature branch, rnmobile/add/tiled-gallery-block.

For this specific PR, the purpose is to define the aspect ratio of images in the editor, so that they appear as squares.

Changes proposed in this Pull Request:

With this PR, the images in the Tiled Gallery block will appear as squares in the editor by default.

Brief overview of code changes proposed in this Pull Request:

A new context, imageCrop, has been added to the index.js file. This is an existing context in the image block, which translates to resizeMode=cover, which is necessary in order for us to be able to change the image's shape.
A className prop is passed to the inner image blocks, which in turn will be passed to the image block's shapeStyle prop and used to pass styles.
The is-style-squared CSS selector is defined directly in the Tiled Gallery's styles.native.css file and its styles are the ones being passed to the inner image blocks, making use of the fact that aspect ratio is supported by React Native.

Jetpack product discussion

Tiled Gallery project thread: p9ugOq-1Tb-p2

Does this pull request change what data or activity we track or use?

No, it doesn't.

Testing instructions:

With these branches checked out and the Gutenberg Mobile demo app running, add the Tiled Gallery block.
Add a few images to the block.
Observe the shape of the images and verify that they're squares.

A few ideas for testing:

Add varying numbers of images to ensure the images are squares even when there are different numbers per row.
Switch between portrait and landscape to check for any inconsistencies.
Test the core image and gallery blocks for any regressions.

Screenshots

Before	After

github-actions · 2021-09-23T13:35:27Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ All commits were linted before commit.
🔴 Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
🔴 Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

🔴 Action required: Please add missing changelog entries for the following projects: projects/plugins/jetpack

Use the Jetpack CLI tool to generate changelog entries by running the following command: jetpack changelog add.
Guidelines: /docs/writing-a-good-changelog-entry.md

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

Next scheduled release: December 7, 2021.
Scheduled code freeze: November 30, 2021.

By adding an "imageCrop" and setting it to true, images in the Tiled Gallery will now appear cropped by default. This is important so that images don't appear unusually distorted when their shape is changed to be square.

As the image block's caption component is not a part of the Tiled Gallery block's design, it's hidden with this commit. This is done by adding an "hideImageCaption" context and setting it to true. This is passed back to the image block, which is set to hide the component when this context is detected.

"imageCrop" and "hideImageCaption" don't need to be set as props.

With this commit, the image's aspect ratio is being set in Jetpack's CSS files. It was previously set in Gutenberg's files, but this wasn't inline with our guidelines for Jetpack code appearing in the open source project.

The previous version of this code wasn't working as expected, as it included quotation marks. With this commit, the issue is fixed and the reference to the CSS class now works as expected.

guarani · 2021-09-30T01:04:30Z

projects/plugins/jetpack/extensions/blocks/tiled-gallery/index.js

+	hideImageCaption: {
+		type: 'boolean',
+		default: true,
+	},


This file is shared between native and web so hideImageCaption is being added as a new block attribute on both platforms. Do you think we should copy this file into a new index.native.js and only add this attribute there?

That seems like a good idea, I've gone ahead to create a native-specific file in 128667a, let me know if anything looks off with that to you.

projects/plugins/jetpack/extensions/blocks/tiled-gallery/index.js

guarani

I left a few comments, we might need to make an index.native.js file to avoid impacting the web, but overall the approach is solid and I learned about these block contexts 🙇‍♂️

In order to limit the PR's scope to the square aspect ratio, this commit removes the 'hideImageCaption'. New contexts for hiding the image block's caption and settings will be moved to #21278.

As a native-specific index file was introdued in 128667a, we no longer need to include contexts in the web-only index file, as they're only used with the apps.

SiobhyB · 2021-10-04T14:02:52Z

Thank you for taking a first pass @guarani 🙇‍♀️

I've gone ahead to add the mobile-specific file in 128667a.

As we've chatted about, too, I've also removed the hideImageCaption context from this PR, so that the focus is solely on the aspect ratio of the images. I'll create a separate PR for the introduction of other contexts, like hideImageCaption.

…ared-aspect-ratio

The removal of the image's caption goes beyond the scope of this specific PR, so its removal is reverted with this PR.

SiobhyB · 2021-10-28T16:01:35Z

@guarani, this PR is ready for review again whenever you have the chance. Thanks in advance! 🙇‍♀️

guarani · 2021-10-28T16:52:33Z

This PR is one part of a wider effort to port the Jetpack Tiled Gallery block to Mobile Gutenberg. It will be merged into a feature branch, for which the main PR can be found at #20831.

I just realized that #20831 should actually be closed since its changes have already been merged to the feature branch, so I closed it now.

guarani

I tested this on iOS and Android and it looks perfect 💯. I've held off approving so I can get clarification on whether a single index file would still be preferable, but apart from that this is ready to approve.

projects/plugins/jetpack/extensions/blocks/tiled-gallery/index.native.js

projects/plugins/jetpack/extensions/blocks/tiled-gallery/edit.native.js

guarani · 2021-10-28T21:22:46Z

@SiobhyB, I think the three failing PR checks are not an issue. The failing changelog check can be ignored because we're merging to a feature branch, the failing wpcom check is not an issue for the same reason. The failing PHP test could be flaky, but I don't think it's required.

As discussed here, there isn't currently a compelling need to keep the changes to the index file in a native-specific file: #21166 (comment) This change therefore reverts the use of a native-specific file, combining its changes into the shared index file.

As the layout constant doesn't currently have any values other than "squared", it's removed with this commit.

SiobhyB · 2021-10-29T14:01:22Z

@guarani, thanks for your review! I've addressed all your comments and updated the PR description 🙇‍♀️

guarani

Looks great! 👍 Thanks @SiobhyB!

…ared-aspect-ratio

Pass 'squared' class back to image for styling

64f1950

matticbot added the Touches WP.com Files label Sep 23, 2021

github-actions bot added [Block] Tiled Gallery [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ labels Sep 23, 2021

github-actions bot added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Sep 23, 2021

SiobhyB mentioned this pull request Sep 23, 2021

Tiled Gallery Block: Aspect Ratio for Square Layout wordpress-mobile/gutenberg-mobile#4017

Merged

2 tasks

SiobhyB added the [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack label Sep 26, 2021

Siobhan added 6 commits September 27, 2021 11:15

Crop images in Tiled Gallery by default

f2ea815

By adding an "imageCrop" and setting it to true, images in the Tiled Gallery will now appear cropped by default. This is important so that images don't appear unusually distorted when their shape is changed to be square.

Remove uncessary code

1f16029

"imageCrop" and "hideImageCaption" don't need to be set as props.

Set aspect ratio of images so that they are squares

5f068ac

With this commit, the image's aspect ratio is being set in Jetpack's CSS files. It was previously set in Gutenberg's files, but this wasn't inline with our guidelines for Jetpack code appearing in the open source project.

Remove caption, as it's unused

19139b0

Fix reference to CSS class

5f6a329

The previous version of this code wasn't working as expected, as it included quotation marks. With this commit, the issue is fixed and the reference to the CSS class now works as expected.

SiobhyB mentioned this pull request Sep 27, 2021

[RNMobile] Enable Third-Party Blocks to Set Aspect Ratio of Image via Inner Blocks WordPress/gutenberg#35089

Closed

7 tasks

SiobhyB requested review from guarani and illusaen September 27, 2021 22:17

SiobhyB marked this pull request as ready for review September 27, 2021 22:17

SiobhyB changed the title ~~[RNMobile] Tiled Gallery Block: Aspect Ratio~~ [RNMobile] Tiled Gallery Block: Aspect Ratio for Square Layout Sep 28, 2021

guarani reviewed Sep 30, 2021

View reviewed changes

projects/plugins/jetpack/extensions/blocks/tiled-gallery/index.js Show resolved Hide resolved

guarani reviewed Sep 30, 2021

View reviewed changes

Siobhan added 3 commits October 4, 2021 10:32

Remove 'hideImageCaption' context from PR's scope

1500648

In order to limit the PR's scope to the square aspect ratio, this commit removes the 'hideImageCaption'. New contexts for hiding the image block's caption and settings will be moved to #21278.

Create native-specific index file to avoid impact to web

128667a

Remove unused contexts from web-only index file

95f0e5c

As a native-specific index file was introdued in 128667a, we no longer need to include contexts in the web-only index file, as they're only used with the apps.

SiobhyB mentioned this pull request Oct 4, 2021

[RNMobile] Inner Block Contexts for Image Block WordPress/gutenberg#35315

Closed

7 tasks

SiobhyB requested a review from guarani October 5, 2021 09:29

Siobhan added 2 commits October 28, 2021 16:39

Merge branch 'rnmobile/add/tiled-gallery-block' into rnmobile/add/squ…

b773084

…ared-aspect-ratio

Revert unecessary code removal

0e69603

The removal of the image's caption goes beyond the scope of this specific PR, so its removal is reverted with this PR.

guarani reviewed Oct 28, 2021

View reviewed changes

projects/plugins/jetpack/extensions/blocks/tiled-gallery/index.native.js Outdated Show resolved Hide resolved

projects/plugins/jetpack/extensions/blocks/tiled-gallery/edit.native.js Outdated Show resolved Hide resolved

Siobhan added 2 commits October 29, 2021 14:51

Remove layout constant, for clarity

b463560

As the layout constant doesn't currently have any values other than "squared", it's removed with this commit.

guarani self-requested a review October 29, 2021 14:16

guarani approved these changes Oct 29, 2021

View reviewed changes

Merge branch 'rnmobile/add/tiled-gallery-block' into rnmobile/add/squ…

03a8724

…ared-aspect-ratio

SiobhyB merged commit b11a395 into rnmobile/add/tiled-gallery-block Nov 2, 2021

SiobhyB deleted the rnmobile/add/squared-aspect-ratio branch November 2, 2021 14:49

github-actions bot removed the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Nov 2, 2021

SiobhyB mentioned this pull request Jan 4, 2022

The Mosaic layout should respect image aspect ratios wordpress-mobile/gutenberg-mobile#4300

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RNMobile] Tiled Gallery Block: Aspect Ratio for Square Layout #21166

[RNMobile] Tiled Gallery Block: Aspect Ratio for Square Layout #21166

SiobhyB commented Sep 23, 2021 •

edited

Loading

github-actions bot commented Sep 23, 2021 •

edited

Loading

guarani Sep 30, 2021

SiobhyB Oct 4, 2021

guarani left a comment

SiobhyB commented Oct 4, 2021

SiobhyB commented Oct 28, 2021

guarani commented Oct 28, 2021 •

edited

Loading

guarani left a comment

guarani commented Oct 28, 2021

SiobhyB commented Oct 29, 2021

guarani left a comment

[RNMobile] Tiled Gallery Block: Aspect Ratio for Square Layout #21166

[RNMobile] Tiled Gallery Block: Aspect Ratio for Square Layout #21166

Conversation

SiobhyB commented Sep 23, 2021 • edited Loading

Changes proposed in this Pull Request:

Brief overview of code changes proposed in this Pull Request:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Screenshots

github-actions bot commented Sep 23, 2021 • edited Loading

guarani Sep 30, 2021

Choose a reason for hiding this comment

SiobhyB Oct 4, 2021

Choose a reason for hiding this comment

guarani left a comment

Choose a reason for hiding this comment

SiobhyB commented Oct 4, 2021

SiobhyB commented Oct 28, 2021

guarani commented Oct 28, 2021 • edited Loading

guarani left a comment

Choose a reason for hiding this comment

guarani commented Oct 28, 2021

SiobhyB commented Oct 29, 2021

guarani left a comment

Choose a reason for hiding this comment

SiobhyB commented Sep 23, 2021 •

edited

Loading

github-actions bot commented Sep 23, 2021 •

edited

Loading

guarani commented Oct 28, 2021 •

edited

Loading