diff --git a/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts b/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts index 87b1e30f076a..72281776df82 100644 --- a/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts +++ b/src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts @@ -1728,6 +1728,7 @@ export class CanvasElementManager { editable ); this.alignControlFrameWithActiveElement(); + this.adjustTarget(this.activeElement); } // Determine which of the side handles, if any, should have the class "bloom-currently-cropped" diff --git a/src/BloomBrowserUI/bookEdit/toolbox/games/GamePromptDialog.tsx b/src/BloomBrowserUI/bookEdit/toolbox/games/GamePromptDialog.tsx index de0b8eecb6ef..f64f3b3a550a 100644 --- a/src/BloomBrowserUI/bookEdit/toolbox/games/GamePromptDialog.tsx +++ b/src/BloomBrowserUI/bookEdit/toolbox/games/GamePromptDialog.tsx @@ -44,6 +44,10 @@ import { import { splitIntoGraphemes } from "../../../utils/textUtils"; import { kCanvasElementClass } from "../overlay/canvasElementUtils"; import { kBloomCanvasSelector } from "../../js/bloomImages"; +import OverflowChecker from "../../OverflowChecker/OverflowChecker"; +import { doesContainingPageHaveSameSizeMode } from "./gameUtilities"; +import { max } from "underscore"; +import { _0 } from "../../../react_components/Progress/ProgressBar.stories"; export const GamePromptDialog: React.FunctionComponent = props => { const promptL10nId = props.prompt?.getAttribute("data-caption-l10nid"); @@ -357,6 +361,7 @@ const initializeDialog = (prompt: HTMLElement, tg: HTMLElement | null) => { if (!promptEditable) { throw new Error("No promptEditable in dragActivity"); } + promptEditable.innerHTML = editable.innerHTML; // copy back to the permanent element so it gets saved. const promptText = editable.textContent ?? ""; // Split the prompt text into letter groups consisting of a base letter and any combining marks. @@ -368,77 +373,134 @@ const initializeDialog = (prompt: HTMLElement, tg: HTMLElement | null) => { // any letters. (Can become display:none if we have no letters.) setDraggableText(draggables[0], letters[0]); draggables[0].classList.remove("bloom-unused-in-lang"); - const separation = draggables[0].offsetWidth + 15; // enhance: may want to increase this - // How many can we fit in one row inside the parent? - const maxBubbles = Math.floor( - (draggables[0].parentElement?.offsetWidth ?? 0 - draggableX) / - separation - ); + const separation = 15; // enhance: may want to increase this + // truncate to the number of draggables we can display // This is important because (e.g., with autorepeat or paste) we can get a massive number of draggables // very quickly, and performance degrades badly, making it hard to recover. Also, until the page relaods, // ones beyond this would be off-page and difficult to deal with. And when it does reload they will all // be on top of each other and only just visible. - letters.splice(maxBubbles); const newBubbles: HTMLElement[] = []; - if (draggables.length < letters.length) { + const makeNewDraggable = () => { // We have more letters than draggables. We'll add more draggables. const lastDraggable = draggables[draggables.length - 1]; - for (let i = draggables.length; i < letters.length; i++) { - const newDraggable = lastDraggable.cloneNode( - true - ) as HTMLElement; - setGeneratedDraggableId(newDraggable); - // Ensure the new draggable starts out empty. See BL-14348. - // (This covers all languages present, visible or not.) - const paras = newDraggable.querySelectorAll( - "div.Letter-style>p" - ); - paras.forEach(p => { - p.textContent = ""; - GameTool.setBlankClass(p.parentElement as HTMLElement); - }); - lastDraggable.parentElement?.appendChild(newDraggable); - makeTargetForDraggable(newDraggable); - // It's available to push letter groups into - draggables.push(newDraggable); - // It needs refreshBubbleEditing to be called on it later. - newBubbles.push(newDraggable); - // It should be removed if we Cancel. This list has a longer lifetime. - createdBubbles.push(newDraggable); - } - } + const newDraggable = lastDraggable.cloneNode(true) as HTMLElement; + setGeneratedDraggableId(newDraggable); + // Ensure the new draggable starts out empty. See BL-14348. + // (This covers all languages present, visible or not.) + const paras = newDraggable.querySelectorAll("div.Letter-style>p"); + paras.forEach(p => { + p.textContent = ""; + GameTool.setBlankClass(p.parentElement as HTMLElement); + }); + lastDraggable.parentElement?.appendChild(newDraggable); + makeTargetForDraggable( + newDraggable, + page.getElementsByClassName("bloom-target-row")[0] as + | HTMLElement + | undefined + ); + // It's available to push letter groups into + draggables.push(newDraggable); + // It needs refreshBubbleEditing to be called on it later. + newBubbles.push(newDraggable); + // It should be removed if we Cancel. This list has a longer lifetime. + createdBubbles.push(newDraggable); + }; + // We need this loop to run over all the draggables and all the letters. + // When there are more letters than draggables, we will add a new one if there is room. + // When there are more draggables, or no more room, the remaining ones will be set + // empty for this language. + const iterations = Math.max(letters.length, draggables.length); + const minHeight = 50; // Enhance: get from page somehow + const minWidth = 50; // Enhance: get from page somehow + let newHeight = minHeight; // We deliberately don't remove draggables we don't need for this word. They might be in use // in some other language. This loop, as well as making the ones we want have the right content, // makes the ones we don't want invisible and empty. - for (let i = 0; i < draggables.length; i++) { - setDraggableText(draggables[i], letters[i]); - // up to the number of letters we have, they should be visible; others, not. - draggables[i].classList.toggle( - "bloom-unused-in-lang", - i >= letters.length - ); - getTarget(draggables[i])?.classList.toggle( - "bloom-unused-in-lang", - i >= letters.length - ); - copyContentToTarget(draggables[i]); + for (let i = 0; i < iterations; i++) { + if (i >= draggables.length) { + makeNewDraggable(); + } + const draggable = draggables[i]; + const target = getTarget(draggable) as HTMLElement; + let stopMakingLetters = false; + if (i < letters.length) { + draggable.classList.remove("bloom-unused-in-lang"); + target.classList.remove("bloom-unused-in-lang"); + setDraggableText(draggable, letters[i]); + copyContentToTarget(draggable); + target.style.width = `${minWidth}px`; + target.style.height = `${minHeight}px`; + // We'll measure the overflow of the target, because it has the extra border, + // so it can actually overflow when the draggable doesn't. + const editable = target?.getElementsByClassName( + "bloom-visibility-code-on" + )[0] as HTMLElement; + // as currently implemented these indicate the overflow of the editable. But it may be bigger than the + // target, and certainly does not allow for a border on the target. + const [ + overflowX, + overflowY + ] = OverflowChecker.getSelfOverflowAmounts(editable); + const row = target.parentElement as HTMLElement; + const bloomCanvas = row.parentElement as HTMLElement; + // The width the editable is currently, plus any overflow, plus the target border and padding in that direction. + const newWidth = + overflowX + + editable.offsetWidth + + (target.offsetWidth - target.clientWidth); + if ( + row.offsetLeft + target.offsetLeft + newWidth > + bloomCanvas.clientWidth + ) { + // This one won't fit. We'll stop without adding it. + stopMakingLetters = true; + } else { + draggable.style.width = `${newWidth}px`; + target.style.width = `${newWidth}px`; + newHeight = Math.max( + newHeight, + // again, the actual height of the editable, plus our overflow estimate, plus any padding and border of the target. + overflowY + + editable.offsetHeight + + (target.offsetHeight - target.clientHeight) + ); + } + } else { + stopMakingLetters = true; + } + if (stopMakingLetters) { + // either added them all or ran out of space + // We have more draggables than letters. Set the extra ones empty and invisible. + // (Don't delete them, because they might be in use in some other language.) + setDraggableText(draggable, ""); + copyContentToTarget(draggable); + draggable.classList.add("bloom-unused-in-lang"); + target.classList.add("bloom-unused-in-lang"); + draggable.style.width = `${minWidth}px`; + draggable.style.height = `${minHeight}px`; + target.style.width = `${minWidth}px`; + target.style.height = `${minHeight}px`; + } + } + for (const d of draggables) { + d.style.height = `${newHeight}px`; + const target = getTarget(d) as HTMLElement; + target.style.height = `${newHeight}px`; } - const shuffledDraggables = draggables.slice(); - shuffledDraggables.splice(letters.length); // don't want any invisible ones taking up space + const shuffledDraggables = draggables.slice( + 0, + letters.length + ) as HTMLElement[]; // don't want any invisible ones taking up space shuffle(shuffledDraggables, isTheTextInDraggablesTheSame); + let left = draggableX; for (let i = 0; i < shuffledDraggables.length; i++) { - shuffledDraggables[i].style.left = `${draggableX + - i * separation}px`; + shuffledDraggables[i].style.left = `${left}px`; + left += shuffledDraggables[i].offsetWidth + separation; shuffledDraggables[i].style.top = `${draggableY}px`; - // Note that we use draggables, not shuffledDraggables, here. We want the targets - // in the correct order, not the random order. - const t = getTarget(draggables[i]); - if (t) { - t.style.left = `${targetX + i * separation}px`; - t.style.top = `${targetY}px`; - } } + // Target shouldn't need adjusting, but we'll show the arrow for this one. adjustTarget(draggables[0], getTarget(draggables[0]), true); // Must do this AFTER we finish setting the content. Among many other things it does, // it will attach a ckeditor to the new editable. That does very complex things @@ -453,6 +515,7 @@ const initializeDialog = (prompt: HTMLElement, tg: HTMLElement | null) => { false // don't make it active. ); }); + // This seems to at least somewhat reduce the likelihood of losing focus // after a keystroke, especially with Keyman multi-character inserts (BL-14098). // I think it is harmless, since I can't think of any case where the text in the diff --git a/src/BloomBrowserUI/bookEdit/toolbox/games/GameTool.tsx b/src/BloomBrowserUI/bookEdit/toolbox/games/GameTool.tsx index 21fa25afd717..693fba6c0b32 100644 --- a/src/BloomBrowserUI/bookEdit/toolbox/games/GameTool.tsx +++ b/src/BloomBrowserUI/bookEdit/toolbox/games/GameTool.tsx @@ -277,31 +277,37 @@ const makeArrowShape = ( // These values make a line from the center of the draggable to the center of the target. const startX = draggable.offsetLeft + draggable.offsetWidth / 2; const startY = draggable.offsetTop + draggable.offsetHeight / 2; - const endXCenter = target.offsetLeft + target.offsetWidth / 2; - const endYCenter = target.offsetTop + target.offsetHeight / 2; + let targetLeft = target.offsetLeft; + let targetTop = target.offsetTop; + if (target.parentElement !== draggable.parentElement) { + targetLeft += target.parentElement!.offsetLeft; + targetTop += target.parentElement!.offsetTop; + } + const endXCenter = targetLeft + target.offsetWidth / 2; + const endYCenter = targetTop + target.offsetHeight / 2; // Figure out where the tip of the arrow should be. At least one of these will be changed, // so that we end up at a corner or side midpoint of the target. // (This assumes the two don't overlap. If they do, we don't make an arrow at all.) let endX = endXCenter; let endY = endYCenter; - if (target.offsetLeft > startX) { + if (targetLeft > startX) { // The target is entirely to the right of the center of the draggable. // We will go for one of the points on the left of the target. - endX = target.offsetLeft; - } else if (target.offsetLeft + target.offsetWidth < startX) { + endX = targetLeft; + } else if (targetLeft + target.offsetWidth < startX) { // The target is entirely to the left of the center of the draggable. // We will go for one of the points on the right of the target. - endX = target.offsetLeft + target.offsetWidth; + endX = targetLeft + target.offsetWidth; } - if (target.offsetTop > startY) { + if (targetTop > startY) { // The target is entirely below the center of the draggable. // We will go for one of the points on the top of the target. - endY = target.offsetTop; - } else if (target.offsetTop + target.offsetHeight < startY) { + endY = targetTop; + } else if (targetTop + target.offsetHeight < startY) { // The target is entirely above the center of the draggable. // We will go for one of points on the bottom of the target. - endY = target.offsetTop + target.offsetHeight; + endY = targetTop + target.offsetHeight; } if (!arrow) { @@ -1964,7 +1970,8 @@ function pxToNumber(dimension: string): number { } export const makeTargetForDraggable = ( - canvasElement: HTMLElement + canvasElement: HTMLElement, + parentForTarget?: HTMLElement ): HTMLElement => { const id = canvasElement.getAttribute("data-draggable-id"); if (!id) { @@ -1973,28 +1980,35 @@ export const makeTargetForDraggable = ( // don't simplify to 'document.createElement'; may be in a different iframe const target = canvasElement.ownerDocument.createElement("div"); target.setAttribute("data-target-of", id); - const left = pxToNumber(canvasElement.style.left); - const top = pxToNumber(canvasElement.style.top); const width = pxToNumber(canvasElement.style.width); const height = pxToNumber(canvasElement.style.height); - let newLeft = left + 20; - let newTop = top + height + 30; - if (newTop + height > canvasElement.parentElement!.clientHeight) { - newTop = Math.max(0, top - height - 30); - } - if (newLeft + width > canvasElement.parentElement!.clientWidth) { - newLeft = Math.max(0, left - width - 30); + if (!parentForTarget) { + // if we're putting it in some other parent, then positioning it relative to the + // thing for which it's a target doesn't make sense. It definitely isn't wanted for + // the current application, making a row of them in the letter game. + const left = pxToNumber(canvasElement.style.left); + const top = pxToNumber(canvasElement.style.top); + + let newLeft = left + 20; + let newTop = top + height + 30; + if (newTop + height > canvasElement.parentElement!.clientHeight) { + newTop = Math.max(0, top - height - 30); + } + if (newLeft + width > canvasElement.parentElement!.clientWidth) { + newLeft = Math.max(0, left - width - 30); + } + + // Review: can we do any more to make sure it's visible and not overlapping canvas element? + // Should we try to avoid overlapping other canvas elements and/or targets? + target.style.left = `${newLeft}px`; + target.style.top = `${newTop}px`; } - // Review: can we do any more to make sure it's visible and not overlapping canvas element? - // Should we try to avoid overlapping other canvas elements and/or targets? - target.style.left = `${newLeft}px`; - target.style.top = `${newTop}px`; target.style.width = `${width}px`; target.style.height = `${height}px`; // This allows it to get focus, which allows it to get the shadow effect we want when // clicked. But is that really right? We can't actually type there. target.setAttribute("tabindex", "0"); - canvasElement.parentElement!.appendChild(target); + (parentForTarget ?? canvasElement.parentElement)!.appendChild(target); enableDraggingTargets(target); adjustTarget(canvasElement, target); return target; diff --git a/src/BloomBrowserUI/bookEdit/toolbox/games/gameUtilities.tsx b/src/BloomBrowserUI/bookEdit/toolbox/games/gameUtilities.tsx index 2730059b4eeb..d7f3cfc4c802 100644 --- a/src/BloomBrowserUI/bookEdit/toolbox/games/gameUtilities.tsx +++ b/src/BloomBrowserUI/bookEdit/toolbox/games/gameUtilities.tsx @@ -14,7 +14,12 @@ function doesPageHaveSameSizeMode( if (!page) { return false; } - return page.getAttribute("data-same-size") !== "false"; + // Don't support same-size mode for this game. The new source page sets this false, but earlier + // ones just didn't have it, since same-size is usually the default. + return ( + page.getAttribute("data-same-size") !== "false" && + page.getAttribute("data-activity") !== "drag-letter-to-target" + ); } // Returns true if we need to take into account that this element must be kept the same size diff --git a/src/BloomBrowserUI/utils/measureText.ts b/src/BloomBrowserUI/utils/measureText.ts index 3bb28f09b32c..8d0fafffcf9b 100644 --- a/src/BloomBrowserUI/utils/measureText.ts +++ b/src/BloomBrowserUI/utils/measureText.ts @@ -77,9 +77,10 @@ export class MeasureText { .bottom; const baselineOfTextWithDefaultLineSpace = block.getBoundingClientRect() .bottom; - const fontDescent = + const fontDescent = Math.ceil( bottomOfTextWithDefaultLineSpace - - baselineOfTextWithDefaultLineSpace; + baselineOfTextWithDefaultLineSpace + ); if (lineHeight !== null) div.style.lineHeight = lineHeight; const bottomOfTextWithActualLineSpace = div.getBoundingClientRect() @@ -103,7 +104,9 @@ export class MeasureText { // line, but we decided it was not worth the effort to be that perfectionistic. // We only need enough space to draw what the browser thinks is the descent. - const canvasHeight = fontDescent; + // Plus a little extra because sometimes the font descent is not actually enough + // for the lowest possible descenders. + const canvasHeight = fontDescent + 4; const testingCanvas = this.createCanvas(width, canvasHeight); // The number of pixels of descent is one more than the index of // the bottom line on which we found part of a descender. @@ -200,8 +203,11 @@ export class MeasureText { fontSize: number ): void { const context = canvas.getContext("2d"); - if (context != null) { + if (context !== null) { context.textAlign = "start"; // was "top", which is not a valid choice + // means that the baseline of the text will align with the y coordinate passed + // as the last argument to fillText, which is zero, so basically we draw just + // the descenders at the top of the canvas. context.textBaseline = "alphabetic"; context.font = `${fontSize}px '${fontFamily}'`; context.fillStyle = "white"; diff --git a/src/content/templates/template books/Games/Games.html b/src/content/templates/template books/Games/Games.html index a0a6052ea316..1085df78f633 100644 --- a/src/content/templates/template books/Games/Games.html +++ b/src/content/templates/template books/Games/Games.html @@ -1,4 +1,4 @@ - + @@ -274,6 +274,7 @@ data-tool-id="game" data-pagelineage="3325A8B6-EA15-4FB7-9F8D-271D7B3C8D57" data-page-number="1" + data-same-size="false" lang="" data-correct-sound="correct_tada-fanfare-a_pixabay.webm" data-wrong-sound="wrong_twoDownElectronic2_pixabay.webm" @@ -350,10 +351,15 @@
+ class="bloom-target-row" + style="left: 30px; top: 254px;" + > +
+