cds-snc / notification-planning

Project planning for GC Notify Team
4 stars 0 forks source link

Wide branding are stretched in template preview #819

Open sastels opened 1 year ago

sastels commented 1 year ago

Describe the bug

Wide brandings (say, over 1100 x 108) are stretched vertically in the template preview. The emails use the correct aspect ratio.

Bug Severity

See examples in the documentation

SEV-3 Minor - everything works in the end, but service owners might not know that.

To Reproduce

Steps to reproduce the behavior:

  1. set your email branding to a wide image
  2. preview an email

Expected behavior

image is not stretched

Impact

Screenshots

Branding here is 1149 × 108 pixels

image.png

Additional context

https://cds-snc.freshdesk.com/a/tickets/9524 https://cds-snc.freshdesk.com/a/tickets/9578

andrewleith commented 1 year ago
amazingphilippe commented 1 year ago

Instead of closing this issue, we could pull it back to the backlog and integrate it into the branding epic.

There is a problem to fix, where we have 2 different templates to generate the branding in a preview, and in the email. This should not happen: the preview should be reliable.

The bug is ocated in these 2 files, and we can work on fixing both files, and maybe simplify them once we know more about the branding requirements in our federal and provincial context. The current files follows branding standards from the UK, which are not relevant here. See this documentation in the same repo

The 2 problem files:

  1. https://github.com/cds-snc/notification-utils/blob/f86f51874a1873e4d4a7c6df59c84bafad882f09/notifications_utils/jinja_templates/email/_custom_logo_no_background_colour.jinja2
  2. https://github.com/cds-snc/notification-utils/blob/5c5b86f782a8f75df7a5ace48a120eee5d46752a/notifications_utils/jinja_templates/email/email_preview_template.jinja2
yaelberger-commits commented 1 year ago

Jimmy Prince also experienced a similar problem, possibly the same problem, when adding a logo to a template

yaelberger-commits commented 1 year ago

There are no width requirements for logos

jprince-cds commented 1 year ago

I have attached the problematic logo from ticket https://cds-snc.freshdesk.com/a/tickets/15086

It is 108 pixels high.

dfo-mpo-eng.png

jprince-cds commented 1 year ago

The preview in Notify using the above logo showed a logo much too big.

I ended up resizing as follows for it to work:

dfo-mpo-eng-resized.png

But it was hard to find a width/height ratio that showed well in the preview.

yaelberger-commits commented 4 months ago

@amazingphilippe and @andrewleith I believe this is now fixed with our release yesterday, correct?

amazingphilippe commented 4 months ago

No! This bug happens in a different place.