Skip to content

Commit

Permalink
Fix pause/play button in review step for iOS
Browse files Browse the repository at this point in the history
It worked in other browsers only by coincidence. I assume that pausing
the video in other browsers always causes a last `timeupdate`, which
lead to a rerender. On Safari this doesn't happen, so we have to make
sure that pausing/playing the video actually causes a rerender.
  • Loading branch information
LukasKalbertodt committed Oct 4, 2023
1 parent ffa07f2 commit af44c8a
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
2 changes: 2 additions & 0 deletions src/steps/review/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const Review: React.FC<StepProps> = ({ goToFirstStep, goToNextStep }) =>
const previewController = useRef<PreviewHandle>(null);
const [currentTime, setCurrentTime] = useState(0);
const [previewReady, setPreviewReady] = useState(false);
const [_isPaused, setIsPaused] = useState(true);

const expectedRecordings = match(videoChoice, {
"both": () => 2,
Expand Down Expand Up @@ -81,6 +82,7 @@ export const Review: React.FC<StepProps> = ({ goToFirstStep, goToNextStep }) =>
onTimeUpdate={event => {
setCurrentTime(event.currentTarget.currentTime);
}}
onPausePlay={paused => setIsPaused(paused)}
onReady={() => setPreviewReady(true)}
/>

Expand Down
10 changes: 9 additions & 1 deletion src/steps/review/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { SHORTCUTS, useShortcut } from "../../shortcuts";

type PreviewProps = {
onTimeUpdate: (event: SyntheticEvent<HTMLVideoElement, Event>) => void;
onPausePlay: (paused: boolean) => void;
onReady: () => void;
};

Expand All @@ -25,7 +26,10 @@ export type PreviewHandle = {
pause(): void;
};

export const Preview = forwardRef<PreviewHandle, PreviewProps>(({ onTimeUpdate, onReady }, ref) => {
export const Preview = forwardRef<PreviewHandle, PreviewProps>((
{ onTimeUpdate, onReady, onPausePlay },
ref,
) => {
const { recordings, start, end } = useStudioState();
const { t } = useTranslation();
const { isHighContrast } = useColorScheme();
Expand Down Expand Up @@ -86,9 +90,11 @@ export const Preview = forwardRef<PreviewHandle, PreviewProps>(({ onTimeUpdate,
},
play() {
allVideos.forEach(r => r.current?.play());
onPausePlay(false);
},
pause() {
allVideos.forEach(r => r.current?.pause());
onPausePlay(true);
},
}));

Expand Down Expand Up @@ -175,6 +181,7 @@ export const Preview = forwardRef<PreviewHandle, PreviewProps>(({ onTimeUpdate,
useShortcut(SHORTCUTS.review.forwardsFrame, () => jumpInTime(1 / fps));
useShortcut(SHORTCUTS.review.backwardsFrame, () => jumpInTime(-1 / fps));


const children = recordings.map((recording, index) => ({
dimensions: () => recording.dimensions,
body: (
Expand Down Expand Up @@ -247,6 +254,7 @@ export const Preview = forwardRef<PreviewHandle, PreviewProps>(({ onTimeUpdate,
});
}
}}

// For iOS: without the autoplay attribute, the `loadeddata` event is
// never fired for some reason. Adding this does not seem to actually
// cause Safari to autoplay.
Expand Down

0 comments on commit af44c8a

Please sign in to comment.