From 2a737523a6da34f493a0c11adc7cba6435860e63 Mon Sep 17 00:00:00 2001 From: Dima Voytenko Date: Tue, 10 Sep 2019 16:00:18 -0700 Subject: [PATCH 1/3] Lightbox: work in progress --- index-react.html | 116 ++++++++++++++- index.html | 36 +++++ src/amp-element.js | 8 + src/amp-react-lightbox.js | 243 +++++++++++++++++++++++++++++++ src/amp-react-utils.js | 3 + src/react-compat-base-element.js | 106 ++++++++++++-- 6 files changed, 501 insertions(+), 11 deletions(-) create mode 100644 src/amp-react-lightbox.js diff --git a/index-react.html b/index-react.html index 8035510..ac90b9f 100644 --- a/index-react.html +++ b/index-react.html @@ -6,15 +6,18 @@ - + + @@ -28,11 +31,75 @@ class App extends React.Component { state = { slides: [0, 1, 2, 3, 4, 5], + lightbox1Open: false, }; + constructor(props) { + super(props); + this.backButtonService = new BackButtonServiceFake(); + this.buttonRef = React.createRef(); + } + render() { + const lightbox1BlockedMessage = + 'This lightbox has unsaved changes. Are you sure you want to close it?'; return (
+ + + {props => ( + this.setState({lightbox1Open: false})} + closeConfirm={this.state.lightbox1Blocked && lightbox1BlockedMessage} + deps={{backButton: this.backButtonService}} + // TBD: no way to inject ref.current instead? No re-render + // on hydration... + transitionAnchor={this.buttonRef} + > +
This is a lightbox
+ + + + +
this.setState({lightbox1Blocked: true})} + onReset={() => this.setState({lightbox1Blocked: false})} + > + + + +
+ +
scroll down...
+ + + + + )} + + + +
+ +
+ +

amp-react-img

amp-react-img } } -ReactDOM.render(, document.getElementById('app')) +// TODO: redo this service via Router.history object. +class BackButtonServiceFake { + /** @return BackButtonMarker */ + push() { + return new BackButtonMarkerFake(); + } +} + +class BackButtonMarkerFake { + constructor() { + this.closed = false; + /** @type {?function()} */ + this.onPop = null; + + /** @private {boolean} */ + this.popped_ = false; + this.popHandler_ = () => { + this.popped_ = true; + this.pop(); + }; + window.addEventListener('popstate', this.popHandler_); + history.pushState({index: 1}, ''); + } + + pop() { + if (this.closed) { + return; + } + this.closed = true; + if (this.onPop) { + this.onPop(); + this.onPop = null; + } + if (!this.popped_) { + this.popped_ = true; + history.go(-1); + } + if (this.popHandler_) { + window.removeEventListener('popstate', this.popHandler_); + this.popHandler_ = null; + } + } +} + + +ReactDOM.render(, document.getElementById('app')) diff --git a/index.html b/index.html index f1b81fb..3c4cd30 100644 --- a/index.html +++ b/index.html @@ -174,9 +174,12 @@ + + + @@ -200,6 +203,39 @@ + + +
+ +
+

amp-react-img

