Skip to content

Save page when creating custom layout (BL-15989) #7731

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.

Sign up for GitHub

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

Merged
StephenMcConnel merged 1 commit into from
Mar 16, 2026
Merged

Save page when creating custom layout (BL-15989) #7731

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 55 additions & 32 deletions src/BloomBrowserUI/bookEdit/toolbox/canvas/customXmatterPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import { EditableDivUtils } from "../../js/editableDivUtils";
import { kBloomCanvasClass, kCanvasElementClass } from "./canvasElementUtils";
import { getAsync, postString } from "../../../utils/bloomApi";
import { Bubble, BubbleSpec } from "comicaljs";
import { recomputeSourceBubblesForPage } from "../../js/bloomEditing";
import {
recomputeSourceBubblesForPage,
wrapWithRequestPageContentDelay,
} from "../../js/bloomEditing";
import BloomSourceBubbles from "../../sourceBubbles/BloomSourceBubbles";
import { getToolboxBundleExports } from "../../js/workspaceFrames";
import { ILanguageNameValues } from "../../bookSettings/FieldVisibilityGroup";
Expand Down Expand Up @@ -57,18 +60,22 @@ import { isLegacyThemeCssLoaded } from "../../bookSettings/appearanceThemeUtils"
visibility of different languages.
*/

export function convertXmatterPageToCustom(page: HTMLElement): void {
export async function convertXmatterPageToCustom(
page: HTMLElement,
): Promise<void> {
const marginBox = page.getElementsByClassName(
"marginBox",
)[0] as HTMLElement;
if (!marginBox) return; // paranoia and lint

const languageNameValues = await getLanguageNameValues();

theOneCanvasElementManager?.turnOffCanvasElementEditing();
// we need to get rid of the old ones before we switch things around,
// since the remove code makes use of the existing divs that the
// source bubbles are connected to.
BloomSourceBubbles.removeSourceBubbles(page);

const marginBox = page.getElementsByClassName(
"marginBox",
)[0] as HTMLElement;
if (!marginBox) return; // paranoia and lint

const contentElements = Array.from(
marginBox.querySelectorAll("[data-book], [data-derived]"),
);
Expand Down Expand Up @@ -125,9 +132,11 @@ export function convertXmatterPageToCustom(page: HTMLElement): void {
e.classList.remove("bloom-visibility-code-on");
}
}
// We don't need to await this. We just want it done before the next time
// we save the page, and the async result should arrive almost instantly.
setDataDefault(ceContent as HTMLElement, lang || "");
setDataDefault(
ceContent as HTMLElement,
lang || "",
languageNameValues,
);

// Don't let the appearance system mess with which languages are visible here.
ceContent.removeAttribute("data-visibility-variable");
Expand Down Expand Up @@ -212,17 +221,20 @@ export function convertXmatterPageToCustom(page: HTMLElement): void {
finishReactivatingPage(page);
}

async function setDataDefault(
async function getLanguageNameValues(): Promise<ILanguageNameValues> {
return (await getAsync("settings/languageNames"))
.data as ILanguageNameValues;
}

function setDataDefault(
ceContent: HTMLElement,
lang: string,
): Promise<void> {
languageNameValues: ILanguageNameValues,
): void {
// We also want it to stay that way when C# code later updates visibility codes.
// This is based on a setting in the TG. Using these generic codes means that if
// the collection languages change, or we make a derivative, the right language
// should still be made visible in each box.
// settings/languageNames
const languageNameValues = (await getAsync("settings/languageNames"))
.data as ILanguageNameValues;
if (languageNameValues.language1Tag === lang) {
ceContent.setAttribute("data-default-languages", "V");
} else if (languageNameValues.language2Tag === lang) {
Expand Down Expand Up @@ -414,32 +426,43 @@ function renderPageLayoutMenu(page: HTMLElement, container: HTMLElement): void {
<CustomPageLayoutMenu
isCustom={isCustomPage}
disableCustomPage={usingLegacyTheme}
setCustom={(selection) => {
setCustom={async (selection) => {
if (usingLegacyTheme && selection !== "standard") {
return;
}
if (selection === "customStartOver") {
convertXmatterPageToCustom(page);
await wrapWithRequestPageContentDelay(
() => convertXmatterPageToCustom(page),
"customPageLayout-convertStartOver",
);
renderPageLayoutMenu(page, container);
return;
}
postString(
const response = await postString(
"editView/toggleCustomPageLayout",
page.getAttribute("id")!,
).then((response) => {
if (
selection === "custom" &&
response &&
// C# returns the string "false" if we don't have any saved state for custom mode,
// but currently something in axios converts that to a boolean false.
// I'm not sure that might not change one day, so we check for both.
(response.data === "false" || response.data === false)
) {
// making a custom cover for the first time
convertXmatterPageToCustom(page);
renderPageLayoutMenu(page, container);
}
});
);
if (
selection === "custom" &&
response &&
// C# returns the string "false" if we don't have any saved state for custom mode,
// but currently something in axios converts that to a boolean false.
// I'm not sure that might not change one day, so we check for both.
(response.data === "false" || response.data === false)
) {
// making a custom cover for the first time
await wrapWithRequestPageContentDelay(
() => convertXmatterPageToCustom(page),
"customPageLayout-convertFirstTime",
);
// Persist the newly created custom layout state so a later toggle back
// to standard has matching server-side state to work from.
await postString(
"editView/jumpToPage",
page.getAttribute("id")!,
);
renderPageLayoutMenu(page, container);
Comment on lines +458 to +464
Copy link

@devin-ai-integration devin-ai-integration bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 editView/jumpToPage triggers a page save AND reload, not just a save

The new code at lines 451-454 uses editView/jumpToPage to persist the custom layout state. Looking at the C# handler (EditingViewApi.cs:195-200), HandleJumpToPage calls request.PostSucceeded() then View.Model.SaveThen(() => pageId, () => { }). SaveThen saves the current page DOM and then navigates to pageId. Since the page ID is the current page, this will cause a full page reload after the save completes. This means renderPageLayoutMenu at line 455 is called but will be immediately wiped by the reload. The reload also means setupPageLayoutMenu() will run again on the reloaded page, re-rendering the menu. This is wasteful but not harmful — it just causes a brief visual flash. If the intent is only to save (not reload), a more targeted save mechanism may be preferable.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}
}}
/>,
container,
Expand Down