Skip to content

LG-13196 Enhanced IPP Ready to Verify Email - Take Two#10839

Merged
gina-yamada merged 6 commits intomainfrom
yamada/LG-13196-enhanced-ipp-ready-to-verify-email-take-two
Jun 21, 2024
Merged

LG-13196 Enhanced IPP Ready to Verify Email - Take Two#10839
gina-yamada merged 6 commits intomainfrom
yamada/LG-13196-enhanced-ipp-ready-to-verify-email-take-two

Conversation

@gina-yamada
Copy link
Contributor

@gina-yamada gina-yamada commented Jun 20, 2024

🎫 Ticket

  • Pull Request to address comments in PR# 10816 that were made after the pull request had been merged into main
  • Later also discovered (while testing on staging) that the images on my email did not display (even though they displayed locally on email template)

Screenshot 2024-06-20 at 2 03 48 PM

Original ticket: LG-13196 Implementation: EIPP "You're ready to verify in person" Barcode email

🛠 Summary of changes

  • Delete deprecated width attribute on td and replaced it with CSS style="width:xxpx" to achieve the same layout
    (I had not previously modified reset_password_instructions but it used the same deprecated attribute so I fixed it.)
  • Round down on image width where a decimal was used
  • Used USWDS class
  • For each SVG image in the mailer- I make a png version. The Ready to Verify mailer is now using the png image. The SVG images did not get deleted because they are still being used on the Ready to Verify view.

📜 Testing Plan

  1. Create an EIPP Enrollment by starting up Sinatra.
  2. Inspect the Ready To Verify Email Template (EIPP Version). Layout/style should not have changed and should match mocks. Confirm Ready to Verify is not affected just because it is just one more click.
  3. If you prefer not to run through the entire EIPP flow you can jump to rails/mailer view once you get the sever going:
  1. Once this is merged into staging- run through and create an EIPP enrollment. Open the email from staging that got delivered and confirm you can see the icons

👀 Screenshots

Screenshot 2024-06-21 at 9 19 31 AM

Screenshot 2024-06-21 at 9 19 48 AM

Screenshot 2024-06-21 at 9 19 52 AM

<tbody>
<tr>
<td align="left" width="110.46px">
<td align="left" style="width:110px;">
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sorta curious if it's necessary to specify the width at all, or would the cell grow automatically to accommodate the image's defined dimensions? The only things that gives me pause here is:

  1. Usually style attributes won't work in production due to CSP style-src, but that's probably not an issue for emails, particularly given the second point below.
  2. Rails mailers will automagically inline CSS styling as style attribute, but I'm not sure how it works if we also specify our own style attribute value.

(This isn't a blocker)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aduth Thanks for the comments here. I need to dig a little more but here is what I know so far...

The cell does not grow to accommodate the images defined dimensions. When I removed style="width:110px; on line 54- you can see the Real ID image below Option 1 shrunk.

Screenshot 2024-06-20 at 2 00 30 PM

Screenshot 2024-06-20 at 1 53 24 PM

Worth mentioning- I created an enrollment in staging and the email I received did not have working images on it. (This EIPP email is only in staging.) I was thinking it was the decimal pixels but am wondering (and will look into) CSP.

Copy link
Contributor

@aduth aduth Jun 20, 2024

Choose a reason for hiding this comment

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

Ohhhh, I know what it is. You're trying to use SVG images but those aren't broadly supported in email:

https://www.caniemail.com/features/image-svg/

This is also why we have app/assets/images/email folder, which includes copies of images specifically because of this support limitation (noted also in folder README):

https://github.com/18F/identity-idp/tree/main/app/assets/images/email

This folder contains images for exclusive use by mailer templates. This includes email-specific imagery, and also variants of existing assets.

For example, since SVG images are not well-supported in all email clients, this folder may include rasterized versions of common SVG images.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great tip! Okay, each image on the email template is pointing at a png file inside app/assets/images/email. I did not delete the svg files because they each are being used in app/views/idv/in_person/ready_to_verify/show.html.erb.

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@rutvigupta-design
Copy link

This LGTM! 🎉

@WilliamBirdsall
Copy link
Contributor

LGTM as well!

@gina-yamada gina-yamada merged commit 59c4b26 into main Jun 21, 2024
@gina-yamada gina-yamada deleted the yamada/LG-13196-enhanced-ipp-ready-to-verify-email-take-two branch June 21, 2024 18:14
@gina-yamada
Copy link
Contributor Author

I just confirmed icons in the Ready to Verify email in staging display.

Screenshot 2024-06-24 at 11 28 12 AM

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants