Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tiled gallery block: add stacked layout #13130

Closed
wants to merge 2 commits into from
Closed

Tiled gallery block: add stacked layout #13130

wants to merge 2 commits into from

Conversation

simison
Copy link
Member

@simison simison commented Jul 25, 2019 •
edited
Loading

Add stacked layout to tiled gallery block:

Screenshot 2019-07-25 at 14 32 10

Changes proposed in this Pull Request:

  • Add new stacked layout to the Tiled gallery block.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • enhancement

Testing instructions:

  • Add Tiled gallery with some images
  • From styles, change to "stacked" layout
  • Confirm it works in frontend of the site
  • Confirm no columns setting available
  • Confirm that full alignment works

Proposed changelog entry for your changes:

  • Adds stacked layout to Tiled gallery block

@simison simison added [Status] In Progress [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Block] Tiled Gallery labels Jul 25, 2019
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello simison! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D30813-code before merging this PR. Thank you!

@jetpackbot
Copy link

jetpackbot commented Jul 25, 2019 •
edited
Loading

Warnings
⚠️ The Privacy section is missing for this PR. Please specify whether this PR includes any changes to data or privacy.

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-13130

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 31187a1

@simison simison force-pushed the add/tiled-gallery-stacked branch from d682c0d to bea8699 Compare October 4, 2019 07:14
@matticbot
Copy link
Contributor

simison, Your synced wpcom patch D30813-code has been updated.

@simison

This comment has been minimized.

Sign in to view
@htdat htdat mentioned this pull request Dec 31, 2019
Open
@stale

This comment has been minimized.

Sign in to view
@stale stale bot added the [Status] Stale label Jan 2, 2020
@simison simison requested a review from iamtakashi January 17, 2020 07:51
@stale stale bot removed the [Status] Stale label Jan 17, 2020
@simison
Copy link
Member Author

simison commented Jan 17, 2020

Curious to hear from @iamtakashi if this would suffice as a replacement of the CoBlocks Stacked Gallery block? :-)

@iamtakashi
Copy link
Contributor

Nice! One thing I like about CoBlocks one is that I can change the gutter size to remove it completely, especially for the stack gallery. Is it possible to have the setting?

@simison simison added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Feb 16, 2020
@simison
Copy link
Member Author

simison commented Feb 16, 2020 •
edited
Loading

One thing I like about CoBlocks one is that I can change the gutter size to remove it completely, especially for the stack gallery. Is it possible to have the setting?

Maybe! Tried at bit at #14705 (issue #9548). That's a separate technical concern from this PR tho. Does the layout make sense without that setting?

Folks can also create columns layout with 1 column, does this improve discoverability? Maybe, since we added "Stacked" CoBlocks block to .com too.

@simison simison force-pushed the add/tiled-gallery-stacked branch from bea8699 to 18a0723 Compare February 16, 2020 22:17
@matticbot
Copy link
Contributor

simison, Your synced wpcom patch D30813-code has been updated.

@simison simison marked this pull request as ready for review February 16, 2020 22:20
@simison simison requested a review from a team February 16, 2020 22:20
@jeherve jeherve added this to the 8.3 milestone Feb 17, 2020
@simison simison force-pushed the add/tiled-gallery-stacked branch from 18a0723 to 3b39465 Compare February 17, 2020 13:41
@matticbot
Copy link
Contributor

simison, Your synced wpcom patch D30813-code has been updated.

jeherve
jeherve previously approved these changes Feb 17, 2020
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works well, but part of me wonders: what's the difference between the stacked style and a column style with only one column? Is that new style necessary?

@jeherve jeherve removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Feb 17, 2020
@jeherve jeherve 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 Feb 17, 2020
@iamtakashi
Copy link
Contributor

Folks can also create columns layout with 1 column

That's a good point. As Mikael says, the style option might improve the discoverability, it might not be absolutely necessary :)

@jeherve
Copy link
Member

jeherve commented Feb 20, 2020

@simison Should we proceed and merge this, or get other opinions about whether or not the option is useful?

@simison
Copy link
Member Author

simison commented Feb 20, 2020 via email

@jeherve jeherve removed this from the 8.3 milestone Feb 20, 2020
@jeherve jeherve added [Status] Blocked / Hold and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Feb 20, 2020
@stale
Copy link

stale bot commented May 20, 2020

This PR has been marked as stale. This happened because:

  • It has been inactive in the past 3 months.
  • It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this PR has clear testing instructions, is it up to date with master, and it is still valid. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

@stale stale bot added the [Status] Stale label May 20, 2020
@ockham ockham force-pushed the add/tiled-gallery-stacked branch from 3b39465 to 31187a1 Compare November 13, 2020 18:27
@stale stale bot removed the [Status] Stale label Nov 13, 2020
@ockham
Copy link
Contributor

ockham commented Nov 13, 2020

Rebased.

@jeherve
Copy link
Member

jeherve commented Mar 2, 2022

I'll close this PR for now because of the lack of activity on this. We can always reopen in the future if needed, but it will need a rebase, so it may be easier to start a new PR at this point.

@jeherve jeherve closed this Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeherve jeherve jeherve left review comments

@iamtakashi iamtakashi Awaiting requested review from iamtakashi

Assignees

@simison simison

Labels
[Block] Tiled Gallery [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Status] Blocked / Hold Touches WP.com Files
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@simison @matticbot @jetpackbot @iamtakashi @jeherve @ockham