Skip to content

🖍 [Story performance] Added CSS rules for units, and exaple stories #36007

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.

Sign up for GitHub

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

Closed
mszylkowski wants to merge 5 commits into from
+129 −14
Closed

🖍 [Story performance] Added CSS rules for units, and exaple stories #36007

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
51e1ed9
Added CSS rules for units
mszylkowski Sep 7, 2021
00e2b1c
Removed 50vw
mszylkowski Sep 9, 2021
17f6cf7
Merge branch 'main' of github.com:ampproject/amphtml into css_units_m…
mszylkowski Sep 9, 2021
9942c91
Added font and example files
mszylkowski Sep 10, 2021
6bd9a22
Removed noamp example
mszylkowski Sep 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Added CSS rules for units
  • Loading branch information
@mszylkowski
mszylkowski committed Sep 7, 2021
commit 51e1ed9b4b07dc9310a64c02c206e23a175d299d
10 changes: 2 additions & 8 deletions extensions/amp-story/1.0/amp-story-page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,9 +653,6 @@ export class AmpStoryPage extends AMP.BaseElement {
state.width = width;
state.vh = height / 100;
state.vw = width / 100;
state.fiftyVw = Math.round(width / 2);
state.vmin = Math.min(state.vh, state.vw);
state.vmax = Math.max(state.vh, state.vw);
},
mutate: (state) => {
const {height, width} = state;
Expand All @@ -671,11 +668,8 @@ export class AmpStoryPage extends AMP.BaseElement {
}
this.cssVariablesStyleEl_.textContent =
`:root {` +
`--story-page-vh: ${px(state.vh)};` +
`--story-page-vw: ${px(state.vw)};` +
`--story-page-vmin: ${px(state.vmin)};` +
`--story-page-vmax: ${px(state.vmax)};` +
`--i-amphtml-story-page-50vw: ${px(state.fiftyVw)};` +
`--story-page-vh: ${px(state.vh)} !important;` +
`--story-page-vw: ${px(state.vw)} !important;` +
`}`;
},
},
Expand Down
7 changes: 7 additions & 0 deletions extensions/amp-story/1.0/amp-story.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@
@import './pagination-buttons.css';

/** Common */
:root {
--story-page-vw: 1vw;
--story-page-vh: 1vh;
Comment thread
mszylkowski marked this conversation as resolved.
--story-page-vmin: min(var(--story-page-vw), var(--story-page-vh));
--story-page-vmax: max(var(--story-page-vw), var(--story-page-vh));
--i-amphtml-story-page-50vw: calc(var(--story-page-vw) * 50);
Copy link
Copy Markdown
Contributor

@gmajoulet gmajoulet Sep 8, 2021

Choose a reason for hiding this comment

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

The round() was really needed, but I think it was for the 3 panels desktop so hopefully we can get rid of this variable entirely.

Copy link
Copy Markdown
Contributor Author

@mszylkowski mszylkowski Sep 9, 2021

Choose a reason for hiding this comment

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

True, it gets removed entirely on #35932 so I'll also remove it from here, wait for that PR to merge, and then sync this branch and ensure everything is ok

}
amp-story, amp-story-page, amp-story-cta-layer {
contain: strict !important;
overflow: hidden !important;
Expand Down