MediaCarousel functionality improvements (#64)

* Fix MediaCarousel to correctly show images of varying heights/widths

* Allow for thumbnails to be on any side of the carousel

* PR cleanup

* Version bump

Author: kevhender

package.json

@@ -1,6 +1,6 @@
 {
   "name": "react-storefront",
-  "version": "7.10.0",
+  "version": "7.11.0",
   "description": "Build and deploy e-commerce progressive web apps (PWAs) in record time.",
   "module": "./index.js",
   "license": "Apache-2.0",

src/carousel/Carousel.js

@@ -17,6 +17,7 @@ const styles = theme => ({
     flexDirection: 'column',
     alignItems: 'stretch',
     position: 'relative',
+    flexBasis: '100%',
     '& img': {
       display: 'block',
     },

src/carousel/CarouselThumbnails.js

@@ -3,7 +3,8 @@ import React from 'react'
 import clsx from 'clsx'
 import Tabs from '@material-ui/core/Tabs'
 import Tab from '@material-ui/core/Tab'
-import { makeStyles } from '@material-ui/core/styles'
+import { makeStyles, useTheme } from '@material-ui/core/styles'
+import useMediaQuery from '@material-ui/core/useMediaQuery'
 import Image from '../Image'
 
 export const styles = theme => ({
@@ -35,7 +36,47 @@ export const styles = theme => ({
   /**
    * Styles applied to the root element of the Tabs element
    */
-  tabsRoot: {
+  tabsRoot: {},
+  /**
+   * Styles applied the to the root element of the Tabs element when `thumbnailPosition` is `left` or `right`.
+   */
+  tabsVertical: {
+    [theme.breakpoints.up('sm')]: {
+      flexDirection: 'column',
+    },
+  },
+  /**
+   * Styles applied to the root element of the Tabs element when `thumbnailPosition` is `left`.
+   */
+  tabsRootLeft: {
+    [theme.breakpoints.down('xs')]: {
+      marginTop: theme.spacing(2),
+    },
+    [theme.breakpoints.up('sm')]: {
+      marginRight: theme.spacing(2),
+    },
+  },
+  /**
+   * Styles applied to the root element of the Tabs element when `thumbnailPosition` is `right`.
+   */
+  tabsRootRight: {
+    [theme.breakpoints.down('xs')]: {
+      marginTop: theme.spacing(2),
+    },
+    [theme.breakpoints.up('sm')]: {
+      marginLeft: theme.spacing(2),
+    },
+  },
+  /**
+   * Styles applied to the root element of the Tabs element when `thumbnailPosition` is `top`.
+   */
+  tabsRootTop: {
+    marginBottom: theme.spacing(2),
+  },
+  /**
+   * Styles applied to the root element of the Tabs element when `thumbnailPosition` is `bottom`.
+   */
+  tabsRootBottom: {
     marginTop: theme.spacing(2),
   },
   /**
@@ -79,17 +120,34 @@ const useStyles = makeStyles(styles, { name: 'RSFCarouselThumbnails' })
  * be clicked to switch to the given slide. Internally, `CarouselThumbnails` uses MaterialUI's
  * [`Tabs`](https://material-ui.com/api/tabs) component to indicate which slide is selected
  */
-function CarouselThumbnails({ thumbnails, selected, setSelected, classes, className }) {
+function CarouselThumbnails({
+  thumbnails,
+  selected,
+  setSelected,
+  classes,
+  className,
+  thumbnailPosition,
+}) {
   const styles = useStyles({ classes })
+  const theme = useTheme()
+  const isSmall = useMediaQuery(theme.breakpoints.down('xs'))
+  const isVertical = !isSmall && ['left', 'right'].includes(thumbnailPosition)
 
   return (
     <div className={clsx(className, styles.thumbs)}>
       <Tabs
         value={selected}
         variant="scrollable"
         onChange={(_, index) => setSelected(index)}
+        orientation={isVertical ? 'vertical' : 'horizontal'}
         classes={{
-          root: styles.tabsRoot,
+          root: clsx(styles.tabsRoot, {
+            [styles.tabsVertical]: isVertical,
+            [styles.tabsRootLeft]: thumbnailPosition === 'left',
+            [styles.tabsRootRight]: thumbnailPosition === 'right',
+            [styles.tabsRootTop]: thumbnailPosition === 'top',
+            [styles.tabsRootBottom]: thumbnailPosition === 'bottom',
+          }),
           indicator: styles.tabsIndicator,
         }}
       >
@@ -134,14 +192,19 @@ CarouselThumbnails.propTypes = {
   setSelected: PropTypes.func,
 
   /**
-   * Array of objects containing the data for an image to be used for each thumbnail
+   * Array of objects containing the data for an image to be used for each thumbnail.
    */
   thumbnails: PropTypes.arrayOf(
     PropTypes.shape({
       src: PropTypes.string,
       alt: PropTypes.string,
     }),
   ),
+
+  /**
+   * Position of the thumbnails, relative to the main carousel image.
+   */
+  thumbnailPosition: PropTypes.oneOf(['bottom', 'top', 'left', 'right']),
 }
 
 export default React.memo(CarouselThumbnails)

src/carousel/Media.js

@@ -2,19 +2,65 @@ import PropTypes from 'prop-types'
 import React from 'react'
 import Image from '../Image'
 import ReactImageMagnify from 'react-image-magnify'
+import clsx from 'clsx'
+import { makeStyles } from '@material-ui/core/styles'
+
+const useStyles = makeStyles(theme => ({
+  rimRoot: {
+    height: '100% !important',
+    width: '100% !important',
+  },
+  rimSmallImage: {
+    height: '100% !important',
+    width: '100% !important',
+  },
+}))
 
 /**
  * An element that determines the proper tag to use for a media node within a
  * [`Carousel`](/apiReference/carousel%f2Carousel).
  */
-export default function Media({ magnifyProps, imageProps, src, alt, magnify, type = 'image' }) {
+export default function Media({
+  magnifyProps,
+  imageProps,
+  src,
+  alt,
+  magnify,
+  poster,
+  type = 'image',
+}) {
+  const classes = useStyles()
+
+  const adjustMagnifyProps = () => {
+    const appliedMagnifyProps = { ...(magnifyProps || {}) }
+    appliedMagnifyProps.style = {
+      ...((magnifyProps && magnifyProps.style) || {}),
+      display: 'flex',
+      objectFit: 'contain',
+    }
+    appliedMagnifyProps.imageStyle = {
+      ...((magnifyProps && magnifyProps.imageStyle) || {}),
+      objectFit: 'contain',
+    }
+    appliedMagnifyProps.className = clsx(magnifyProps && magnifyProps.className, classes.rimRoot)
+    appliedMagnifyProps.imageClassName = clsx(
+      magnifyProps && magnifyProps.imageClassName,
+      classes.rimSmallImage,
+    )
+    appliedMagnifyProps.enlargedImageStyle = {
+      ...((magnifyProps && magnifyProps.enlargedImageStyle) || {}),
+      height: '100%',
+    }
+    return appliedMagnifyProps
+  }
+
   if (type === 'video') {
-    return <video src={src} alt={alt} />
+    return <video src={src} alt={alt} poster={poster} />
   } else if (magnify) {
     return (
       <ReactImageMagnify
         enlargedImagePosition="over"
-        {...magnifyProps}
+        {...adjustMagnifyProps()}
         smallImage={{
           src: src,
           alt: alt,
@@ -36,7 +82,7 @@ Media.propTypes = {
 
   /**
    * Props passed to the [`ReactImageMagnify`](https://github.com/ethanselzer/react-image-magnify#usage)
-   * element for an `'image'` type when [`magnify`](#prop-magnify) is `true`.
+   * element for an `'image'` type when [`magnify`](#prop-magnify) is defined.
    */
   magnifyProps: PropTypes.object,
 
@@ -46,15 +92,20 @@ Media.propTypes = {
   imageProps: PropTypes.object,
 
   /**
-   * Used as the `alt` tag for an `'image'` type.
+   * Used as the `alt` attribute for the `<img>` or `<video>`.
    */
   alt: PropTypes.string,
 
   /**
-   * Used as the `src` tag for an `'image'` type.
+   * Used as the `src` attribute for the `<img>` or `<video>`.
    */
   src: PropTypes.string,
 
+  /**
+   * Used as the `poster` attribute for a `<video>`.
+   */
+  poster: PropTypes.string,
+
   /**
    * An object to pass to pass to `ReactImageMagnify` containing the data for the magnified image.
    * If `false`, the media is not able to be magnified.