Skip to content

Commit

Permalink
fix: Get games to not play embedded videos twice (20241202)
Browse files Browse the repository at this point in the history
Also clean up playAllVideos in narration.ts a bit(which is now used only
by Bloom Desktop)
  • Loading branch information
StephenMcConnel committed Dec 2, 2024
1 parent fb18ab6 commit 8f62151
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 9 deletions.
6 changes: 5 additions & 1 deletion src/activities/dragActivities/DragToDestination.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ class DragToDestinationActivity implements IActivityObject {
}

public doInitialSoundAndAnimation(activityContext: ActivityContext) {
playInitialElements(activityContext.pageElement);
// As noted by the name of this method, we want to do the sound here not the video.
// Any video on the page has already been played by the time this is called by the
// Video.HandlePageVisible method. In fact, this is triggered by the final video
// ending event handler.
playInitialElements(activityContext.pageElement, false);
}

// Do just those things that we only want to do once per read of the book.
Expand Down
14 changes: 9 additions & 5 deletions src/dragActivityRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ function changePageButtonClicked(e: MouseEvent) {
currentChangePageAction?.(next);
}

export function playInitialElements(page: HTMLElement) {
export function playInitialElements(page: HTMLElement, playVideos: boolean) {
const initialFilter = (e) => {
const top = e.closest(".bloom-textOverPicture") as HTMLElement;
if (!top) {
Expand Down Expand Up @@ -387,9 +387,6 @@ export function playInitialElements(page: HTMLElement) {
}
return true;
};
const videoElements = Array.from(page.getElementsByTagName("video")).filter(
initialFilter,
);
const audioElements = getVisibleEditables(page).filter(initialFilter);

//Slider: // This is used in drag-word-chooser-slider to mark the text item the user is currently
Expand All @@ -402,7 +399,14 @@ export function playInitialElements(page: HTMLElement) {
// audioElements.push(activeTextBox);
// }
const playables = getAudioSentences(audioElements);
playAllVideo(videoElements, () => playAllAudio(playables, page));
if (playVideos) {
const videoElements = Array.from(
page.getElementsByTagName("video"),
).filter(initialFilter);
playAllVideo(videoElements, () => playAllAudio(playables, page));
} else {
playAllAudio(playables, page);
}
}

function getAudioSentences(editables: HTMLElement[]) {
Expand Down
6 changes: 3 additions & 3 deletions src/narration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,7 @@ export function hidingPage() {
// Play the specified elements, one after the other. When the last completes (or at once if the array is empty),
// perform the 'then' action (typically used to play narration, which we put after videos).
//
// Note, there is a very similar function in narration.ts. It would be nice to combine them, but
// Note, there is a very similar function in video.ts. It would be nice to combine them, but
// there are various reasons that is difficult at the moment. e.g.:
// 1. See comment below about sharing code with Bloom Desktop.
// 2. The other version handles play/pause which doesn't apply in BloomDesktop.
Expand All @@ -1254,7 +1254,7 @@ export function playAllVideo(elements: HTMLVideoElement[], then: () => void) {
video.readyState === HTMLMediaElement.HAVE_NOTHING
) {
showVideoError(video);
this.playAllVideo(elements.slice(1));
playAllVideo(elements.slice(1), then);
} else {
hideVideoError(video);
// Review: do we need to do something to let the rest of the world know about this?
Expand Down Expand Up @@ -1282,7 +1282,7 @@ export function playAllVideo(elements: HTMLVideoElement[], then: () => void) {
.catch((reason) => {
console.error("Video play failed", reason);
showVideoError(video);
this.playAllVideo(elements.slice(1), then);
playAllVideo(elements.slice(1), then);
});
}
}
Expand Down

0 comments on commit 8f62151

Please sign in to comment.