Update BlockStylesSelector component and implement in Eventbrite

[scruffian]

This makes some changes to the BlockStylesSelector component and implements it in the Eventbrite block.

Fixes #14527

Changes proposed in this Pull Request:

• The preview for the button Embed Type on the EventBrite block will now be a real block preview.
• The EventBrite block also now allows you to change the Embed Type from the BlockControls.
• The useModal attribute in the block is now deprecated and replaced with a style attribute.
• The component now also includes the inspector controls and creates a the block toolbar

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

• adding to existing features

Testing instructions:

• Add an Eventbrite block
• Add an event
• Check that you can still toggle between a button and an inline embed

• Check that the Calendly, OpenTable and EventBrite blocks all have a Toolbar which allows you to change the embed style:

• You also need to test the deprecation for old Eventbrite blocks using the old attributes.
• Create an Eventbrite block using master
• Set the Embed type to be "Modal"
• Save the post
• Check out this branch
• Reload the editor
• Check that the block is still in modal type
• Open the editor in code view and check you see "style":"modal" in the block attributes.

Proposed changelog entry for your changes:

• no changelog

[matticbot]

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

[matticbot]

scruffian, Your synced wpcom patch D38351-code has been updated.

[jetpackbot]

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: April 7, 2020.
Scheduled code freeze: March 31, 2020

Generated by 🚫 dangerJS against b1d75b0

[matticbot]

scruffian, Your synced wpcom patch D38351-code has been updated.

[matticbot]

scruffian, Your synced wpcom patch D38351-code has been updated.

[creativecoder]

While I appreciate the code consolidation, I think an image is a better fit for the Eventbrite in page embed preview

• The preview is small enough that it's hard to read the text, so there's not really a benefit to having "live" updates
• We don't have to worry about maintaining an external event that could expire or be deleted and break the preview
• There's a significant delay for the embed to render, long enough that it feels like it might be broken when it first loads

The Button & Modal embed does benefit from a live preview, because it shows the button with the style and color defaults of the active theme.

Two ideas to reconcile this

• Modify the BlockStylesSelector to accept a customized preview (like an image), OR
• Expose a more granular component, like BlockStylesSelectorItem, that can be used for just one of the block styles

Then we could keep the image for the In Page preview and have a dynamic preview for the Button & Modal. What do you think?

[scruffian]

We don't have to worry about maintaining an external event that could expire or be deleted and break the preview

I'm not sure what you mean by this, sorry

[creativecoder]

We don't have to worry about maintaining an external event that could expire or be deleted and break the preview

I mean the example url for the block. If the Eventbrite event at that url is in the past, deleted, or otherwise not working, the preview for the In Page embed won't show correctly.

[scruffian]

I mean the example url for the block. If the Eventbrite event at that url is in the past, deleted, or otherwise not working, the preview for the In Page embed won't show correctly.

Ah yes, good point. I'm also not sure why we use the example attributes, rather than previewing with the actual URL the user enters. That's how the core block works, but it doesn't make a lot of sense to me.

I'll think about it some more...

[scruffian]

The BlockStyleSelector component now renders a preview if one is passed, otherwise it uses the BlockPreview component, so I think this is ready for another review.

[Copons]

I think we have an issue with how we slot-fill the various block controls, that is causing the toolbar controls to never show up.

The way we are using the BlockStylesSelector in the various blocks is inside the InspectorControls slot:

<InspectorControls>
  <PanelBody>
    <BlockStylesSelector />
  </PanelBody>
</InspectorControls>

But then, inside of BlockStylesSelector we also define the Style toolbar button, which is supposed to be rendered inside the BlockControls slot:

<BlockControls>
  <Toolbar />
</BlockControls>

In my tests, the Style button never shows up in the block toolbar.
I've tried moving a bunch of stuff around, and it turns out we could solve the issue and DRY-fy this a bit more.

// block edit.js
<BlockStylesSelector />

// BlockStylesSelector
<BlockControls>
  <Toolbar />
</BlockControls>

<InspectorControls>
  <PanelBody>
    <div className="block-editor-block-styles jetpack-block-styles-selector">
      { /* ... */ }
    </div>
  </PanelBody>
</InspectorControls>

[scruffian]

In my tests, the Style button never shows up in the block toolbar.

Interesting, it shows for me:

If removing the nesting fixes it for you though then we might as well do it. Maybe you're running an older version of Gutenberg?

[Copons]

Maybe you're running an older version of Gutenberg?

That was it.
I was testing with whatever version is included in WP 5.3.2.
I've tried with Guten plugin 7.4, and indeed the button shows up regardless.

If removing the nesting fixes it for you though then we might as well do it.

Yeah, it would be a nice compatibility fix, while also helping to keep things simple. 🙂

[Copons]

This works fine, but there seems to be some visual inconsistencies with the latest Gutenberg (7.7, but checked out from master right now), that doesn't happen without the Gutenberg plugin.

Block	No Plugin	Gutenberg 7.7
Calendly
Eventbrite
OpenTable

[scruffian]

Thanks for finding this @Copons. This will be an issue even if we don't merge this PR, so I think we should go ahead and merge and then fix in a different issue.

[Copons]

This will be an issue even if we don't merge this PR, so I think we should go ahead and merge and then fix in a different issue.

Makes sense @scruffian, go for it!

[jeherve]

This looks good to me. I agree that the inconsistencies between Gutenberg versions aren't ideal though, but we can address this in a follow-up PR.

[creativecoder]

This works fine, but there seems to be some visual inconsistencies with the latest Gutenberg (7.7, but checked out from master right now), that doesn't happen without the Gutenberg plugin.

@Copons Thanks for finding these! Can you please open a Jetpack issue for these (if there isn't one already) and add to our global backlog?

[matticbot]

scruffian, Your synced wpcom patch D38351-code has been updated.

[scruffian]

r204888-wpcom