feat(ras-acc): update newsletters palette on theme primary color updates

[chickenn00dle]

All Submissions:

• Have you followed the Newspack Contributing guideline?
• Does your code follow the WordPress' coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Part of https://app.asana.com/0/1206943664367847/1205668937699542/f

This PR is meant to be reviewed alongside these newsletters changes: Automattic/newspack-newsletters#1487

These changes ensure we are updating the newsletter color palette option with the latest primary and secondary theme colors before creating transactional emails from templates OR before triggering an update to email HTML when updating a sites primary or secondary newspack theme colors.

How to test the changes in this Pull Request:

1. With both the changes in the newsletters pr as well as this one checked out, go to Newspack > Engagement > Show advanced settings and reset the one time password email.
2. Go to the post editor for the OTP email by selecting the edit link
3. Confirm the social icons are filled either white or black depending on the color of the email footer (should be contrasting)
4. Send a test email to yourself
5. Confirm the social icons are also filled with the same color in the email
6. Go to Newspack > Site Design and change the sites primary color to something of the opposite darkness/lightness than it previously was and save
7. Go back to the OTP email edit page and confirm the social icons are now the opposite color than they were before (black or white)
8. Send another test email to yourself
9. Confirm the social icons are also filled with the same color in the email

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully ran tests with your changes locally?

[adekbadek]

After updating the theme color, the primary email color is visible when editing the email, but not in the sent email HTML.

[adekbadek]

Despite this check, the test are failing.

[chickenn00dle]

Ah. I think this is because the Plugin_Manager downloads the latest release for newsletters, which does not yet have the required method:

newspack-plugin/includes/class-plugin-manager.php

Line 144 in 725786b

'Download' => 'https://github.com/Automattic/newspack-newsletters/releases/latest/download/newspack-newsletters.zip',

I guess we can hold off on moving forward with this one until the next newsletters release.

[chickenn00dle]

Thanks for testing this @adekbadek!

After updating the theme color, the primary email color is visible when editing the email, but not in the sent email HTML.

That's strange. I am seeing the colors change in both the editor as well as the email:

Screen.Recording.2024-05-07.at.15.03.20.mov

Are you resetting the email template once again after changing the primary/secondary colors by any chance?

[adekbadek]

@chickenn00dle –

Are you resetting the email template once again after changing the primary/secondary colors by any chance?

No. I tested by requesting the OTP as a reader, but when testing with the "Testing" sidebar panel (as on the video), the effect is the same. I've inspected the newspack_email_html post meta, and even after saving the post in the editor it still has the old color.

FWIW, I've found that on updating the theme color in the Site Design wizard, the first condition in the get_email_config_by_type is always satisfied, so Newspack_Newsletters::update_color_palette is never called.

[chickenn00dle]

FWIW, I've found that on updating the theme color in the Site Design wizard, the first condition in the get_email_config_by_type is always satisfied, so Newspack_Newsletters::update_color_palette is never called.

Ah. We actually update the newsletters color palette in two places in this PR. The first is where you pointed out here, where we need to be sure the palette is updated with the latest before we initially create our email templates (after a template is reset for example).

The second is here:

newspack-plugin/includes/emails/class-emails.php

Lines 656 to 665 in 9932035

	if ( $previous_value['primary_color_hex'] !== $updated_value['primary_color_hex'] || $previous_value['secondary_color_hex'] !== $updated_value['secondary_color_hex'] ) {
	// Update the newsletters color palette.
	$updated = \Newspack_Newsletters::update_color_palette(
	[
	'primary' => $updated_value['primary_color_hex'],
	'secondary' => $updated_value['secondary_color_hex'],
	'primary-text' => newspack_get_color_contrast( $updated_value['primary_color_hex'] ),
	'secondary-text' => newspack_get_color_contrast( $updated_value['secondary_color_hex'] ),
	]
	);

This triggers whenever the sites primary or secondary color are updated.

That said, I spoke with @dkoo about this and he pointed out the issue is actually related to a limitation with the mjml library in which newsletter HTML meta won't actually update unless the editor is open and the send/test button is clicked. Luckily, he was able to get a workaround for this added in 7463b3c

So this should be working as expected now, regardless of how you trigger the email after updating site color.

[adekbadek]

Just noticed that maybe_update_email_templates will only be called if no child theme is used (due to get_template() use in the hook). The hook won't be called if a child theme is set, and so the email templates won't get updated.

[chickenn00dle]

Just noticed that maybe_update_email_templates will only be called if no child theme is used (due to get_template() use in the hook). The hook won't be called if a child theme is set, and so the email templates won't get updated.

Good catch @adekbadek! I've fixed in 96aaf98

[adekbadek]

Child theme is now handled, but noticed a different issue 😬

[adekbadek]

newspack_get_theme_colors does not return primary_text_color and secondary_text_color keys:

newspack-plugin/includes/util.php

Lines 549 to 552 in 725786b

	return [
	'primary_color' => $primary_color,
	'primary_text_color' => newspack_get_color_contrast( $primary_color ),
	];

So it should be:

'primary-text'   => newspack_get_color_contrast( $theme_colors['primary_color'] ),
'secondary-text' => newspack_get_color_contrast( $theme_colors['secondary_color'] ),

[chickenn00dle]

Ah! That's right. We removed the secondary color from the email templates, so now this should only reference primary_color and primary_text_color. I've updated in e898500.

Thanks for the keen eye!

[matticbot]

🎉 This PR is included in version 3.9.0-epic-ras-acc.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[matticbot]

🎉 This PR is included in version 4.1.0-epic-ras-acc.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[matticbot]

🎉 This PR is included in version 4.2.0-epic-ras-acc.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