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

fix: set viewport-fit=cover to enable CSS env() variables in PWAs #20836

Merged
merged 5 commits into from
Jan 20, 2025

Conversation

vursen
Copy link
Contributor

@vursen vursen commented Jan 13, 2025

Description

Using env() variables in PWAs requires viewport-fit=cover to be set in the <meta name="viewport">. Without this setting, some parts of such components as app-layout get cut off on iOS devices. Adding viewport-fit follows MDN's documentation about env() variables and helps fix such issues .

Fixes vaadin/web-components#8449

Type of change

  • Bugfix

Copy link

github-actions bot commented Jan 13, 2025

Test Results

1 163 files  ± 0  1 163 suites  ±0   1h 31m 9s ⏱️ -20s
7 620 tests ± 0  7 564 ✅ +15  56 💤 ±0  0 ❌  - 2 
7 977 runs  +21  7 912 ✅ +36  65 💤 ±0  0 ❌  - 2 

Results for commit edc052a. ± Comparison against base commit 3fe8592.

♻️ This comment has been updated with latest results.

@vursen vursen changed the title fix: set viewport-fit: cover to enable CSS env() variables in PWAs fix: set viewport-fit=cover to enable CSS env() variables in PWAs Jan 15, 2025
@vursen vursen marked this pull request as ready for review January 15, 2025 07:33
@vaadin-bot vaadin-bot added +0.0.1 and removed +1.0.0 labels Jan 15, 2025
@vursen
Copy link
Contributor Author

vursen commented Jan 16, 2025

I wonder if it's potentially a breaking change?

@vursen vursen requested a review from mshabarov January 16, 2025 05:32
@mshabarov
Copy link
Contributor

mshabarov commented Jan 16, 2025

I wonder if it's potentially a breaking change?

I see a benefit of setting this option by default, because this gives a flexibility and customisability to developers to setup the viewport best alignment on modern side-to-side device screens, e.g. with env(safe-area-inset-*) - LGTM.

Though this should be tested on various devices before released. And if we spot any significant changes in displaying, we may want to postpone this change to Vaadin 25.

Do you @vursen or someone from Design System team mind to test this on various devices or shall Flow team do?

@vursen
Copy link
Contributor Author

vursen commented Jan 20, 2025

Do you @vursen or someone from Design System team mind to test this on various devices or shall Flow team do?

Based on our internal testing across various devices (iOS, Android), this change only has an effect on PWAs on iOS, where it enables the env variables so that they start behaving the same as in regular iOS Safari. So, it seems to be safe.

@mshabarov mshabarov merged commit fa584e1 into main Jan 20, 2025
26 checks passed
@mshabarov mshabarov deleted the set-viewport-fit-cover branch January 20, 2025 08:40
vaadin-bot pushed a commit that referenced this pull request Jan 20, 2025
…0836)

Using env() variables in PWAs requires viewport-fit=cover to be set in the <meta name="viewport">. Without this setting, some parts of such components as app-layout get cut off on iOS devices. Adding viewport-fit follows MDN's documentation about env() variables and helps fix such issues .

Fixes vaadin/web-components#8449
vaadin-bot added a commit that referenced this pull request Jan 20, 2025
…0836) (#20881)

Using env() variables in PWAs requires viewport-fit=cover to be set in the <meta name="viewport">. Without this setting, some parts of such components as app-layout get cut off on iOS devices. Adding viewport-fit follows MDN's documentation about env() variables and helps fix such issues .

Fixes vaadin/web-components#8449

Co-authored-by: Sergey Vinogradov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AppLayout marginals look broken on iPhone 15
3 participants