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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃殌 [Story performance] Use dvh if available, instead of vh #37003

Merged
merged 5 commits into from Nov 24, 2021

Conversation

mszylkowski
Copy link
Contributor

We want to introduce a CSS variable that accurately reflects the viewport height, --story-dvh, that will allow us to inject a script inlined into the document to set the variable when the document loads, so on mobile browsers the pages won't have any CLS

@mszylkowski mszylkowski self-assigned this Nov 18, 2021
@mszylkowski mszylkowski added this to In progress in wg-stories Sprint via automation Nov 18, 2021
@amp-owners-bot
Copy link

amp-owners-bot bot commented Nov 18, 2021

Hey @gmajoulet, @newmuis! These files were changed:

extensions/amp-story/1.0/amp-story-access.css
extensions/amp-story/1.0/amp-story-desktop-one-panel.css
extensions/amp-story/1.0/amp-story-page.js
extensions/amp-story/1.0/amp-story-share-menu.css
extensions/amp-story/1.0/amp-story.css
extensions/amp-story/1.0/amp-story.js
extensions/amp-story/1.0/pagination-buttons.css
extensions/amp-story/1.0/test/test-amp-story.js

@gmajoulet gmajoulet requested review from jridgewell and erwinmombay and removed request for estherkim November 18, 2021 21:59
@@ -159,25 +159,25 @@ amp-story[standalone].amp-notbuilt {
}

amp-story[standalone]:-webkit-full-screen {
height: var(--story-100dvh) !important;
height: 100vh !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why isn't this using var(--story-100dvh)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Full screen pages don't have any issues with vh since the browser appbar is not showing, it should be always the same 1dvh = 1vh.

extensions/amp-story/1.0/amp-story.js Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants