Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blank Canvas: Add the "Subscription Form and Links" pattern #3416

Merged
merged 5 commits into from
Mar 10, 2021

Conversation

kjellr
Copy link
Contributor

@kjellr kjellr commented Mar 8, 2021

Adds the Jetpack Subscribe + Links pattern from #3161, now that Automattic/jetpack#18085 has been resolved. This PR also properly escapes the wpcom-only block pattern strings + URLs to match up with the changes made in #3345.

Two small caveats:

  • The design is changed slightly from the mockup in the other thread: I've removed the noisy background so that the form field text is more readable.
  • It's pretty obvious that the two 50% buttons don't add up to the full width of the container. This is a Gutenberg bug, and there's a potential fix here. I don't think that should hold this PR up though.

Screenshots

Editor:

kjellreigstad wordpress com_wp-admin_post php_post=2075 action=edit(1200x900)

Front End:

Desktop Mobile
kjellreigstad wordpress com_(1200x900) kjellreigstad wordpress com_(iPhone X)

@kjellr kjellr requested a review from a team March 8, 2021 17:34
@kjellr kjellr self-assigned this Mar 8, 2021
@jffng
Copy link
Contributor

jffng commented Mar 8, 2021

I got a block validation error when I added this pattern:

Screen Shot 2021-03-08 at 1 52 12 PM

Clicking "Attempting Block Recovery" worked though.

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.

LGTM. I didn't see the validation error. The jetpack form looks very different on the front/back but that's outside of the scope of this...

@kjellr
Copy link
Contributor Author

kjellr commented Mar 9, 2021

@jffng were you testing on WP.com? I just want to make sure we can't reproduce before merge.

@jffng
Copy link
Contributor

jffng commented Mar 9, 2021

Yes — tested on a simple site. I will give it another spin on different site to see if I can still recreate it.

@jffng
Copy link
Contributor

jffng commented Mar 9, 2021

Confirm this happens every time I insert the pattern on a sandboxed simple site. This is the console error:

index.js:13 Block validation: Block validation failed for `jetpack/subscriptions` (Object).

Content generated by `save` function:

<div class="wp-block-jetpack-subscriptions wp-block-jetpack-subscriptions__supports-newline">
			[jetpack_subscription_form
				subscribe_placeholder="Enter your email address"
				show_subscribers_total="false"
				button_on_newline="false"
				submit_button_text="Sign Up"
				custom_background_emailfield_color="undefined"
				custom_background_button_color="undefined"
				custom_text_button_color="undefined"
				custom_font_size="16"
				custom_border_radius="0"
				custom_border_weight="1"
				custom_border_color="undefined"
				custom_padding="16"
				custom_spacing="10"
				submit_button_classes="has-text-color has-primary-color has-background has-background-background-color"
				email_field_classes=""
				show_only_email_and_button="true"
			]</div>

Content retrieved from post body:

<div class="wp-block-jetpack-subscriptions wp-block-jetpack-subscriptions__supports-newline">[jetpack_subscription_form subscribe_placeholder="Enter your email address" show_subscribers_total="false" button_on_newline="false" custom_font_size="undefinedpx" custom_border_radius="0" custom_border_weight="1" custom_padding="16" custom_spacing="10" submit_button_classes="has-undefinedpx-font-size has-text-color has-primary-color has-background has-background-background-color" email_field_classes="has-undefinedpx-font-size" show_only_email_and_button="true"]</div>

@kjellr
Copy link
Contributor Author

kjellr commented Mar 9, 2021

@jffng I tried on a couple test sites and was unable to reproduce. Is your sandbox up to date?

@jffng
Copy link
Contributor

jffng commented Mar 9, 2021

It was not! Working now 🚢

@kjellr kjellr merged commit 2e3029d into trunk Mar 10, 2021
@kjellr kjellr deleted the add/new-link-in-bio-pattern branch March 10, 2021 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants