Skip to content

🖍 [Story performance] Remove standalone classes #36127

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

Merged
mszylkowski merged 18 commits into from
Oct 8, 2021
Merged

🖍 [Story performance] Remove standalone classes #36127

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
278b389
Remove standalone classes
mszylkowski Sep 21, 2021
8b16778
Updated specificity
mszylkowski Sep 22, 2021
43840c2
Added specificity again
mszylkowski Sep 22, 2021
9eb6688
Merge branch 'main' of github.com:ampproject/amphtml into remove_runt…
mszylkowski Sep 22, 2021
b7b7036
Merge branch 'main' of github.com:ampproject/amphtml into remove_runt…
mszylkowski Sep 22, 2021
41575d6
Remove standalone classes
mszylkowski Sep 21, 2021
c519391
Updated specificity
mszylkowski Sep 22, 2021
cd44147
Added specificity again
mszylkowski Sep 22, 2021
2b47f13
Merge branch 'remove_runtimeclasses' of github.com:mszylkowski/amphtm…
mszylkowski Sep 22, 2021
e824e7f
Control element lifecycle with height auto
mszylkowski Sep 23, 2021
4d8805c
Started changing tests to use AmpStory
mszylkowski Sep 24, 2021
2636c3a
Reverted to automatic height
mszylkowski Oct 5, 2021
e87efc4
Remove unused test setup
mszylkowski Oct 5, 2021
c39680f
Merge branch 'main' of github.com:ampproject/amphtml into remove_runt…
mszylkowski Oct 5, 2021
37f0a80
Updated comment to make it more clear
mszylkowski Oct 6, 2021
cade19e
Merge branch 'main' of github.com:ampproject/amphtml into remove_runt…
mszylkowski Oct 7, 2021
50476ef
Remove unused rule
mszylkowski Oct 7, 2021
504c12f
Merge branch 'main' of github.com:ampproject/amphtml into remove_runt…
mszylkowski Oct 8, 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
7 changes: 3 additions & 4 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 @@ -76,12 +76,11 @@ amp-story {
touch-action: manipulation !important;
}

html.i-amphtml-story-standalone {
html:root {
font-size: calc(2.5 * var(--story-page-vmax, 8px));
}

html.i-amphtml-story-standalone,
html.i-amphtml-story-standalone body {
html:root, html:root body {
font-size: calc(2.5 * var(--story-page-vh, 8px));
height: 100% !important;
margin: 0 !important;
Expand Down Expand Up @@ -119,7 +118,7 @@ h6 {
font-size: 0.67rem;
}

html.i-amphtml-story-standalone #i-amphtml-wrapper body {
html:root #i-amphtml-wrapper body {
/** AMP runtime adds a 1px border on iOS iframes, causing the body to be
1px bigger than the viewport. */
border-top: none !important;
Expand Down
2 changes: 0 additions & 2 deletions extensions/amp-story/1.0/amp-story.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,8 +503,6 @@ export class AmpStory extends AMP.BaseElement {
* @private
*/
initializeStandaloneStory_() {
const html = this.win.document.documentElement;
html.classList.add('i-amphtml-story-standalone');
// Lock body to prevent overflow.
this.lockBody_();
// Standalone CSS affects sizing of the entire page.
Expand Down
12 changes: 10 additions & 2 deletions extensions/amp-story/1.0/test/test-amp-story.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ describes.realWin(
* @param {Array<string>=} ids
* @return {!Array<!Element>}
*/
async function createStoryWithPages(count, ids = [], autoAdvance = false) {
function createStoryWithPages(count, ids = [], autoAdvance = false) {
element = win.document.createElement('amp-story');
element.getAmpDoc = () => env.ampdoc;

const pageArray = Array(count)
.fill(undefined)
Expand All @@ -67,7 +68,7 @@ describes.realWin(
});

win.document.body.appendChild(element);
story = await element.getImpl();
story = new AmpStory(element);

return pageArray;
}
Expand Down Expand Up @@ -126,13 +127,15 @@ describes.realWin(
it('should build with the expected number of pages', async () => {
const pagesCount = 2;
await createStoryWithPages(pagesCount, ['cover', 'page-1']);
await story.buildCallback();
await story.layoutCallback();
expect(story.getPageCount()).to.equal(pagesCount);
});

it('should activate the first page when built', async () => {
await createStoryWithPages(2, ['cover', 'page-1']);

await story.buildCallback();
await story.layoutCallback();
// Getting all the AmpStoryPage objets.
const pageElements = story.element.getElementsByTagName('amp-story-page');
Expand Down Expand Up @@ -163,12 +166,14 @@ describes.realWin(

const buildShareMenuStub = env.sandbox.stub(story.shareMenu_, 'build');

await story.buildCallback();
await story.layoutCallback();
expect(buildShareMenuStub).to.have.been.calledOnce;
});

it('should return a valid page index', async () => {
await createStoryWithPages(4, ['cover', 'page-1', 'page-2', 'page-3']);
await story.buildCallback();
await story.layoutCallback();
// Getting all the AmpStoryPage objets.
const pageElements = story.element.getElementsByTagName('amp-story-page');
Expand All @@ -185,6 +190,7 @@ describes.realWin(
it('should pause/resume pages when switching pages', async () => {
await createStoryWithPages(2, ['cover', 'page-1']);

await story.buildCallback();
await story.layoutCallback();
// Getting all the AmpStoryPage objects.
const pageElements = story.element.getElementsByTagName('amp-story-page');
Expand Down Expand Up @@ -237,6 +243,7 @@ describes.realWin(
it('lock body when amp-story is initialized', async () => {
await createStoryWithPages(2, ['cover', 'page-1']);

await story.buildCallback();
await story.layoutCallback();
story.lockBody_();
expect(win.document.body.style.getPropertyValue('overflow')).to.be.equal(
Expand All @@ -250,6 +257,7 @@ describes.realWin(
it('checks if pagination buttons exist ', async () => {
await createStoryWithPages(2, ['cover', 'page-1']);

await story.buildCallback();
await story.layoutCallback();
expect(
story.element.querySelectorAll('.i-amphtml-story-button-container')
Expand Down