Sidebar to add its own intersection observer to the scheduler (#33503)

ampproject · Mar 25, 2021 · 30dba7c · 30dba7c
1 parent 5d90476
commit 30dba7c
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 0 deletions.
diff --git a/extensions/amp-sidebar/0.1/amp-sidebar.js b/extensions/amp-sidebar/0.1/amp-sidebar.js
@@ -42,6 +42,7 @@ import {removeFragment} from '../../../src/url';
 import {setModalAsClosed, setModalAsOpen} from '../../../src/modal';
 import {setStyles, toggle} from '../../../src/style';
 import {toArray} from '../../../src/types';
+import {unmountAll} from '../../../src/utils/resource-container-helper';
 
 /** @private @const {string} */
 const TAG = 'amp-sidebar toolbar';
@@ -469,6 +470,9 @@ export class AmpSidebar extends AMP.BaseElement {
     this.triggerEvent_(SidebarEvents.OPEN, trust);
     this.element.setAttribute('i-amphtml-sidebar-opened', '');
     this.getMaskElement_().setAttribute('i-amphtml-sidebar-opened', '');
+
+    // Set as a container for scheduler to load children elements.
+    this.setAsContainer();
   }
 
   /**
@@ -504,6 +508,13 @@ export class AmpSidebar extends AMP.BaseElement {
       this.getRealChildren()
     );
     this.triggerEvent_(SidebarEvents.CLOSE, trust);
+
+    // Undo `setAsContainer`.
+    this.removeAsContainer();
+
+    // Unmount all children when the sidebar is closed. They will automatically
+    // remount when the sidebar is opened again.
+    unmountAll(this.element, /* includeSelf */ false);
   }
 
   /**

diff --git a/extensions/amp-sidebar/0.1/test/test-amp-sidebar.js b/extensions/amp-sidebar/0.1/test/test-amp-sidebar.js
@@ -226,6 +226,8 @@ describes.realWin(
       it('should open sidebar on button click', async () => {
         const sidebarElement = await getAmpSidebar();
         const impl = await sidebarElement.getImpl(false);
+        env.sandbox.stub(sidebarElement, 'setAsContainerInternal');
+        env.sandbox.stub(sidebarElement, 'removeAsContainerInternal');
         const screenReaderCloseButton = sidebarElement.querySelector(
           'button.i-amphtml-screen-reader'
         );
@@ -273,6 +275,9 @@ describes.realWin(
         expect(owners.scheduleLayout).to.be.calledOnce;
         expect(historyPushSpy).to.be.calledOnce;
         expect(historyPopSpy).to.have.not.been.called;
+
+        expect(sidebarElement.setAsContainerInternal).to.be.calledOnce;
+        expect(sidebarElement.removeAsContainerInternal).to.not.be.called;
       });
 
       it('ignore repeated calls to open', async () => {
@@ -335,9 +340,19 @@ describes.realWin(
       it('should close sidebar on button click', async () => {
         const sidebarElement = await getAmpSidebar({'stubHistory': true});
         const impl = await sidebarElement.getImpl(false);
+        env.sandbox.stub(sidebarElement, 'setAsContainerInternal');
+        env.sandbox.stub(sidebarElement, 'removeAsContainerInternal');
         clock = fakeTimers.withGlobal(impl.win).install({
           toFake: ['Date', 'setTimeout'],
         });
+
+        // Sidebar has a child.
+        const child = createElementWithAttributes(doc, 'amp-img', {
+          layout: 'nodisplay',
+        });
+        sidebarElement.appendChild(child);
+        env.sandbox.stub(child, 'unmount');
+
         owners.schedulePause = env.sandbox.spy();
         const historyPushSpy = env.sandbox.spy();
         const historyPopSpy = env.sandbox.spy();
@@ -374,6 +389,9 @@ describes.realWin(
         execute(impl, 'close');
         expect(owners.schedulePause).to.be.calledOnce;
         expect(historyPopSpy).to.be.calledOnce;
+
+        expect(sidebarElement.removeAsContainerInternal).to.be.calledOnce;
+        expect(child.unmount).to.be.calledOnce;
       });
 
       it('should toggle sidebar on button click', async () => {

diff --git a/extensions/amp-sidebar/0.2/amp-sidebar.js b/extensions/amp-sidebar/0.2/amp-sidebar.js
@@ -43,6 +43,7 @@ import {removeFragment} from '../../../src/url';
 import {setModalAsClosed, setModalAsOpen} from '../../../src/modal';
 import {setStyles, toggle} from '../../../src/style';
 import {toArray} from '../../../src/types';
+import {unmountAll} from '../../../src/utils/resource-container-helper';
 
 /** @private @const {string} */
 const TAG = 'amp-sidebar toolbar';
