From 97d01c7c3d250fa5b675a057b0c346f19639b4b3 Mon Sep 17 00:00:00 2001 From: Noel Chou Date: Fri, 20 Feb 2026 18:33:52 -0500 Subject: [PATCH] real BL-15721 wip --- src/BloomBrowserUI/bookEdit/css/editMode.less | 38 ++++++++++++++----- .../bookEdit/js/CanvasElementManager.ts | 3 +- src/BloomBrowserUI/bookEdit/js/bloomVideo.ts | 26 ++++++++----- src/content/bookLayout/basePage.less | 3 -- 4 files changed, 48 insertions(+), 22 deletions(-) diff --git a/src/BloomBrowserUI/bookEdit/css/editMode.less b/src/BloomBrowserUI/bookEdit/css/editMode.less index 1de1b90b9a90..e92f9c3aa3c5 100644 --- a/src/BloomBrowserUI/bookEdit/css/editMode.less +++ b/src/BloomBrowserUI/bookEdit/css/editMode.less @@ -1642,21 +1642,34 @@ svg.bloom-videoControl { } } +// Detector above the canvas so we can detect hover and clicks of playback controls +.bloom-videoMouseDetector { + 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 { + .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); + } } } @@ -1674,6 +1687,13 @@ svg.bloom-videoControl { } } +// TODO copilot did this and it works but I'm not sure it's the right fix. Let's see if HandlePlayClick gets fixed in BL-15936 +// For draggable videos in Play Mode, disable pointer events on the mouse detector +// so clicks can pass through to the drag system which handles playback +.drag-activity-play [data-draggable-id] .bloom-videoMouseDetector { + pointer-events: none; +} + // If the background image hasn't been set in a game, we don't want to show the placeholder image. .bloom-page[data-tool-id="game"]:not([data-activity="simple-dom-choice"]) { .bloom-canvas diff --git a/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts b/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts index 9ae5722241b5..a00cca4d13c0 100644 --- a/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts +++ b/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts @@ -6535,7 +6535,8 @@ export class CanvasElementManager { .find(".bloom-ui") .filter( (_, x) => - !x.classList.contains("bloom-videoControlContainer"), + !x.classList.contains("bloom-videoControlContainer") && + !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) diff --git a/src/BloomBrowserUI/bookEdit/js/bloomVideo.ts b/src/BloomBrowserUI/bookEdit/js/bloomVideo.ts index ac06a7cb975c..fc81da378ac7 100644 --- a/src/BloomBrowserUI/bookEdit/js/bloomVideo.ts +++ b/src/BloomBrowserUI/bookEdit/js/bloomVideo.ts @@ -31,9 +31,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 @@ -44,13 +49,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", ); @@ -302,17 +307,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; } @@ -336,6 +341,7 @@ export function handlePlayClick(ev: MouseEvent, forcePlay?: boolean) { // becuse the click might be a drag on the canvas element. We'll let CanvasElementManager // decide and call playVideo if appropriate. That is, if we're not in Play mode, // where dragging is not applicable, or being called FROM the CanvasElementManager. + // TODO The above comment is out of date; there if ( !forcePlay && video.closest(kCanvasElementSelector) && @@ -391,12 +397,14 @@ function handlePauseClick(ev: MouseEvent) { // be a natural bit of code to put in dragActivityRuntime.ts, except we don't need // it there, because BloomPlayer has this behavior for all videos, not just in Games.) const handleVideoClick = (ev: MouseEvent) => { - const video = ev.currentTarget as HTMLVideoElement; + const video = (ev.currentTarget as HTMLElement) + ?.closest(".bloom-videoContainer") + ?.getElementsByTagName("video")[0]; // If we're not in Play mode, we don't need these behaviors. // At least I don't think so. Outside Play mode, clicking on canvas elements is mainly about moving // them, but we have a visible Play button in case you want to play one. In BP (and Play mode), you - // can't move them (unless one day we make them something you can drag to a target), so it + // can't move them if they aren't draggable, so it // makes sense that a click anywhere on the video would play it; there's nothing else useful // to do in response. if (!video.closest(".drag-activity-play")) { diff --git a/src/content/bookLayout/basePage.less b/src/content/bookLayout/basePage.less index 05615e468e4c..67ff2a2a5258 100644 --- a/src/content/bookLayout/basePage.less +++ b/src/content/bookLayout/basePage.less @@ -336,9 +336,6 @@ 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; - video { // I don't know exactly why this works, but it makes the video shrink to fit, // keep its aspect ratio, and center in whatever direction is not filled.