✨ Bento Carousel: "orientation" feature (#31286)

* Add Storybook samples * Support "orientation" attr/prop * Add e2e test * Code cleanups * Add inline gallery Storybook usage * 2019 -> 2020 * Update styling for mixedLength + vertical * Remove slide `height: 100%` entirely * Remove axis state * Round all dimensions
ampproject · Dec 2, 2020 · 39fd323 · 39fd323
1 parent f14ac3c
commit 39fd323
Show file tree

Hide file tree

Showing 12 changed files with 253 additions and 30 deletions.
diff --git a/extensions/amp-base-carousel/1.0/amp-base-carousel.js b/extensions/amp-base-carousel/1.0/amp-base-carousel.js
@@ -100,6 +100,12 @@ AmpBaseCarousel['props'] = {
   },
   'autoAdvanceLoops': {attr: 'auto-advance-loops', type: 'number', media: true},
   'controls': {attr: 'controls', type: 'string', media: true},
+  'orientation': {
+    attr: 'orientation',
+    type: 'string',
+    media: true,
+    default: 'horizontal',
+  },
   'loop': {attr: 'loop', type: 'boolean', media: true},
   'mixedLength': {attr: 'mixed-length', type: 'boolean', media: true},
   'outsetArrows': {attr: 'outset-arrows', type: 'boolean', media: true},

diff --git a/extensions/amp-base-carousel/1.0/base-carousel.js b/extensions/amp-base-carousel/1.0/base-carousel.js
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 import * as Preact from '../../../src/preact';
-import {Alignment} from './dimensions';
+import {Alignment, Axis, Orientation} from './dimensions';
 import {Arrow} from './arrow';
 import {CarouselContext} from './carousel-context';
 import {ContainWrapper} from '../../../src/preact/component';
@@ -88,6 +88,7 @@ function BaseCarouselWithRef(
     onMouseEnter,
     onSlideChange,
     onTouchStart,
+    orientation = Orientation.HORIZONTAL,
     outsetArrows,
     snap = true,
     snapAlign = Alignment.START,
@@ -107,9 +108,11 @@ function BaseCarouselWithRef(
   const setCurrentSlide =
     carouselContext.setCurrentSlide ?? setCurrentSlideState;
   const {slides, setSlides} = carouselContext;
+
   const scrollRef = useRef(null);
   const containRef = useRef(null);
   const contentRef = useRef(null);
+
   const autoAdvanceTimesRef = useRef(0);
   const autoAdvanceInterval = useMemo(
     () => Math.max(customAutoAdvanceInterval, MIN_AUTO_ADVANCE_INTERVAL),
@@ -261,6 +264,7 @@ function BaseCarouselWithRef(
         advanceCount={advanceCount}
         alignment={snapAlign}
         autoAdvanceCount={autoAdvanceCount}
+        axis={orientation == Orientation.HORIZONTAL ? Axis.X : Axis.Y}
         loop={loop}
         mixedLength={mixedLength}
         restingIndex={currentSlide}

diff --git a/extensions/amp-base-carousel/1.0/base-carousel.jss.js b/extensions/amp-base-carousel/1.0/base-carousel.jss.js
@@ -43,6 +43,13 @@ const horizontalScroll = {
   },
 };
 
+const verticalScroll = {
+  flexDirection: 'column',
+  scrollSnapTypeY: 'mandatory', // Firefox/IE
+  scrollSnapType: 'y mandatory',
+  overflowX: 'hidden',
+};
+
 /*
  * Styles to hide scrollbars, with three different methods:
  *
@@ -61,8 +68,6 @@ const hideScrollbar = {
   scrollbarWidth: 'none',
 
   boxSizing: '',
-  height: '100%',
-  paddingBottom: '20px',
 
   // Chrome, Safari
   '&::-webkit-scrollbar': {
@@ -72,7 +77,6 @@ const hideScrollbar = {
 };
 
 const slideElement = {
-  height: '100%',
   position: 'relative',
   overflow: 'hidden',
   display: 'flex',
@@ -247,6 +251,7 @@ const JSS = {
   scrollContainer,
   hideScrollbar,
   horizontalScroll,
+  verticalScroll,
   slideElement,
   thumbnails,
   enableSnap,

diff --git a/extensions/amp-base-carousel/1.0/base-carousel.type.js b/extensions/amp-base-carousel/1.0/base-carousel.type.js
@@ -33,6 +33,7 @@ var BaseCarouselDef = {};
  *   loop: (boolean|undefined),
  *   mixedLength: (boolean|undefined),
  *   onSlideChange: (function(number):undefined|undefined),
+ *   orientation: (string|undefined),
  *   snap: (boolean|undefined),
  *   snapAlign: (string|undefined),
  *   snapBy: (number|undefined),
@@ -44,6 +45,7 @@ BaseCarouselDef.Props;
 /**
  * @typedef {{
  *   advanceCount: (number|undefined),
+ *   axis: number,
  *   children: !Array<PreactDef.Renderable>,
  *   loop: (boolean|undefined),
  *   mixedLength: (boolean|undefined),

diff --git a/extensions/amp-base-carousel/1.0/dimensions.js b/extensions/amp-base-carousel/1.0/dimensions.js
@@ -32,6 +32,14 @@ export const Alignment = {
   CENTER: 'center',
 };
 
+/**
+ * @enum {string}
+ */
+export const Orientation = {
+  HORIZONTAL: 'horizontal',
+  VERTICAL: 'vertical',
+};
+
 /**
  * @typedef {{
  *   start: number,
@@ -57,9 +65,9 @@ export function getDimension(axis, el) {
   } = el./*OK*/ getBoundingClientRect();
 
   return {
-    start: axis == Axis.X ? left : top,
-    end: axis == Axis.X ? right : bottom,
-    length: axis == Axis.X ? width : height,
+    start: Math.round(axis == Axis.X ? left : top),
+    end: Math.round(axis == Axis.X ? right : bottom),
+    length: Math.round(axis == Axis.X ? width : height),
   };
 }
 
@@ -105,7 +113,7 @@ export function getPosition(axis, alignment, el) {
 export function overlaps(axis, el, position) {
   const {start, end} = getDimension(axis, el);
   // Ignore the end point, since that is shared with the adjacent Element.
-  return Math.round(start) <= position && position < Math.round(end);
+  return start <= position && position < end;
 }
 
 /**

diff --git a/extensions/amp-base-carousel/1.0/scroller.js b/extensions/amp-base-carousel/1.0/scroller.js
@@ -31,7 +31,6 @@ import {
   useLayoutEffect,
   useMemo,
   useRef,
-  useState,
 } from '../../../src/preact';
 import {useStyles} from './base-carousel.jss';
 
@@ -56,6 +55,7 @@ function ScrollerWithRef(
     advanceCount,
     alignment,
     autoAdvanceCount,
+    axis,
     children,
     loop,
     mixedLength,
@@ -71,7 +71,6 @@ function ScrollerWithRef(
 ) {
   // We still need our own ref that we can always rely on to be there.
   const containerRef = useRef(null);
-  const [axis] = useState(Axis.X);
 
   /**
    * The number of slides we want to place before the reference or resting index.
@@ -240,7 +239,9 @@ function ScrollerWithRef(
     <div
       ref={containerRef}
       onScroll={handleScroll}
-      class={`${classes.scrollContainer} ${classes.hideScrollbar} ${classes.horizontalScroll}`}
+      class={`${classes.scrollContainer} ${classes.hideScrollbar} ${
+        axis === Axis.X ? classes.horizontalScroll : classes.verticalScroll
+      }`}
       tabindex={0}
       {...rest}
     >
@@ -332,8 +333,10 @@ function renderSlides(
           snap && mod(index, snapBy) === 0
             ? classes.enableSnap
             : classes.disableSnap
-        } ${_thumbnails ? classes.thumbnails : ''}`}
-        style={{flex: mixedLength ? '0 0 auto' : `0 0 ${100 / visibleCount}%`}}
+        } ${_thumbnails ? classes.thumbnails : ''} `}
+        style={{
+          flex: mixedLength ? '0 0 auto' : `0 0 ${100 / visibleCount}%`,
+        }}
       >
         {child}
       </div>

diff --git a/extensions/amp-base-carousel/1.0/storybook/Basic.amp.js b/extensions/amp-base-carousel/1.0/storybook/Basic.amp.js
@@ -19,6 +19,8 @@ import {boolean, number, select, text, withKnobs} from '@storybook/addon-knobs';
 import {withA11y} from '@storybook/addon-a11y';
 import {withAmp} from '@ampproject/storybook-addon';
 
+const ORIENTATIONS = ['horizontal', 'vertical'];
+
 export default {
   title: 'amp-base-carousel',
   decorators: [withKnobs, withA11y, withAmp],
@@ -42,6 +44,7 @@ export const Default = () => {
   const visibleCount = text('visible count', '(min-width: 400px) 2, 1');
   const outsetArrows = text('outset arrows', '(min-width: 400px) true, false');
   const controls = select('show controls', ['auto', 'always', 'never']);
+  const orientation = select('orientation', ORIENTATIONS, 'vertical');
   const slideCount = number('slide count', 5, {min: 0, max: 99});
   const colorIncrement = Math.floor(255 / (slideCount + 1));
 
@@ -55,20 +58,20 @@ export const Default = () => {
         auto-advance-interval={autoAdvanceInterval}
         auto-advance-loops={autoAdvanceLoops}
         controls={controls}
+        orientation={orientation}
         outset-arrows={outsetArrows}
-        width="880"
-        height="225"
+        width="450"
+        height="450"
         snap={String(snap)}
         snap-align={snapAlign}
         snap-by={snapBy}
         loop={loop}
-        layout="responsive"
         visible-count={visibleCount}
       >
         {Array.from({length: slideCount}, (x, i) => {
           const v = colorIncrement * (i + 1);
           return (
-            <amp-layout width="440" height="225" layout="responsive">
+            <amp-layout width="225" height="225" layout="responsive">
               <div
                 style={{
                   backgroundColor: `rgb(${v}, 100, 100)`,
@@ -97,8 +100,8 @@ export const Default = () => {
 };
 
 export const mixedLength = () => {
-  const width = number('width', 440);
-  const height = number('height', 225);
+  const width = number('width', 300);
+  const height = number('height', 300);
   const slideCount = number('slide count', 7, {min: 0, max: 99});
   const colorIncrement = Math.floor(255 / (slideCount + 1));
   const loop = boolean('loop', true);
@@ -113,12 +116,15 @@ export const mixedLength = () => {
     [252, 113, 115, 186, 248, 188, 162, 104, 100, 109, 175, 227, 143, 249, 280],
   ];
   const preset = select('random preset', [1, 2, 3]);
+  const orientation = select('orientation', ORIENTATIONS, 'vertical');
+  const horizontal = orientation == 'horizontal';
 
   return (
     <amp-base-carousel
       controls={controls}
       mixed-length={mixedLength}
       loop={loop}
+      orientation={orientation}
       snap={String(snap)}
       snap-align={snapAlign}
       snap-by={snapBy}
@@ -132,8 +138,12 @@ export const mixedLength = () => {
             style={{
               backgroundColor: `rgb(${v}, 100, 100)`,
               border: 'solid white 1px',
-              width: `${randomPreset[preset - 1 || 0][i]}px`,
-              height: `100px`,
+              width: horizontal
+                ? `${randomPreset[preset - 1 || 0][i]}px`
+                : '100px',
+              height: horizontal
+                ? '100px'
+                : `${randomPreset[preset - 1 || 0][i]}px`,
             }}
           ></div>
         );

diff --git a/extensions/amp-base-carousel/1.0/storybook/Basic.js b/extensions/amp-base-carousel/1.0/storybook/Basic.js
@@ -21,6 +21,7 @@ import {withA11y} from '@storybook/addon-a11y';
 
 const CONTROLS = ['auto', 'always', 'never'];
 const SNAP_ALIGN = ['start', 'center'];
+const ORIENTATIONS = ['horizontal', 'vertical'];
 
 export default {
   title: 'BaseCarousel',
@@ -49,12 +50,13 @@ function CarouselWithActions(props) {
 }
 
 export const _default = () => {
-  const width = number('width', 440);
-  const height = number('height', 225);
+  const width = number('width', 225);
+  const height = number('height', 440);
   const slideCount = number('slide count', 5, {min: 0, max: 99});
   const snap = boolean('snap', true);
   const snapAlign = select('snap alignment', SNAP_ALIGN, 'start');
   const snapBy = number('snap by', 1);
+  const orientation = select('orientation', ORIENTATIONS, 'vertical');
   const loop = boolean('loop', true);
   const advanceCount = number('advance count', 1, {min: 1});
   const visibleCount = number('visible count', 2, {min: 1});
@@ -66,6 +68,7 @@ export const _default = () => {
       advanceCount={advanceCount}
       controls={controls}
       loop={loop}
+      orientation={orientation}
       outsetArrows={outsetArrows}
       snap={snap}
       snapAlign={snapAlign}
@@ -83,7 +86,7 @@ export const _default = () => {
               height,
               textAlign: 'center',
               fontSize: '48pt',
-              lineHeight: height + 'px',
+              lineHeight: height / visibleCount + 'px',
             }}
           >
             {i}
@@ -95,8 +98,8 @@ export const _default = () => {
 };
 
 export const mixedLength = () => {
-  const width = number('width', 440);
-  const height = number('height', 225);
+  const width = number('width', 300);
+  const height = number('height', 300);
   const slideCount = 15;
   const colorIncrement = Math.floor(255 / (slideCount + 1));
   const autoAdvance = boolean('auto advance', true);
@@ -115,6 +118,8 @@ export const mixedLength = () => {
     [252, 113, 115, 186, 248, 188, 162, 104, 100, 109, 175, 227, 143, 249, 280],
   ];
   const preset = select('random preset', [1, 2, 3]);
+  const orientation = select('orientation', ORIENTATIONS, 'vertical');
+  const horizontal = orientation == 'horizontal';
   return (
     <BaseCarousel
       autoAdvance={autoAdvance}
@@ -124,6 +129,7 @@ export const mixedLength = () => {
       controls={controls}
       mixedLength={mixedLength}
       loop={loop}
+      orientation={orientation}
       snap={snap}
       snapAlign={snapAlign}
       snapBy={snapBy}
@@ -136,8 +142,12 @@ export const mixedLength = () => {
             style={{
               backgroundColor: `rgb(${v}, 100, 100)`,
               border: 'solid white 1px',
-              width: `${randomPreset[preset - 1 || 0][i]}px`,
-              height: `100px`,
+              width: horizontal
+                ? `${randomPreset[preset - 1 || 0][i]}px`
+                : '100px',
+              height: horizontal
+                ? '100px'
+                : `${randomPreset[preset - 1 || 0][i]}px`,
             }}
           ></div>
         );