Skip to content

Commit e95f491

Browse files
committed
Refactor canvas elements: registry-driven controls
CanvasElementManager had grown too large and UI affordances (context menu + mini toolbar) were being assembled imperatively, which made ordering/section dividers hard to reason about and encouraged cross-bundle imports. This change introduces a declarative canvas element registry that drives which buttons and menus are available per element type. It also makes context menu/mini-toolbar composition deterministic: fixed section ordering, exactly one divider/spacer between non-empty sections, and Duplicate/Delete always last. To reduce runtime import-cycle risk across the edit view + toolbox bundles, DOM selectors/constants move to a dependency-light module (canvasElementConstants) while canvasElementUtils is narrowed to a cross-frame bridge (getCanvasElementManager) with type-only imports. CanvasElementManager is partially decomposed into focused helper modules (Geometry/Positioning/Alternates) plus public-function wrappers, and related call sites were updated. Misc hardening: safer MUI Menu anchoring, avoid non-null assertions, fix closest() selector typo, and remove duplicate pxToNumber helper. Follow-ups in this series: - Make mini-toolbar + menu more declarative and consistent - Make `toolbarButtons` the sole source of truth for the mini-toolbar (including explicit spacers) and normalize spacer runs. - Share menu + toolbar definitions via a single command registry to keep icons/tooltips/click behavior in sync. - Replace “Set Up Hyperlink” with the “Set Destination” command in this context, and do not show either on simple image elements.
1 parent d2df208 commit e95f491

36 files changed

Lines changed: 1726 additions & 947 deletions

src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import { kBloomYellow } from "../../bloomMaterialUITheme";
4141
import { RenderRoot } from "./AudioHilitePage";
4242
import { RenderCanvasElementRoot } from "./CanvasElementFormatPage";
4343
import { CanvasElementManager } from "../js/CanvasElementManager";
44-
import { kCanvasElementSelector } from "../toolbox/canvas/canvasElementUtils";
44+
import { kCanvasElementSelector } from "../toolbox/canvas/canvasElementConstants";
4545
import { getPageIFrame } from "../../utils/shared";
4646

4747
// Controls the CSS text-align value

src/BloomBrowserUI/bookEdit/editViewFrame.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export { showRegistrationDialogForEditTab as showRegistrationDialog };
6161
import { showAboutDialog } from "../react_components/aboutDialog";
6262
export { showAboutDialog };
6363
import { reportError } from "../lib/errorHandler";
64-
import { IToolboxFrameExports } from "./toolbox/toolboxBootstrap";
64+
import type { IToolboxFrameExports } from "./toolbox/toolboxBootstrap";
6565
import { showCopyrightAndLicenseInfoOrDialog } from "./copyrightAndLicense/CopyrightAndLicenseDialog";
6666
import { showTopicChooserDialog } from "./TopicChooser/TopicChooserDialog";
6767
import * as ReactDOM from "react-dom";

src/BloomBrowserUI/bookEdit/js/CanvasElementContextControls.tsx

Lines changed: 707 additions & 529 deletions
Large diffs are not rendered by default.

src/BloomBrowserUI/bookEdit/js/CanvasElementKeyboardProvider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Originally this was wired into CanvasSnapProvider.ts, but we're going to do that PR separately and later.
22
// And the way it was wired in, just using the grid size, may not be enough. We may need to ask the snap provider
33
// to give us the snap location. We'll see.
4-
import { kBackgroundImageClass } from "./CanvasElementManager";
4+
import { kBackgroundImageClass } from "../toolbox/canvas/canvasElementConstants";
55
import { CanvasSnapProvider } from "./CanvasSnapProvider";
66

