Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
31 changes: 22 additions & 9 deletions src/BloomBrowserUI/bookEdit/css/editMode.less
Original file line number Diff line number Diff line change
Expand Up @@ -1640,21 +1640,34 @@ svg.bloom-videoControl {
}
}

// Detector above the canvas so we can detect hover and clicks of playback controls
.bloom-videoMouseDetector {
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 don't know what to call this. "videoInteractionDetector"? "videoMouseEventDetector"? If we decide not to preserve hover detection (see below) then we could call it "videoClickDetector"

height: 100%;
width: 100%;
position: absolute;
top: 0;
z-index: @canvasElementCanvasZIndex + 1;
}

// Show pause and replay when playing and hovered.
.bloom-videoContainer.playing:hover {
.bloom-videoControlContainer {
&.bloom-videoPauseIcon,
&.bloom-videoReplayIcon {
display: flex;
.bloom-videoContainer.playing {
.bloom-videoMouseDetector:hover {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like we decided not to do hover behavior in BloomPlayer (e.g. BloomBooks/bloom-player@c31f0fc) but kept it in BloomDesktop. Do we still want this?

.bloom-videoControlContainer {
&.bloom-videoPauseIcon,
&.bloom-videoReplayIcon {
display: flex;
}
}
}
}

// Show play, centered, when hovering a video that is not paused or playing
.bloom-videoContainer:not(.paused):not(.playing):hover {
.bloom-videoPlayIcon {
display: flex;
left: calc(50% - @videoIconSize / 2);
.bloom-videoContainer:not(.paused):not(.playing) {
.bloom-videoMouseDetector:hover {
.bloom-videoPlayIcon {
display: flex;
left: calc(50% - @videoIconSize / 2);
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6426,7 +6426,8 @@ export class CanvasElementManager {
.find(".bloom-ui")
.filter(
(_, x) =>
!x.classList.contains("bloom-videoControlContainer"),
!x.classList.contains("bloom-videoControlContainer") &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what's going on here, we have a "Not sure about keeping this" comment above, but now there's no point in keeping a bloom-videoControlContainer without a bloom-videoMouseDetector, it won't work

!x.classList.contains("bloom-videoMouseDetector"),
)
.remove();
thisCanvasElement.find(".bloom-dragHandleTOP").remove(); // BL-7903 remove any left over drag handles (this was the class used in 4.7 alpha)
Expand Down
19 changes: 12 additions & 7 deletions src/BloomBrowserUI/bookEdit/js/bloomVideo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,14 @@ export function SetupVideoEditing(container) {
// debugging, and just might prevent a problem in normal operation.
videoElement.parentElement?.classList.remove("playing");
videoElement.parentElement?.classList.remove("paused");
videoElement.addEventListener("click", handleVideoClick);
const mouseDetector =
videoElement.ownerDocument.createElement("div");
mouseDetector.classList.add("bloom-videoMouseDetector");
mouseDetector.classList.add("bloom-ui"); // don't save as part of document
mouseDetector.addEventListener("click", handleVideoClick);
videoElement.parentElement?.appendChild(mouseDetector);
const playButton = wrapVideoIcon(
videoElement,
mouseDetector,
// Alternatively, we could import the Material UI icon, make this file a TSX, and use
// ReactDom.render to render the icon into the div. But just creating the SVG
// ourselves (as these methods do) seems more natural to me. We would not be using
Expand All @@ -43,13 +48,13 @@ export function SetupVideoEditing(container) {
);
playButton.addEventListener("click", handlePlayClick);
const pauseButton = wrapVideoIcon(
videoElement,
mouseDetector,
getPauseIcon("#ffffff", videoElement),
"bloom-videoPauseIcon",
);
pauseButton.addEventListener("click", handlePauseClick);
const replayButton = wrapVideoIcon(
videoElement,
mouseDetector,
getReplayIcon("#ffffff", videoElement),
"bloom-videoReplayIcon",
);
Expand Down Expand Up @@ -296,17 +301,17 @@ export function updateVideoInContainer(container: Element, url: string): void {
// configure one of the icons we display over videos. We put a div around it and apply
// various classes and append it to the parent of the video.
function wrapVideoIcon(
videoElement: HTMLVideoElement,
parent: HTMLElement,
icon: HTMLElement,
iconClass: string,
): HTMLElement {
const wrapper = videoElement.ownerDocument.createElement("div");
const wrapper = parent.ownerDocument.createElement("div");
wrapper.classList.add("bloom-videoControlContainer");
wrapper.classList.add("bloom-ui"); // don't save as part of document
wrapper.appendChild(icon);
wrapper.classList.add(iconClass);
icon.classList.add("bloom-videoControl");
videoElement.parentElement?.appendChild(wrapper);
parent.appendChild(wrapper);
return icon;
}

Expand Down
9 changes: 7 additions & 2 deletions src/content/bookLayout/basePage.less
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,13 @@ books may contain divs with box-header-off, so we need a rule to hide them.*/
background-size: contain;
}

// above canvas so we can click playback controls
z-index: @canvasElementCanvasZIndex + 1;
// When the book is viewed in an older version of BloomPlayer, we want to set z-index to make the video controls
// clickable like we used to do before BL-15721, even that means any speech bubbles will be behind the video.
// Newer versions of BloomPlayer will add a .videoMouseDetector element to detect clicks so that in that case
// we can avoid setting z-index here and so allow speech bubbles to be in front of the video.
&:not(:has(.bloom-videoMouseDetector)):not(:has(.videoMouseDetector)) {
z-index: @canvasElementCanvasZIndex + 1;
}

video {
// I don't know exactly why this works, but it makes the video shrink to fit,
Expand Down