-
-
Notifications
You must be signed in to change notification settings - Fork 18
Make page gutter understandable #7712
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,6 +68,42 @@ body { | |
| transition: transform 20ms; | ||
| } | ||
|
|
||
| .bloom-page-tooltip { | ||
| position: fixed; | ||
| z-index: @formatButtonZIndex; | ||
| display: none; | ||
| pointer-events: none; | ||
| white-space: nowrap; | ||
| padding: 2px 6px; | ||
| border: none; | ||
| border-radius: 3px; | ||
| background-color: black; | ||
| color: white; | ||
| font-family: @UIFontStack; | ||
| font-size: 8pt; | ||
| } | ||
|
|
||
| .bloom-page-tooltip-target { | ||
| position: absolute; | ||
| pointer-events: auto; | ||
| background-color: transparent; | ||
| } | ||
|
|
||
| .bloom-gutter-tooltip-target { | ||
| top: 0; | ||
| bottom: 0; | ||
| width: var(--page-gutter); | ||
| z-index: 3; | ||
| } | ||
|
|
||
| .bloom-gutter-target-side-left { | ||
| right: 0; | ||
| } | ||
|
|
||
| .bloom-gutter-target-side-right { | ||
| left: 0; | ||
| } | ||
|
|
||
| // See comments on .bloom-mediaBox in basePage.less for a description of the mediaBox. | ||
| // Here, we are causing it to be visible when desired. | ||
| .bloom-mediaBox { | ||
|
|
@@ -142,6 +178,41 @@ body:has(.MuiPaper-root:hover) > .qtip { | |
| outline: none !important; | ||
| } | ||
|
|
||
| // Show the paper-page gutter (binding space) in edit mode. | ||
| // Show this only while the page is active (hover/focus) like other edit affordances. | ||
| .bloom-page:is(.side-left, .side-right):not(.outsideFrontCover):not( | ||
| .outsideBackCover | ||
| )::before { | ||
| content: ""; | ||
| position: absolute; | ||
| top: 0; | ||
| bottom: 0; | ||
| width: var(--page-gutter); | ||
| background-color: var(--page-structure-color); | ||
| box-sizing: border-box; | ||
| pointer-events: none; | ||
| z-index: 2; | ||
| opacity: 0; | ||
|
Comment on lines
+183
to
+195
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚩 ::before pseudo-element on .bloom-page conflicts with MXB-Book-Literacy-Prepub branding The new gutter visualization uses Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| } | ||
| .bloom-page.side-left:not(.outsideFrontCover):not(.outsideBackCover)::before { | ||
| right: 0; | ||
| border-left: 2px dashed white; | ||
| } | ||
| .bloom-page.side-right:not(.outsideFrontCover):not(.outsideBackCover)::before { | ||
| left: 0; | ||
| border-right: 2px dashed white; | ||
| } | ||
|
|
||
| .bloom-page:is(:hover, :focus-within):is(.side-left, .side-right):not( | ||
| .outsideFrontCover | ||
| ):not(.outsideBackCover)::before, | ||
| body:has(#canvas-element-context-controls:hover) | ||
| .bloom-page:is(.side-left, .side-right):not(.outsideFrontCover):not( | ||
| .outsideBackCover | ||
| )::before { | ||
| opacity: 1; | ||
| } | ||
|
|
||
| // Keep together here all the effects we want when hovering (previously: also when focus-within) | ||
| // the bloom-page. All of them need to also apply when the canvas element context controls are hovered, | ||
| // even though that is no longer a child of the bloom-page. (That's what the second rule is for.) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,207 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import $ from "jquery"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import theOneLocalizationManager from "../../lib/localizationManager/localizationManager"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const kTooltipTargetClass = "bloom-page-tooltip-target"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const kGutterTargetClass = "bloom-gutter-tooltip-target"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const kGutterLeftClass = "bloom-gutter-target-side-left"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const kGutterRightClass = "bloom-gutter-target-side-right"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const kTooltipTextAttribute = "data-bloom-page-tooltip-text"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const kTooltipBubbleClass = "bloom-page-tooltip"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const kTooltipUiMarkerAttribute = "data-bloom-ui"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const kTooltipUiMarkerValue = "page-hover-tooltip"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const kDefaultGutterTooltipText = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Page Gutter: space for staples or other binding."; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let pageHoverTooltip: JQuery | undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let gutterTooltipText = kDefaultGutterTooltipText; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let formatDialogTitleTooltipText = "Format"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let activeTooltipText: string | undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let tooltipLocalizationInitialized = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| interface MousePositionEvent { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clientX: number; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clientY: number; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // This module supports tooltips for page editing regions that may not have their own interactive DOM elements. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // For those cases, we create temporary "target" elements (class bloom-page-tooltip-target) and position | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // them over the page region (currently, the paper gutter). These targets all get the bloom-ui class so they | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // are treated as edit-time markup. Also, when saving, bloomEditing.ts uses | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // getBodyInnerHtmlWithoutPageHoverTooltips() so these temporary elements are removed from the HTML that is | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // written to disk, even though they remain available in the live editing DOM. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function hidePageHoverTooltip(): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| activeTooltipText = undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pageHoverTooltip?.hide(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function ensurePageHoverTooltipBubble(): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (pageHoverTooltip) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pageHoverTooltip = $(`<div class='bloom-ui ${kTooltipBubbleClass}'></div>`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pageHoverTooltip.attr(kTooltipUiMarkerAttribute, kTooltipUiMarkerValue); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pageHoverTooltip.text(gutterTooltipText); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $("body").append(pageHoverTooltip); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function removeTooltipTargets(): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $(`.${kTooltipTargetClass}`).remove(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function makeTooltipTarget( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| page: HTMLElement, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| extraClasses: string[], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tooltipText: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const target = document.createElement("div"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| target.classList.add("bloom-ui", kTooltipTargetClass, ...extraClasses); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| target.setAttribute(kTooltipTextAttribute, tooltipText); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| target.setAttribute(kTooltipUiMarkerAttribute, kTooltipUiMarkerValue); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| page.appendChild(target); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function addGutterTooltipTargets(): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| document.querySelectorAll("div.bloom-page").forEach((pageElement) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const page = pageElement as HTMLElement; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| page.classList.contains("outsideFrontCover") || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| page.classList.contains("outsideBackCover") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (page.classList.contains("side-left")) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| makeTooltipTarget( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| page, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [kGutterTargetClass, kGutterLeftClass], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gutterTooltipText, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (page.classList.contains("side-right")) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| makeTooltipTarget( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| page, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [kGutterTargetClass, kGutterRightClass], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gutterTooltipText, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function updateGutterTooltipTexts(): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| document | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .querySelectorAll(`.${kGutterTargetClass}`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .forEach((targetElement) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| targetElement.setAttribute( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| kTooltipTextAttribute, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gutterTooltipText, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function getTooltipTextForTarget(target: HTMLElement): string | undefined { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (target.id === "formatButton") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return formatDialogTitleTooltipText; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const value = target.getAttribute(kTooltipTextAttribute); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return value || undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function setActiveTooltipText(tooltipText: string | undefined): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| activeTooltipText = tooltipText; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!activeTooltipText) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hidePageHoverTooltip(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pageHoverTooltip?.text(activeTooltipText).show(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function moveTooltipBubble(event: MousePositionEvent): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!activeTooltipText) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pageHoverTooltip?.css({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| left: event.clientX + 12, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| top: event.clientY + 12, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function setupTooltipLocalization(): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (tooltipLocalizationInitialized) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tooltipLocalizationInitialized = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| theOneLocalizationManager | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .asyncGetText("EditTab.Tooltip.Gutter", kDefaultGutterTooltipText, "") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .done((result) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gutterTooltipText = result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| updateGutterTooltipTexts(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| theOneLocalizationManager | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .asyncGetText("EditTab.FormatDialog.Format", "Format", "") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .done((result) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| formatDialogTitleTooltipText = result; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function setupPageHoverTooltips(): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ensurePageHoverTooltipBubble(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| removeTooltipTargets(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| addGutterTooltipTargets(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setupTooltipLocalization(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| $(document) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .off("mouseenter.bloomPageTooltip") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .on( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "mouseenter.bloomPageTooltip", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `.${kTooltipTargetClass}, #formatButton`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const target = this as HTMLElement; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setActiveTooltipText(getTooltipTextForTarget(target)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| moveTooltipBubble(e as unknown as MousePositionEvent); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .off("mousemove.bloomPageTooltip") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .on("mousemove.bloomPageTooltip", function (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| moveTooltipBubble(e as unknown as MousePositionEvent); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .off("mouseleave.bloomPageTooltip") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .on( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "mouseleave.bloomPageTooltip", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| `.${kTooltipTargetClass}, #formatButton`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setActiveTooltipText(undefined); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+171
to
+182
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | |
| ) | |
| .off("mousemove.bloomPageTooltip") | |
| .on("mousemove.bloomPageTooltip", function (e) { | |
| moveTooltipBubble(e as unknown as MousePositionEvent); | |
| }) | |
| .off("mouseleave.bloomPageTooltip") | |
| .on( | |
| "mouseleave.bloomPageTooltip", | |
| `.${kTooltipTargetClass}, #formatButton`, | |
| () => { | |
| setActiveTooltipText(undefined); | |
| // Attach mousemove handler only while a tooltip target is hovered | |
| $(document) | |
| .off("mousemove.bloomPageTooltip") | |
| .on( | |
| "mousemove.bloomPageTooltip", | |
| function (moveEvent) { | |
| moveTooltipBubble( | |
| moveEvent as unknown as MousePositionEvent, | |
| ); | |
| }, | |
| ); | |
| }, | |
| ) | |
| .off("mouseleave.bloomPageTooltip") | |
| .on( | |
| "mouseleave.bloomPageTooltip", | |
| `.${kTooltipTargetClass}, #formatButton`, | |
| () => { | |
| setActiveTooltipText(undefined); | |
| // Detach mousemove handler when tooltip is no longer active | |
| $(document).off("mousemove.bloomPageTooltip"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚩 Clone-based save replaces direct innerHTML access — performance and correctness tradeoff
The previous code at src/BloomBrowserUI/bookEdit/js/bloomEditing.ts:1348 was simply document.body.innerHTML. The new code calls getBodyInnerHtmlWithoutPageHoverTooltips() which does a full document.body.cloneNode(true), then removes tooltip markup from the clone, then returns clone.innerHTML. This is a deep clone of the entire page body on every save. For complex pages with many elements (canvas elements, images, translation groups), this could be noticeably more expensive than the prior direct property access. The function comment at line 1311 notes "We don't want this to become an async method" suggesting performance sensitivity. The clone is synchronous so correctness is preserved, but the cost scales with page complexity. An alternative approach — temporarily removing tooltip elements from the live DOM, getting innerHTML, then re-inserting them — would avoid the clone cost but would be riskier if an error occurs mid-operation.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a typo in the arrow function syntax. It should be "() =>" not "() ==>".