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

Start autoplay when video is in view #2839

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/sharp-monkeys-cry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@comet/cms-site": minor
---

Play videos on auto play only when visible

Start videos depending on their visibility in `DamVideoBlock`, `YoutubeVideoBlock` and `VimeoVideoBlock`. Videos that are not in the viewport of the user, pause.
36 changes: 24 additions & 12 deletions packages/site/cms-site/src/blocks/DamVideoBlock.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
"use client";

import { ReactElement, ReactNode, useState } from "react";
import { ReactElement, ReactNode, useRef, useState } from "react";
import styled, { css } from "styled-components";

import { DamVideoBlockData } from "../blocks.generated";
import { withPreview } from "../iframebridge/withPreview";
import { PreviewSkeleton } from "../previewskeleton/PreviewSkeleton";
import { pauseDamVideo, playDamVideo } from "./helpers/controlVideos";
import { useIsElementVisible } from "./helpers/useIsElementVisible";
import { VideoPreviewImage, VideoPreviewImageProps } from "./helpers/VideoPreviewImage";
import { PropsWithData } from "./PropsWithData";

Expand Down Expand Up @@ -33,6 +35,13 @@ export const DamVideoBlock = withPreview(
const [showPreviewImage, setShowPreviewImage] = useState(true);
const hasPreviewImage = Boolean(previewImage && previewImage.damFile);

const inViewRef = useRef<HTMLDivElement>(null);
Copy link
Member

@thomasdax98 thomasdax98 Dec 16, 2024

Choose a reason for hiding this comment

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

Do you need the extra inViewRef? Wouldn't it be possible to use the videoRef directly to check if it's in view?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I already tried that, but using only one ref is not possible, as controlling the videos does not work in that case.

const videoRef = useRef<HTMLVideoElement>(null);

const inView = useIsElementVisible(inViewRef);

inView && autoplay ? playDamVideo(videoRef) : pauseDamVideo(videoRef);
Copy link
Member

@thomasdax98 thomasdax98 Dec 16, 2024

Choose a reason for hiding this comment

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

This should probably be in a useEffect

nvm since the state in useIsElementInViewport only changes if the element get in view, not triggering unnecessary rerenders


return (
<>
{hasPreviewImage && showPreviewImage ? (
Expand All @@ -56,17 +65,20 @@ export const DamVideoBlock = withPreview(
/>
)
) : (
<Video
autoPlay={autoplay || (hasPreviewImage && !showPreviewImage)}
controls={showControls}
loop={loop}
playsInline
muted={autoplay}
$aspectRatio={aspectRatio.replace("x", " / ")}
$fill={fill}
>
<source src={damFile.fileUrl} type={damFile.mimetype} />
</Video>
<div ref={inViewRef}>
<Video
autoPlay={autoplay || (hasPreviewImage && !showPreviewImage)}
controls={showControls}
loop={loop}
playsInline
muted={autoplay}
ref={videoRef}
$aspectRatio={aspectRatio.replace("x", " / ")}
$fill={fill}
>
<source src={damFile.fileUrl} type={damFile.mimetype} />
</Video>
</div>
)}
</>
);
Expand Down
14 changes: 10 additions & 4 deletions packages/site/cms-site/src/blocks/VimeoVideoBlock.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
"use client";
import { ReactNode, useState } from "react";
import { ReactNode, useEffect, useRef, useState } from "react";
import styled, { css } from "styled-components";

import { VimeoVideoBlockData } from "../blocks.generated";
import { withPreview } from "../iframebridge/withPreview";
import { PreviewSkeleton } from "../previewskeleton/PreviewSkeleton";
import { pauseVimeoVideo, playVimeoVideo } from "./helpers/controlVideos";
import { useIsElementVisible } from "./helpers/useIsElementVisible";
import { VideoPreviewImage, VideoPreviewImageProps } from "./helpers/VideoPreviewImage";
import { PropsWithData } from "./PropsWithData";

Expand Down Expand Up @@ -43,15 +45,19 @@ export const VimeoVideoBlock = withPreview(
}: VimeoVideoBlockProps) => {
const [showPreviewImage, setShowPreviewImage] = useState(true);
const hasPreviewImage = !!(previewImage && previewImage.damFile);
const inViewRef = useRef(null);
const inView = useIsElementVisible(inViewRef);

useEffect(() => {
inView && autoplay ? playVimeoVideo() : pauseVimeoVideo();
}, [autoplay, inView]);

if (!vimeoIdentifier) return <PreviewSkeleton type="media" hasContent={false} aspectRatio={aspectRatio} />;

const identifier = parseVimeoIdentifier(vimeoIdentifier);

const searchParams = new URLSearchParams();

if (autoplay !== undefined || (hasPreviewImage && !showPreviewImage))
searchParams.append("autoplay", Number(autoplay || (hasPreviewImage && !showPreviewImage)).toString());
if (autoplay) searchParams.append("muted", "1");

if (loop !== undefined) searchParams.append("loop", Number(loop).toString());
Expand Down Expand Up @@ -87,7 +93,7 @@ export const VimeoVideoBlock = withPreview(
/>
)
) : (
<VideoContainer $aspectRatio={aspectRatio.replace("x", "/")} $fill={fill}>
<VideoContainer ref={inViewRef} $aspectRatio={aspectRatio.replace("x", "/")} $fill={fill}>
<VimeoContainer src={vimeoUrl.toString()} allow="autoplay" allowFullScreen style={{ border: 0 }} />
</VideoContainer>
)}
Expand Down
17 changes: 12 additions & 5 deletions packages/site/cms-site/src/blocks/YouTubeVideoBlock.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
"use client";

import { ReactElement, ReactNode, useState } from "react";
import { ReactElement, ReactNode, useEffect, useRef, useState } from "react";
import styled, { css } from "styled-components";

