Skip to content

Conversation

@Copons
Copy link
Contributor

@Copons Copons commented Jan 31, 2020

Changes proposed in this Pull Request:

  • Increase the CSS specificity of the workaround that improve the Eventbrite and OpenTable block previews in the Inserter.

Screenshot 2020-01-31 at 16 27 09

We added this workaround in #14075, but it's too generic and affects other block previews, such as the WordPress.com Page Layout Selector.

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

  • Bug fix.

Testing instructions:

  • Open the editor and click on the top bar's block insterter.
  • Hover on various blocks, especially Jetpack ones, and make sure that they look good enough.

Notes:

  • Some blocks (e.g. Image, Slideshow, Map) might not be positioned correctly at first hover. Try again!
  • Pinterest is incorrectly positioned, period. display: table doesn't help.

Proposed changelog entry for your changes:

  • N/A

Related

Automattic/wp-calypso#39198

@Copons Copons added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review This PR is ready for review. [Block] Eventbrite [Block] OpenTable labels Jan 31, 2020
@Copons Copons requested review from a team, creativecoder and iamtakashi January 31, 2020 16:28
@Copons Copons self-assigned this Jan 31, 2020
@matticbot
Copy link
Contributor

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

@jetpackbot
Copy link
Collaborator

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 3b4aa21

@creativecoder creativecoder added this to the 8.2 milestone Jan 31, 2020
Copy link
Contributor

@creativecoder creativecoder left a comment

Choose a reason for hiding this comment

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

Approving with some caveats

Jetpack sites
Works as expected

Simple Sites
(testing wpcom diff D38409-code)

Doesn't seem to work, though I think that might have to do with a need to rebase that diff

Screen Shot 2020-01-31 at 11 24 02

wp-calypso issue

Automattic/wp-calypso#39198

This helps the spacing issue identified, there, but doesn't fix it. Seems like this Eventbrite block style was not the root cause.

Screen Shot 2020-01-31 at 11 16 53

Also (non-blocking) I'd prefer that spacing changes be merged separately, since it confuses the code change history.

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.

This seems to test well for me in Jetpack. It should be good to merge.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! [Status] Needs Cherry-Pick and removed [Status] Needs Review This PR is ready for review. labels Jan 31, 2020
@creativecoder creativecoder merged commit 632cdd9 into master Jan 31, 2020
@creativecoder creativecoder deleted the fix/eventbrite-breaking-layout-selector-preview branch January 31, 2020 20:42
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Jan 31, 2020
@creativecoder
Copy link
Contributor

r202393-wpcom

@jeherve
Copy link
Member

jeherve commented Feb 3, 2020

Cherry-picked to branch-8.2 in 03a2ab1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

6 participants