diff --git a/src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementBackgroundImageManager.test.ts b/src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementBackgroundImageManager.test.ts new file mode 100644 index 000000000000..5193c5e7dd8e --- /dev/null +++ b/src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementBackgroundImageManager.test.ts @@ -0,0 +1,255 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; + +// Comical wants paper.js and a real , which jsdom doesn't give us. Nothing in the +// code under test needs it to do anything. +vi.mock("comicaljs", () => ({ + Bubble: class {}, + Comical: { + setSelectorForBubblesWhichTailMidpointMayOverlap: () => {}, + activateElement: () => {}, + update: () => {}, + }, +})); + +// jsdom gives every element a zero bounding rectangle, so the real getExactClientSize +// could only ever report the zero-area case. We control the reported size instead, so +// the test can also show what happens when the bloom-canvas does have a size. +const reportedSize = { width: 0, height: 0 }; +vi.mock("../../../utils/elementUtils", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + getExactClientSize: () => ({ + width: reportedSize.width, + height: reportedSize.height, + }), + }; +}); + +// These imports deliberately come after the vi.mock calls above, so that the module graph +// they pull in gets the stubbed modules. +import { + adjustBackgroundImageSize, + getBackgroundCanvasElement, +} from "./CanvasElementBackgroundImageManager"; +import type { BackgroundImageManagerState } from "./CanvasElementBackgroundImageManager"; + +// The inline styles a background image already has when something asks for a refit. +const initialCanvasElementStyle = { + width: "300px", + height: "200px", + left: "10px", + top: "20px", +}; + +function setUpBackgroundImage(): { + bloomCanvas: HTMLElement; + bgCanvasElement: HTMLElement; + img: HTMLImageElement; +} { + document.body.innerHTML = ` +
+
+
+
+ +
+
+
+
`; + const bloomCanvas = document.querySelector(".bloom-canvas") as HTMLElement; + const bgCanvasElement = document.querySelector( + ".bloom-backgroundImage", + ) as HTMLElement; + bgCanvasElement.style.width = initialCanvasElementStyle.width; + bgCanvasElement.style.height = initialCanvasElementStyle.height; + bgCanvasElement.style.left = initialCanvasElementStyle.left; + bgCanvasElement.style.top = initialCanvasElementStyle.top; + return { + bloomCanvas, + bgCanvasElement, + img: bgCanvasElement.getElementsByTagName("img")[0], + }; +} + +function makeState(): BackgroundImageManagerState { + return { bgImageLoadListeners: new WeakMap() }; +} + +function refit( + bloomCanvas: HTMLElement, + bgCanvasElement: HTMLElement, +): Promise { + return adjustBackgroundImageSize( + makeState(), + bloomCanvas, + bgCanvasElement, + false, + () => undefined, // nothing is the active element, so no controls get rendered + () => {}, + ); +} + +describe("adjustBackgroundImageSize", () => { + beforeEach(() => { + reportedSize.width = 0; + reportedSize.height = 0; + }); + + test("leaves the background image alone when the bloom-canvas has no area", async () => { + const { bloomCanvas, bgCanvasElement, img } = setUpBackgroundImage(); + // Sanity check: the styles we expect to survive the call are there to start with. + expect(bgCanvasElement.style.width).toBe( + initialCanvasElementStyle.width, + ); + expect(bgCanvasElement.style.left).toBe(initialCanvasElementStyle.left); + expect(img.style.width).toBe(""); + + await refit(bloomCanvas, bgCanvasElement); + + expect(bgCanvasElement.style.width).toBe( + initialCanvasElementStyle.width, + ); + expect(bgCanvasElement.style.height).toBe( + initialCanvasElementStyle.height, + ); + expect(bgCanvasElement.style.left).toBe(initialCanvasElementStyle.left); + expect(bgCanvasElement.style.top).toBe(initialCanvasElementStyle.top); + // and it did not start cropping the image either + expect(img.style.width).toBe(""); + }); + + test("leaves the background image alone when the bloom-canvas has width but no height", async () => { + const { bloomCanvas, bgCanvasElement } = setUpBackgroundImage(); + reportedSize.width = 400; + reportedSize.height = 0; + + await refit(bloomCanvas, bgCanvasElement); + + expect(bgCanvasElement.style.width).toBe( + initialCanvasElementStyle.width, + ); + expect(bgCanvasElement.style.height).toBe( + initialCanvasElementStyle.height, + ); + }); + + // A hidden bloom-canvas that has a border reports a negative size, because + // getExactClientSize subtracts the border from a zero bounding rectangle. + test("leaves the background image alone when the bloom-canvas reports a negative size", async () => { + const { bloomCanvas, bgCanvasElement } = setUpBackgroundImage(); + reportedSize.width = -2; + reportedSize.height = -2; + + await refit(bloomCanvas, bgCanvasElement); + + expect(bgCanvasElement.style.width).toBe( + initialCanvasElementStyle.width, + ); + expect(bgCanvasElement.style.height).toBe( + initialCanvasElementStyle.height, + ); + }); + + // This is the guard against the tests above passing for the wrong reason: given a + // bloom-canvas that does have a size, the same call really does resize the background + // image. (The image here has failed to load, which is the one case the code can size + // synchronously, since it then fills the container to show the error message.) + test("fits the background image to a bloom-canvas that has a size", async () => { + const { bloomCanvas, bgCanvasElement } = setUpBackgroundImage(); + reportedSize.width = 400; + reportedSize.height = 500; + + await refit(bloomCanvas, bgCanvasElement); + + expect(bgCanvasElement.style.width).toBe("400px"); + expect(bgCanvasElement.style.height).toBe("500px"); + expect(bgCanvasElement.style.left).toBe("0px"); + expect(bgCanvasElement.style.top).toBe("0px"); + }); +}); + +describe("getBackgroundCanvasElement", () => { + test("finds the background image that is a direct child of the bloom-canvas", () => { + const { bloomCanvas, bgCanvasElement } = setUpBackgroundImage(); + expect(getBackgroundCanvasElement(bloomCanvas)).toBe(bgCanvasElement); + }); + + test("returns undefined when the bloom-canvas has no background image", () => { + document.body.innerHTML = ` +
+
+
`; + const bloomCanvas = document.querySelector( + ".bloom-canvas", + ) as HTMLElement; + expect(getBackgroundCanvasElement(bloomCanvas)).toBeUndefined(); + }); + + test("does not take a nested bloom-canvas's background image for the outer one", () => { + document.body.innerHTML = ` +
+
+
+
+
+
+
+
+
`; + const outer = document.getElementById("outer") as HTMLElement; + const inner = document.getElementById("inner") as HTMLElement; + const innerBg = document.getElementById("innerBg") as HTMLElement; + // Sanity check: a plain descendant search would have found the inner one. + expect(outer.getElementsByClassName("bloom-backgroundImage")[0]).toBe( + innerBg, + ); + + expect(getBackgroundCanvasElement(outer)).toBeUndefined(); + expect(getBackgroundCanvasElement(inner)).toBe(innerBg); + }); + + test("finds each bloom-canvas's own background image when both have one", () => { + document.body.innerHTML = ` +
+
+
+
+
+
+
+
+
+
+
+
`; + const outer = document.getElementById("outer") as HTMLElement; + const inner = document.getElementById("inner") as HTMLElement; + expect(getBackgroundCanvasElement(outer)).toBe( + document.getElementById("outerBg"), + ); + expect(getBackgroundCanvasElement(inner)).toBe( + document.getElementById("innerBg"), + ); + }); + + // The Image Description tool wraps the background image in a bloom-describedImage + // while it is active, so the background image is not a direct child then. + test("still finds the background image when the Image Description tool has wrapped it", () => { + document.body.innerHTML = ` +
+
+
+
+
+
+
`; + const bloomCanvas = document.querySelector( + ".bloom-canvas", + ) as HTMLElement; + expect(getBackgroundCanvasElement(bloomCanvas)).toBe( + document.getElementById("bg"), + ); + }); +}); diff --git a/src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementBackgroundImageManager.ts b/src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementBackgroundImageManager.ts index 70a5fc379a6a..02e6340c3364 100644 --- a/src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementBackgroundImageManager.ts +++ b/src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementBackgroundImageManager.ts @@ -85,6 +85,26 @@ export function revertBackgroundCanvasElements(): void { } } +/** + * Find the canvas element that holds the background image of this bloom-canvas, if any. + * + * A descendant search is not enough once a bloom-canvas can be nested inside another: + * the outer canvas would find the inner canvas's background image and treat it as its + * own. A direct-child search is not enough either, because the Image Description tool + * temporarily wraps the background image in a bloom-describedImage. So we take the + * first background image whose nearest bloom-canvas is this one. + */ +export function getBackgroundCanvasElement( + bloomCanvas: HTMLElement, +): HTMLElement | undefined { + return Array.from( + bloomCanvas.getElementsByClassName(kBackgroundImageClass), + ).find( + (candidate) => + candidate.closest(`.${kBloomCanvasClass}`) === bloomCanvas, + ) as HTMLElement | undefined; +} + export function handleResizeAdjustments( state: BackgroundImageManagerState, bloomCanvases: HTMLElement[], @@ -117,9 +137,7 @@ function switchBackgroundToCanvasElementIfNeeded( getActiveElement: () => HTMLElement | undefined, alignControlFrameWithActiveElement: () => void, ) { - const bgCanvasElement = bloomCanvas.getElementsByClassName( - kBackgroundImageClass, - )[0] as HTMLElement; + const bgCanvasElement = getBackgroundCanvasElement(bloomCanvas); if (bgCanvasElement) { // I think this is redundant, but it got added by mistake at one point, // and will hide the placeholder if it's there, so make sure it's not. @@ -141,9 +159,7 @@ function switchBackgroundToCanvasElement( alignControlFrameWithActiveElement: () => void, ) { const oldBgImage = getImageFromContainer(bloomCanvas); - let bgCanvasElement = bloomCanvas.getElementsByClassName( - kBackgroundImageClass, - )[0] as HTMLElement; + let bgCanvasElement = getBackgroundCanvasElement(bloomCanvas); if (!bgCanvasElement) { // various legacy behavior, such as hiding the old-style background placeholder. bloomCanvas.classList.add(kHasCanvasElementClass); @@ -228,9 +244,7 @@ export function setupBackgroundImageAttributes( useSizeOfNewImage = false, ): Promise { if (!bgElement) { - bgElement = bloomCanvas.getElementsByClassName( - kBackgroundImageClass, - )[0] as HTMLElement; + bgElement = getBackgroundCanvasElement(bloomCanvas); } if (bgElement?.getAttribute("data-bubble")) { return Promise.resolve(); // setup has already been done (data-bubble is added by putBubbleBefore) @@ -375,6 +389,16 @@ function adjustBackgroundImageSizeToFit( ): Promise { const { width: bloomCanvasWidth, height: bloomCanvasHeight } = getExactClientSize(bloomCanvas); + if (bloomCanvasWidth <= 0 || bloomCanvasHeight <= 0) { + // Nothing useful can be computed from a box with no area, and the numbers + // we would write are not neutral: they become the baseline that later + // resizes scale, so the picture ends up somewhere arbitrary. A bloom-canvas + // that is hidden, or that has not been laid out yet, is in this state, so + // the right thing to do is wait until it has a real size and fit then. + // (A hidden bloom-canvas with a border reports a negative size, since + // getExactClientSize subtracts the border from a zero bounding rectangle.) + return Promise.resolve(); + } let imgAspectRatio = bgCanvasElement.clientWidth / bgCanvasElement.clientHeight; const img = getImageFromCanvasElement(bgCanvasElement); diff --git a/src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementManager.ts b/src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementManager.ts index a8165a66fe6b..19e6a3ab8de5 100644 --- a/src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementManager.ts +++ b/src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementManager.ts @@ -97,6 +97,7 @@ import { CanvasElementEditingSuspension } from "./CanvasElementEditingSuspension import { adjustCanvasElementChildrenIfSizeChanged } from "./CanvasElementResizeAdjustments"; import { adjustBackgroundImageSize as adjustCanvasBackgroundImageSize, + getBackgroundCanvasElement, handleResizeAdjustments as handleBackgroundResizeAdjustments, setupBackgroundImageAttributes, type BackgroundImageManagerState, @@ -2845,6 +2846,23 @@ export class CanvasElementManager { // but dragging gets stopped by mouse up, so we need to do it here. theOneCanvasElementManager.handleResizeAdjustments(); } + /** + * Run the same adjustments on every bloom-canvas on the page that an origami splitter + * drag runs on its panes: convert a legacy background image to a canvas element if + * needed, then rescale every canvas element to the size the bloom-canvas has now. + * A host that resizes a container with JavaScript after Bloom's page-load pass has + * run needs this, so that a picture follows its container the way an origami image + * follows its pane. + * + * The rescale only touches canvas elements that already have a position. A background + * image whose bloom-canvas had no size when it was created never got one (see the + * zero-area guard in adjustBackgroundImageSizeToFit), so this method leaves it alone; + * use refitBackgroundImage for that canvas. + */ + public adjustAfterContainerResize(): void { + this.handleResizeAdjustments(); + } + private handleResizeAdjustments(): void { handleBackgroundResizeAdjustments( this.backgroundImageManagerState, @@ -3132,6 +3150,27 @@ export class CanvasElementManager { ); } + /** + * Re-fit the background image of one bloom-canvas to the size the canvas has now. + * + * The general resize path (AdjustChildrenIfSizeChanged) keeps each child's offsets + * and scales them, which is right for canvas elements the user placed but wrong for + * a background image that was fitted while its bloom-canvas had no real size yet. + * That is the case whenever something lays out a container with JavaScript after + * the page-load pass, so the bloom-canvas inside it gets its real size later. + */ + public refitBackgroundImage(bloomCanvas: HTMLElement): Promise { + const bgCanvasElement = getBackgroundCanvasElement(bloomCanvas); + if (!bgCanvasElement) return Promise.resolve(); + // The promise settles once the image has loaded and been fitted, so a caller + // that lays out other things around the picture can wait for it. + return this.adjustBackgroundImageSize( + bloomCanvas, + bgCanvasElement, + false, + ); + } + public AdjustChildrenIfSizeChanged(bloomCanvas: HTMLElement): void { adjustCanvasElementChildrenIfSizeChanged( bloomCanvas, diff --git a/src/BloomBrowserUI/placeHolderImages.less b/src/BloomBrowserUI/placeHolderImages.less index 4fd3ba5bff30..9e87e4171fc0 100644 --- a/src/BloomBrowserUI/placeHolderImages.less +++ b/src/BloomBrowserUI/placeHolderImages.less @@ -52,18 +52,24 @@ // Use the easel placeholder instead of the usual flower placeholder for the background image whenever there is data-tool-id="canvas" // (there are no legacy-style templates that should have the easel icon) .bloom-canvas[data-tool-id="canvas"] - .bloom-backgroundImage.bloom-canvas-element { + > .bloom-backgroundImage.bloom-canvas-element { .bloom-imageContainer:has(img[src*="placeHolder.png"]) { background-image: @canvas-placeholder; } } // In a bloom-canvas that has canvas elements other than the background image, we want to -// hide the placeholder except for when the background image is active/selected +// hide the placeholder except for when the background image is active/selected. +// Both steps are child combinators on purpose. A bloom-canvas nested inside another +// bloom-canvas would otherwise satisfy the :has() from the outer canvas's elements, and +// so lose its own placeholder. A canvas element is always a direct child of the +// bloom-canvas it belongs to, except while the Image Description tool has it wrapped in +// a bloom-describedImage, and that case hides the placeholder anyway (see +// bloom-describedImage in editMode.less). .bloom-canvas.bloom-canvas:has( - .bloom-canvas-element:not(.bloom-backgroundImage) + > .bloom-canvas-element:not(.bloom-backgroundImage) ) { - .bloom-canvas-element.bloom-backgroundImage:not( + > .bloom-canvas-element.bloom-backgroundImage:not( [data-bloom-active="true"] ):has(img[src*="placeHolder.png"]) { .bloom-imageContainer {