Allow image placeholders to get "edit with AI" (BL-16744) - #8230
Conversation
https://issues.bloomlibrary.org/youtrack/issue/BL-16744 "Edit with AI..." was disabled on an empty image slot, so the only way to fill a placeholder with an AI image was to put some other image there first. The host side already supported empty slots: EnumerateBookImages lists them with isPlaceholder, the commit path can write into them, and the AI image editor has "create" tools that make an image with no source image. Only the menu rule stood in the way. Changes: - canvasControlAvailabilityRules.editWithAi is now enabled when the user can modify the slot AND the slot is either an empty placeholder or a real image of a format the editor can open. A broken image stays disabled, as before. - IControlContext gains isPlaceholderImage, set from isPlaceHolderImage(img.src). It is needed because !hasRealImage is also true for an image that failed to load, and those two cases now have to be told apart. - aiEditorOverlay sends a placeholder slot's id as selectedBookImageId instead of withholding it. Enabling the menu item exposed a trap: with no selectedBookImageId the AI image editor targets the FIRST image of the book, which is normally the front cover, so a user who launched on an empty slot and ran an edit tool would have changed the cover. Sending the id targets the slot the user clicked. A "create" tool ignores the target, so the ordinary flow is unchanged. No change was needed in C# or in the separate bloom-ai-image-tools editor. Tests: added four cases to canvasControlAvailabilityRules.test.ts (placeholder enabled; placeholder enabled whatever the format check says; placeholder disabled when unmodifiable; broken image still disabled) and rewrote the overlay's placeholder target test. Ran the whole front-end suite with vitest: 736 passed, 5 skipped, 0 failed. Ran pnpm typecheck (passed) and eslint on the changed files (clean).
The AI image editor now recognizes an empty slot from isPlaceholder: it puts nothing in "Image to Edit" and opens its "Create an Image" tool, keeping the slot so the created image can be committed into it. The comment here described the older behavior. Needs a bloom-ai-image-tools build newer than dist-v0.1.5.
Every empty image slot looks the same in the AI image editor, so a book with two of them on one page gave the user no way to know which slot they were filling. Bloom now sends a label with each slot: "Page 3" for a numbered page, the page's own English name (e.g. "Front Cover") for front or back matter, and "Page 3 - image 2" when a page offers more than one image. The label was previously the raw data-page-number attribute, which said nothing at all for xmatter pages and never distinguished two slots on one page. The name is English because the AI image editor's user interface is English only; a localized page name inside it would look out of place, and it would need a new localizable string for "Page". GetPageNameForImageSlotLabel deliberately does not call HtmlDom.GetNumberOrLabelOfPageWhereElementLives: that helper returns the number bare, with no "Page", and answers "unknown" for a page whose data-page-number attribute is missing rather than empty, which HtmlDom itself can produce (see BL-12903). No label is better than "unknown". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A bloom-canvas can hold one background image with pictures on top of it. A page with a background and one picture reported "Page 1 - image 1" and "Page 1 - image 2", which tells the user nothing about which is which. It now reports "Page 1 - Canvas Background" and "Page 1 - Image 1". The background is named, not numbered, so the pictures on top of it start at 1 whether or not the page has a background. A page with a single image slot still gets the page name alone, which is the common case: a full-page picture is one canvas background image and nothing else. HtmlDom.IsBackgroundImage answers which slot is the background. A test pins the canvas markup that question is asked about, so a change to that DOM breaks here rather than quietly mislabelling every page. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The labels Bloom sends to the AI image editor for its strip of the book's
images were English, on the grounds that the editor's own user interface is
English. But these labels name the user's own book, so they now come from
BloomMediumPriority.xlf in the user interface language.
Four new entries: "Page {0}", "Canvas Background", "Image {0}", and "{0} - {1}"
to join the two halves. The separate join lets a translator reorder the halves
or change the separator.
A front or back matter page is named by its page label, which is localized the
same way the Edit tab's page list does it, under the dynamic id
"TemplateBooks.PageLabel." plus the English label.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A Picture Dictionary page holds six bloom-canvases, so six canvas background images. Every one of them was labelled "Page 4 - Canvas Background", which is the confusion these labels exist to remove. "Canvas Background" now names a slot only when the page has exactly one canvas. A page with several numbers them all instead, which keeps every label distinct without a new localizable string. The naming of a page's slots moved into BuildImageSlotLabelsForPage, because a slot's name depends on what else the page holds. That also makes it testable without a Book. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
| Filename | Overview |
|---|---|
| src/BloomBrowserUI/bookEdit/aiImageEditor/aiEditorOverlay.ts | Sends placeholders as selected targets, but the required newer iframe editor is not included by the dependency pin. |
| src/BloomExe/web/controllers/AiImageEditorApi.cs | Adds localized page/slot labels while preserving full-holder ordinals for deterministic replacement matching. |
| src/BloomBrowserUI/bookEdit/toolbox/canvas/canvasControlAvailabilityRules.ts | Correctly enables editable placeholders while retaining disabled behavior for broken or unsupported images. |
| src/BloomBrowserUI/bookEdit/toolbox/canvas/buildCanvasElementControlRegistryContext.ts | Adds placeholder state using the existing shared placeholder-recognition helper. |
| DistFiles/localization/en/BloomMediumPriority.xlf | Adds the four label strings with the repository-required temporary translate="no" convention. |
| src/BloomTests/web/controllers/AiImageEditorApiTests.cs | Covers page naming and slot labeling across single-image, canvas, and multi-canvas layouts. |
Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/mas..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The doc said imageNumber counts only the slots that are not the canvas background. A page with several canvases names none of them as the background, so there every slot is counted. The labels were right; the comment was not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The editor half of this work is now released: bloom-ai-image-tools 0.1.6 opens its "Create an Image" tool when Bloom launches it on an empty picture box, puts the created image back into that box, and shows the per-slot labels this branch sends. Bloom pins the editor as an immutable dist- tag, so it keeps using the old build until the pin moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every empty picture box shows placeHolder.png, and the overlay found the clicked box by page and file name alone. On a page with two empty boxes that always found the first one, so a user who chose "Edit with AI..." on the second box got the first as the target, and the image they made landed there. Devin caught this. The page frame now counts how many same-named boxes come before the one the user clicked and sends that count along with the file name; C# carries it back to the overlay, which takes that one out of the same-named boxes on the page. Counting only the same-named boxes is what makes the count safe: the extra images Bloom injects into the live page, and the pictures C# leaves out of the book image list, carry other file names, so neither can shift it. A count that overshoots falls back to the first same-named box. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The count of same-named boxes ran over every picture on the live page, but the list it indexes into leaves out the branding, license, and QR-code slots. An empty one of those shows placeHolder.png like any other empty box, so a page that had one before the box the user clicked put the count one too high, the overlay fell back to the first empty box, and the image landed there. Devin caught this. The count now skips those three classes, mirroring IsUserChangeableImageElement, and skips the controls Bloom injects into the live page, which are in no saved book. Each side names the other in a comment, so the two lists stay together. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
[Claude Opus 5 from Hatton's machine during preflight] Consulted Devin on 2026-08-25, up to commit 5b116fe. It raised two bugs, both real, both about aiming at the wrong empty picture box on a page that has more than one. Each has its own thread here, and each is fixed and resolved:
Devin now marks both as fixed. Its one remaining Investigate flag was that Bloom still pinned an older AI image editor; the pin is now at CI ( |
This editor release makes the local dummy model draw the page label of the launched slot in a random high-contrast color, gives the launched book image a wider and brighter thumbnail border, and scrolls that thumbnail into view when the editor opens. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Accepting an image from the AI editor now registers with the image undo system, the same way Paste Image does. To keep that undo alive, the overlay no longer saves the page after a commit: a save reloads the page frame and discards the undo stack. Each "Edit with AI..." launch already saves first, so a later editor session still reads a fresh DOM. Dropping the save exposed a retry problem: when a commit partially fails and the user retries, the slot's src no longer matches the oldSrc the editor remembers. A WeakSet now marks each element the AI already swapped, and the matcher accepts the ordinal match on such an element despite the stale filename. The swap also makes its element the active canvas element, because the undo button stays disabled without one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The comment said package.json pins dist-v0.1.6, and the pin had already moved to dist-v0.1.7. The comment now names only the version where the behavior arrived, so a later pin bump cannot make it wrong again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
[Claude Fable 5 from Hatton's machine during preflight] Consulted Devin on 2026-08-25 up to commit ad7a92e. It raised one bug on this round — a comment naming the pinned editor tag dist-v0.1.6 after the pin moved to dist-v0.1.7 — fixed in ad7a92e, thread resolved: #8230 (comment). Its re-review of that commit marks all three of its bugs resolved; its one standing Investigate flag (the menu enabled before the editor pin moved) was answered earlier by the pin bump. Nine informational items, none needing action. CI (pr-automation) is green. |
…nto BL-16744-placeholder-edit-with-ai # Conflicts: # src/BloomBrowserUI/bookEdit/aiImageEditor/aiEditorOverlay.ts
hatton
left a comment
hatton
left a comment
There was a problem hiding this comment.
@hatton partially reviewed 10 files and made 3 comments.
Reviewable status: 0 of 16 files reviewed, 7 unresolved discussions.
DistFiles/localization/en/BloomMediumPriority.xlf line 53 at r5 (raw file):
<note>Context-menu item on an image in the Edit tab; opens the AI image editor in an overlay. The "..." indicates further UI will open, as in the sibling item "Choose image from your computer...".</note> </trans-unit> <trans-unit id="AiImageEditor.SlotLabel.Page" translate="no">
Do we already have "Page" and "Image"? then just reuse those and append the number. Don't worry that some language might prefer to change the order or something.
DistFiles/localization/en/BloomMediumPriority.xlf line 68 at r5 (raw file):
<note>Names one picture of a page that has several, shown over the picture in the AI image editor's strip of the book's images. {0} is replaced with the number of the picture on its page, counting from 1 and not counting the background picture. Combined with the page name by AiImageEditor.SlotLabel.PageAndSlot, giving e.g. "Page 3 - Image 2".</note> </trans-unit> <trans-unit id="AiImageEditor.SlotLabel.PageAndSlot" translate="no">
Let's not have this as a localizable thing. It's too confusing for the translator and not needed.
src/BloomBrowserUI/bookEdit/aiImageEditor/aiEditorPageCommands.ts line 60 at r5 (raw file):
// AiImageEditorApi.cs). An empty one of those shows placeHolder.png like any other empty slot, // so the count below has to skip them or it would run ahead of the list C# sent. const kNotUserChangeableClasses = ["branding", "licenseImage", "bloom-qrcode"];
Is this the best place to keep this list? If so, okay.
Next question on this: None of these show up in Bloom image containers, right? So if that's true, why do we need to even have this list? Could we instead limit the list of images to those that are inside of Bloom image containers?
hatton
left a comment
hatton
left a comment
There was a problem hiding this comment.
@hatton+JHAI made 3 comments and resolved 3 discussions.
Reviewable status: 0 of 16 files reviewed, 4 unresolved discussions.
DistFiles/localization/en/BloomMediumPriority.xlf line 53 at r5 (raw file):
Previously, hatton (John Hatton) wrote…
Do we already have "Page" and "Image"? then just reuse those and append the number. Don't worry that some language might prefer to change the order or something.
[Claude Opus 5 from Hatton's machine during reviewable-replies]
Yes, both existed. It now reuses ReaderSetup.PageHeader ("Page") and EditTab.CustomPage.Image ("Image") and appends the number. The three new trans-units are deleted; only AiImageEditor.SlotLabel.CanvasBackground remains.
DistFiles/localization/en/BloomMediumPriority.xlf line 68 at r5 (raw file):
Previously, hatton (John Hatton) wrote…
Let's not have this as a localizable thing. It's too confusing for the translator and not needed.
[Claude Opus 5 from Hatton's machine during reviewable-replies]
Done. AiImageEditor.SlotLabel.PageAndSlot is gone. The C# now joins with a literal " - ", so no translator ever sees a format string.
src/BloomBrowserUI/bookEdit/aiImageEditor/aiEditorPageCommands.ts line 60 at r5 (raw file):
Previously, hatton (John Hatton) wrote…
Is this the best place to keep this list? If so, okay.
Next question on this: None of these show up in Bloom image containers, right? So if that's true, why do we need to even have this list? Could we instead limit the list of images to those that are inside of Bloom image containers?
[Claude Opus 5 from Hatton's machine during reviewable-replies]
You are right, and the list is gone. Your second question led to a bigger simplification of the whole slot-numbering design, which John approved for this card:
- A slot is now simply a
bloom-imageContaineron the page. The ordered list of those containers is the numbering, and the index is the whole identity. - Branding, license and QR-code images sit outside an image container, so they are excluded by construction. No class list, no filename, no fallback.
aiEditorSlotMatching.tsand its test are deleted,sameNameOrdinalOnPageis deleted, andIAiImageEditorTargetis now{ pageId, slotIndex }.- C# numbers the slots the same way (
SelectImageSlotsOnPage), so the two ends agree without sharing anything but the index.
The only exclusion left on the live-page side is .bloom-ui, because Bloom injects temporary controls that the saved book does not have.
hatton
left a comment
hatton
left a comment
There was a problem hiding this comment.
@hatton+JHAI made 2 comments and resolved 2 discussions.
Reviewable status: 0 of 16 files reviewed, 2 unresolved discussions.
Review asked why a slot needed a file name and an ordinal among same-named
images, plus a list of classes to skip. It does not. A slot is now simply the
Nth bloom-imageContainer on the page, in document order, and that index is the
whole of its identity on both sides of the wire.
An image container is exactly what a user may replace, so the branding, license
and QR-code images, which live outside any container, are excluded by
construction rather than by name. C# numbers the containers in
SelectImageSlotsOnPage and the page frame numbers the same containers in
slotIndexOnPage, so the two lists agree without sharing anything but the index.
The ordinal stays positional: a slot C# declines to offer still holds its place.
Gone with it: aiEditorSlotMatching.ts and its test, sameNameOrdinalOnPage,
fileNameOf, the elementsAlreadySwappedByAi WeakSet, and
IsUserChangeableImageElement. IAiImageEditorTarget is now { pageId, slotIndex }.
The only exclusion left on the live-page side is .bloom-ui, for the controls
Bloom injects that no saved book has.
Also, per review: the slot label no longer uses a localizable "{0} - {1}"
joiner, which was confusing for translators. It reuses the existing "Page" and
"Image" strings and appends the number, so the three new trans-units are
removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
[Claude Opus 5 from Hatton's machine during preflight] Consulted Devin on 2026-08-26, up to commit It raised two things worth a thread on this round, both about the change that now identifies a picture box by its position on the page. Each has its own thread here, and I judged both to be non-issues and said why:
Devin now marks all three of its earlier bugs as fixed, and their threads were already resolved. Its one remaining Investigate flag, that the menu was enabled before the editor pin moved, is answered by the pin now sitting at CI ( |
1 participant
Problem
"Edit with AI..." was offered only on a picture box that already held an image, so a user who
wanted the AI to make a picture for an empty box could not start there. When Bloom did hand the
editor a book's pictures, they arrived as an unnamed list, so on a book with several pictures the
user could not tell which was which. And an image the user accepted could not be undone.
What the PR does
and back matter, "Page 3 - Canvas Background" for the picture behind a page, and
"Page 3 - Image 2" for the pictures on top of it. The ai-image-editor labels each slot with that
name.
is the box the editor works on and fills, even on a page holding several empty ones. That index
is the slot's whole identity, and C# and the page frame number the same containers.
the editor closes, because that save discarded the undo.
dist-v0.1.7, the release that keeps a placeholder out of "Image toEdit", opens the tool that makes an image from a description, and returns the image to the box
the user launched on.
Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16744
Devin review
This change is