Revue Block #14645

Copons · 2020-02-11T18:43:59Z

Changes proposed in this Pull Request:

Add a new Revue block.

For this simple implementation we have decided to not use the Revue API, but simply show a subscription form, linked to a Revue user through its username, that on submit opens Revue in a new tab.

Editor	Front End

Notes

I've decided to reimplement (yet again) the submit button logic, copying it from the latest Gutenberg version (as of last week) that also has gradient backgrounds available, instead of using our own SubmitButton.
This is mostly to be as consistent as possible to Core Gutenberg, both on the edit side and on the SSR.
I recognize this is definitely not ideal, and I'd rather use a single button implementation that was as similar to Core Gutenberg's Button block.
@pablinos (since you're working on it) do you think the work in this PR could be a good starting point for a SubmitButton update?
Form styles aren't typically exposed by the theme to the editor to avoid conflicts with the editor styles. For this reason, the design is slightly different between editor and front end.
I don't have a good solution for this off the top of my head, but maybe I'm missing something. 🤔
Since the submit opens a new tab, going back to the post and seeing the form as nothing happened felt a bit awkward to me.
Currently, 1 second after the submission, when the user is supposedly on the Revue page, the form is replaced by a "You can complete your registration in the new page." message.
This message is not permanent, and it's enough to reload the post to see the form again.
I'm not sold on the copy or the look, and I guess we could embellish them in several ways.
What do you think? (cc @davemart-in)

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

pb5gDS-jZ-p2

Testing instructions:

Create an account on Revue.
Open the editor and insert a Revue block.
Enter your Revue username.
Try customizing the submit button.
Save and view the post.
Make sure the button show up with the same customizations in the front end.
Fill the form with an email address (not the same as the Revue account).
On submit you should open a Revue page with the next steps instructions.
Check your email for a Revue email to confirm that the subscription happened (whatever happens next is in their hands).

Proposed changelog entry for your changes:

Add a new Revue block.

jetpackbot · 2020-02-11T18:47:45Z

	Warnings
⚠️	`pre-commit` hook was skipped for one or more commits

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 🤖

Generated by 🚫 dangerJS against b9aa5a0

extensions/blocks/revue/index.js

scruffian · 2020-02-20T17:14:03Z

This block looks very similar to our Jetpack Contact Form block. Is there any way we could reuse some of that here?

extensions/blocks/revue/edit.js

extensions/blocks/revue/index.js

extensions/blocks/revue/revue.php

scruffian · 2020-02-20T17:32:54Z

For the button, is there any reason we can't use the core button block as a child block?

matticbot · 2020-02-20T17:47:33Z

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 D39155-code before merging this PR. Thank you!

Copons · 2020-02-20T17:50:12Z

This block looks very similar to our Jetpack Contact Form block. Is there any way we could reuse some of that here?

@scruffian It occurred to me as well, but then in this case we don't really want for the user to customize the form at all.

The email field is required (I'd have to double check if Revue also expects a member[subscribe] property from the submit button).
The fields need to have a precise name (e.g. member[email]).
JPCF lets user change the input names but only by assigning an ID to them, which is not great if we want to have multiple Revue subscription blocks in the same page.

This said, I've tried to keep the two as visually consistent as possible in the front end.

For the button, is there any reason we can't use the core button block as a child block?

Admittedly I haven't tried, but off the top of my head, the Button block outputs an anchor element, which AFAIK can't be used to submit a form without unnecessarily involving JS.

Also, even if Button saved as button or input type="submit", it would be stripped away by WPCOM's KSES on post save.
I haven't tested it yet, but server-side-rendering it straight when the post is displayed should avoid it.

extensions/blocks/revue/button-preview.js

extensions/blocks/revue/revue.php

davemart-in · 2020-02-21T12:33:01Z

Thanks @Copons! Here are some thoughts after my review:

Can we update the block icon to use this SVG?
In the blank slate let's add a link under the username field that says "Need help finding your Revue username?" which will link to the support doc.
In the sidebar, let's change the description from "Add a Revue signup form." to "Add a subscription form for your Revue newsletter."
The blank slate input and submit button are not vertically aligned. Can we update these to match how we've done it on the Calendly block?
Can we update the blank slate button text from "Insert" to "Add Form"?
Since first and last name are optional, can we show toggles for them in the sidebar?
I'm not entirely sure what the bold, italic, and rich text controls are meant to apply to. Clicking them doesn't seem to do anything. Can we remove them?
"Edit as HTML" in the toolbar dropdown produces this. Do we want to just hide that option?
Let's change the on submission text from "You can complete your registration in the new page." to Subscription received!
Please check your email to confirm your newsletter subscription.

davemart-in · 2020-02-24T12:09:51Z

The form's behaviour in the editor (with the "Preview only" placeholder) is very different from what we're used to with other form blocks. I expected to be able to click around in the form while in the editor, to change some of the labels, change placeholders, reorder fields.

Since we already have our own form blocks like Mailchimp, Subscription, and the Form block, I think we should aim for a somewhat consistent experience between all those.

You can read about the rationale in pb5gDS-iT-p2

Copons · 2020-02-24T13:27:37Z

@jeherve @davemart-in
In 8e665f9 I've tried having the form work almost like the Contact Form block.

The two blocks' needs are slightly different, so the implementation is slightly different as well.
Revue format is strict: a required email field called member[email] and two optional name fields called member[first_name] and member[last_name].
Contact Form can be customized entirely by adding all kinds of fields, removing them, changing their IDs, etc.

So, instead of having a container block with fields as children blocks (like Contact Form), I've hardcoded the three fields (names are still toggleable from the sidebar), and made the label and placeholder customizable.

To customize the label, I've had to add a new prop to Contact Form's JetpackFieldLabel; it's not a breaking change, but it adds a few lines of code to the diff.

Now: I realize this is a very last minute change, so if we think it's too big to be tested properly, I can drop it from the PR and open it in a follow up.

jeherve

This looks good to me. I only have some minor comments.

jeherve · 2020-02-24T16:17:46Z

extensions/blocks/revue/revue.php

+	>
+		<div>
+			<label>
+				<?php echo $email_label; ?>


Should we escape this?

You're right, this was a big distraction 😞

Updated in b9aa5a0

extensions/blocks/revue/revue.php

glendaviesnz · 2020-02-24T18:39:33Z

@davemart-in with Calendly, OpenTable and Google Cal, we removed the Edit option in the block tool bar which took the block back to preview mode to update the embed code, and instead added the embed code setting to the sidebar.

In the Review block it has the edit option in the toolbar to update the username, should this be moved into the settings panel also for consistency with the other blocks?

glendaviesnz · 2020-02-24T20:53:54Z

extensions/blocks/revue/edit.js

+					<TextControl
+						label={
+							<JetpackFieldLabel
+								label={ emailLabel }


A very minor thing, but in the edit view of the label field the text is not vertically centred, but in the other form fields it is:

glendaviesnz · 2020-02-24T21:38:45Z

extensions/blocks/revue/edit.js

+
+	const saveUsername = event => {
+		event.preventDefault();
+		setAttributes( { revueUsername: username } );


Should we add a .trim() to username here? currently if a user copies and pastes some spaces along with their username by mistake they get a 404 from revue when trying to submit the form.

jeherve

I think we're in a good place now, and I would suggest merging this as is and addressing the feedback that was left in future PRs.

jeherve · 2020-02-24T21:41:31Z

extensions/blocks/revue/revue.php

+	ob_start();
+	?>
+
+<div class="wp-block-jetpack-revue">


Let's use Jetpack_Gutenberg::block_classes() here maybe?

glendaviesnz

I agree with @jeherve, this is in a good place to merge now and then follow up with another PR for the other suggested changes.

jeherve · 2020-02-24T23:12:16Z

Merging for now. @Copons I'll let you handle the WordPress.com part as well as the remaining feedback? Thank you!

davemart-in · 2020-02-25T10:10:20Z

@davemart-in with Calendly, OpenTable and Google Cal, we removed the Edit option in the block tool bar which took the block back to preview mode to update the embed code, and instead added the embed code setting to the sidebar.
In the Review block it has the edit option in the toolbar to update the username, should this be moved into the settings panel also for consistency with the other blocks?

As @jeherve suggested, I think this is good enough to ship.

I thinking circling back around and updating this block to remove the Edit option is a good idea after this first iteration ships. Let's go ahead and move that to the sidebar at that point.

Copons · 2020-02-25T12:16:45Z

r203320-wpcom

* 8.3 release: changelog * Changelog: add #14516 * Changelog: add #14574 * Bring in changes from 8.2.1 and 8.2.2 * Update stable version * Bring in 8.2.3 changes * Changelog: add #14714 * Changelog: add #14639 * Changelog: add #14678 * Changelog: add #14673 * Changelog: add #14687 * Changelog: add #14704 * Changelog: add #14702 * Changelog: add #14541 * Changelog: add #14657 * Changelog: add #14622 * Changelog: add #14582 * Changelog: add #14638 * Changelog: add #14633 * Changelog: add #14571 * Changelog: add #14592 * Changelog: add #14539 * Changelog: add #14514 * Changelog: add #14643 * Changelog: add #14494 * Changelog: add #13739 * Changelog: add #14707 * Changelog: add #14736 * Changelog: add #14706 * Changelog: add #14730 * Changelog: add #14685 * Changelog: add #14727 * Changelog: add #14711 * Changelog: add #14742 * Changelog: add #14746 * Changelog: add #14725 * Changelog: add #13999 * Changelog: add #14740 * Changelog: add #14759 * Changelog: add #14703 * Changelog: add #14753 * Changelog: add #14754 * Changelog: add #14645 * Cahngelog: add #14599

Copons added the [Status] In Progress label Feb 11, 2020

Copons requested a review from a team as a code owner February 11, 2020 18:43

Copons self-assigned this Feb 11, 2020

matticbot added the Touches WP.com Files label Feb 11, 2020

jeherve added [Block] Revue [Type] Feature Request [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Feb 11, 2020

Copons force-pushed the add/revue-block branch from b1ebc3a to a80efae Compare February 13, 2020 18:42

Copons requested a review from a team February 19, 2020 16:13

Copons added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Feb 19, 2020

Copons changed the title ~~[WIP] Revue Block~~ Revue Block Feb 19, 2020

Copons force-pushed the add/revue-block branch from 376a59d to 4eae567 Compare February 20, 2020 17:02

scruffian reviewed Feb 20, 2020

View reviewed changes

extensions/blocks/revue/index.js Outdated Show resolved Hide resolved

scruffian reviewed Feb 20, 2020

View reviewed changes

extensions/blocks/revue/edit.js Show resolved Hide resolved

scruffian reviewed Feb 20, 2020

View reviewed changes

extensions/blocks/revue/index.js Outdated Show resolved Hide resolved

scruffian reviewed Feb 20, 2020

View reviewed changes

extensions/blocks/revue/revue.php Show resolved Hide resolved

glendaviesnz reviewed Feb 21, 2020

View reviewed changes

extensions/blocks/revue/button-preview.js Outdated Show resolved Hide resolved

glendaviesnz reviewed Feb 21, 2020

View reviewed changes

extensions/blocks/revue/button-preview.js Outdated Show resolved Hide resolved

glendaviesnz reviewed Feb 21, 2020

View reviewed changes

extensions/blocks/revue/revue.php Outdated Show resolved Hide resolved

jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Feb 21, 2020

Copons added 2 commits February 21, 2020 15:55

[not verified] [not verified] Create a new Revue block

bbdc557

[not verified] Use the official subscription form

5af45e8

jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Feb 24, 2020

Copons added 3 commits February 24, 2020 12:16

Simplify the block's view

b49c0bf

Improve button preview and save

f1a155d

In the editor, make the form work similar to the Contact Form block

8e665f9

jeherve requested changes Feb 24, 2020

View reviewed changes

Properly escape all the strings

b9aa5a0

glendaviesnz reviewed Feb 24, 2020

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Feb 24, 2020

jeherve approved these changes Feb 24, 2020

View reviewed changes

glendaviesnz approved these changes Feb 24, 2020

View reviewed changes

jeherve merged commit c70ed04 into master Feb 24, 2020

jeherve deleted the add/revue-block branch February 24, 2020 23:10

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 24, 2020

jeherve added [Status] Needs Testing We need to add this change to the testing call for this month's release [Status] Has Changelog and removed [Status] Needs Changelog [Status] Needs Testing We need to add this change to the testing call for this month's release labels Feb 24, 2020

jeherve added a commit that referenced this pull request Feb 25, 2020

Changelog: add #14645

08999cc

Copons mentioned this pull request Feb 25, 2020

Revue Block: Minor Fixes #14792

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revue Block #14645

Revue Block #14645

Copons commented Feb 11, 2020 •

edited

Loading

jetpackbot commented Feb 11, 2020 •

edited

Loading

scruffian commented Feb 20, 2020

scruffian commented Feb 20, 2020

matticbot commented Feb 20, 2020

Copons commented Feb 20, 2020

davemart-in commented Feb 21, 2020 •

edited

Loading

davemart-in commented Feb 24, 2020 •

edited by kraftbj

Loading

Copons commented Feb 24, 2020 •

edited

Loading

jeherve left a comment

jeherve Feb 24, 2020

Copons Feb 24, 2020

glendaviesnz commented Feb 24, 2020

glendaviesnz Feb 24, 2020

glendaviesnz Feb 24, 2020

jeherve left a comment

jeherve Feb 24, 2020

glendaviesnz left a comment

jeherve commented Feb 24, 2020

davemart-in commented Feb 25, 2020 •

edited

Loading

Copons commented Feb 25, 2020

Revue Block #14645

Revue Block #14645

Conversation

Copons commented Feb 11, 2020 • edited Loading

Changes proposed in this Pull Request:

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

Testing instructions:

Proposed changelog entry for your changes:

jetpackbot commented Feb 11, 2020 • edited Loading

scruffian commented Feb 20, 2020

scruffian commented Feb 20, 2020

matticbot commented Feb 20, 2020

Copons commented Feb 20, 2020

davemart-in commented Feb 21, 2020 • edited Loading

davemart-in commented Feb 24, 2020 • edited by kraftbj Loading

Copons commented Feb 24, 2020 • edited Loading

jeherve left a comment

Choose a reason for hiding this comment

jeherve Feb 24, 2020

Choose a reason for hiding this comment

Copons Feb 24, 2020

Choose a reason for hiding this comment

glendaviesnz commented Feb 24, 2020

glendaviesnz Feb 24, 2020

Choose a reason for hiding this comment

glendaviesnz Feb 24, 2020

Choose a reason for hiding this comment

jeherve left a comment

Choose a reason for hiding this comment

jeherve Feb 24, 2020

Choose a reason for hiding this comment

glendaviesnz left a comment

Choose a reason for hiding this comment

jeherve commented Feb 24, 2020

davemart-in commented Feb 25, 2020 • edited Loading

Copons commented Feb 25, 2020

Copons commented Feb 11, 2020 •

edited

Loading

jetpackbot commented Feb 11, 2020 •

edited

Loading

davemart-in commented Feb 21, 2020 •

edited

Loading

davemart-in commented Feb 24, 2020 •

edited by kraftbj

Loading

Copons commented Feb 24, 2020 •

edited

Loading

davemart-in commented Feb 25, 2020 •

edited

Loading