setOpen(false); + } + + function closeOnEsc(e) { + if (e.key == 'Escape') { + setOpen(false); + } + } + + function closeOnClick(e) { + if (e.target == backdropRef.current) { + setOpen(false); + } + } + + // An effect whenever the isOpen state changes. + useEffect(() => { + if (isOpen) { + // Do open. + if (backButtonService) { + const backButtonMarker = backButtonService.push(); + backButtonMarker.onPop = () => setOpen(false); + backButtonStateRef.current = backButtonMarker; + } + document.addEventListener('keyup', closeOnEsc); + // TODO: Transition: a pretty stupid sample code. + const transitionAnchorEl = transitionAnchor && transitionAnchor.current; + if (transitionAnchorEl) { + const backdrop = backdropRef.current; + backdrop.style.opacity = 0; + backdrop.style.transition = 'opacity 0.3s'; + transitionAnchorEl.style.transition = 'transform 0.3s'; + transitionAnchorEl.style.transform = 'scale(3) translate(50px, 50px)'; + transitionAnchorEl.addEventListener('transitionend', () => { + backdrop.style.opacity = 1; + backdrop.addEventListener('transitionend', () => { + backdrop.style.transition = ''; + transitionAnchorEl.style.transition = ''; + transitionAnchorEl.style.transform = ''; + }, {once: true}); + }, {once: true}); + } + } else { + // Do close. + setOpen(false); + const backButtonMarker = backButtonStateRef.current; + if (backButtonMarker) { + backButtonMarker.pop(); + backButtonStateRef.current = null; + } + document.removeEventListener('keyup', closeOnEsc); + if (onClose) { + onClose(); + } + } + }, [isOpen]); + + + + // Render. Imagine JSX here. + + // Navigation arrows. + const closeButton = () => { + // Default button. + const outs = { + onClick: () => setOpen(false), + }; + outs['style'] = { + position: 'absolute', + width: '32px', + height: '32px', + top: '8px', + right: '8px', + // Other styles. + background: 'rgba(0, 0, 0, 0.25)', + }; + return React.createElement('button', outs, 'x'); + }; + + // Creates scroller element with `overflow-x: auto`. + const container = () => { + const outs = { + ref: backdropRef, + onClick: closeOnClick, + }; + outs['style'] = { + display: isOpen ? 'block' : 'none', + // TBD: how is lightbox's background customized with the backdrop? + backgroundColor: 'rgba(255, 255, 255, 0.97)', + border: '1px dotted black', + paddingTop: '16px', + zIndex: 10000000, + position: 'fixed', + top: 0, + left: 0, + right: 0, + bottom: 0, + boxSizing: 'border-box', + // Lightbox is immediately scrollable by default. + overscrollBehavior: 'contain', + overflowX: 'auto', + overflowY: 'auto', + WebkitOverflowScrolling: 'touch', + }; + return React.createElement('div', outs, closeButton(), content); + }; + + return container(); +} + +function useOpenState(prop, closeConfirm) { + const [isOpen, setOpen] = useStateFromProp(prop || false); + return [ + isOpen, + // TBD/TODO: make a stable function. Per React's docs: + // "React guarantees that setState function identity is stable and won't + // change on re-renders." + function set(value) { + if (isOpen && !value && closeConfirm) { + const msg = + typeof closeConfirm == 'string' ? + closeConfirm : + 'Are you sure you want to close?'; + if (window.confirm(msg)) { + setOpen(value); + } + } else { + setOpen(value); + } + }, + ]; +} + +const AmpReactLightbox = ReactCompatibleBaseElement(AmpLightbox, { + attrs: { + 'transition-anchor': { + prop: 'transitionAnchor', + type: 'Element', + }, + }, + passthrough: true, + children: { + 'children': '*', + }, + // TBD: do property->dep mapping instead? + // TBD: required vs optional? + deps: ['backButton'], + + // TBD: this API extensions may warrant overriding the base-base + // ReactBaseElement. Any other approach better here? + init(reactBaseElement) { + const {element} = reactBaseElement; + element.style.position = 'fixed'; + element.style.zIndex = 11111111; + return { + onClose() { + element.setAttribute('hidden', ''); + reactBaseElement.mutateProps({open: false}); + }, + }; + }, + executeAction(reactBaseElement, invocation) { + // TODO: keep ugly private calls until the API extension question is + // resolved. + const {element} = reactBaseElement; + switch (invocation.action) { + case 'activate': + case 'open': + element.removeAttribute('hidden'); + reactBaseElement.mutateProps({open: true}); + break; + } + }, +}); +customElements.define('amp-react-lightbox', AmpReactLightbox); diff --git a/src/amp-react-utils.js b/src/amp-react-utils.js index f39097d..68da81a 100644 --- a/src/amp-react-utils.js +++ b/src/amp-react-utils.js @@ -36,6 +36,9 @@ export function useStateFromProp(prop) { } return [ valueRef.current, + // TBD/TODO: make a stable function. Per React's docs: + // "React guarantees that setState function identity is stable and won't + // change on re-renders." function set(value) { if (!Object.is(value, valueRef.current)) { valueRef.current = value; diff --git a/src/react-compat-base-element.js b/src/react-compat-base-element.js index 3c9dd77..9295235 100644 --- a/src/react-compat-base-element.js +++ b/src/react-compat-base-element.js @@ -60,6 +60,8 @@ export default function ReactCompatibleBaseElement(Component, opts) { /** @private {?Node} */ this.container_ = null; + this.customProps_ = {}; + /** @private {?Component} */ this.el_ = null; @@ -71,6 +73,15 @@ export default function ReactCompatibleBaseElement(Component, opts) { renderable: false, playable: false, }; + + // TBD: Yep. Very ugly. See notes in `opts` decl in + // `amp-react-lightbox.js`. + if (opts.init) { + const props = opts.init(this); + if (props) { + Object.assign(this.customProps_, props); + } + } } /** @@ -133,6 +144,11 @@ export default function ReactCompatibleBaseElement(Component, opts) { // el.setState({ inViewport }); } + mutateProps(props) { + Object.assign(this.customProps_, props); + this.rerender_(); + } + /** @private */ scheduleRender_() { if (!this.renderScheduled_) { @@ -148,7 +164,7 @@ export default function ReactCompatibleBaseElement(Component, opts) { rerender_() { if (!this.container_) { // TBD: create container/shadow in the amp-element.js? - if (opts.children) { + if (opts.children || opts.passthrough) { this.container_ = this.element.attachShadow({mode: 'open'}); } else { this.container_ = this.getWin().document.createElement('i-amphtml-c'); @@ -162,7 +178,7 @@ export default function ReactCompatibleBaseElement(Component, opts) { } } - const props = collectProps(this.element, opts); + const props = {...collectProps(this.element, opts), ...this.customProps_}; // While this "creates" a new element, React's diffing will not create // a second instance of Component. Instead, the existing one already @@ -319,10 +335,6 @@ export default function ReactCompatibleBaseElement(Component, opts) { throw new Error('unimplemented'); } - activate(unusedInvocation) { - throw new Error('unimplemented'); - } - activationTrust() { throw new Error('unimplemented'); } @@ -348,7 +360,9 @@ export default function ReactCompatibleBaseElement(Component, opts) { } executeAction(invocation, unusedDeferred) { - throw new Error('unimplemented'); + if (opts.executeAction) { + return opts.executeAction(this, invocation); + } } getDpr() { @@ -491,7 +505,14 @@ function collectProps(element, opts) { const { name, value } = attributes[i]; const def = defs[name]; if (def) { - const v = def.type == 'number' ? Number(value) : value; + const v = + def.type == 'number' ? + Number(value) : + def.type == 'Element' ? + // TBD: what's the best way for element referencing compat between + // React and AMP? Currently modeled as a Ref. + {current: element.getRootNode().getElementById(value)} : + value; props[def.prop] = v; } else if (name == 'class') { props.className = value; @@ -507,7 +528,9 @@ function collectProps(element, opts) { // as separate properties. Thus in a carousel the plain "children" are // slides, and the "arrowNext" children are passed via a "arrowNext" // property. - if (opts.children) { + if (opts.passthrough) { + props.children = [React.createElement('slot')]; + } else if (opts.children) { const children = []; for (let i = 0; i < element.children.length; i++) { const childElement = element.children[i]; @@ -534,9 +557,74 @@ function collectProps(element, opts) { props.children = children; } + // Dependencies. + // TBD: what would really happen is that we will subscribe to dependencies + // via revamp. + // TBD: direct props vs context props. + if (opts.deps) { + props.deps = {}; + opts.deps.forEach(id => { + props.deps[id] = resolveDep(id); + }); + } + return props; } +/** + * Stub implementation. This will actually be something very different. + * @param {!Id} id + * @return {?T} + */ +function resolveDep(id) { + switch (id) { + case 'backButton': + class BackButtonMarkerFake { + constructor() { + this.closed = false; + /** @type {?function()} */ + this.onPop = null; + + /** @private {boolean} */ + this.popped_ = false; + this.popHandler_ = () => { + this.popped_ = true; + this.pop(); + }; + window.addEventListener('popstate', this.popHandler_); + history.pushState({index: 1}, ''); + } + + pop() { + if (this.closed) { + return; + } + this.closed = true; + if (this.onPop) { + this.onPop(); + this.onPop = null; + } + if (!this.popped_) { + this.popped_ = true; + history.go(-1); + } + if (this.popHandler_) { + window.removeEventListener('popstate', this.popHandler_); + this.popHandler_ = null; + } + } + } + class BackButtonServiceFake { + /** @return BackButtonMarker */ + push() { + return new BackButtonMarkerFake(); + } + } + return new BackButtonServiceFake(); + } + return null; +} + /** * @param {!Element} element * @param {!Object} defs From fa63c17f47fd16523dd525b7f3b4edce296d01f2 Mon Sep 17 00:00:00 2001 From: Dima Voytenko Date: Tue, 17 Sep 2019 13:14:57 -0700 Subject: [PATCH 2/3] review fixes --- src/amp-react-lightbox.js | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/amp-react-lightbox.js b/src/amp-react-lightbox.js index 39ffe19..fb2b934 100644 --- a/src/amp-react-lightbox.js +++ b/src/amp-react-lightbox.js @@ -69,18 +69,6 @@ export function AmpLightbox(props) { backButtonStateRef.current.onPop = () => setOpen(false); } - function closeOnEsc(e) { - if (e.key == 'Escape') { - setOpen(false); - } - } - - function closeOnClick(e) { - if (e.target == backdropRef.current) { - setOpen(false); - } - } - // An effect whenever the isOpen state changes. useEffect(() => { if (isOpen) { @@ -90,6 +78,11 @@ export function AmpLightbox(props) { backButtonMarker.onPop = () => setOpen(false); backButtonStateRef.current = backButtonMarker; } + function closeOnEsc(e) { + if (e.key == 'Escape') { + setOpen(false); + } + } document.addEventListener('keyup', closeOnEsc); // TODO: Transition: a pretty stupid sample code. const transitionAnchorEl = transitionAnchor && transitionAnchor.current; @@ -108,6 +101,9 @@ export function AmpLightbox(props) { }, {once: true}); }, {once: true}); } + return function unlisten() { + document.removeEventListener('keyup', closeOnEsc); + }; } else { // Do close. setOpen(false); @@ -116,7 +112,6 @@ export function AmpLightbox(props) { backButtonMarker.pop(); backButtonStateRef.current = null; } - document.removeEventListener('keyup', closeOnEsc); if (onClose) { onClose(); } @@ -149,7 +144,11 @@ export function AmpLightbox(props) { const container = () => { const outs = { ref: backdropRef, - onClick: closeOnClick, + onClick: e => { + if (e.target == backdropRef.current) { + setOpen(false); + } + }, }; outs['style'] = { display: isOpen ? 'block' : 'none', @@ -207,9 +206,6 @@ const AmpReactLightbox = ReactCompatibleBaseElement(AmpLightbox, { }, }, passthrough: true, - children: { - 'children': '*', - }, // TBD: do property->dep mapping instead? // TBD: required vs optional? deps: ['backButton'], From 14e418800bc4ed4ce3ae5aee40f476211b443af7 Mon Sep 17 00:00:00 2001 From: Dima Voytenko Date: Tue, 17 Sep 2019 13:18:24 -0700 Subject: [PATCH 3/3] fixes --- src/react-compat-base-element.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/react-compat-base-element.js b/src/react-compat-base-element.js index 9295235..3bfcf1e 100644 --- a/src/react-compat-base-element.js +++ b/src/react-compat-base-element.js @@ -146,7 +146,7 @@ export default function ReactCompatibleBaseElement(Component, opts) { mutateProps(props) { Object.assign(this.customProps_, props); - this.rerender_(); + this.scheduleRender_(); } /** @private */