Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Bento Lightbox Gallery #32008

Merged
merged 31 commits into from
Feb 22, 2021
Merged
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
1aed0e1
Prototype
caroqliu Jan 15, 2021
5e43aec
Merge branch 'master' into bento-lightbox-gallery
caroqliu Feb 1, 2021
ff3a400
Update with deeply nested example
caroqliu Feb 1, 2021
296e7a4
Preact skeleton
caroqliu Feb 1, 2021
2941bc2
cloneElement takes a children param
caroqliu Feb 1, 2021
490e8e5
Merge branch 'master' into bento-lightbox-gallery
caroqliu Feb 11, 2021
e536acd
Adopt new file structure
caroqliu Feb 11, 2021
d627d32
Only use WithLightbox
caroqliu Feb 11, 2021
ff2394b
Make WithLightbox more customizable
caroqliu Feb 11, 2021
25536d3
Lightbox content should support scrolling
caroqliu Feb 11, 2021
f94b78c
Support lightboxing BaseCarousel
caroqliu Feb 12, 2021
0fd6a24
Fix types
caroqliu Feb 12, 2021
752fea6
Merge branch 'master' into bento-lightbox-gallery
caroqliu Feb 12, 2021
86cf9ff
Specify cross-component file dependencies
caroqliu Feb 12, 2021
f5fc8f7
Dedupe contentRef and lightboxRef
caroqliu Feb 12, 2021
d453e24
With lightbox does not need a ref
caroqliu Feb 12, 2021
bdec869
Merge branch 'master' into bento-lightbox-gallery
caroqliu Feb 18, 2021
e6a3baf
Fix a11y attributes
caroqliu Feb 18, 2021
708771e
Don't render elements until the LightboxGallery opens
caroqliu Feb 18, 2021
b739ce3
Don't take thumbnailSrc for lightbox
caroqliu Feb 18, 2021
3926d94
Split up files
caroqliu Feb 18, 2021
97b3256
LightboxGallery -> LightboxGalleryProvider
caroqliu Feb 18, 2021
9fc9c42
Update dependency list
caroqliu Feb 18, 2021
df95ff4
LightboxGalleryProvider render prop
caroqliu Feb 18, 2021
b1ab988
Remove key
caroqliu Feb 18, 2021
1e5b26a
Rename autoLightbox as enableActivation
caroqliu Feb 19, 2021
0145e2b
Update type definitions
caroqliu Feb 19, 2021
e99044e
Always render <Comp> layer
caroqliu Feb 19, 2021
c9cc0dd
Export directly
caroqliu Feb 19, 2021
83e6a02
Merge branch 'master' into bento-lightbox-gallery
caroqliu Feb 22, 2021
d976861
lightbox.js moved to component.js
caroqliu Feb 22, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
92 changes: 92 additions & 0 deletions extensions/amp-lightbox-gallery/1.0/storybook/Basic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/**
* Copyright 2021 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as Preact from '../../../../src/preact';
// import {LightboxGallery} from '../lightbox-gallery';
import {Lightbox} from './../../../amp-lightbox/1.0/lightbox';
import {withA11y} from '@storybook/addon-a11y';
import {withKnobs} from '@storybook/addon-knobs';

export default {
title: 'LightboxGallery',
// component: LightboxGallery,
decorators: [withKnobs, withA11y],
};

const LightboxGalleryContext = Preact.createContext({});

/**
* @param {!Object} props
* @return {*}
*/
function WithLightbox({children}) {
const {open, pushChild} = Preact.useContext(LightboxGalleryContext);
Preact.useLayoutEffect(() => {
pushChild(children);
}, []);
return <div onClick={() => open()}>{children}</div>;
}

/**
* @param {!Object} props
* @return {*}
*/
function LightboxGallery({children}) {
const lightboxRef = Preact.useRef(null);
const lightboxElements = Preact.useRef([]);
const pushChild = (el) => lightboxElements.current.push(el);
const context = {pushChild, open: () => lightboxRef.current.open()};
return (
<>
<Lightbox ref={lightboxRef}>
<div onClick={() => lightboxRef.current.close()}>close lightbox</div>
<div>{lightboxElements.current}</div>
</Lightbox>
<LightboxGalleryContext.Provider value={context}>
{Preact.toChildArray(children).map((child) =>
child.props.lightbox ? <WithLightbox>{child}</WithLightbox> : child
)}
</LightboxGalleryContext.Provider>
</>
);
}

