BL-16822 Inline images: the gestures, the toolbar, and Add Image in the text menu

[hatton]

What a person actually does to a picture in a text block. Right-click text and choose
Add Image; drag the picture to any of the four docks or down through the lines of the
block; drag a corner handle to resize it; use the toolbar it carries, which is the
canvas element menu (rendered through the module the first commit in this stack
extracted). The offered target excludes editables inside a canvas element, editables
that are not direct children of a translation group, and data-book and xmatter
fields, where the field's markup goes through the data div and a wrapper would end up
in the stored book title.

The two units the drag has to reconcile are worth knowing: getBoundingClientRect and
a pointer event's clientX/clientY are viewport pixels, while clientHeight, scrollTop
and the offset custom property are layout pixels. Bloom draws the edit view's zoom by
scaling a container around the page, so the ratio between them IS the zoom, and
computeViewportPxPerLayoutPx measures it once at pointerdown.

Two more places had to learn about the feature:

• removeEditingDebris clears bloom-inlineImage-selected, which is editing UI but not
  a bloom-ui class, so nothing else would strip it on the way to a save; from the
  book's HTML it would reach spreadsheet exports and published books.
• ckeditor's undo command is intercepted when an inline image is the active thing.
  Nothing in Bloom binds ctrl+z, so the key reaches ckeditor's undo plugin, which
  restores the saved HTML of ONE editable -- putting the focused language's copy back
  and leaving the others at the geometry just undone. Ordinary typing keeps its
  ctrl+z: the gate is the same one the top-bar Undo button consults, and typing after
  a deletion hands the key back.

Card: https://issues.bloomlibrary.org/youtrack/issue/BL-16822

---

🤖 Generated with Claude Code

https://claude.ai/code/session_014DCBGajN5YyAYPBenEf1yy

Devin review

---

This change is 

[hatton]

✅ Handled — see the outcome at the end of this comment.

[Devin] Investigate: Localization priority needs confirmation

DistFiles/localization/en/Bloom.xlf:2042 -- outside the diff, so this is not resolvable as a thread.

The new command uses the highest-priority catalog. Confirm that this feature-specific menu action belongs there instead of the medium-priority catalog.

---

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Fair question, and now answered: the developer chose medium priority, so the entry has moved to BloomMediumPriority.xlf. Nothing was lost in the move -- it is a new entry marked translate="no", so it had no translations, and it keeps both of its notes. It now sits beside EditTab.TextContextMenu.NoIndent, the other command on that same right-click menu.

[hatton]

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Consulted Devin on 2026-09-07 12:46 MDT up to commit aec5849cf2.

It raised 5 things. Each has its own review thread with the reasoning and the outcome; in short:

• Partial setup erases image undo — real: adding a text box over a picture made the picture's last move un-undoable. Fixed.
• Width-only layouts skip image refitting — real: splitting a text box in Change Layout left the text after the picture running off the end of the block, permanently. Fixed.
• Toolbar retains discarded image targets — real: after Delete the bar of buttons stayed on screen offering commands for a picture that was gone. Fixed.
• Localization priority needs confirmation — a developer call, now made: the new string sits in BloomMediumPriority.xlf.
• Feature status crosses page lifetimes — not an issue; it is a collection-wide flag, so the value cannot differ between pages.

The fixes were amended into this branch after the review, so the commit above is not the current head; a fresh review is running against the new one. The only GitHub check on this PR is pr-automation (the review trigger), which passed; the front-end unit suite (915 tests), the C# spreadsheet tests (415) and the end-to-end suite (85) were all run locally and are green.

[hatton]

✅ Assessed, no change needed — see the outcome at the end of this comment.

[Devin] Investigate: Shared toolbar lacks effect justification

src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementContextControls.tsx:180 -- outside the diff, so this is not resolvable as a thread.

Inline images now exercise CanvasElementContextControls outside its original context. Its bare effects lack the repository-required justification for using effects.

---

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Both useEffects in CanvasElementContextControls already carry comments saying why they are needed, and neither is touched by this stack -- they are byte-for-byte what master has. The only change this PR makes to that file is the afterToolbarCommand prop and its comment. So there is nothing here to justify that is not justified already.

[hatton]

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Consulted Devin again on 2026-09-07 14:00 MDT, on commit 1411da9d68 (the head after the first round of fixes).

It raised 5 new things. Each has its own review thread with the reasoning and the outcome; in short:

• Dragging outside the editor never ends — real: a button released over another document or outside the window never ended the gesture. Fixed with a pointer capture, with a test.
• Undo strands the image toolbar — real: the toolbar went on offering commands for a wrapper undo had replaced. Fixed, with two tests.
• Undo stops after restoring deletion — the same bug as the undo gate one on PR 8320, fixed there; the toolbar half is fixed here.
• Test relies on a prohibited timeout — right, and against the repo's rule; that setTimeout is now a wait on the state the assertion is about.
• Shared toolbar lacks effect justification — not an issue: both effects already carry justification comments and are unchanged from master.

It also re-reported the findings the reviewed commit already fixes; those are the first round's threads, which carry their outcomes.

