Fix Video component and add autoplay (#76)

* fix: Video component and add autplay * add sources support to video * Remove poster from video. * Version bump Co-authored-by: kevhender <[email protected]>
storefront-foundation · May 8, 2020 · 31c1b05 · 31c1b05
1 parent d960644
commit 31c1b05
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 23 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "react-storefront",
-  "version": "7.12.0",
+  "version": "7.12.1",
   "description": "Build and deploy e-commerce progressive web apps (PWAs) in record time.",
   "module": "./index.js",
   "license": "Apache-2.0",

diff --git a/src/carousel/Media.js b/src/carousel/Media.js
@@ -23,9 +23,11 @@ const useStyles = makeStyles(theme => ({
 export default function Media({
   magnifyProps,
   imageProps,
+  videoProps,
   src,
   alt,
   magnify,
+  sources,
   poster,
   type = 'image',
 }) {
@@ -55,7 +57,17 @@ export default function Media({
   }
 
   if (type === 'video') {
-    return <video src={src} alt={alt} poster={poster} />
+    if (sources && sources.length) {
+      return (
+        <video alt={alt} {...videoProps}>
+          {sources.map(source => (
+            <source key={source.src} src={source.src} type={source.type} />
+          ))}
+        </video>
+      )
+    } else {
+      return <video src={src} alt={alt} {...videoProps} />
+    }
   } else if (magnify) {
     return (
       <ReactImageMagnify
@@ -86,6 +98,11 @@ Media.propTypes = {
    */
   magnifyProps: PropTypes.object,
 
+  /**
+   * Other props to pass to the video component.
+   */
+  videoProps: PropTypes.object,
+
   /**
    * Other props to pass to the [`Image`](/apiReference/Image) for an `'image'` type.
    */
@@ -102,9 +119,9 @@ Media.propTypes = {
   src: PropTypes.string,
 
   /**
-   * Used as the `poster` attribute for a `<video>`.
+   * Used as the source inside the video `<video>`.
    */
-  poster: PropTypes.string,
+  sources: PropTypes.array,
 
   /**
    * An object to pass to pass to `ReactImageMagnify` containing the data for the magnified image.

diff --git a/src/carousel/MediaCarousel.js b/src/carousel/MediaCarousel.js
@@ -131,6 +131,7 @@ function MediaCarousel(props) {
     thumbsClassName,
     thumbnailPosition,
     magnifyHintClassName,
+    videoProps,
     imageProps,
     lightboxProps,
     lightboxClassName,
@@ -149,6 +150,7 @@ function MediaCarousel(props) {
   const ref = useRef(null)
   const [over, setOver] = useState(false)
   const [selected, setSelected] = useState(0)
+  const [video, setVideo] = useState(false)
   const [lightboxActive, setLightboxActive] = useState()
   const theme = useTheme()
   const isSmall = useMediaQuery(theme.breakpoints.down('xs'))
@@ -160,6 +162,14 @@ function MediaCarousel(props) {
     setSelected(0)
   }, [media])
 
+  useEffect(() => {
+    if (media && media.full && media.full[selected].type === 'video') {
+      setVideo(true)
+    } else {
+      setVideo(false)
+    }
+  }, [selected])
+
   const timeout = useRef(null)
 
   const handleMouseEnter = useCallback(() => {
@@ -182,7 +192,7 @@ function MediaCarousel(props) {
   }, [])
 
   const onClickCarousel = useCallback(evt => {
-    if (!evt.defaultPrevented) {
+    if (!evt.defaultPrevented && !video) {
       setLightboxActive(true)
     }
   })
@@ -215,7 +225,7 @@ function MediaCarousel(props) {
         showHint = false
       }
     }
-    if (showHint) {
+    if (showHint && !video) {
       belowAdornments.push(
         <MagnifyHint
           key="magnify-hint"
@@ -280,8 +290,9 @@ function MediaCarousel(props) {
                         contain: true,
                         src: get(item, 'magnify.src', item.src),
                       }
-                    : item.imageProps
+                    : imageProps
                 }
+                videoProps={videoProps}
               />
             </Fill>
           ))}
@@ -356,6 +367,10 @@ MediaCarousel.propTypes = {
    * An optional `className` to use for the [`MagnifyHint`](/apiReference/carousel/MagnifyHint) component.
    */
   magnifyHintClassName: PropTypes.string,
+  /**
+   * Props passed through to each [`Media`](/apiReference/carousel/Media)'s video component.
+   */
+  videoProps: PropTypes.object,
   /**
    * Props passed through to each [`Media`](/apiReference/carousel/Media)'s
    * [`imageProps`](/apiReference/carousel/Media#prop-imageProps).
@@ -402,6 +417,10 @@ MediaCarousel.defaultProps = {
   MediaComponent: Media,
   CarouselComponent: Carousel,
   CarouselThumbnailsComponent: CarouselThumbnails,
+  videoProps: {
+    controls: true,
+    autoPlay: false,
+  },
 }
 
 export default React.memo(MediaCarousel)
diff --git a/test/carousel/Media.test.js b/test/carousel/Media.test.js
@@ -25,6 +25,12 @@ describe('Media', () => {
     expect(wrapper.find('video')).toExist()
   })
 
+  it('should render sources inside video if defined', () => {
+    wrapper = mount(<Media type="video" sources={[{ src: 'test1' }]} />)
+
+    expect(wrapper.find('source')).toExist()
+  })
+
   it('should render ReactImageMagnify if magnify prop is passed', () => {
     wrapper = mount(
       <Media magnify={{ src: 'test', width: 1000, height: 1000 }} src="test" alt="test" />,

diff --git a/test/carousel/MediaCarousel.test.js b/test/carousel/MediaCarousel.test.js
@@ -14,12 +14,29 @@ jest.useFakeTimers()
 
 describe('MediaCarousel', () => {
   let wrapper
+  let media
 
   const mockMediaQuery = jest.spyOn(useMediaQuery, 'default')
 
   beforeEach(() => {
     window.innerWidth = 1024
     window.dispatchEvent(new Event('resize'))
+
+    media = {
+      full: [1, 2, 3].map(key => ({
+        src: `test${key}`,
+        alt: `test${key}`,
+        magnify: {
+          height: 1200,
+          width: 1200,
+          src: `test${key}`,
+        },
+      })),
+      thumbnails: [1, 2, 3].map(key => ({
+        src: `testThumb${key}`,
+        alt: `testThumb${key}`,
+      })),
+    }
   })
 
   afterEach(() => {
@@ -34,22 +51,6 @@ describe('MediaCarousel', () => {
     jest.restoreAllMocks()
   })
 
-  const media = {
-    full: [1, 2, 3].map(key => ({
-      src: `test${key}`,
-      alt: `test${key}`,
-      magnify: {
-        height: 1200,
-        width: 1200,
-        src: `test${key}`,
-      },
-    })),
-    thumbnails: [1, 2, 3].map(key => ({
-      src: `testThumb${key}`,
-      alt: `testThumb${key}`,
-    })),
-  }
-
   it('should have MagnifyHint and thumbnails when media has those', () => {
     wrapper = mount(<MediaCarousel media={media} />)
 
@@ -85,6 +86,24 @@ describe('MediaCarousel', () => {
     expect(wrapper.find(Lightbox).prop('open')).toBe(true)
   })
 
+  it('should not open lightbox on MediaCarousel click if active media is video', () => {
+    media.full[0].type = 'video'
+
+    wrapper = mount(<MediaCarousel media={media} />)
+
+    expect(wrapper.find(Lightbox).prop('open')).toBe(false)
+    wrapper.find(Carousel).simulate('click')
+    expect(wrapper.find(Lightbox).prop('open')).toBe(false)
+  })
+
+  it('should not show MagnifyHint if active media is video', () => {
+    media.full[0].type = 'video'
+
+    wrapper = mount(<MediaCarousel media={media} />)
+
+    expect(wrapper.find(MagnifyHint)).not.toExist()
+  })
+
   it('should not open lightbox on MediaCarousel click if event is default prevented', () => {
     wrapper = mount(<MediaCarousel media={media} />)