77
export interface ICanvasElementKeyboardActions {

src/BloomBrowserUI/bookEdit/js/CanvasElementManager.ts

Lines changed: 118 additions & 307 deletions
Large diffs are not rendered by default.
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Small public helpers that other modules can call without importing the full
2+
// CanvasElementManager class.
3+
//
4+
// This file exists to keep CanvasElementManager.ts smaller and to reduce coupling
5+
// between the page bundle and toolbox UI code.
6+
7+
import { kCanvasToolId } from "../toolbox/toolIds";
8+
import { getToolboxBundleExports } from "./bloomFrames";
9+
import { kImageContainerClass } from "./bloomImages";
10+
11+
// This is just for debugging. It produces a string that describes the canvas element, generally
12+
// well enough to identify it in console.log.
13+
export const canvasElementDescription = (
14+
e: Element | null | undefined,
15+
): string => {
16+
const elt = e as HTMLElement;
17+
if (!elt) {
18+
return "no canvas element";
19+
}
20+
const result =
21+
"canvas element at (" + elt.style.left + ", " + elt.style.top + ") ";
22+
const imageContainer = elt.getElementsByClassName(kImageContainerClass)[0];
23+
if (imageContainer) {
24+
const img = (imageContainer as HTMLElement).getElementsByTagName(
25+
"img",
26+
)[0];
27+
if (img) {
28+
return result + "with image : " + img.getAttribute("src");
29+
}
30+
}
31+
const videoSrc = elt.getElementsByTagName("source")[0];
32+
if (videoSrc) {
33+
return result + "with video " + videoSrc.getAttribute("src");
34+
}
35+
// Enhance: look for videoContainer similarly
36+
return result + "with text " + elt.innerText;
37+
};
38+
39+
export const showCanvasTool = () => {
40+
getToolboxBundleExports()
41+
?.getTheOneToolbox()
42+
.activateToolFromId(kCanvasToolId);
43+
};

src/BloomBrowserUI/bookEdit/js/CanvasGuideProvider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// This class that helps visually align elements during drag operations by showing red lines
22
// and highlighting elements with equal dimensions during resize operations.
33

4-
import { kBackgroundImageClass } from "./CanvasElementManager";
4+
import { kBackgroundImageClass } from "../toolbox/canvas/canvasElementConstants";
55

66
// ALIGNMENT RULES:
77
// 1. When a dragged element aligns horizontally (top/middle/bottom) or vertically (left/center/right)

src/BloomBrowserUI/bookEdit/js/bloomEditing.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ import {
2424
initializeCanvasElementManager,
2525
theOneCanvasElementManager,
2626
} from "./CanvasElementManager";
27+
import { getCanvasElementManager } from "../toolbox/canvas/canvasElementUtils";
2728
import {
28-
getCanvasElementManager,
2929
kCanvasElementClass,
3030
kCanvasElementSelector,
31-
} from "../toolbox/canvas/canvasElementUtils";
31+
} from "../toolbox/canvas/canvasElementConstants";
3232
import { showTopicChooserDialog } from "../TopicChooser/TopicChooserDialog";
3333
import "../../modified_libraries/jquery-ui/jquery-ui-1.10.3.custom.min.js";
3434
import "./jquery.hasAttr.js"; //reviewSlog for CenterVerticallyInParent

src/BloomBrowserUI/bookEdit/js/bloomFrames.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313
to hide the details so that we can easily change it later.
1414
*/
1515

16-
import { IPageFrameExports } from "../editablePage";
17-
import { IEditViewFrameExports } from "../editViewFrame";
18-
import { IToolboxFrameExports } from "../toolbox/toolboxBootstrap";
16+
import type { IPageFrameExports } from "../editablePage";
17+
import type { IEditViewFrameExports } from "../editViewFrame";
18+
import type { IToolboxFrameExports } from "../toolbox/toolboxBootstrap";
1919

2020
interface WindowWithExports extends Window {
2121
editTabBundle: any;

src/BloomBrowserUI/bookEdit/js/bloomImages.ts

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,11 @@ import theOneLocalizationManager from "../../lib/localizationManager/localizatio
1010

1111
import {
1212
kBackgroundImageClass,
13-
theOneCanvasElementManager,
14-
updateCanvasElementClass,
15-
} from "./CanvasElementManager";
16-
import {
17-
kCanvasElementSelector,
1813
kBloomCanvasClass,
1914
kBloomCanvasSelector,
20-
} from "../toolbox/canvas/canvasElementUtils";
15+
kCanvasElementSelector,
16+
} from "../toolbox/canvas/canvasElementConstants";
17+
import { updateCanvasElementClass } from "../toolbox/canvas/canvasElementDomUtils";
2118

2219
import { farthest } from "../../utils/elementUtils";
2320
import { EditableDivUtils } from "./editableDivUtils";
@@ -415,7 +412,7 @@ function DisableImageTooltip(container: HTMLElement | undefined | null) {
415412
}
416413

417414
// If the canvas element manager hasn't been set up at all we can at least clear the current one.
418-
const bloomCanvas = container.closest(kBloomCanvasClass) as HTMLElement; // this is the one we want to clear the title on, if any
415+
const bloomCanvas = container.closest(kBloomCanvasSelector) as HTMLElement; // this is the one we want to clear the title on, if any
419416

420417
if (bloomCanvas) {
421418
bloomCanvas.title = "";
@@ -821,18 +818,17 @@ export function SetupResizableElement(element) {
821818
// caption centered, but currently we are NOT centering it. However, it makes sense
822819
// to resize the picture and its captions together anyway. We at least want the text
823820
// boxes to stay the same size as the bloom-canvas.)
821+
const canvasElementManager = getCanvasElementManager();
824822
const img = $(bloomCanvas).find("img");
825823
$(element).resizable({
826824
handles: "nw, ne, sw, se",
827825
containment: "parent",
828826
alsoResize: bloomCanvas,
829827
start(e, ui) {
830-
theOneCanvasElementManager.suspendComicEditing(
831-
"forJqueryResize",
832-
);
828+
canvasElementManager?.suspendComicEditing("forJqueryResize");
833829
},
834830
stop(e, ui) {
835-
theOneCanvasElementManager.resumeComicEditing();
831+
canvasElementManager?.resumeComicEditing();
836832
},
837833
});
838834
}

0 commit comments

Comments
 (0)