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.

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

Make lack of plaintext emails explicit #5485

Merged
merged 3 commits into from
Jan 20, 2025
Merged

Make lack of plaintext emails explicit #5485

merged 3 commits into from
Jan 20, 2025

Conversation

Vinnl
Copy link
Collaborator

@Vinnl Vinnl commented Jan 13, 2025

SubPlat apparently sends plaintext emails:
https://storage.googleapis.com/mozilla-storybooks-fxa/commits/d06a74b249ebfe979642fdc94f4086a3ed031312/fxa-auth-server/index.html?path=/story/subplat-emails-layout--layout-with-was-deleted

We currently don't, although technically, we easily could: we'd just have to add a text property to the options object passed to gTransporter.sendMail. Since there's currently discussion about whether to send them ourselves, I figured I'd at least adopt SubPlat's style for Storybook, to make it top-of-mind that we could send them, and to make it apparent where we're not and, in case we do start sending them, to make it ease to compare and see where we include information in the HTML email that we don't in the plaintext version.

@Vinnl Vinnl added the Review: µ Code review time: 5 minutes or less label Jan 13, 2025
@Vinnl Vinnl requested review from codemist and flozia January 13, 2025 10:38
@Vinnl Vinnl self-assigned this Jan 13, 2025
SubPlat apparently sends plaintext emails:
https://storage.googleapis.com/mozilla-storybooks-fxa/commits/d06a74b249ebfe979642fdc94f4086a3ed031312/fxa-auth-server/index.html?path=/story/subplat-emails-layout--layout-with-was-deleted

We currently don't, although technically, we easily could: we'd
just have to add a `text` property to the options object passed to
`gTransporter.sendMail`. Since there's currently discussion about
whether to send them ourselves, I figured I'd at least adopt
SubPlat's style for Storybook, to make it top-of-mind that we could
send them, and to make it apparent where we're not and, in case we
do start sending them, to make it ease to compare and see where we
include information in the HTML email that we don't in the
plaintext version.
@Vinnl Vinnl force-pushed the plaintext-email-note branch from 1c5ca0a to 2951213 Compare January 13, 2025 10:53
Copy link

Copy link
Collaborator

@flozia flozia left a comment

Choose a reason for hiding this comment

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

Nice addition to the email stories.

</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!

@Vinnl Vinnl merged commit 5e924e9 into main Jan 20, 2025
16 checks passed
@Vinnl Vinnl deleted the plaintext-email-note branch January 20, 2025 10:31
Copy link

Cleanup completed - database 'blurts-server-pr-5485' destroyed, cloud run service 'blurts-server-pr-5485' destroyed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review: µ Code review time: 5 minutes or less
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants