Add new invite email template#12130
Conversation
|
Storybook is ready:
|
|
Build files for 9676996 have been deleted. |
83d24dc to
22c6c2e
Compare
…or additional emails in this sprint.
tofumatt
left a comment
There was a problem hiding this comment.
One question about the asset strings here, and a few text comments/suggestions, but otherwise this looks good 👍🏻
| /** | ||
| * Asset paths mapping. | ||
| * | ||
| * Maps asset filenames to their versioned folder paths. | ||
| * Format: 'asset-filename.png' => 'version/template-name' | ||
| * | ||
| * @since n.e.x.t | ||
| * @var array | ||
| */ | ||
| const EMAIL_ASSET_PATHS = array( | ||
| // email-report assets (1.168.0). | ||
| 'site-kit-logo.png' => '1.168.0/email-report', | ||
| 'shooting-stars-graphic.png' => '1.168.0/email-report', | ||
| 'icon-conversions.png' => '1.168.0/email-report', | ||
| 'icon-growth.png' => '1.168.0/email-report', | ||
| 'icon-link-arrow.png' => '1.168.0/email-report', | ||
| 'icon-search.png' => '1.168.0/email-report', | ||
| 'icon-views.png' => '1.168.0/email-report', | ||
| 'icon-visitors.png' => '1.168.0/email-report', | ||
| 'conversions-timeline-green.png' => '1.168.0/email-report', | ||
| 'conversions-timeline-red.png' => '1.168.0/email-report', | ||
| 'notification-icon-star.png' => '1.168.0/email-report', | ||
| // invitation-email assets (1.172.0). | ||
| 'invitation-envelope-graphic.png' => '1.172.0/invitation-email', | ||
| ); |
There was a problem hiding this comment.
Is there some more documentation somewhere on how this works or how these versions should be updated? I'm surprised we're not using n.e.x.t here. How do developers know how to add new assets here?
There was a problem hiding this comment.
I had already discussed a refactor of this to move away from linking assets to version numbers and to simplify the asset loading by using a key-based asset map. Lmk what you think of this refactor.
…add-invitation-email-template.
Co-authored-by: Matthew Riley MacPherson <hi@tofumatt.com>
Co-authored-by: Matthew Riley MacPherson <hi@tofumatt.com>
…bust new Email_Assets class.
|
Thanks @tofumatt. I have asked @sigal-teller to review/approve the copy changes, as they will need to be brought back to Figma to prevent being caught in QA later. |
Co-authored-by: Matthew Riley MacPherson <hi@tofumatt.com>
…ate.php Co-authored-by: Matthew Riley MacPherson <hi@tofumatt.com>
…add-invitation-email-template.
Summary
Addresses issue:
Relevant technical choices
As part of this ticket, I'd worked on some initial optimisation to allow for future simple email templates that follow the same design as this one (three will be added to this feature in this epic). I've created shared simple template methods throughout the render process and also included copy to be used in the plain text renderer in a centralised class
Body_Content_MapTest.This can be extended upon in upcoming tickets, we should consider sharing the template between each similar email with only configurable body copy within a centralised class.
PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist