Tiled Gallery: Set Max Columns

[illusaen]

Sets the max columns allowed to the number of images.
Allows columns to resize when used in landscape/portrait mode.

Fixes #4147
Fixes #4138

Gutenberg Mobile PR: wordpress-mobile/gutenberg-mobile#4441

Changes proposed in this Pull Request:

• Sets the max columns allowed to either the number of images of the MAX_COLUMNS value of 20 (defined here in the block's constant.js file).
• Sets the max number of displayed columns in the editor to either two (for portrait mode) or four (for landscape mode). This brings the block's behaviour inline with that of the gallery block's behaviour.
• Allows columns to resize when used in landscape/portrait mode.

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

N/A

Screenshots

screen-20211123-144305.mp4

Testing instructions:

Test 1

• Add tiled gallery block.
• Add three images.
• Open settings and verify that you can go from 1-3 columns.
• Add another image.
• Open settings and verify that you can go to 4 columns.

Test 2

• Add tiled gallery block.
• Add four images.
• open settings and change number of columns to 4.
• Rotate to landscape and verify that the block shows 4 columns and expands to fit the space.

[github-actions]

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: January 11, 2022.
• Scheduled code freeze: January 3, 2022.

[guarani]

Since this PR aims to fix two issues, I'll respond to each separately.

Fixes #4147

My initial impression is that this PR allows the editor to display the number of columns set in the block attributes. This differs from the core Gallery, which on my phone uses two columns in portrait orientation, and four columns in landscape orientation.

This issue describes an implementation matching that of the core Gallery. Given our goal of having a shippable block by Dec 1, for now, I think it's best to stick to this.

If we agree, the next step might be to implement the same logic the core Gallery uses to control how many columns to display.

Fixes #4138

Going through the behavior described in wordpress-mobile/gutenberg-mobile#4138 (comment):

The columns setting's upper limit is equal to the number of images

This is working for me, e.g. if I have 10 images I can set the columns block attribute to 10.

The columns setting's default value is dynamic

In my testing of this PR, the default columns block attribute always stays at two. In the above mentioned comment, we discussed matching the Tiled Gallery web by making the column block attribute default to the number of images – if there are three or less images – or default to three if there are more than three images.

The columns settings is only shown when there are multiple images

This doesn't appear to be implemented.

---

For clarity, I think it would be best to handle each of these issues in a separate PR. We could then either re-work this PR to fix just one of the issues, or close it in favor of new PRs.

[guarani]

I'm just un-assigning myself reviewer since I should have made the above comment a review.

[SiobhyB]

@guarani, this PR's ready for review again. :) It includes the following functionality:

• Allows users to set the columns to either the number of images in the gallery (up to the web's max value of 20).
• The number of columns displayed within the editor itself is set to a max of four for landscape and two for portrait.
• The columns resize depending on whether they're in portrait or landscape mode.

I've created separate issues for tracking the outstanding issues you've noted, including updating the default value for columns to match the web and hiding the setting when there's only one image:

• Jetpack Tiled Gallery: Default column setting should match web wordpress-mobile/gutenberg-mobile#4431
• Tiled Gallery Block: Hide columns setting when there is only one image wordpress-mobile/gutenberg-mobile#4430

Looking forward to hearing your thoughts!

[guarani]

Thanks @SiobhyB! I gave this a test and the functionality is working as described, although I ran into one blocker and one very minor issue.

Images don't resize properly when changing columns setting

In the below video, as I switch between one and two columns, the Tiled Gallery's layout seems to break. Do you know if this is a separate issue, or is it specific to this branch?

layout-breaking.mov

Column stepper increment button isn't disabled when at max columns

The Gallery block disables the increment button (of the column stepper) when the user column setting is at its maximum value. This is definitely not a blocker and feel free to create a separate issue for it to address later (e.g. after Phase 1B).

[guarani]

A heads-up that the failure in the JS tests and Code coverage CI checks should be fixed after #22252 is merged and this PR is updated to include its change. The Jetpack E2E Tests / E2E connection tests CI check failure might be due to flakiness (you can retry it), and the other failures are expected (see note here).

[SiobhyB]

@guarani, thank you for the review! I was able to replicate the issue with images not resizing correctly on the main feature branch, so will create a separate issue for it. I'll also create a separate issue for the increment button 🙇‍♀️

[guarani]

@guarani, thank you for the review! I was able to replicate the issue with images not resizing correctly on the main feature branch, so will create a separate issue for it. I'll also create a separate issue for the increment button 🙇‍♀️

Ok, thanks for checking that! Approving now 🙇