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

adding note about viewport size #435

Merged
merged 2 commits into from
Jul 29, 2024
Merged

adding note about viewport size #435

merged 2 commits into from
Jul 29, 2024

Conversation

M-J-Robbins
Copy link
Collaborator

@M-J-Robbins M-J-Robbins commented Jun 3, 2024

Added a note based on this conversation here https://emailgeeks.slack.com/archives/C1Z733K1P/p1717319677837359?thread_ts=1717175491.511369&cid=C1Z733K1P

I've not been able to test fastmail, protonmail or SFR webmail clients.

@husseinalhammad
Copy link
Contributor

I don't think "buggy" is the best description here. It works exactly as expected on the webpage as a whole. Perhaps something more specific along the lines of:

The HTML of the email message is embedded directly on the webmail (not in an <iframe>) and does not fill the full viewport's width. In this case, the vw values are relevant to the viewport (browser window) not the email message.

Perhaps it needs to be even more detailed. A lot of webmail clients allow you to open the message on a separate window. In that case, the email message width equals the full viewport width.

@M-J-Robbins
Copy link
Collaborator Author

@husseinalhammad yeah that sounds much better. How about "may not fill the full viewport's width" instead of "does not fill the full viewport's width" to account for opening in a new window.

The HTML of the email message is embedded directly on the webmail (not in an <iframe>) and may not fill the full viewport's width. In this case, the vw values are relevant to the viewport (browser window) not the email message.

@husseinalhammad
Copy link
Contributor

I like that, @M-J-Robbins. The phrasing is better and more future proof.

@M-J-Robbins
Copy link
Collaborator Author

Just pushed the updated wording

@hteumeuleu hteumeuleu merged commit 014891e into main Jul 29, 2024
0 of 4 checks passed
@hteumeuleu
Copy link
Owner

Good stuff, thank you both!

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.

3 participants