@@ -467,6 +468,9 @@ export class AmpSidebar extends AMP.BaseElement {
     this.triggerEvent_(SidebarEvents.OPEN, trust);
     this.element.setAttribute('i-amphtml-sidebar-opened', '');
     this.getMaskElement_().setAttribute('i-amphtml-sidebar-opened', '');
+
+    // Set as a container for scheduler to load children elements.
+    this.setAsContainer();
   }
 
   /**
@@ -508,6 +512,13 @@ export class AmpSidebar extends AMP.BaseElement {
       this.historyId_ = -1;
     }
     this.triggerEvent_(SidebarEvents.CLOSE, trust);
+
+    // Undo `setAsContainer`.
+    this.removeAsContainer();
+
+    // Unmount all children when the sidebar is closed. They will automatically
+    // remount when the sidebar is opened again.
+    unmountAll(this.element, /* includeSelf */ false);
   }
 
   /**

diff --git a/extensions/amp-sidebar/0.2/test/test-amp-sidebar.js b/extensions/amp-sidebar/0.2/test/test-amp-sidebar.js
@@ -21,6 +21,7 @@ import {Keys} from '../../../../src/utils/key-codes';
 import {Services} from '../../../../src/services';
 import {assertScreenReaderElement} from '../../../../testing/test-helper';
 import {clearModalStack, getModalStackLength} from '../../../../src/modal';
+import {createElementWithAttributes} from '../../../../src/dom';
 import {toggleExperiment} from '../../../../src/experiments';
 
 // Represents the correct value of KeyboardEvent.which for the Escape key
@@ -221,6 +222,8 @@ describes.realWin(
       it('should open sidebar on button click', async () => {
         const sidebarElement = await getAmpSidebar();
         const impl = await sidebarElement.getImpl(false);
+        env.sandbox.stub(sidebarElement, 'setAsContainerInternal');
+        env.sandbox.stub(sidebarElement, 'removeAsContainerInternal');
         const screenReaderCloseButton = sidebarElement.querySelector(
           'button.i-amphtml-screen-reader'
         );
@@ -268,6 +271,9 @@ describes.realWin(
         expect(owners.scheduleLayout).to.be.calledOnce;
         expect(historyPushSpy).to.be.calledOnce;
         expect(historyPopSpy).to.have.not.been.called;
+
+        expect(sidebarElement.setAsContainerInternal).to.be.calledOnce;
+        expect(sidebarElement.removeAsContainerInternal).to.not.be.called;
       });
 
       it('ignore repeated calls to open', async () => {
@@ -293,9 +299,19 @@ describes.realWin(
       it('should close sidebar on button click', async () => {
         const sidebarElement = await getAmpSidebar({'stubHistory': true});
         const impl = await sidebarElement.getImpl(false);
+        env.sandbox.stub(sidebarElement, 'setAsContainerInternal');
+        env.sandbox.stub(sidebarElement, 'removeAsContainerInternal');
         clock = fakeTimers.withGlobal(impl.win).install({
           toFake: ['Date', 'setTimeout'],
         });
+
+        // Sidebar has a child.
+        const child = createElementWithAttributes(doc, 'amp-img', {
+          layout: 'nodisplay',
+        });
+        sidebarElement.appendChild(child);
+        env.sandbox.stub(child, 'unmount');
+
         owners.schedulePause = env.sandbox.spy();
         const historyPushSpy = env.sandbox.spy();
         const historyPopSpy = env.sandbox.spy();
@@ -332,6 +348,9 @@ describes.realWin(
         execute(impl, 'close');
         expect(owners.schedulePause).to.be.calledOnce;
         expect(historyPopSpy).to.be.calledOnce;
+
+        expect(sidebarElement.removeAsContainerInternal).to.be.calledOnce;
+        expect(child.unmount).to.be.calledOnce;
       });
 
       it('should toggle sidebar on button click', async () => {