export const _default = () => {
return (
<LightboxGallery>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think I understood your process here. And it makes sense to me. One nuance - it cannot traverse nested children. But we could enable it with making WithLightbox public. E.g. the user code could look like this:

Somewhere once:

<LightboxGallery>  // could also call it `(Auto)LightboxProvider`
  {children}
</LightboxGallery>

And elsewhere:

function SomeComponent() {
  return (
    <div>
      <WithLightbox>
         <img ..../>
      </WithLightbox>
    </div>
  );
}

This might be pretty clean from Preact point of view. Some of our components could even call WithLightbox internally.

In AMP we could enable this somehow too:

  1. The LightboxProvider attachment is very easy for us now - all it needs to do is to propagate context. We do this today with InlineGallery.
  2. The WithLightbox would be a bit harder. But I think we can figure it out with "detached" mode. Or maybe we'd just always require related components to call WithLightbox internally for now.

The important question here: do we currently clone DOM elements to put them inside the lightbox? Or do we reparent them? Or just position? This is important, because if we clone/reposition, then in React that'd probably be taken care of by portalling, which we haven't worked with yet here, but could open some doors for us.

Copy link
Contributor Author

@caroqliu caroqliu Feb 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can get nested children if we are willing to clone elements, but this is costly and somewhat hacky to drill it down like this:

<LightboxGalleryContext.Provider value={context}>
  <WithDeepLightbox>{children}</WithDeepLightbox>
</LightboxGalleryContext.Provider>
function WithDeepLightbox({children}) {
  return toChildArray(children).map((child) =>
    child.props?.lightbox ? (
      <WithLightbox>{child}</WithLightbox>
    ) : child.props?.children ? (
      Preact.cloneElement(child, null, <WithDeepLightbox>{child.props.children}</WithDeepLightbox>)
    ) : (
      child
    )
  );
}

To your point about portals - 0.1 clones and cleans lightboxable DOM elements:

/**
* Return a cleaned clone of the given element for building
* carousel slides with.
* @param {!Element} element
* @return {!Node}
* @private
*/
cloneLightboxableElement_(element) {
const fallback = element.getFallback();
const shouldCloneFallback =
element.classList.contains('amp-notsupported') && !!fallback;
if (shouldCloneFallback) {
element = fallback;
}
const deepClone = !element.classList.contains('i-amphtml-element');
const clonedNode = element.cloneNode(deepClone);
clonedNode.removeAttribute('on');
clonedNode.removeAttribute('id');
clonedNode.removeAttribute('i-amphtml-layout');
clonedNode.removeAttribute('fallback');
return clonedNode;
}
Can you describe more how portals helps this use case? I imagine we would want lightboxed elements both to be rendered where they are in the DOM as well as in the LightboxGallery when it is opened. From what I've read about portals, it doesn't seem like you can have it both ways?

Edit: Discussed portals offline and prefer taking render prop for inside the lightbox.

<style>{`
img {
width: 240px;
height: 160px;
}
`}</style>
<img
lightbox
src="https://images.unsplash.com/photo-1583511655857-d19b40a7a54e?ixlib=rb-1.2.1&ixid=eyJhcHBfaWQiOjEyMDd9&auto=format&fit=crop&w=1498&q=80"
/>
<p>abc</p>
<img
lightbox
src="https://images.unsplash.com/photo-1583511666407-5f06533f2113?ixlib=rb-1.2.1&auto=format&fit=crop&w=1498&q=80"
/>
<p>abc</p>
<img
lightbox
src="https://images.unsplash.com/photo-1599839575945-a9e5af0c3fa5?ixlib=rb-1.2.1&ixid=eyJhcHBfaWQiOjQwMzA0fQ&auto=format&fit=crop&w=1498&q=80"
/>
</LightboxGallery>
);
};