Browse Source

ref(autoplayvideo): convert to functional component (#31276)

* ref(autoplayvideo): functional component + tests

* fix(autoplayvideo): removed orphaned console log

* test(autoplayvideo): simplify component and use test-id

* test(autoplayvideo): make title required

* Update static/app/views/issueList/noGroupsHandler/congratsRobots.tsx

Co-authored-by: Priscila Oliveira <priscilawebdev@gmail.com>

* fix(autoplayvideo): import t

Co-authored-by: Priscila Oliveira <priscilawebdev@gmail.com>
Jonas 3 years ago
parent
commit
1e61fdcba3

+ 13 - 27
static/app/components/autoplayVideo.tsx

@@ -1,7 +1,8 @@
 import * as React from 'react';
 
-type Props = React.HTMLProps<HTMLVideoElement>;
-
+interface AutoplayVideoProps extends React.VideoHTMLAttributes<HTMLVideoElement> {
+  'aria-label': string;
+}
 /**
  * Wrapper for autoplaying video.
  *
@@ -11,40 +12,25 @@ type Props = React.HTMLProps<HTMLVideoElement>;
  * Note, video needs `muted` for `autoplay` to work on Chrome
  * See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/video
  */
-class AutoplayVideo extends React.Component<Props> {
-  componentDidMount() {
-    if (this.videoRef.current) {
+function AutoplayVideo(props: AutoplayVideoProps) {
+  const videoRef = React.useRef<HTMLVideoElement>(null);
+
+  React.useEffect(() => {
+    if (videoRef.current) {
       // Set muted as more browsers allow autoplay with muted video.
       // We can't use the muted prop because of a react bug.
       // https://github.com/facebook/react/issues/10389
       // So we need to set the muted property then trigger play.
-      this.videoRef.current.muted = true;
-      const playPromise = this.videoRef.current.play();
+      videoRef.current.muted = true;
 
       // non-chromium Edge and jsdom don't return a promise.
-      playPromise?.catch(() => {
+      videoRef.current.play()?.catch(() => {
         // Do nothing. Interrupting this playback is fine.
       });
     }
-  }
-  private videoRef = React.createRef<HTMLVideoElement>();
-
-  render() {
-    const {className, src, ...props} = this.props;
+  }, []);
 
-    return (
-      <video
-        className={className}
-        ref={this.videoRef}
-        playsInline
-        disablePictureInPicture
-        loop
-        {...props}
-      >
-        <source src={src} type="video/mp4" />
-      </video>
-    );
-  }
+  return <video ref={videoRef} playsInline disablePictureInPicture loop {...props} />;
 }
 
-export default AutoplayVideo;
+export {AutoplayVideo};

+ 3 - 2
static/app/views/issueList/noGroupsHandler/congratsRobots.tsx

@@ -2,7 +2,8 @@ import styled from '@emotion/styled';
 
 import video from 'sentry-images/spot/congrats-robots.mp4';
 
-import AutoplayVideo from 'sentry/components/autoplayVideo';
+import {AutoplayVideo} from 'sentry/components/autoplayVideo';
+import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
 
 /**
@@ -12,7 +13,7 @@ import space from 'sentry/styles/space';
 function CongratsRobots() {
   return (
     <AnimatedScene>
-      <StyledAutoplayVideo src={video} />
+      <StyledAutoplayVideo aria-label={t('Congratulations video')} src={video} />
     </AnimatedScene>
   );
 }

+ 71 - 0
tests/js/spec/components/autoplayVideo.spec.tsx

@@ -0,0 +1,71 @@
+import * as React from 'react';
+
+import {mountWithTheme, screen} from 'sentry-test/reactTestingLibrary';
+
+import {AutoplayVideo} from 'sentry/components/autoplayVideo';
+
+jest.mock('react', () => {
+  return {
+    ...jest.requireActual('react'),
+    useRef: jest.fn(),
+  };
+});
+
+// Use a proxy to prevent <video ref={ref}/> from overriding our ref.current
+const makeProxyMock = (video: Partial<HTMLVideoElement>) => {
+  return new Proxy(
+    {current: video},
+    {
+      get(obj, prop) {
+        return obj[prop];
+      },
+      set(obj, prop) {
+        if (prop === 'current') {
+          obj.current = obj.current;
+        }
+        return true;
+      },
+    }
+  );
+};
+
+describe('autoplayVideo', () => {
+  it('sets mute and calls play', () => {
+    const mock = makeProxyMock({
+      muted: false,
+      play: jest.fn().mockReturnValue(Promise.resolve()),
+    });
+
+    // @ts-ignore we are mocking useRef
+    React.useRef.mockImplementation(() => mock);
+
+    mountWithTheme(
+      <AutoplayVideo aria-label="video" src="https://example.com/video.mp4" />
+    );
+
+    expect(screen.getByLabelText('video')).toBeInTheDocument();
+    expect(mock.current.muted).toBe(true);
+    expect(mock.current.play).toHaveBeenCalledTimes(1);
+  });
+
+  it('handles non promise-like return from play', () => {
+    const mock = makeProxyMock({
+      muted: false,
+      play: jest.fn().mockReturnValue(null),
+    });
+
+    // @ts-ignore we are mocking useRef
+    React.useRef.mockImplementation(() => mock);
+
+    mountWithTheme(
+      <AutoplayVideo aria-label="video" src="https://example.com/video.mp4" />
+    );
+
+    expect(screen.getByLabelText('video')).toBeInTheDocument();
+    expect(mock.current.muted).toBe(true);
+
+    // Seems that useEffect runs, so no mocking or tick is needed.
+    // Was tested manually by removing the ?.catch safe access and the test fails
+    expect(mock.current.play).toHaveBeenCalledTimes(1);
+  });
+});