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

Subscriptions block updates #15107

Merged
merged 58 commits into from
May 1, 2020
Merged

Subscriptions block updates #15107

merged 58 commits into from
May 1, 2020

Conversation

apeatling
Copy link
Member

@apeatling apeatling commented Mar 24, 2020

Update the email subscription block to significantly improve the settings for customizing the display and style of the form.

Before After
before after

Fixes #15076

Testing instructions:

  • Insert a subscriptions form block on your site and try all of the new settings in the sidebar for customization.
  • Save the post and confirm the settings and design updates pass through to the front end of the site.
  • Add the subscription form widget to a widget area and confirm this still renders and works correctly.

To test old blocks:

  • Add the block in master
  • Check out this branch
  • Check that the block looks ok in the editor and in the frontend

Proposed changelog entry for your changes:

  • Will update this later.

@matticbot
Copy link
Contributor

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

@Automattic Automattic deleted a comment from jetpackbot Mar 25, 2020
@Automattic Automattic deleted a comment from matticbot Mar 25, 2020
@Automattic Automattic deleted a comment from matticbot Mar 25, 2020
@apeatling
Copy link
Member Author

apeatling commented Mar 25, 2020

Status so far:

  • Removed the subscriber count setting from the canvas to the sidebar.
  • Moved the input field and submit button to a single line.
  • Added support for gradients, border radius, and font sizing to the submit button.

In terms of styling I also think there needs to be control of:

  • Input field border radius and font sizing
  • Padding on both the submit button and input field
  • Space between the input field and submit button
  • Dropping the submit button down onto a new line
  • Positioning and possibly more fine grained design control over the subscriber count text.

/cc @mtias

@Automattic Automattic deleted a comment from jetpackbot Mar 25, 2020
@Automattic Automattic deleted a comment from jetpackbot Mar 25, 2020
@mtias
Copy link
Member

mtias commented Mar 25, 2020

Nice work!

Input field border radius and font sizing

I think this should be a single control that always applies to both the button and the input to make it feel in concert.

@mtias
Copy link
Member

mtias commented Mar 25, 2020

I guess with the submit button being a "shared" resource we need to check impact on other blocks consuming it.

@apeatling
Copy link
Member Author

Thanks! Will review this feedback this afternoon. With all the shared button changes the other blocks will definitely need an update, I can handle this once all the changes to this block are done. If it ends up being a large amount of work, I can separate the button changes and work on that in a different PR.

@Automattic Automattic deleted a comment from jetpackbot Mar 25, 2020
@apeatling
Copy link
Member Author

I've now switched this to not use the shared submit button. Instead using standard controls and bringing the settings into the actual block itself. Styles are passed through and applied to both elements.

@Automattic Automattic deleted a comment from jetpackbot Mar 26, 2020
@Automattic Automattic deleted a comment from jetpackbot Mar 26, 2020
@jetpackbot
Copy link

jetpackbot commented Mar 26, 2020

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 f0635f4

@apeatling
Copy link
Member Author

Range control components don't seem to work correctly on zero values. For example try the buttons block and drag the border radius slider to zero. The value in the textfield remains at 5? The actual border radius is set as zero though. Known bug?

@mtias
Copy link
Member

mtias commented Mar 26, 2020

Mmm, yes, I'm seeing the same issue. Perhaps related to: WordPress/gutenberg#20247

…er's website until they have at least one subscriber.
jeherve
jeherve previously approved these changes May 1, 2020
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.

Approving once again after a rebase.

@jeherve 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 May 1, 2020
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.

Missed a commit in my last rebase.

@jeherve jeherve merged commit 3466b06 into master May 1, 2020
@jeherve jeherve deleted the update/subscriptions-block branch May 1, 2020 16:23
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels May 1, 2020
@jeherve jeherve added the [Status] Needs Testing We need to add this change to the testing call for this month's release label May 1, 2020
@apeatling
Copy link
Member Author

🎉

@Copons
Copy link
Contributor

Copons commented May 5, 2020

I've noticed that this PR hasn't been synced to WPCOM yet!
When you do this please note that I've done a tiny fix to the block icon in #15664.

@apeatling
Copy link
Member Author

Will do this later today 👍

@apeatling
Copy link
Member Author

@Copons Looks like this can't be merged until June 2nd according to @jeherve on the Phab diff.

@jeherve jeherve added [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 May 26, 2020
jeherve added a commit that referenced this pull request May 26, 2020
@jeherve
Copy link
Member

jeherve commented Jun 2, 2020

r208345-wpcom

@mkaz
Copy link
Contributor

mkaz commented Sep 2, 2020

I came to the repo to submit a feature request, and here it is already done. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Subscriptions Blocks V2 [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.

Block: Subscription Form Improvements