Calendly Block: Replace SubmitButton with new button block

[aaronrobertshaw]

Fixes #15711, Fixes #14564

Changes proposed in this Pull Request:

• Replace the SubmitButton in the Calendly block with new shared Button block.
• Fixes minor bug where Calendly spinner for inline style widget isn't hidden when there are multiple Calendly blocks on the page.
• Switch to shared Button block also fixes issue where custom text color wasn't shown in editor.

Prerequisites:

• Calendly account to create blocks and embed codes
• Post with Calendly blocks created prior to applying this PR's changes.
  1. Create a post.
  2. Add two Calendly blocks using the link style.
  3. Configure one of those blocks setting its various options e.g. background color.
  4. Add one inline Calendly block.
  5. Save post.
• Have the AMP plugin installed and active

Testing instructions:

1. On the frontend, visit post containing previous versions of Calendly block.
2. Confirm the blocks display and function as they did when you set them up.
   Should still be using simple <a> markup.
3. Edit the post.
4. Confirm the Calendly blocks still appear correctly and their settings are correct.
5. Update the link style blocks using the new settings controls provided by button block.
6. Add new Calendly block using simple URL e.g. https://calendly.com/<username>
7. Add new Calendly block using customized embed code for a link style widget.
8. Check that the customized attributes from the embed code have been set on the button.
9. Save the post.
10. Re-visit the post on the frontend and confirm display and function of blocks.
11. Switch back to the editor and update the embed url under the Inspector Controls > Calender Settings with a link style URL containing custom colors.
12. Confirm button colors update, save and switch to the frontend view to check there as well.
13. Click the AMP link at the top of the page and confirm links work on the AMP request view.
    ( inline style block will show as a simple link )
14. Run tests in extensions/blocks/calendly/test/utils.js

Before:

After:

Proposed changelog entry for your changes:

• Update Calendly block to use new shared Button block.

[jetpackbot]

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

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 🤖

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

Generated by 🚫 dangerJS against f450dba

[Copons]

This works well! @aaronrobertshaw
I've left a few minor comments, and then I've got a major one below. 😅

Just one thing:

Fixes minor bug where Calendly spinner for inline style widget isn't hidden when there are multiple Calendly blocks on the page.

This is happening to me regardless. 🤔
Tested with two Calendly blocks: a widget and a button.

---

I've took the liberty of assigning you to #15498, which is directly related to this PR.

As it is right now, the button doesn't work in AMP.
(You can try it yourself with the AMP plugin, previewing the post with the AMP button).

The idea proposed in #15498 was to replace the button with a link when in AMP.
But, looking at the Calendly block, I don't see any good reasons to render the Button as a button.
Instead, we could set it as an element: 'a' and pass the Calendly link to the url attribute.
In AMP it should just work as-is, while we could simply add a event.preventDefault() when we attach the click handler.
Having the Button as a link, it wouldn't be stripped away by the WPCOM KSES, and so we could even go as far as to save in the post content (handy Button saveInPostContent attribute 😄).

There is a gotcha though: to pass the Calendly link from the parent block to the child Button, and keep the value in sync, we need a small update to the Button block, based upon an old discussion.
PR opened at #15931

[aaronrobertshaw]

Thanks @Copons, appreciate all the suggestions and direction on this one. Especially the AMP side of things, I was a little at a loss on how to test that.

But, looking at the Calendly block, I don't see any good reasons to render the Button as a button. Instead, we could set it as an element: 'a' and pass the Calendly link to the url attribute.
In AMP it should just work as-is, while we could simply add a event.preventDefault() when we attach the click handler.

I agree regarding this block only needing to render as a link. I've updated it accordingly and also tweaked it to pass those attributes through to the button block. Happy to come back, rebase this and update to use the passthroughAttributes from #15931 once it's merged or as a separate PR.

While making these updates I found I'd missed updating the button block's attributes when the embed URL from the sidebar calendar settings section was changed and contained embedded styles. These sound like another candidate to use the passthroughAttributes with as well.

The buttons work ok for me now via AMP although if there are no custom colors used to provide inline styles, their appearance could be sketchy. The block's inline style, that embeds an iframe, obviously wasn't working either, so I've made that just fallback to a simple link. Do you think it's worth just making the block render a simple link for all AMP requests to avoid styling inconsistencies?

Fixes minor bug where Calendly spinner for inline style widget isn't hidden when there are multiple Calendly blocks on the page.

This is happening to me regardless. 🤔
Tested with two Calendly blocks: a widget and a button.

I might not have been clear on that one, sorry. The spinner was just appearing outside the bounds of the block's container for me. The fix was just adding a simple position: relative on that inline style container so its positioning appeared ok until it was eventually covered by the iframed widget.

Before:

After:

Can you confirm it's still an issue?

[Copons]

As mentioned internally, I've noticed that for some reasons using the Button with InnerBlocks is preventing the Calendly block from being selected.
I'm not sure why this happens, but I've noticed that it's a regression from one of last night / yesterday commits, since in 265ec83 blocks are selectable.