import { YouTubeVideoBlockData } from "../blocks.generated";
import { withPreview } from "../iframebridge/withPreview";
import { PreviewSkeleton } from "../previewskeleton/PreviewSkeleton";
import { pauseYoutubeVideo, playYoutubeVideo } from "./helpers/controlVideos";
import { useIsElementVisible } from "./helpers/useIsElementVisible";
import { VideoPreviewImage, VideoPreviewImageProps } from "./helpers/VideoPreviewImage";
import { PropsWithData } from "./PropsWithData";

Expand Down Expand Up @@ -40,6 +42,12 @@ export const YouTubeVideoBlock = withPreview(
}: YouTubeVideoBlockProps) => {
const [showPreviewImage, setShowPreviewImage] = useState(true);
const hasPreviewImage = !!(previewImage && previewImage.damFile);
const inViewRef = useRef(null);
const inView = useIsElementVisible(inViewRef);

useEffect(() => {
inView && autoplay ? playYoutubeVideo() : pauseYoutubeVideo();
}, [autoplay, inView]);
Copy link
Member

Choose a reason for hiding this comment

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

useIsElementInViewport has a local state and re-renders the Video Block once it gets into view. But you don't have to re-render, as you only call the JS api inside the iframe.

You could argue that performance doesn't matter here, it's only re-rendered once - but I'd argue that it makes the code more complicated than it has to be.

Copy link
Collaborator

Choose a reason for hiding this comment

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

How would you design useIsElementInViewport to remove the state and effect? Something like this?

useIsElementInViewport(ref, (inView) => {
  if(autoplay) {
    if(inView) {
      playYouTubeVideo();
    } else {
      pauseYouTubeVideo();
    }
  }
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it better like this?
23411e5

Copy link
Member

Choose a reason for hiding this comment

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

Is it better like this?

yes, much simpler - isn't it?


if (!youtubeIdentifier) {
return <PreviewSkeleton type="media" hasContent={false} aspectRatio={aspectRatio} />;
Expand All @@ -49,9 +57,8 @@ export const YouTubeVideoBlock = withPreview(
const searchParams = new URLSearchParams();
searchParams.append("modestbranding", "1");
searchParams.append("rel", "0");
searchParams.append("enablejsapi", "1");

if (autoplay !== undefined || (hasPreviewImage && !showPreviewImage))
searchParams.append("autoplay", Number(autoplay || (hasPreviewImage && !showPreviewImage)).toString());
if (autoplay) searchParams.append("mute", "1");

if (showControls !== undefined) searchParams.append("controls", Number(showControls).toString());
Expand Down Expand Up @@ -87,8 +94,8 @@ export const YouTubeVideoBlock = withPreview(
/>
)
) : (
<VideoContainer $aspectRatio={aspectRatio.replace("x", "/")} $fill={fill}>
<YouTubeContainer src={youtubeUrl.toString()} allow="autoplay" />
<VideoContainer ref={inViewRef} $aspectRatio={aspectRatio.replace("x", "/")} $fill={fill}>
<YouTubeContainer src={youtubeUrl.toString()} />
</VideoContainer>
)}
</>
Expand Down
41 changes: 41 additions & 0 deletions packages/site/cms-site/src/blocks/helpers/controlVideos.ts
Copy link
Collaborator

Choose a reason for hiding this comment

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

These functions should be in the respective blocks. There's no need for a separate file IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed here: 42b53fb

Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { RefObject } from "react";

export const playDamVideo = (videoRef: RefObject<HTMLVideoElement>) => {
if (videoRef.current) {
videoRef.current.play();
}
};

export const pauseDamVideo = (videoRef: RefObject<HTMLVideoElement>) => {
if (videoRef.current) {
videoRef.current.pause();
}
};

export const pauseYoutubeVideo = () => {
const iframe = document.getElementsByTagName("iframe")[0];
if (iframe?.contentWindow) {
iframe.contentWindow.postMessage(`{"event":"command","func":"pauseVideo","args":""}`, "*");
}
};

export const playYoutubeVideo = () => {
const iframe = document.getElementsByTagName("iframe")[0];
if (iframe?.contentWindow) {
iframe.contentWindow.postMessage(`{"event":"command","func":"playVideo","args":""}`, "*");
}
};

export const pauseVimeoVideo = () => {
const iframe = document.getElementsByTagName("iframe")[0];
if (iframe?.contentWindow) {
iframe.contentWindow.postMessage(JSON.stringify({ method: "pause" }), "https://player.vimeo.com");
}
};

export const playVimeoVideo = () => {
const iframe = document.getElementsByTagName("iframe")[0];
if (iframe?.contentWindow) {
iframe.contentWindow.postMessage(JSON.stringify({ method: "play" }), "https://player.vimeo.com");
}
};
20 changes: 20 additions & 0 deletions packages/site/cms-site/src/blocks/helpers/useIsElementVisible.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { RefObject, useEffect, useState } from "react";

export const useIsElementVisible = (ref: RefObject<HTMLDivElement>) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Naming: useIsElementInViewport would probably be better. An element can still be hidden using display: none.

BTW, nice implementation using IntersectionObserver 👏🏼

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you :)

Naming changed here: 0ebb7d3

const [isVisible, setIsVisible] = useState(false);

useEffect(() => {
const options = { root: null, rootMargin: "0px", threshold: 1.0 };
const observer = new IntersectionObserver((entries) => {
setIsVisible(entries[0].isIntersecting);
}, options);
if (ref.current) observer.observe(ref.current);
const inViewRefValue = ref.current;

return () => {
if (inViewRefValue) observer.unobserve(inViewRefValue);
};
}, [ref]);

return isVisible;
};