Skip to content

Contact Form Block: Fix Missing Button Color Attributes#14898

Merged
Copons merged 3 commits intomasterfrom
fix/contact-form-button-colors
Mar 5, 2020
Merged

Contact Form Block: Fix Missing Button Color Attributes#14898
Copons merged 3 commits intomasterfrom
fix/contact-form-button-colors

Conversation

@Copons
Copy link
Copy Markdown
Contributor

@Copons Copons commented Mar 5, 2020 •
edited
Loading

Changes proposed in this Pull Request:

The Contact Form block was missing the textButtonColor and backgroundButtonColor attributes, necessary to make it work with the SubmitButton shared component.
This practically means that, currently, you can change the colors of the Contact Form submit button, you will see the button preview updating correctly (as the colors are applied as CSS classes in the editor), but if you save the post and reload, the colors are not saved.
This only happens for "named" colors; custom colors work fine, as their attributes are not missing.

This PR adds those missing two attributes to the Contact Form block.
Also, externalizes the colourValidator utility that was being carelessly copypasted around.
I've also americanized the function name, as it's more consistent with the fact that we use color everywhere, and I'm ready to fight my British colleagues about it! 😄

Note: most changed lines here are automatic Prettier changes on the test file. 😭

Testing instructions:

  • Insert a Contact Form block.
  • Modify the button colors, picking from the named colors palette.
  • Make sure the button updates correctly.
  • Save and reload the editor.
  • Make sure the button shows up with the correct colors.
  • Try again with custom colors, and make sure there are no regressions.

Proposed changelog entry for your changes:

  • Contact Form Block: Fix button colors not saving correctly.

@Copons Copons added the [Status] Needs Review This PR is ready for review. label Mar 5, 2020
@Copons Copons requested review from a team March 5, 2020 16:01
@Copons Copons self-assigned this Mar 5, 2020
@matticbot
Copy link
Copy Markdown
Contributor

matticbot commented Mar 5, 2020

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

@Copons Copons added [Feature] Forms [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Bug When a feature is broken and / or not performing as intended labels Mar 5, 2020
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Mar 5, 2020 •
edited
Loading

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 67b209e

@Copons Copons mentioned this pull request Mar 5, 2020
Closed
scruffian
scruffian reviewed Mar 5, 2020
View reviewed changes
scruffian
scruffian previously approved these changes Mar 5, 2020
View reviewed changes
Copy link
Copy Markdown
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.

This looks good, just one nitpick about spelling!

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Mar 5, 2020
@jeherve jeherve added this to the 8.4 milestone Mar 5, 2020
jeherve
jeherve approved these changes Mar 5, 2020
View reviewed changes
Copy link
Copy Markdown
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.

Looks good in my tests. 👍

@Copons Copons merged commit 698a72b into master Mar 5, 2020
@Copons Copons deleted the fix/contact-form-button-colors branch March 5, 2020 18:48
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Mar 5, 2020
@Copons
Copy link
Copy Markdown
Contributor Author

Copons commented Mar 5, 2020

r203932-wpcom

@pablinos
Copy link
Copy Markdown
Contributor

pablinos commented Mar 6, 2020

Good catch! It looks like we didn't pick up the Contact Form with this change which is probably because it doesn't explicitly use the SubmitButton component.

AFAIK we shouldn't be validating the colours has hex values though. I had to remove that validation from another block as the colours can be values like light-gray coming from the theme pallette. I think this may be contributing to the problems we're seeing in the AMP change

@Copons
Copy link
Copy Markdown
Contributor Author

Copons commented Mar 6, 2020

@pablinos I think you're right: we should only validate custom colors (that are supposed to always be hex codes), but not named ones.
To be honest, I've done that on Contact Form, and simply updated all the other colorValidator uses with the new shared utility, without thinking much about it.

@pablinos
Copy link
Copy Markdown
Contributor

pablinos commented Mar 6, 2020

Yeah looking at it again, I think I got confused by the all the colour related attributes for the Calendly block. I think there might be a couple we can deprecate now.

jeherve added a commit that referenced this pull request Mar 23, 2020
@jeherve
Changelog: add #14898
a4297a9
jeherve added a commit that referenced this pull request Mar 31, 2020
@jeherve
8.4 Changelog (#15075)
0eb0b35 
* Initial changelog entry

* Changelog: add #14904

* Changelog: add #14910

* Changelog: add #14913

* Changelog: add #14916

* Changelog: add #14922

* Changelog: add #14924

* Changelog: add #14925

* Changelog: add #14928

* Changelog: add #14840

* Changelog: add #14841

* Changelog: add #14842

* Changelog: add #14826

* Changelog: add #14835

* Changelog: add #14859

* Changelog: add #14884

* Changelog: add #14888

* Changelog: add #14817

* Changelog: add #14814

* Changelog: add #14819

* Changelog;: add #14797

* Changelog: add #14798

* Changelog: add #14802

* Changelog: add #13676

* Changelog: add #13744

* Changelog: add #13777

* Changelog: add #14446

* Changelog: add #14739

* Changelog: add #14770

* Changelog: add #14784

* Changelog: add #14897

* Changelog: add #14898

* Changelog: add #14968

* Changelog: add #14985

* Changelog: add #15044

* Changelog: add #15052

* Update to remove Podcast since it remains in Beta

* Changelog: add #14803

* Changelog: add #15028

* Changelog: add #15065

* Changelog:add #14886

* Changelog: add #15118

* Changelog: add #14990

* Changelog: add #14528

* Changelog: add #15120

* Changelog: add #15126

* Changelog: add #15049

* Chanegelog: add #14852

* Changelog: add #15090

* Changelog: add #15138

* Changelog: add #15124

* Changelog:add #15055

* Changelog: add #15017

* Changelog: add #15109

* Changelog: add #15145

* Changelog:add #15096

* Changelog:add #15153

* Changelog: add #15133

* Changelog: add #14960

* Changelog: add #15127

* Changelog: add #15056

* Copy current changelog to changelog archive.

* Clarify changelog description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeherve jeherve jeherve approved these changes

@scruffian scruffian scruffian left review comments

Assignees

@Copons Copons

Labels

Bug When a feature is broken and / or not performing as intended [Feature] Forms [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Touches WP.com Files

Projects

None yet

Milestone

8.4

Development

Successfully merging this pull request may close these issues.

7 participants

@Copons @matticbot @jetpackbot @pablinos @scruffian @jeherve @creativecoder