Skip to content

Show canvas tool when making cover page custom layout (BL-15977)#7767

Merged
JohnThomson merged 1 commit intomasterfrom
BL-15977-CanvasCoverEnablesCanvasTool
Mar 25, 2026
Merged

Show canvas tool when making cover page custom layout (BL-15977)#7767
JohnThomson merged 1 commit intomasterfrom
BL-15977-CanvasCoverEnablesCanvasTool

Conversation

@StephenMcConnel
Copy link
Contributor

@StephenMcConnel StephenMcConnel commented Mar 24, 2026

This change is Reviewable


Open with Devin

devin-ai-integration[bot]

This comment was marked as resolved.

@StephenMcConnel StephenMcConnel force-pushed the BL-15977-CanvasCoverEnablesCanvasTool branch from 6b1661a to ce6c05f Compare March 24, 2026 18:00
devin-ai-integration[bot]

This comment was marked as resolved.

@StephenMcConnel StephenMcConnel force-pushed the BL-15977-CanvasCoverEnablesCanvasTool branch from ce6c05f to de39919 Compare March 24, 2026 18:16
Copy link
Contributor

@JohnThomson JohnThomson left a comment

Choose a reason for hiding this comment

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

@JohnThomson reviewed 1 file and made 1 comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on StephenMcConnel).


src/BloomExe/web/controllers/EditingViewApi.cs line 182 at r3 (raw file):

            if (switchingToCustom)
            {
                pageElt.SetAttribute("data-tool-id", "canvas");

It's not clear to me that we need to do all this here as well as in JS. The state of the toolbox and the data-tool-id that we set in TS should be saved automatically. I think it's at least worth a try to see if we can do without this, and comment on the apparently duplication if it is necessary.

@StephenMcConnel StephenMcConnel force-pushed the BL-15977-CanvasCoverEnablesCanvasTool branch from de39919 to 183857f Compare March 25, 2026 18:20
Copy link
Contributor Author

@StephenMcConnel StephenMcConnel left a comment

Choose a reason for hiding this comment

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

@StephenMcConnel made 1 comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on JohnThomson).


src/BloomExe/web/controllers/EditingViewApi.cs line 182 at r3 (raw file):

Previously, JohnThomson (John Thomson) wrote…

It's not clear to me that we need to do all this here as well as in JS. The state of the toolbox and the data-tool-id that we set in TS should be saved automatically. I think it's at least worth a try to see if we can do without this, and comment on the apparently duplication if it is necessary.

Setting the data-tool-id needs to stay in C#, but the BookInfo settings aren't needed once I figured out the sequence of events in JS and fixed things there.

@StephenMcConnel StephenMcConnel force-pushed the BL-15977-CanvasCoverEnablesCanvasTool branch from 183857f to 0ca4b54 Compare March 25, 2026 19:07
devin-ai-integration[bot]

This comment was marked as resolved.

@StephenMcConnel StephenMcConnel force-pushed the BL-15977-CanvasCoverEnablesCanvasTool branch from 0ca4b54 to 95fe3c2 Compare March 25, 2026 19:35
Copy link
Contributor

@JohnThomson JohnThomson left a comment

Choose a reason for hiding this comment

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

@JohnThomson reviewed 2 files and all commit messages, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on StephenMcConnel).

@JohnThomson JohnThomson merged commit e03e43f into master Mar 25, 2026
2 checks passed
@JohnThomson JohnThomson deleted the BL-15977-CanvasCoverEnablesCanvasTool branch March 25, 2026 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants