From b114cc0bb6ef988d43972ec4087762be7e125ac9 Mon Sep 17 00:00:00 2001 From: Corey Masanto Date: Tue, 29 Mar 2022 13:57:21 -0400 Subject: [PATCH 1/4] Hide the page attachment's container element by default; unhide it when the attachment's owner has been set to itself --- .../0.1/amp-story-draggable-drawer.js | 9 +++++++-- .../0.1/amp-story-page-attachment.js | 4 ---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js b/extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js index 66c0a9910aed..9979d932001a 100644 --- a/extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js +++ b/extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js @@ -40,7 +40,7 @@ export const DrawerState = { const renderDrawerElement = () => { return (
-
+
@@ -135,6 +135,7 @@ export class DraggableDrawer extends AMP.BaseElement { this.containerEl = dev().assertElement( templateEl.querySelector('.i-amphtml-story-draggable-drawer-container') ); + this.contentEl = dev().assertElement( this.containerEl.querySelector( '.i-amphtml-story-draggable-drawer-content' @@ -177,7 +178,11 @@ export class DraggableDrawer extends AMP.BaseElement { Services.ownersForDoc(this.element).setOwner(el, this.element); } } - return Promise.resolve(); + + // `containerEl` is hidden by default, to ensure that its content is not + // rendered/loaded by the AMP Resources manager before we can set a + // different owner. Now that the owner has been set, we can unhide it. + toggle(dev().assertElement(this.containerEl), true); } /** diff --git a/extensions/amp-story-page-attachment/0.1/amp-story-page-attachment.js b/extensions/amp-story-page-attachment/0.1/amp-story-page-attachment.js index ac2f7c80adc8..31f61f8795f4 100644 --- a/extensions/amp-story-page-attachment/0.1/amp-story-page-attachment.js +++ b/extensions/amp-story-page-attachment/0.1/amp-story-page-attachment.js @@ -247,10 +247,6 @@ export class AmpStoryPageAttachment extends DraggableDrawer { while (this.element.firstChild && this.element.firstChild !== templateEl) { this.contentEl.appendChild(this.element.firstChild); } - - // Ensures the content of the attachment won't be rendered/loaded until we - // actually need it. - toggle(dev().assertElement(this.containerEl), true); } /** From 21aa9147c7c938713c30d5904e0a09ea5adc7529 Mon Sep 17 00:00:00 2001 From: Corey Masanto Date: Wed, 6 Apr 2022 15:28:31 -0400 Subject: [PATCH 2/4] Allow all story pages and grid layers in preview mode --- extensions/amp-story/1.0/amp-story-grid-layer.js | 5 +++++ extensions/amp-story/1.0/amp-story-page.js | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/extensions/amp-story/1.0/amp-story-grid-layer.js b/extensions/amp-story/1.0/amp-story-grid-layer.js index 1ab88483d446..ac80e2cbcbcd 100644 --- a/extensions/amp-story/1.0/amp-story-grid-layer.js +++ b/extensions/amp-story/1.0/amp-story-grid-layer.js @@ -64,6 +64,11 @@ export class AmpStoryGridLayer extends AmpStoryBaseLayer { return isPrerenderActivePage(element.parentElement); } + /** @override */ + static previewAllowed() { + return true; + } + /** @param {!AmpElement} element */ constructor(element) { super(element); diff --git a/extensions/amp-story/1.0/amp-story-page.js b/extensions/amp-story/1.0/amp-story-page.js index 48a5ba39b194..b0225c220b55 100644 --- a/extensions/amp-story/1.0/amp-story-page.js +++ b/extensions/amp-story/1.0/amp-story-page.js @@ -165,6 +165,11 @@ export class AmpStoryPage extends AMP.BaseElement { return isPrerenderActivePage(element); } + /** @override */ + static previewAllowed() { + return true; + } + /** @param {!AmpElement} element */ constructor(element) { super(element); From d8db4932c8429de55d0e1f25a658ce695d97d4f6 Mon Sep 17 00:00:00 2001 From: Corey Masanto Date: Wed, 6 Apr 2022 16:06:35 -0400 Subject: [PATCH 3/4] Revert unrelated change --- .../amp-story-page-attachment/0.1/amp-story-draggable-drawer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js b/extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js index 3321dc3a1402..16c35276dd62 100644 --- a/extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js +++ b/extensions/amp-story-page-attachment/0.1/amp-story-draggable-drawer.js @@ -42,7 +42,7 @@ export const DrawerState = { const renderDrawerElement = () => { return (
- From f6340df5b769095267f593cddf3b51868667ad4e Mon Sep 17 00:00:00 2001 From: Corey Masanto Date: Wed, 6 Apr 2022 18:31:02 -0400 Subject: [PATCH 4/4] Add amp-story-page tests for preview vs prerender mode --- .../amp-story/1.0/test/test-amp-story-page.js | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/extensions/amp-story/1.0/test/test-amp-story-page.js b/extensions/amp-story/1.0/test/test-amp-story-page.js index f158ba847265..a4ae07aa6a8c 100644 --- a/extensions/amp-story/1.0/test/test-amp-story-page.js +++ b/extensions/amp-story/1.0/test/test-amp-story-page.js @@ -29,6 +29,7 @@ describes.realWin('amp-story-page', {amp: {extensions}}, (env) => { let html; let gridLayerEl; let page; + let story; let storeService; let isPerformanceTrackingOn; @@ -67,7 +68,7 @@ describes.realWin('amp-story-page', {amp: {extensions}}, (env) => { }; }); - const story = win.document.createElement('amp-story'); + story = win.document.createElement('amp-story'); story.getImpl = () => Promise.resolve(mediaPoolRoot); // Makes whenUpgradedToCustomElement() resolve immediately. story.createdCallback = Promise.resolve(); @@ -713,4 +714,26 @@ describes.realWin('amp-story-page', {amp: {extensions}}, (env) => { expect(startMeasuringStub).to.not.have.been.called; }); + + it('should only allow the prerender visibility state if it is the first page', async () => { + const pageElement2 = win.document.createElement('amp-story-page'); + const pageElement3 = win.document.createElement('amp-story-page'); + story.appendChild(pageElement2); + story.appendChild(pageElement3); + + expect(AmpStoryPage.prerenderAllowed(element)).to.be.true; + expect(AmpStoryPage.prerenderAllowed(pageElement2)).to.be.false; + expect(AmpStoryPage.prerenderAllowed(pageElement3)).to.be.false; + }); + + it('should always allow the preview visibility state', async () => { + const pageElement2 = win.document.createElement('amp-story-page'); + const pageElement3 = win.document.createElement('amp-story-page'); + story.appendChild(pageElement2); + story.appendChild(pageElement3); + + expect(AmpStoryPage.previewAllowed(element)).to.be.true; + expect(AmpStoryPage.previewAllowed(pageElement2)).to.be.true; + expect(AmpStoryPage.previewAllowed(pageElement3)).to.be.true; + }); });