These fixes were amended into this branch after the review, so the commit above is not the current head. The only GitHub check on this PR is pr-automation (the review trigger). Run locally and green after these fixes: the front-end unit suite (920 tests), the end-to-end suite (85 passed, 1 skipped), and the C# spreadsheet and translation-group tests (417 and 50).

[hatton]

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Consulted Devin a third time on 2026-09-07 15:55 MDT, on commit daf1f21d76 (the head after the second round of fixes).

It raised 5 new things. Each has its own review thread with the reasoning and the outcome; in short:

• Transparency changes skip image undo — real: the commands come from the canvas-element registry, which knows nothing about this undo layer, so ctrl+z after making a picture transparent removed the picture. Fixed, with a test.
• Text menu leaves image undo active — real: a right-click in the text left a picture selected, which says the commands apply to it and keeps ctrl+z on the wrong layer. Fixed, with a test.
• Second touch corrupts active gesture — real: the handlers never compared the pointer id, so a second finger (or a brush of a trackpad) moved the picture and ended the drag. Fixed, with a test.
• Localization indentation breaks repository check — there is no such check: .gitattributes makes git diff --check complain, but nothing enforces it, and every existing line of those files is the same.
• Page setup rewrites saved geometry — intended: an offset is an absolute distance, so a block that changed size has to be re-measured, and there is no user action to undo. The reasoning is on the thread.

It also re-reported the findings the reviewed commit already fixes; those are the earlier rounds' threads, which carry their outcomes.

These fixes were amended into this branch after the review, so the commit above is not the current head. The only GitHub check on this PR is pr-automation (the review trigger). Run locally and green after these fixes: the front-end unit suite (927 tests), the C# spreadsheet and translation-group tests (470), and the end-to-end suite.

[hatton]

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Consulted Devin a fourth time on 2026-09-07 16:20 MDT, on commit 7de94f8341 (the head after the third round of fixes).

It raised 2 new things. Each has its own review thread with the reasoning and the outcome; in short:

• Text editing permanently blocks image undo — the same bug as the first one on PR 8320, seen from this side, and fixed there.
• Resize handles persist in saved books — they do not: the frame and its handles are bloom-ui, and HtmlDom.ProcessPageAfterEditing strips every .bloom-ui element on the way to disk. The reasoning is on the thread.

It also re-reported the findings the reviewed commit already fixes; those are the earlier rounds' threads, which carry their outcomes.

These fixes were amended into this branch after the review, so the commit above is not the current head. The only GitHub check on this PR is pr-automation (the review trigger). Run locally and green after these fixes: the inline-image unit tests (47), the C# spreadsheet tests (422), and the nine end-to-end specs the cleanup touched.

[hatton]

[Devin] Bug: Older text history blocks image undo

src/BloomBrowserUI/bookEdit/js/inlineImages.ts:732 -- outside the diff, so this is not resolvable as a thread.

After newer text edits are undone, ckeditorHasSomethingToUndo treats older text history as newer than the image operation. Undo then removes earlier text while leaving the image change intact.

[hatton]

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Consulted Devin a fifth time on 2026-09-07 16:50 MDT, on commit dbb6f90705 (the head after the fourth round of fixes).

It raised 1 new thing. Each has its own review thread with the reasoning and the outcome; in short:

• Older text history blocks image undo — the same bug as on PR 8320, fixed there. One test in this PR had to say out loud what it always assumed (that CKEditor is holding the typing it reports), which is how I found that my previous round left it red — I had re-run only the model's own test file. The whole front-end suite is green now.

It also re-reported the findings the reviewed commit already fixes; those are the earlier rounds' threads, which carry their outcomes.

These fixes were amended into this branch after the review, so the commit above is not the current head. The only GitHub check on this PR is pr-automation (the review trigger). Run locally and green after these fixes: the whole front-end unit suite, the C# spreadsheet tests (423), and the inline-image end-to-end specs.

[hatton]

[Devin] Bug: Full text history breaks undo ordering

src/BloomBrowserUI/bookEdit/js/inlineImages.ts:774 -- outside the diff, so this is not resolvable as a thread.

Once the 20-entry history is full, index remains 19 when newer snapshots evict older ones. The bundled UndoManager.save shifts the oldest snapshot before reusing that final index. Undo then restores the image before later typing.

[hatton]

[Claude Opus 5 (1M context) from Hatton's machine during devin-review]

Consulted Devin a sixth time on 2026-09-07 17:30 MDT, on commit 27c255d19f (the head after the fifth round of fixes).

It raised 1 new thing. Each has its own review thread with the reasoning and the outcome; in short:

• Full text history breaks undo ordering — the same bug as on PR 8320, fixed there.

It also re-reported the findings the reviewed commit already fixes; those are the earlier rounds' threads, which carry their outcomes.

These fixes were amended into this branch after the review, so the commit above is not the current head. The only GitHub check on this PR is pr-automation (the review trigger). Run locally and green after these fixes: the whole front-end unit suite, the C# spreadsheet tests (424), and the inline-image end-to-end specs.