Bento: add container to the lightbox 1.0 (ampproject#33488)

extensions/amp-lightbox/0.1/amp-lightbox.js

@@ -623,7 +623,7 @@ class AmpLightbox extends AMP.BaseElement {
 
     // Unmount all children when the lightbox is closed. They will automatically
     // remount when the lightbox is opened again.
-    unmountAll(this.element);
+    unmountAll(this.element, /* includeSelf */ false);
 
     Services.ownersForDoc(this.element).schedulePause(
       this.element,

extensions/amp-lightbox/1.0/base-element.js

@@ -20,6 +20,7 @@ import {PreactBaseElement} from '../../../src/preact/base-element';
 import {dict} from '../../../src/utils/object';
 import {toggle} from '../../../src/style';
 import {toggleAttribute} from '../../../src/dom';
+import {unmountAll} from '../../../src/utils/resource-container-helper';
 
 export class BaseElement extends PreactBaseElement {
   /** @param {!AmpElement} element */
@@ -33,24 +34,48 @@ export class BaseElement extends PreactBaseElement {
   /** @override */
   init() {
     return dict({
-      'onBeforeOpen': this.toggle_.bind(this, true),
-      'onAfterClose': this.toggle_.bind(this, false),
+      'onBeforeOpen': () => this.beforeOpen_(),
+      'onAfterOpen': () => this.afterOpen_(),
+      'onAfterClose': () => this.afterClose_(),
     });
   }
 
+  /** @override */
+  unmountCallback() {
+    this.removeAsContainer();
+  }
+
   /** @override */
   updatePropsForRendering(props) {
     props['closeButtonAs'] = () => props['closeButton'];
   }
 
-  /**
-   * Toggle open/closed attributes.
-   * @param {boolean} opt_state
-   */
-  toggle_(opt_state) {
-    this.open_ = toggleAttribute(this.element, 'open', opt_state);
-    toggle(this.element, this.open_);
-    this.triggerEvent(this.element, this.open_ ? 'open' : 'close');
+  /** @private */
+  beforeOpen_() {
+    this.open_ = true;
+    toggleAttribute(this.element, 'open', true);
+    toggle(this.element, true);
+    this.triggerEvent(this.element, 'open');
+  }
+
+  /** @private */
+  afterOpen_() {
+    const scroller = this.element.shadowRoot.querySelector('[part=scroller]');
+    this.setAsContainer(scroller);
+  }
+
+  /** @private */
+  afterClose_() {
+    this.open_ = false;
+    toggleAttribute(this.element, 'open', false);
+    toggle(this.element, false);
+    this.triggerEvent(this.element, 'close');
+
+    this.removeAsContainer();
+
+    // Unmount all children when the lightbox is closed. They will automatically
+    // remount when the lightbox is opened again.
+    unmountAll(this.element, /* includeSelf */ false);
   }
 
   /** @override */
@@ -72,7 +97,6 @@ BaseElement['props'] = {
   'animation': {attr: 'animation', media: true, default: 'fade-in'},
   'closeButton': {selector: '[slot="close-button"]', single: true},
   'children': {passthrough: true},
-  'id': {attr: 'id'},
 };
 
 /** @override */

extensions/amp-lightbox/1.0/component.js

@@ -46,6 +46,8 @@ const ANIMATION_PRESETS = {
 
 const DEFAULT_CLOSE_LABEL = 'Close the modal';
 
+const CONTENT_PROPS = {'part': 'scroller'};
+
 /**
  * @param {!LightboxDef.Props} props
  * @param {{current: (!LightboxDef.LightboxApi|null)}} ref
@@ -58,6 +60,7 @@ function LightboxWithRef(
     closeButtonAs,
     onBeforeOpen,
     onAfterClose,
+    onAfterOpen,
     ...rest
   },
   ref
@@ -77,14 +80,13 @@ function LightboxWithRef(
   const animationRef = useValueRef(animation);
   const onBeforeOpenRef = useValueRef(onBeforeOpen);
   const onAfterCloseRef = useValueRef(onAfterClose);
+  const onAfterOpenRef = useValueRef(onAfterOpen);
 
   useImperativeHandle(
     ref,
     () => ({
       open: () => {
-        if (onBeforeOpenRef.current) {
-          onBeforeOpenRef.current();
-        }
+        onBeforeOpenRef.current?.();
         setMounted(true);
         setVisible(true);
       },
@@ -108,6 +110,7 @@ function LightboxWithRef(
         setStyle(element, 'opacity', 1);
         setStyle(element, 'visibility', 'visible');
         tryFocus(element);
+        onAfterOpenRef.current?.();
       };
       if (!element.animate) {
         postVisibleAnim();
@@ -147,7 +150,7 @@ function LightboxWithRef(
         animation.cancel();
       }
     };
-  }, [visible, animationRef, onAfterCloseRef]);
+  }, [visible, animationRef, onAfterCloseRef, onAfterOpenRef]);
 
   return (
     mounted && (
@@ -159,6 +162,7 @@ function LightboxWithRef(
         part="lightbox"
         contentClassName={classes.content}
         wrapperClassName={classes.wrapper}
+        contentProps={CONTENT_PROPS}
         role="dialog"
         tabIndex="0"
         onKeyDown={(event) => {

extensions/amp-lightbox/1.0/component.type.js

@@ -27,6 +27,7 @@ var LightboxDef = {};
  *   scrollable: (boolean),
  *   initialOpen: (boolean),
  *   onBeforeOpen: (function|undefined),
+ *   onAfterOpen: (function|undefined),
  *   onAfterClose: (function|undefined),
  * }}
  */

extensions/amp-lightbox/1.0/test/test-amp-lightbox.js

@@ -19,6 +19,7 @@ import {ActionTrust, DEFAULT_ACTION} from '../../../../src/action-constants';
 import {htmlFor} from '../../../../src/static-template';
 import {poll} from '../../../../testing/iframe';
 import {toggleExperiment} from '../../../../src/experiments';
+import {whenCalled} from '../../../../testing/test-helper';
 
 describes.realWin(
   'amp-lightbox:1.0',
@@ -87,6 +88,9 @@ describes.realWin(
       }
 
       it('should open with default action', async () => {
+        env.sandbox.stub(element, 'setAsContainerInternal');
+        env.sandbox.stub(element, 'removeAsContainerInternal');
+
         expect(element.hasAttribute('open')).to.be.false;
         expect(element.hasAttribute('hidden')).to.be.true;
 
@@ -105,9 +109,18 @@ describes.realWin(
         expect(contentEls[0].textContent).to.equal('Hello World');
 
         expect(eventSpy).to.be.calledOnce;
+
+        await whenCalled(element.setAsContainerInternal);
+        const scroller = element.shadowRoot.querySelector('[part=scroller]');
+        expect(scroller).to.exist;
+        expect(element.setAsContainerInternal).to.be.calledWith(scroller);
+        expect(element.removeAsContainerInternal).to.not.be.called;
       });
 
       it('should open and close', async () => {
+        env.sandbox.stub(element, 'setAsContainerInternal');
+        env.sandbox.stub(element, 'removeAsContainerInternal');
+
         expect(element.hasAttribute('open')).to.be.false;
         expect(element.hasAttribute('hidden')).to.be.true;
 
@@ -139,6 +152,8 @@ describes.realWin(
 
         expect(openSpy).to.be.calledOnce;
         expect(closeSpy).to.be.calledOnce;
+        expect(element.setAsContainerInternal).to.not.be.called;
+        expect(element.removeAsContainerInternal).to.be.calledOnce;
       });
     });
   }

extensions/amp-lightbox/1.0/test/test-component.js

@@ -43,6 +43,10 @@ describes.sandboxed('Lightbox preact component v1.0', {}, () => {
     const closeButton = buttons.first().getDOMNode();
     expect(closeButton.getAttribute('aria-label')).to.equal('Close the modal');
     expect(closeButton.textContent).to.equal('');
+
+    // Scroller.
+    const scroller = wrapper.getDOMNode().querySelector('[part=scroller]');
+    expect(scroller).to.exist;
   });
 
   it('renders custom close button', () => {

extensions/amp-video/1.0/video-wrapper.js

@@ -157,7 +157,7 @@ function VideoWrapperWithRef(
   }, [readyDeferred]);
 
   const pause = useCallback(() => {
-    readyDeferred.promise.then(() => playerRef.current.pause());
+    readyDeferred.promise.then(() => playerRef.current?.pause());
   }, [readyDeferred]);
 
   const requestFullscreen = useCallback(() => {

src/preact/base-element.js

@@ -1147,12 +1147,16 @@ function parsePropDefs(Ctor, props, propDefs, element, mediaQueryProps) {
     let value;
     if (def.passthrough) {
       devAssert(Ctor['usesShadowDom']);
-      value = [<Slot />];
+      // Use lazy loading inside the passthrough by default due to too many
+      // elements.
+      value = [<Slot loading={Loading.LAZY} />];
     } else if (def.passthroughNonEmpty) {
       devAssert(Ctor['usesShadowDom']);
+      // Use lazy loading inside the passthrough by default due to too many
+      // elements.
       value = element.getRealChildNodes().every(IS_EMPTY_TEXT_NODE)
         ? null
-        : [<Slot />];
+        : [<Slot loading={Loading.LAZY} />];
     } else if (def.attr) {
       value = element.getAttribute(def.attr);
       if (def.media && value != null) {

src/preact/slot.js

@@ -16,6 +16,7 @@
 
 import * as Preact from './index';
 import {CanPlay, CanRender, LoadingProp} from '../core/contextprops';
+import {Loading} from '../core/loading-instructions';
 import {pureDevAssert as devAssert} from '../core/assert';
 import {
   loadAll,
@@ -26,6 +27,8 @@ import {rediscoverChildren, removeProp, setProp} from '../context';
 import {useAmpContext} from './context';
 import {useEffect, useLayoutEffect, useRef} from './index';
 
+const EMPTY = {};
+
 /**
  * @param {!Element} element
  * @param {string} name
@@ -34,7 +37,7 @@ import {useEffect, useLayoutEffect, useRef} from './index';
  */
 export function createSlot(element, name, props) {
   element.setAttribute('slot', name);
-  return <Slot {...(props || {})} name={name} />;
+  return <Slot {...(props || EMPTY)} name={name} />;
 }
 
 /**
@@ -46,7 +49,7 @@ export function createSlot(element, name, props) {
 export function Slot(props) {
   const ref = useRef(/** @type {?Element} */ (null));
 
-  useSlotContext(ref);
+  useSlotContext(ref, props);
 
   useEffect(() => {
     // Post-rendering cleanup, if any.
@@ -60,8 +63,10 @@ export function Slot(props) {
 
 /**
  * @param {{current:?}} ref
+ * @param {!JsonObject=} opt_props
  */
-export function useSlotContext(ref) {
+export function useSlotContext(ref, opt_props) {
+  const {'loading': loading} = opt_props || EMPTY;
   const context = useAmpContext();
 
   // Context changes.
@@ -98,13 +103,17 @@ export function useSlotContext(ref) {
     const slot = ref.current;
     devAssert(slot?.nodeType == 1, 'Element expected');
 
-    // TODO(#31915): switch to `mount`.
-    execute(slot, loadAll);
+    // Mount children, unless lazy loading requested. If so the element should
+    // use `BaseElement.setAsContainer`.
+    if (loading != Loading.LAZY) {
+      // TODO(#31915): switch to `mount`.
+      execute(slot, loadAll);
+    }
 
     return () => {
       execute(slot, unmountAll);
     };
-  }, [ref]);
+  }, [ref, loading]);
 }
 
 /**

src/service/scheduler.js

@@ -18,7 +18,6 @@ import {LayoutPriority} from '../layout';
 import {READY_SCAN_SIGNAL} from './resources-interface';
 import {VisibilityState} from '../visibility-state';
 import {containsNotSelf, hasNextNodeInDocumentOrder, isIframed} from '../dom';
-import {devAssert} from '../log';
 import {getServiceForDoc, registerServiceBuilderForDoc} from '../service';
 import {removeItem} from '../utils/array';
 
@@ -135,7 +134,6 @@ export class Scheduler {
    * @param {!Element=} opt_scroller
    */
   setContainer(container, opt_scroller) {
-    devAssert(!opt_scroller || container.contains(opt_scroller));
     if (this.containerMap_.has(container)) {
       return;
     }