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.

Sign up for GitHub

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

Jump to bottom

Fix custom styles for emails in preview and in sent messages #7595

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from
Open

Fix custom styles for emails in preview and in sent messages #7595

wants to merge 5 commits into from

Conversation

merkushin
Copy link
Member

Resolves #7459

The changes appear in HTML as has-pale-blue-something class for the element. This classes are defined in the global styles.

Proposed Changes

  • Load global styles for inline CSS processor.

Testing Instructions

  1. Go to Sensei LMS > Settings > Emails.
  2. Enable the Course Completed email and then edit it.
  3. Change the styles in some way (e.g. change the button colors and center align it).
  4. Save and preview. Make sure you see your changes in the preview.
  5. Complete a course as a student.
  6. Check your mailbox and make sure that the Course Completed email reflects your changes.

Screenshots

Real email:
(You can see that Sensei logo is not displayed — that's because it refers to my local website.)
CleanShot 2024-04-21 at 22 29 36@2x

Editor:
CleanShot 2024-04-21 at 22 30 09@2x

Preview:
CleanShot 2024-04-21 at 22 30 26@2x

Pre-Merge Checklist

  • PR title and description contain sufficient detail and accurately describe the changes
  • Acceptance criteria is met
  • Decisions are publicly documented
  • Adheres to coding standards (PHP, JavaScript, CSS, HTML)
  • All strings are translatable (without concatenation, handles plurals)
  • Follows our naming conventions (P6rkRX-4oA-p2)
  • Hooks (p6rkRX-1uS-p2) and functions are documented
  • New UIs are responsive and use a mobile-first approach
  • New UIs match the designs
  • Different user privileges (admin, teacher, subscriber) are tested as appropriate
  • Legacy courses (course without blocks) are tested
  • Code is tested on the minimum supported PHP and WordPress versions
  • User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
  • "Needs Documentation" label is added if this change requires updates to documentation
  • Known issues are created as new GitHub issues

@merkushin merkushin added this to the 4.24.0 milestone Apr 22, 2024
@merkushin merkushin requested a review from a team April 22, 2024 04:45
@merkushin merkushin self-assigned this Apr 22, 2024
@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

merkushin
merkushin commented Apr 23, 2024
View reviewed changes
includes/internal/emails/class-email-sender.php
* @param array $placeholders The placeholders.
*
* @return string
*/
private function replace_placeholders( string $string, array $placeholders ): string {
private function replace_placeholders( string $content, array $placeholders ): string {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHPCS was complaining for the variable name.

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

Imran92
Imran92 reviewed Apr 23, 2024
View reviewed changes
Copy link
Contributor

@Imran92 Imran92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason it's not working for me -

Editor:

Screenshot 2024-04-23 at 4 30 02 PM

Gmail:

Screenshot 2024-04-23 at 4 31 42 PM

Temp mail client:

Screenshot 2024-04-23 at 4 32 20 PM

@merkushin
Copy link
Member Author

merkushin commented Apr 23, 2024 •
edited

@Imran92 Thanks for testing!

Could you tell me what are the versions of WordPress and Gutenberg (if installed), and what theme do you use on the test website?

@m1r0
Copy link
Member

m1r0 commented Apr 23, 2024 •
edited

I've tested it and the colors worked, but the alignment didn't. The result was the same with and without the Gutenberg plugin.

@Imran92
Copy link
Contributor

Imran92 commented Apr 24, 2024 •
edited

Could you tell me what are the versions of WordPress and Gutenberg (if installed), and what theme do you use on the test website?

Hi @merkushin 👋 I've shared the creds of a JN site where I reproduced the issue in DM. Hope it'll help! Thanks! Locally I didn't have GB installed and WP version was 6.5.2

@merkushin
Copy link
Member Author

Thanks Imran!

Ah, yes, the same email in Gmail and in the Mail app look so differently.

CleanShot 2024-04-23 at 23 04 06@2x

CleanShot 2024-04-23 at 23 03 52@2x

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@github-actions GitHub Actions
Copy link

Test the previous changes of this PR with WordPress Playground.

@donnapep donnapep removed this from the 4.24.0 milestone Apr 25, 2024
@merkushin merkushin added this to the 5.0.0 milestone May 9, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented May 9, 2024

Test the previous changes of this PR with WordPress Playground.

@merkushin
Copy link
Member Author

merkushin commented May 9, 2024 •
edited

As Imran suggested, I checked that when we don't use default colors and the picker instead we get colors as the HEX value, so I disabled default colors in the editor settings.
2a2ec4e

With alignment is a bit harder. I added classes needed we missed in CSS, now it works in Preview and in Mail app. But Gmail webapp "doesn't work". I found they just don't support some CSS properties (for example, justify-content and align-items—both are used to align buttons).
b8a6aab
And it is impossible to disable in the editor settings.

I also added styles to make text alignment work.

I still see other issues with styles in emails:

  • The size of the button is different in Editor vs Preview/Real Message.
  • Image alignment doesn't work. I tried to fix it quickly by bringing needed classes, but it still misses the width property (not sure if it is the only cause), so it doesn't work.

And I suppose we can find even more tiny issues like those ones. I think we need to add a warning to our documentation that not all styles are supported in emails. And maybe list the ones we know already.

@github-actions GitHub Actions
Copy link

github-actions bot commented May 9, 2024

Test the previous changes of this PR with WordPress Playground.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Imran92 Imran92 Imran92 left review comments

Assignees

@merkushin merkushin

Labels
Emails
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

Style changes made in email editor are not reflected in the preview or in sent emails
4 participants
@merkushin @m1r0 @Imran92 @donnapep