Skip to content

🚀 [Story performance] Add css to show story from beginning#36333

Merged
mszylkowski merged 17 commits intoampproject:mainfrom
mszylkowski:showstory_onlycss
Oct 21, 2021
Merged

🚀 [Story performance] Add css to show story from beginning#36333
mszylkowski merged 17 commits intoampproject:mainfrom
mszylkowski:showstory_onlycss

Conversation

@mszylkowski
Copy link
Copy Markdown
Contributor

Closes #36050

Adds styles to display the first page of the story before the JS loads.

@mszylkowski mszylkowski requested a review from gmajoulet October 12, 2021 19:22
@mszylkowski mszylkowski self-assigned this Oct 12, 2021
@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented Oct 12, 2021

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

extensions/amp-story/1.0/amp-story.css

position: absolute !important;
top: 0;
left: 0;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How come you don't need width/height 100% too? Do you get them from some other CSS?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ampshared.css provides them

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't really have any guarantee that ampshared is loaded if we inline this code in the HEAD of the document. We should add these statements.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a good point, I missed that https://github.com/ampproject/amphtml/blob/main/extensions/amp-story/1.0/amp-story.css#L76 also provides them, so we're good. I don't think there are other styles we need from there, but it opens up the conversation of whether we'd need ampshared.css on stories, or we can save up those extra bytes

@mszylkowski mszylkowski requested a review from gmajoulet October 13, 2021 21:15
position: absolute !important;
top: 0;
left: 0;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't really have any guarantee that ampshared is loaded if we inline this code in the HEAD of the document. We should add these statements.

}

amp-story-page:not([active]) {
amp-story.i-amphtml-story-loaded amp-story-page:not([active]) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do you need this change?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Inactive pages get hidden with the style:

amp-story-page:not(:first-of-type):not([distance]):not([active]) {
    transform: translateY(1000vh)!important;
}

but we want to show the first page with CSS, so we need to say that we only want to hide the non-active pages when the story is loaded. This will make the first page visible, but all the other ones not. When the story loads, all the inactive pages will be hidden (due to this rule on line 235) but before the story is loaded, the CSS I showed above will only show the first page

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As seen offline, let's revisit this and only rely on [distance] and [active] attr

@mszylkowski mszylkowski requested a review from gmajoulet October 14, 2021 20:24
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.

[Story performance] Show story as soon as the JS loads, without waiting for the runtime build callbacks

3 participants