Subscriptions Block: Remove inline styling for colors on status message

[stacimc]

Fixes #19113

Changes proposed in this Pull Request:

The post-submit status message for the Subscriptions block had hard-coded inline styles to set the background-color and text color to the styles defined in the global $themecolors. This can cause bugs if the theme only defines a value for one or the other; for example, if the theme sets the background-color to white but does not define a text color, the message's text color may inherit from the parent block -- and if this also happens to be white, the text is not readable.

This PR removes the hard-coded inline color styles. It should affect only the post-submit message, not the rest of the block. See screenshots.

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

If testing on a sandbox, you will need to manually copy the changes into your sandbox, as it will not be picked up by the awe sync.

First verify that you can see the bug

• Activate the Seedlet theme (which sets $themecolors['bg'] ='FFFFFF' but does not set $themecolors['text'])
• Create a new post and insert a Group block.
• In the Group block's Inspector Controls, set the background-color to black and the text color to white.
• Insert a Subscriptions block into the Group block.
• View the post on the frontend.
• Enter an email address and submit the Subscriptions form (any status will do).
• Observe that the background of the submit message is white (coming from the theme) and the text color is also white (coming from the Group block).

Text partially highlighted so it is visible.

Test that the bug is resolved

• Now checkout this branch and refresh the post.
• Observe that the status message text and background color are now inherited from the Group block.
• Go back to the editor and insert another Subscriptions block outside of the Group block.
• On the frontend, verify that the status message now inherits colors from the theme

The message inside the Group block has black background and white text. The message outside the block has the default green background and dark text from the Twenty Twenty One theme

You can easily test all of the status messages by appending the status type to the URL, like:
<mytestsite.wordpress.com>?p=431&preview=true&blogsub=<status>, where status is each of: confirming, blocked, flooded, spammed, subscribed, pending, confirmed.

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello stacimc! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D59071-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ All commits were linted before commit.
  
• 🔴 Add a "[Status]" label (In Progress, Needs Team Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

---

Jetpack plugin:

• Next scheduled release: April 6, 2021.
• Scheduled code freeze: March 29, 2021.

[stacimc]

I may be missing something with theme configuration; should we actually expect all themes to define both bg and text? I notice Seedlet does define txt.

[ramonjd]

During my tests (using Spearhead) I noticed that, where $themecolors['border'] is empty, the following invalid CSS is output:

border: 1px solid #; padding-left: 5px; padding-right: 5px; margin-bottom: 10px;

Do you think it'd be better to only print the inline style border: 1px solid #; if $themecolors['border'] is empty?

[stacimc]

Agreed 👍 I kept the 1px solid border but removed the empty color value, rather than omitting the border altogether. The border takes on the text-color in this case.

Per the discussion below -- once we can access the theme border color with CSS variables, we can move all of the styling out into the stylesheets and it will be much cleaner :)

[ramonjd]

Curiously, this didn't work for me on Seedlet, but I re-ran using Spearhead and the fix is noticeable.

The background colour inline style is gone and the success message background reflects the group block setting.

I have one question as to whether we should ensure the inline style is valid, that is, ensure no empty values in the border declaration: border: 1px solid #

Before

<div style="background-color: #FFFFFF; border: 1px solid #; color: #; padding-left: 5px; padding-right: 5px; margin-bottom: 10px;">Congrats, you’re subscribed! You’ll get an email with the details of your subscription and an unsubscribe link.</div>

After

<div style="border: 1px solid #; padding-left: 5px; padding-right: 5px; margin-bottom: 10px;">Congrats, you’re subscribed! You’ll get an email with the details of your subscription and an unsubscribe link.</div>

[scruffian]

IMO we should just not output any styles here and let it fall back to the theme defaults. This will be a lot easier once we have this in place: WordPress/gutenberg#29714

[stacimc]

IMO we should just not output any styles here and let it fall back to the theme defaults. This will be a lot easier once we have this in place: WordPress/gutenberg#29714

@scruffian I think we're on the same page, but to clarify -- in this approach we're removing the inline-styles and not explicitly setting the theme colors on the block, so that it only falls back to theme defaults when more specific colors are not set on it/its container blocks. Does that sound correct to you?

The border color is the only style that does not automatically fall back to the theme default, so I'm keeping the inline style for that. The CSS var approach in your linked PR would definitely make that easier.

[scruffian]

Does that sound correct to you?

Yeah, I think this is the best we can do for now.

[apeatling]

Confirmed this works much better now with the default styles removed, and border color kept in place.

[jeherve]

That simplifies quite a bit, nice work. I only have a minor suggestion, below.

[andrewserong]

This is testing nicely for me @stacimc! Nice work cleaning up the code in the process, too 👍

[github-actions]

Great news! One last step: head over to your WordPress.com diff, D59071-code, and commit it.
Once you've done so, come back to this PR and add a comment with your changeset ID.

Thank you!

[stacimc]

r223723-wpcom