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

Eventbrite and OpenTable Blocks: Fix block preview sizing with Gutenberg #14553

Merged
merged 2 commits into from
Feb 10, 2020
Merged

Eventbrite and OpenTable Blocks: Fix block preview sizing with Gutenberg #14553

merged 2 commits into from
Feb 10, 2020

Conversation

creativecoder
Copy link
Contributor

@creativecoder creativecoder commented Feb 1, 2020 •
edited
Loading

Changes proposed in this Pull Request:

Gutenberg 7.3 removes a wrapper div around blocks that changes the display of the block previews for the Eventbrite and OpenTable blocks.

This change adds additional style rules that restore the block previews.

Before

Screen Shot 2020-01-31 at 21 49 32

After

Screen Shot 2020-01-31 at 22 15 29

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

Testing instructions:

  • Install and activate the latest version of the Gutenberg plugin (7.3)
  • Use the block picker to see previews of the Eventbrite and OpenTable blocks
  • Without this PR, the block previews look very small
  • With this PR, the block preview should look correctly sized

Note that there is an extra margin at the top of the block preview--this appears to be a bug in Gutenberg 7.3 since it also appears in the Core block previews.

Proposed changelog entry for your changes:

N/A since these are new blocks in the 8.2 release.

@creativecoder creativecoder added this to the 8.2 milestone Feb 1, 2020
@creativecoder creativecoder requested review from Copons and a team February 1, 2020 03:55
@creativecoder creativecoder self-assigned this Feb 1, 2020
@jetpackbot
Copy link

jetpackbot commented Feb 1, 2020 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: February 11, 2020.
Scheduled code freeze: February 4, 2020

Generated by 🚫 dangerJS against e0e6bbf

@creativecoder creativecoder added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Feb 1, 2020
@matticbot
Copy link
Contributor

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

@creativecoder creativecoder added the [Type] Bug When a feature is broken and / or not performing as intended label Feb 1, 2020
@simison
Copy link
Member

simison commented Feb 3, 2020

That's a big button. 😁 Would something like this look more balanced?

image

@Copons Copons requested a review from apeatling February 3, 2020 20:39
@apeatling
Copy link
Member

Confirmed the button is now big. Agreed with @simison, if we can center it and reduce the size a bit that would be ideal.

@creativecoder
Copy link
Contributor Author

Because of the way the preview is generated automatically from the example attributes registered with the block, I don't know of a way we can control the size of the button preview in the block inserter.

This Gutenberg change from @scruffian would help with that, however!

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

@jeherve jeherve left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks good in my tests. It's a bit big but I wouldn't consider this a blocker. Merging. I'll let you handle the wpcom side of this. 👍

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Feb 10, 2020
@jeherve jeherve merged commit c9ab09c into master Feb 10, 2020
@jeherve jeherve deleted the fix/eventbrite-open-table-block-previews-latest-gutenberg branch February 10, 2020 16:50
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 10, 2020
jeherve pushed a commit that referenced this pull request Feb 10, 2020
@creativecoder @jeherve
[not verified] Eventbrite and OpenTable Blocks: Fix block preview siz…
c90af25 
…ing with… (#14553)

* Eventbrite and OpenTable Blocks: Fix block preview sizing with Gutenberg
7.3

* Adds comment for explanation
@jeherve
Copy link
Member

jeherve commented Feb 10, 2020

Cherry-picked to branch-8.2 in c90af25

@jeherve
Copy link
Member

jeherve commented Feb 10, 2020

r202643-wpcom

@scruffian scruffian mentioned this pull request Feb 13, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeherve jeherve jeherve approved these changes

@Copons Copons Awaiting requested review from Copons

@scruffian scruffian Awaiting requested review from scruffian

@apeatling apeatling Awaiting requested review from apeatling

Assignees

@creativecoder creativecoder

Labels
[Block] Eventbrite [Block] OpenTable Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
8.2
Development

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