🖍 [Story performance] Remove standalone classes (#36127)

* Remove standalone classes * Updated specificity * Added specificity again * Remove standalone classes * Updated specificity * Added specificity again * Control element lifecycle with height auto * Started changing tests to use AmpStory * Reverted to automatic height * Remove unused test setup * Updated comment to make it more clear * Remove unused rule
ampproject · Oct 8, 2021 · 610583b · 610583b
1 parent 70fc163
commit 610583b
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 10 deletions.
diff --git a/extensions/amp-story/1.0/amp-story.css b/extensions/amp-story/1.0/amp-story.css
@@ -82,8 +82,7 @@ amp-story {
   touch-action: manipulation !important;
 }
 
-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;
@@ -121,7 +120,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;

diff --git a/extensions/amp-story/1.0/amp-story.js b/extensions/amp-story/1.0/amp-story.js
@@ -477,8 +477,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.

diff --git a/extensions/amp-story/1.0/test/test-amp-story.js b/extensions/amp-story/1.0/test/test-amp-story.js
@@ -22,6 +22,7 @@ import {createElementWithAttributes} from '#core/dom';
 import {registerServiceBuilder} from '../../../../src/service-helpers';
 import {toggleExperiment} from '#experiments';
 import {waitFor} from '#testing/test-helper';
+import {setImportantStyles} from '#core/dom/style';
 
 // Represents the correct value of KeyboardEvent.which for the Right Arrow
 const KEYBOARD_EVENT_WHICH_RIGHT_ARROW = 39;
@@ -116,6 +117,10 @@ describes.realWin(
       });
 
       AmpStory.isBrowserSupported = () => true;
+
+      // Fakes the size of amp-story so it's not built/laid out until we call build/layoutCallbacks
+      // allowing us to mock function calls before the lifecycle callbacks.
+      setImportantStyles(win.document.documentElement, {'height': 'auto'});
     });
 
     afterEach(() => {
@@ -131,7 +136,6 @@ describes.realWin(
 
     it('should activate the first page when built', async () => {
       await createStoryWithPages(2, ['cover', 'page-1']);
-
       await story.layoutCallback();
       // Getting all the AmpStoryPage objets.
       const pageElements = story.element.getElementsByTagName('amp-story-page');
@@ -161,7 +165,6 @@ describes.realWin(
       await createStoryWithPages(2);
 
       const buildShareMenuStub = env.sandbox.stub(story.shareMenu_, 'build');
-
       await story.layoutCallback();
       expect(buildShareMenuStub).to.have.been.calledOnce;
     });
@@ -183,7 +186,6 @@ describes.realWin(
 
     it('should pause/resume pages when switching pages', async () => {
       await createStoryWithPages(2, ['cover', 'page-1']);
-
       await story.layoutCallback();
       // Getting all the AmpStoryPage objects.
       const pageElements = story.element.getElementsByTagName('amp-story-page');
@@ -235,7 +237,6 @@ describes.realWin(
 
     it('lock body when amp-story is initialized', async () => {
       await createStoryWithPages(2, ['cover', 'page-1']);
-
       await story.layoutCallback();
       story.lockBody_();
       expect(win.document.body.style.getPropertyValue('overflow')).to.be.equal(
@@ -248,7 +249,6 @@ describes.realWin(
 
     it('checks if pagination buttons exist ', async () => {
       await createStoryWithPages(2, ['cover', 'page-1']);
-
       await story.layoutCallback();
       expect(
         story.element.querySelectorAll('.i-amphtml-story-button-container')