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] Set css units on desktop for supports-landscape and regular one-panel #36033

Merged
merged 15 commits into from Oct 6, 2021

Conversation

mszylkowski
Copy link
Contributor

@mszylkowski mszylkowski commented Sep 10, 2021

Follows #36007
Closes #36045 #36046

Brings CSS support for the sizing variables on desktop one panel and desktop landscape.

In order to style the font-size depending on whether the story is supports-landscape or not, the transformers will need to add a data-story-supports-landscape on the root of the document (html tag) so that the rems are set properly. There's no way to know what rems to use (the value for 1-panel or landscape) from the root, since there are no selectors with the logic to select based on the children.

The story will work as expected when the story doesn't support landscape, or when the publisher adds the data-story-supports-landscape (which is added for supports-landscape stories on the runtime to not rely on the creator and ensure built state is good).

@mszylkowski mszylkowski changed the title 馃枍 [Story performance] Css units desktop (WIP) 馃枍 [Story performance] Set css units on desktop for supports-landscape and regular one-panel Sep 13, 2021
@mszylkowski mszylkowski self-assigned this Sep 14, 2021
@mszylkowski mszylkowski added this to In progress in wg-stories Sprint via automation Sep 14, 2021
@mszylkowski mszylkowski marked this pull request as ready for review September 14, 2021 10:22
@amp-owners-bot
Copy link

amp-owners-bot bot commented Sep 14, 2021

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

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-vertical.css
extensions/amp-story/1.0/amp-story.css
extensions/amp-story/1.0/amp-story.js

Copy link
Contributor

@gmajoulet gmajoulet left a comment

Choose a reason for hiding this comment

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

With this implementation you calculate story-page-vh/w in 3 different places. Let's do CSS only, and factorize further so the code can grow in a healthy way.

examples/amp-story/performance/splitcss.html Outdated Show resolved Hide resolved
extensions/amp-story/1.0/amp-story-page.js Outdated Show resolved Hide resolved
@mszylkowski mszylkowski merged commit 2796863 into ampproject:main Oct 6, 2021
wg-stories Sprint automation moved this from In progress to Done Oct 6, 2021
@mszylkowski mszylkowski deleted the css_units_desktop branch October 6, 2021 18:59
samouri pushed a commit to samouri/amphtml that referenced this pull request Oct 7, 2021
鈥 and regular one-panel (ampproject#36033)

* Added CSS rules for units

* Removed 50vw

* Added font and example files

* Add support for desktop panels

* Fix supports-landscape

* Fixed units CSS, removed JS

* Removed unused dep

* Remove unused JS style element

* Fix vertical rendering

* Fixed list.push to list.add

* Fixed one-panel rems
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4 participants