---

The spinner was just appearing outside the bounds of the block's container for me. The fix was just adding a simple position: relative on that inline style container so its positioning appeared ok until it was eventually covered by the iframed widget.

I see, and yes, the spinner is not in that awkward position anymore.
Though, now it's below the initial state of the widget:

It never disappears, so it's not clear what it does.
Though, when we click on an event, the spinner gets covered.

I guess we could reposition it where it would always be covered by the widget.
E.g. we could override the .calendly-spinner{ top: 50% } (set by the widget's own CSS) to something like top: 50px:

(In the screenshot I've also changed the z-index to make it appear above the widget)

[aaronrobertshaw]

Thanks for highlighting the issue with the spinner, I hadn't tried the inline widget with fewer than 2 meeting durations. I've tidied that up.

As mentioned internally, I've noticed that for some reasons using the Button with InnerBlocks is preventing the Calendly block from being selected.
I'm not sure why this happens, but I've noticed that it's a regression from one of last night / yesterday commits, since in 265ec83 blocks are selectable.

I'm not sure I follow regarding the Calendly blocks not being selectable. In the editor, for me, clicking the blank space beside the button block selects the Calendly block itself. Alternatively, moving between the blocks with the arrow keys still lets me select the Calendly block.

On the frontend, I found an inconsistency with which element the block ID was added to, between the deprecated and current versions of the link style blocks. I've fixed that up.

Hopefully, that sorts out the issue.

[Copons]

@aaronrobertshaw Apparently the broken selection only happens with the Guten plugin activated. Tested with 8.1 and 8.2 (freshly checked out this morning).

[Copons]

@aaronrobertshaw I've finally run a git bisect and found what caused the selection issue (but not why).
The bad commit was bd30818 (Remove unnecessary definition of innerButtonBlock), so my best guess is that on recent Gutens, if you don't add the inner blocks to the example, the example will break (silently!), and the sidebar can't open, messing up the selection.
I might be wrong, but I've pushed fdedc24 which fixes the issue for me.

[aaronrobertshaw]

@Copons Thanks for taking the time to look into that one. I understand the issue you were highlighting now, the new commit also has it working properly for me too.

[Copons]

Just a heads up that I've merged #15931 (the passthroughAttributes one)!

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello aaronrobertshaw! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D45004-code before merging this PR. Thank you!
This revision will be updated with each commit to this PR

[jeherve]

In my tests, I seem to be losing the color of the existing buttons when I update to this branch.

Steps to reproduce

1. On master, create a new Calendly block with a button style
   2 Give a custom background color to your button, something that's not among the predefined options.
2. Publish your post.
3. Switch to this branch.
4. On the frontend of your site, the button color is back to the default.

<div class="wp-block-jetpack-calendly aligncenter calendly-style-link"><a id="calendly-block-2" class="wp-block-button__link has-background" href="https://href.li/?https://calendly.com/jeherve?hide_event_type_details=0&amp;background_color=ffffff&amp;text_color=4D5055&amp;primary_color=00A2FF" role="button" rel="noreferrer">Click to book a time slot</a></div>

6. In the editor, the color is visible.

<div aria-label="Block: Button" role="group" id="block-77202acd-6fb7-4965-b6be-04819eb3a9b8" class="block-editor-block-list__block wp-block" data-block="77202acd-6fb7-4965-b6be-04819eb3a9b8" data-type="jetpack/button" data-title="Button" tabindex="0" style="transform-origin: center center;"><div> <div class="wp-block-button wp-block-jetpack-button"><div role="textbox" aria-multiline="true" aria-label="Add text…" class="rich-text block-editor-rich-text__editable wp-block-button__link has-background" contenteditable="true" style="background-color: rgb(6, 172, 9); white-space: pre-wrap;">Click to book a time slot</div></div> </div></div>

7. Opening that post in the editor triggers the migration and we get our color back:

<a class="wp-block-button__link has-background" style="background-color: #06ac09;" data-id-attr="calendly-block-2" id="calendly-block-2" href="https://href.li/?https://calendly.com/jeherve" target="_blank" role="button" rel="noopener noreferrer noreferrer">Click to book a time slot</a>

Is there any way to avoid that?

---

Other than that, everything seems to work well in my tests.

[fgiannar]

Please note the following as well:

I tried to add a new Calendly block using customized embed code with custom page settings, like:
https://d.pr/i/3ls3Vt

The custom page settings were lost.
Seems that the button href attribute was missing the query string : ?background_color=212020&text_color=801249&primary_color=19ff00

[aaronrobertshaw]

The custom styles are now correctly applied to the deprecated button element and custom page settings for embed codes have been fixed.

I also added a style to provide the default font color, that a wrapping .wp-block-button did before the switch to the shared button component. This fixes up an issue where buttons with visited URLs inherited a black font color.

[fgiannar]

Thanks for the fixes @aaronrobertshaw
Tested both for @jeherve and my comments as well and looks fine!

[jeherve]

r210102-wpcom