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

Use the full width in the email header #4905

Merged
merged 2 commits into from
Aug 20, 2024
Merged

Conversation

Vinnl
Copy link
Collaborator

@Vinnl Vinnl commented Aug 5, 2024

References:

Jira: MNTOR-3473
Figma:

Description

I was initially foiled by MJML not supporting auto-growing columns (or rather, them not being able to get that to reliably work in Outlook Desktop:
mjmlio/mjml#1669 (comment)), and thus set the right-hand column (the one containing the "Sign in" link) to 60%, hoping that that would look reasonable enough on most screen sizes — on large screens, it would be fairly right- aligned, and on smaller screens, it wouldn't be pushed to the next line.

However, the second thing that confused me was that MJML's columns don't take effect on viewport widths smaller than 480px, i.e. they're wrapped in media queries starting at that level.

If I just only test on wider viewports, then I can actually just set the columns to together span 100%. The image itself still has a fixed width of 200px, so that won't auto-grow anyway. Only downside is that the "Sign in" link will still be pushed to the next line on screens under 480px, but I guess that's just something we'll have to live with.

Screenshot (if applicable)

Before:

image

After:

image

How to test

Open an email (could be in Storybook), and resize the screen to a small width, but larger than 480px. The Monitor logo and "Sign in" link should stay on the same line.

Checklist (Definition of Done)

  • Localization strings (if needed) have been added.
  • Commits in this PR are minimal and have descriptive commit messages.
  • I've added or updated the relevant sections in readme and/or code comments
  • I've added a unit test to test for potential regressions of this bug. - N/A, visual change
  • If this PR implements a feature flag or experimentation, the Ship Behind Feature Flag status in Jira has been set
  • Product Owner accepted the User Story (demo of functionality completed) or waived the privilege.
  • All acceptance criteria are met.
  • Jira ticket has been updated (if needed) to match changes made during the development process.
  • Jira ticket has been updated (if needed) with suggestions for QA when this PR is deployed to stage.

I was initially foiled by MJML not supporting auto-growing columns
(or rather, them not being able to get that to reliably work in
Outlook Desktop:
mjmlio/mjml#1669 (comment)),
and thus set the right-hand column (the one containing the "Sign
in" link) to 60%, hoping that that would look reasonable enough
on most screen sizes — on large screens, it would be fairly right-
aligned, and on smaller screens, it wouldn't be pushed to the next
line.

However, the second thing that confused me was that MJML's columns
don't take effect on viewport widths smaller than 480px, i.e.
they're wrapped in media queries starting at that level.

If I just only test on wider viewports, then I can actually just
set the columns to together span 100%. The image itself still has
a fixed width of 200px, so that won't auto-grow anyway. Only
downside is that the "Sign in" link will still be pushed to the
next line on screens under 480px, but I guess that's just
something we'll have to live with.
@Vinnl Vinnl added the Review: XS Code review time: up to 30min label Aug 5, 2024
@Vinnl Vinnl requested review from codemist and flozia August 5, 2024 09:57
@Vinnl Vinnl self-assigned this Aug 5, 2024
Copy link

github-actions bot commented Aug 5, 2024

@Vinnl Vinnl merged commit 405d247 into main Aug 20, 2024
15 checks passed
@Vinnl Vinnl deleted the MNTOR-3473-align-email-header branch August 20, 2024 11:58
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review: XS Code review time: up to 30min
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants