Skip to content

Conversation

@pablinos
Copy link
Contributor

As the HOC was used as part of a filter, the className was getting set
by the last filter that was run. This was a lucky accident for OpenTable
as it meant that it wasn't affected by the bug being fixed in #14732

Changes proposed in this Pull Request:

This change passes through the className prop when the block names don't
match, which means that if it's been set by a previous filter, it is
preserved.

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

This is a bug fix

Testing instructions:

  • Use this branch on a free plan simple site.
  • Add a Calendly and OpenTable block.
  • Verify that the containing block-editor-block-list__block div has the has-warning and is-interactive classes for the blocks
  • Check that other blocks that don't require a plan also don't have those classes

This is dependent on #14732, which should be merged first or OpenTable will break for free WPCOM sites.

Proposed changelog entry for your changes:

This only affects WPCOM sites, so I don't think it requires a changelog.

As the HOC was used as part of a filter, the className was getting set
by the last filter that was run. This was a lucky accident for OpenTable
as it meant that it wasn't affected by the bug being fixed in #14732

This change passes through the className prop when the block names don't
match, which means that if it's been set by a previous filter, it is
preserved.
@matticbot
Copy link
Contributor

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

@pablinos pablinos requested a review from scruffian February 19, 2020 21:24
@pablinos pablinos self-assigned this Feb 19, 2020
@pablinos pablinos added [Block] OpenTable [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Status] Needs Review This PR is ready for review. labels Feb 19, 2020
@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: March 3, 2020.
Scheduled code freeze: February 25, 2020

Generated by 🚫 dangerJS against a22fd93

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 work well in my tests on Jetpack and on WordPress.com.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 20, 2020
@jeherve jeherve added this to the 8.3 milestone Feb 20, 2020
Copy link
Member

@scruffian scruffian left a comment

Choose a reason for hiding this comment

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

This "fixes" the issue on WPCOM. Great work 👍 :shipit:

@jeherve jeherve merged commit 714b918 into master Feb 24, 2020
@jeherve jeherve deleted the fix/with-has-warnings-class branch February 24, 2020 07:12
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 24, 2020
@scruffian
Copy link
Member

r203262-wpcom

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

Labels

[Block] OpenTable [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Touches WP.com Files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants