Skip to content

Make lack of plaintext emails explicit #5485

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.

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

Merged
merged 3 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 66 additions & 16 deletions src/emails/StorybookEmailRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { renderToStaticMarkup } from "react-dom/server";

export type Props = {
children: ReactNode;
plainTextVersion: string | null;
emulateDarkMode?: boolean;
};

Expand All @@ -18,7 +19,7 @@ export const StorybookEmailRenderer = (props: Props) => {
const rawMjml = renderToStaticMarkup(props.children);
const renderResult = mjml2html(rawMjml);
return (
<>
<div className="wrapper">
<style>{`
@media (prefers-color-scheme: dark) {
* {
Expand All @@ -31,22 +32,71 @@ export const StorybookEmailRenderer = (props: Props) => {
background-color: #1e293b !important;
color: white !important;
}

.wrapper {
display: flex;
gap: 10px;
}

.badge {
align-self: flex-start;
display: inline-block;
font-family: monospace;
background-color: #eee;
border-radius: 3px;
text-transform: uppercase;
padding: 3px 5px;
color: #444;
}

.wrapper .styled {
flex: 2 0 auto;
width: 66%;
padding: 20px;
display: flex;
flex-direction: column;
align-items: center;
gap: 20px;
}
.wrapper .plaintext {
flex: 1 0 auto;
width: 33%;
font-family: monospace;
border-inline-start: 1px solid #eee;
padding: 20px;
padding-inline-start: calc(10px + 20px);
display: flex;
flex-direction: column;
align-items: flex-start;
gap: 20px;
}

.plaintext pre {
white-space: pre-wrap;
}
`}</style>
<div
dangerouslySetInnerHTML={{
__html:
renderResult.errors.length > 0
? `
<h1>MJML rendering errors:</h1>
<ul style="font-family: monospace;">${renderResult.errors.map((error) => `<li>${error.message}</li>`).join("\n")}</ul>
<hr/>
${renderResult.html}
`
: renderResult.html,
}}
className={props.emulateDarkMode ? "dark-mode-enforced" : ""}
/>
</>
<section className="styled">
<div className="badge">HTML</div>
<div
dangerouslySetInnerHTML={{
__html:
renderResult.errors.length > 0
? `
<h1>MJML rendering errors:</h1>
<ul style="font-family: monospace;">${renderResult.errors.map((error) => `<li>${error.message}</li>`).join("\n")}</ul>
<hr/>
${renderResult.html}
`
: renderResult.html,
}}
className={props.emulateDarkMode ? "dark-mode-enforced" : ""}
/>
</section>
<section className="plaintext">
<div className="badge">Plaintext</div>
<pre>{props.plainTextVersion ?? "Not set"}</pre>
Copy link
Collaborator

@flozia flozia Jan 15, 2025

Choose a reason for hiding this comment

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

Since we are not using plain text versions right now it does not matter too much, but should we already set this up so that the rendered HTML would show?

Suggested change
<pre>{props.plainTextVersion ?? "Not set"}</pre>
{props.plainTextVersion ? (
<div
dangerouslySetInnerHTML={{
__html: props.plainTextVersion,
}}
className={props.emulateDarkMode ? "dark-mode-enforced" : ""}
/>
) : (
"Not set"
)}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@flozia I'm not sure I follow - the plain text version is an alternative to the HTML version, i.e. there shouldn't be HTML in there?

Copy link
Collaborator

@flozia flozia Jan 15, 2025

Choose a reason for hiding this comment

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

@Vinnl In that case I misunderstood. I thought the idea was that it is (semi) plain text: When looking at the FXA email story there are still links in there — how would those be handled? In the specific example there is a “Learn more” link.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ha actually, I think that's a mistake on their side? Generally, it'll be up to the mail client to make spelled-out URLs clickable, or to the user to copy-paste it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense!

</section>
</div>
);
};
/* c8 ignore stop */
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { getL10n } from "../../../app/functions/l10n/storybookAndJest";
const meta: Meta<FC<Props>> = {
title: "Emails/Email boilerplate",
component: (props: Props) => (
<StorybookEmailRenderer>
<StorybookEmailRenderer plainTextVersion={null}>
<BoilerplateEmail {...props} />
</StorybookEmailRenderer>
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { getDashboardSummary } from "../../../app/functions/server/dashboard";
const meta: Meta<FC<RedesignedBreachAlertEmailProps>> = {
title: "Emails/Breach alert",
component: (props: RedesignedBreachAlertEmailProps) => (
<StorybookEmailRenderer>
<StorybookEmailRenderer plainTextVersion={null}>
<RedesignedBreachAlertEmail {...props} />
</StorybookEmailRenderer>
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { getL10n } from "../../../app/functions/l10n/storybookAndJest";
const meta: Meta<FC<Props>> = {
title: "Emails/First data broker removal fixed",
component: (props: Props) => (
<StorybookEmailRenderer>
<StorybookEmailRenderer plainTextVersion={null}>
<FirstDataBrokerRemovalFixed {...props} />
</StorybookEmailRenderer>
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { dataClassKeyMap } from "../../../app/functions/server/dashboard";
const meta: Meta<FC<MonthlyActivityFreeEmailProps>> = {
title: "Emails/Monthly activity (free user)",
component: (props: MonthlyActivityFreeEmailProps) => (
<StorybookEmailRenderer>
<StorybookEmailRenderer plainTextVersion={null}>
<MonthlyActivityFreeEmail {...props} />
</StorybookEmailRenderer>
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ type StoryProps = Props & { emulateDarkMode?: boolean };
const meta: Meta<FC<Props>> = {
title: "Emails/Monthly activity (Plus user)",
component: (props: StoryProps) => (
<StorybookEmailRenderer emulateDarkMode={props.emulateDarkMode}>
<StorybookEmailRenderer
plainTextVersion={null}
emulateDarkMode={props.emulateDarkMode}
>
<MonthlyActivityPlusEmail {...props} />
</StorybookEmailRenderer>
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { createRandomHibpListing } from "../../../apiMocks/mockData";
const meta: Meta<FC<Props>> = {
title: "Emails/Signup Report",
component: (props: Props) => (
<StorybookEmailRenderer>
<StorybookEmailRenderer plainTextVersion={null}>
<SignupReportEmail {...props} />
</StorybookEmailRenderer>
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { getL10n } from "../../../app/functions/l10n/storybookAndJest";
const meta: Meta<FC<Props>> = {
title: "Emails/Verify email address",
component: (props: Props) => (
<StorybookEmailRenderer>
<StorybookEmailRenderer plainTextVersion={null}>
<VerifyEmailAddressEmail {...props} />
</StorybookEmailRenderer>
),
Expand Down
Loading