fix download image button

quran · Sep 1, 2024 · 3d001cc · 3d001cc
1 parent 143e640
commit 3d001cc
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 76 deletions.
diff --git a/src/components/MediaMaker/RenderControls/RenderImageButton.tsx b/src/components/MediaMaker/RenderControls/RenderImageButton.tsx
@@ -1,5 +1,6 @@
-import React, { useEffect, useRef } from 'react';
+import React, { MutableRefObject, useCallback, useEffect, useRef } from 'react';
 
+import { PlayerRef } from '@remotion/player';
 import { useRouter } from 'next/router';
 import useTranslation from 'next-translate/useTranslation';
 
@@ -18,23 +19,21 @@ import { getLoginNavigationUrl, getQuranMediaMakerNavigationUrl } from '@/utils/
 
 type Props = {
   inputProps: any;
-  getCurrentFrame: () => number;
-  getIsPlayerPlaying: () => boolean;
+  playerRef: MutableRefObject<PlayerRef>;
   isFetching: boolean;
 };
 
-const RenderImageButton: React.FC<Props> = ({
-  inputProps,
-  getCurrentFrame,
-  getIsPlayerPlaying,
-  isFetching,
-}) => {
+const RenderImageButton: React.FC<Props> = ({ inputProps, playerRef, isFetching }) => {
   const { t } = useTranslation('media');
   const { renderMedia, state, undo } = useGenerateMediaFile(inputProps);
   const { data, mutate } = useGetMediaFilesCount(MediaType.IMAGE);
   const previousFrame = useRef<number>();
   const router = useRouter();
   const downloadButtonRef = React.useRef<HTMLParagraphElement>();
+  const shouldRerender = useRef<boolean>(false);
+
+  const getCurrentFrame = useCallback(() => playerRef?.current?.getCurrentFrame(), [playerRef]);
+  const getIsPlayerPlaying = () => playerRef?.current?.isPlaying();
 
   const triggerRenderImage = () => {
     const frame = getCurrentFrame();
@@ -65,7 +64,10 @@ const RenderImageButton: React.FC<Props> = ({
       logButtonClick('download_image');
       const isFrameDifferent = previousFrame.current !== getCurrentFrame();
       const isPlaying = getIsPlayerPlaying();
-      if (isPlaying && isFrameDifferent) {
+      if (shouldRerender.current) {
+        undo();
+        shouldRerender.current = false;
+      } else if (isPlaying && isFrameDifferent) {
         undo();
       } else if (!isPlaying && isFrameDifferent) {
         e.preventDefault();
@@ -78,6 +80,11 @@ const RenderImageButton: React.FC<Props> = ({
   // listen to state changes and download the file when it's done
   useEffect(() => {
     if (state?.status === RenderStatus.DONE) {
+      const isFrameDifferent = previousFrame.current !== getCurrentFrame();
+      if (isFrameDifferent) {
+        shouldRerender.current = true;
+      }
+      previousFrame.current = getCurrentFrame();
       downloadButtonRef.current.click();
       mutate(mutateGeneratedMediaCounter, { revalidate: false });
     }

diff --git a/src/components/MediaMaker/RenderControls/index.tsx b/src/components/MediaMaker/RenderControls/index.tsx
@@ -1,5 +1,6 @@
-import React, { useEffect, useState } from 'react';
+import React, { MutableRefObject, useEffect, useState } from 'react';
 
+import { PlayerRef } from '@remotion/player';
 import classNames from 'classnames';
 import clipboardCopy from 'clipboard-copy';
 import useTranslation from 'next-translate/useTranslation';
@@ -16,9 +17,8 @@ import { getCurrentPath } from '@/utils/url';
 
 type Props = {
   inputProps: MediaFileCompositionProps;
-  getCurrentFrame: () => number;
-  getIsPlayerPlaying: () => boolean;
   isFetching: boolean;
+  playerRef: MutableRefObject<PlayerRef>;
 };
 
 export type MediaFileCompositionProps = {
@@ -34,12 +34,7 @@ export type MediaFileCompositionProps = {
 
 const COPY_TIMEOUT_MS = 3000;
 
-const RenderControls: React.FC<Props> = ({
-  inputProps,
-  getCurrentFrame,
-  getIsPlayerPlaying,
-  isFetching,
-}) => {
+const RenderControls: React.FC<Props> = ({ inputProps, isFetching, playerRef }) => {
   const { t } = useTranslation('media');
   const [isCopied, setIsCopied] = useState(false);
 
@@ -74,12 +69,7 @@ const RenderControls: React.FC<Props> = ({
 
       <div className={styles.controlsContainer}>
         <RenderVideoButton isFetching={isFetching} inputProps={inputProps} />
-        <RenderImageButton
-          isFetching={isFetching}
-          inputProps={inputProps}
-          getCurrentFrame={getCurrentFrame}
-          getIsPlayerPlaying={getIsPlayerPlaying}
-        />
+        <RenderImageButton isFetching={isFetching} inputProps={inputProps} playerRef={playerRef} />
         <Button
           className={styles.copyButton}
           prefix={<CopyIcon />}

diff --git a/src/components/MediaMaker/Settings/VideoSettings.tsx b/src/components/MediaMaker/Settings/VideoSettings.tsx
@@ -1,6 +1,7 @@
 /* eslint-disable max-lines */
-import { useCallback, useMemo, useState } from 'react';
+import { MutableRefObject, useCallback, useMemo, useState } from 'react';
 
+import { PlayerRef } from '@remotion/player';
 import classNames from 'classnames';
 import { useRouter } from 'next/router';
 import { useDispatch } from 'react-redux';
@@ -34,10 +35,7 @@ import { logButtonClick, logEvent, logValueChange } from '@/utils/eventLogger';
 type Props = {
   chaptersList: any[];
   reciters: Reciter[];
-  seekToBeginning: () => void;
-  setIsUpdating: (arg: boolean) => void;
-  getCurrentFrame: () => number;
-  getIsPlayerPlaying: () => boolean;
+  playerRef: MutableRefObject<PlayerRef>;
   isFetching: boolean;
   inputProps: any;
   mediaSettings: MediaSettings;
@@ -71,25 +69,32 @@ const MEDIA_SETTINGS_TO_QUERY_PARAM = {
 const VideoSettings: React.FC<Props> = ({
   chaptersList,
   reciters,
-  seekToBeginning,
-  setIsUpdating,
   isFetching,
   inputProps,
-  getCurrentFrame,
-  getIsPlayerPlaying,
   mediaSettings,
+  playerRef,
 }) => {
   const [selectedTab, setSelectedTab] = useState<Tab>(Tab.AUDIO);
   const dispatch = useDispatch();
   const router = useRouter();
   const removeQueryParam = useRemoveQueryParam();
 
+  const seekToBeginning = useCallback(() => {
+    const current = playerRef?.current;
+    if (!current) {
+      return;
+    }
+    if (current.isPlaying) {
+      current.pause();
+    }
+    current.seekTo(0);
+  }, [playerRef]);
+
   const onSettingsUpdate = useCallback(
     (settings: ChangedSettings, key?: keyof MediaSettings, value?: any) => {
       if (key) {
         logValueChange(`media_settings_${key}`, mediaSettings[key], value);
       }
-      setIsUpdating(true);
       seekToBeginning();
       dispatch(updateSettings(settings));
       Object.keys(settings).forEach((settingKey) => {
@@ -103,7 +108,7 @@ const VideoSettings: React.FC<Props> = ({
       });
       router.push({ pathname: router.pathname, query: router.query }, undefined, { shallow: true });
     },
-    [dispatch, mediaSettings, router, seekToBeginning, setIsUpdating],
+    [dispatch, mediaSettings, router, seekToBeginning],
   );
 
   const onResetSettingsClick = useCallback(() => {
@@ -181,12 +186,7 @@ const VideoSettings: React.FC<Props> = ({
         </div>
       </div>
 
-      <RenderControls
-        isFetching={isFetching}
-        getCurrentFrame={getCurrentFrame}
-        getIsPlayerPlaying={getIsPlayerPlaying}
-        inputProps={inputProps}
-      />
+      <RenderControls isFetching={isFetching} inputProps={inputProps} playerRef={playerRef} />
     </>
   );
 };

diff --git a/src/pages/media/index.tsx b/src/pages/media/index.tsx
@@ -83,7 +83,6 @@ const MediaMaker: NextPage<MediaMaker> = ({
   const [isReady, setIsReady] = useState(false);
   const [videoFileReady, setVideoFileReady] = useState(false);
   const [audioFileReady, setAudioFileReady] = useState(false);
-  const [isUpdating, setIsUpdating] = useState(false);
   const TOAST_GENERAL_ERROR = t('common:error.general');
   const areMediaFilesReady = videoFileReady && audioFileReady;
 
@@ -138,17 +137,6 @@ const MediaMaker: NextPage<MediaMaker> = ({
 
   useAddQueryParamsToUrl(getQuranMediaMakerNavigationUrl(queryParams), {});
 
-  const seekToBeginning = useCallback(() => {
-    const current = playerRef?.current;
-    if (!current) {
-      return;
-    }
-    if (current.isPlaying) {
-      current.pause();
-    }
-    current.seekTo(0);
-  }, []);
-
   useEffect(() => {
     const current = playerRef?.current;
     if (!current) {
@@ -159,17 +147,13 @@ const MediaMaker: NextPage<MediaMaker> = ({
       current.toggle(e);
     };
 
-    current.getContainerNode().addEventListener('click', onClick);
+    current?.getContainerNode().addEventListener('click', onClick);
     // eslint-disable-next-line consistent-return
     return () => {
-      current.getContainerNode().removeEventListener('click', onClick);
+      current?.getContainerNode().removeEventListener('click', onClick);
     };
   }, []);
 
-  useEffect(() => {
-    setIsUpdating(false);
-  }, [mediaSettings]);
-
   const API_PARAMS = useMemo(() => {
     return {
       ...DEFAULT_API_PARAMS,
@@ -248,14 +232,6 @@ const MediaMaker: NextPage<MediaMaker> = ({
     return englishChaptersList?.[surah]?.translatedName as string;
   }, [surah, englishChaptersList]);
 
-  const getCurrentFrame = useCallback(() => {
-    return playerRef?.current?.getCurrentFrame();
-  }, []);
-
-  const getIsPlayerPlaying = useCallback(() => {
-    return playerRef?.current?.isPlaying();
-  }, []);
-
   const audioData = useMemo(() => {
     return getCurrentRangesAudioData(currentSurahAudioData, Number(verseFrom), Number(verseTo));
   }, [currentSurahAudioData, verseFrom, verseTo]);
@@ -332,7 +308,6 @@ const MediaMaker: NextPage<MediaMaker> = ({
       // {@see https://www.remotion.dev/docs/troubleshooting/player-flicker#option-6-prefetching-as-base64-to-avoid-network-request-and-local-http-server}
       const { waitUntilDone: waitUntilAudioDone } = prefetch(inputProps.audio.audioUrl, {
         method: 'blob-url',
-        contentType: 'audio/mp3',
       });
 
       waitUntilAudioDone()
@@ -358,7 +333,7 @@ const MediaMaker: NextPage<MediaMaker> = ({
   const renderPoster: RenderPoster = useCallback(() => {
     const video = getBackgroundVideoById(videoId);
 
-    if (isUpdating || isFetching || !areMediaFilesReady) {
+    if (isFetching || !areMediaFilesReady) {
       return (
         <div className={styles.loadingContainer}>
           <Spinner className={styles.spinner} size={SpinnerSize.Large} />
@@ -377,7 +352,7 @@ const MediaMaker: NextPage<MediaMaker> = ({
         />
       </AbsoluteFill>
     );
-  }, [areMediaFilesReady, isFetching, isUpdating, videoId]);
+  }, [areMediaFilesReady, isFetching, videoId]);
 
   const chaptersList = useMemo(() => {
     return Object.entries(chaptersData).map(([id, chapterObj], index) => ({
@@ -428,7 +403,7 @@ const MediaMaker: NextPage<MediaMaker> = ({
               doubleClickToFullscreen
               fps={VIDEO_FPS}
               ref={playerRef}
-              controls={!isUpdating && !isFetching && areMediaFilesReady}
+              controls={!isFetching && areMediaFilesReady}
               bufferStateDelayInMilliseconds={200} // wait for 200ms second before showing the spinner
               renderPoster={renderPoster}
               posterFillMode="player-size"
@@ -443,13 +418,10 @@ const MediaMaker: NextPage<MediaMaker> = ({
           <VideoSettings
             chaptersList={chaptersList}
             reciters={reciters}
-            seekToBeginning={seekToBeginning}
-            getCurrentFrame={getCurrentFrame}
-            getIsPlayerPlaying={getIsPlayerPlaying}
+            playerRef={playerRef}
             isFetching={isFetching}
             inputProps={inputProps}
             mediaSettings={mediaSettings}
-            setIsUpdating={setIsUpdating}
           />
         </div>
       </div>