Rotate and flip an image (BL-16741) - #8227
Conversation
Add a "Rotate right" command (Ctrl+R) and a "Flip" command with horizontal and vertical submenu items to the image menu, and a rotation handle on the control frame of a canvas element. What turns depends on what the image is: - An ordinary canvas element turns as a whole. The angle is an inline `rotate()` transform on the element, which is saved with the book, so the reader and the PDF show it without any Bloom JavaScript. - A background image fills its bloom-canvas and cannot turn as a box, so the picture inside it turns and is shrunk to fit. Flip always works on the picture, because mirroring a box would only move it. The rotation handle is a round knob on a stem above the top edge of the frame, holding the Material UI Refresh icon. Dragging it snaps the angle to every multiple of 45 degrees, within 14 degrees each side; CTRL turns the snapping off, as it does for position snapping. Elements whose outline comicaljs draws (speech and thought bubbles, captions, rectangles, ellipses) get no handle, because that drawn shape stays upright. Rotation also made the existing pointer code rotation-aware: hit testing, the grab offset at the start of a drag, the un-rotation of drag deltas, and the correction that keeps the anchored edge still during a resize. The image menu is now three sections, so the menu shows a line between them, and "Reset Image" is last in the section it undoes. Reset Image now also clears a turn or a mirror and puts the transparency back to "Auto", and it is enabled whenever any of those three things needs undoing. New localizable strings: EditTab.Image.RotateRight, EditTab.Image.Flip, EditTab.Image.FlipHorizontal, EditTab.Image.FlipVertical and EditTab.Toolbox.ComicTool.Handle.Rotate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The page's image undo stack knew only how to restore a replaced picture. Add a second kind of record to it, holding the rotation of the canvas element box, the transform of the img, and the crop. Rotate right and Flip push one of these before they change anything, so the Undo button reaches them through the path it already uses for the other image commands. The crop is part of the record because Rotate right on a background image drops the crop: the cropped region described the picture as it was before the turn. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A drag of the rotation handle now records one undo step, so the Undo button puts the element back to the angle it had before the drag. The record is pushed at the end of the drag, and only when the angle really changed, so a click on the handle leaves nothing for Undo to do. The undo record no longer requires a picture, because the handle also turns text boxes, videos and buttons. For that kind of record the "can undo" test asks instead that the record's own element is the selected one, in place of the old test for an image container, which no text box could pass. Also move setImageTransparencyToAuto below pageBackgroundNeedsTransparency, so that function keeps the comment that describes it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
[Claude Opus 5 from Hatton's machine during preflight] Consulted Devin on 2026-08-23 22:56 UTC up to commit 379ee06. The review job reached |
… item The control frame lies over the canvas element and copies its transform, so everything in the frame turns with the element. Three things did not survive that. The mark that shows which side of a picture is cropped came from comparing getBoundingClientRect of the picture with that of the element. For a turned element that rectangle is the upright box around the turned element, which is larger than the element, so the mark appeared on sides that were not cropped. updateCurrentlyCropped now adds up the offsets from the picture to the element and compares them with clientWidth and clientHeight, as adjustMoveCropHandleVisibility already does. Both are inside the same rotation, so the numbers compare at any angle. The tooltip of each handle is a CSS pseudo-element inside the frame, so its text lay at the angle of the element. The frame now carries the angle as the custom property --canvas-element-rotation, and the tooltip rule turns the text back by the same angle. A cursor cannot turn, so each handle went on showing the cursor of the side it started on. setHandleCursorsForRotation now gives each handle the cursor for the direction it really moves on screen: one of the eight directional cursors for a corner, one of the four axis cursors for a side, taken to the nearest eighth of a turn. Also adds the first unit tests for canvasElementRotation, for imageContentTransform, and for the rotation snapping in CanvasSnapProvider. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Three defects Devin found in the previous commit. Flip named the axis the user sees, but worked it out from the turn of the picture alone. On a canvas element whose box had been turned a quarter turn, Flip horizontal therefore mirrored the picture from top to bottom. flipImageContent now takes the angle of the box as well, and adds the two turns before it decides which axis of the picture to mirror. The angle of the box is taken to the nearest quarter turn, because no mirror of the picture's own axes equals a mirror about the screen axis at an angle in between. The Rotate Right menu item is offered only for a real picture, but Ctrl+R was not. It turned a box that still held the grey placeholder and put a step on the undo stack for it. rotateSelectedImageRight now asks for a real picture too. The table of cursors for a turned handle had the two diagonal axis cursors the wrong way round, so a side handle at 45 degrees showed the diagonal at right angles to the one it moves along. The table now lives in canvasElementRotation.ts beside the rest of the angle arithmetic, with the comment that explains the order, and six tests cover it. Also records in a comment why Reset Image deliberately puts nothing on the undo stack. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
[Claude Opus 5 from Hatton's machine during preflight] Consulted Devin on this pull request, up to Devin raised two bugs and five things to investigate, plus two informational notes that need no action. Each of the seven is now a review thread above, with what came of it:
Devin has been re-triggered for |
AddEditKeyHandlers runs again every time a canvas element is added, because SetupElements does, and two of its handlers listen on the whole document. Each call therefore bound one more copy of them, and one key press ran every copy. That did no harm while every command in those handlers gave the same result however often it ran, but Rotate right does not: on a page where three canvas elements had been added, one Ctrl+R turned the picture three quarter turns and left three steps on the undo stack. The two document-level handlers now live in their own function, which marks the document object the first time it runs and does nothing afterwards. The mark goes on the document object rather than into the page, so a page the user moves to gets its own handlers and nothing is written into the book. Found by Devin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
[Claude Opus 5 from Hatton's machine during preflight] Consulted Devin again, up to It raised one new bug on On One note in the last round is new and is a real question rather than a defect: a turn cannot be undone once the selection moves away. That thread is open, and the question is on the preflight report for Hatton. |
A quarter turn of a page background picture used to drop the crop and shrink the picture inside a page-shaped box, so an author who had cropped a picture lost that framing the moment they turned it, and an author who turned an uncropped picture got it small in the middle of the page. Both came from putting a fit factor into the picture's own transform while leaving the picture's canvas element the shape it already was. Bloom shapes a background element to the picture and centres it in the page, so a turn has to reshape that element in the same way. Then one formula covers every case: turn the whole content of the element, which swaps the element's two dimensions, and scale it back to the area it has to land in. - computeTurnedBackgroundLayout does that arithmetic and is exported, so the tests pin it down on its own. - A turn now gives what the author would have had if the picture had arrived already turned: the whole picture upright, filling the page for a picture that is the page's shape, and any crop kept at the same framing. - A picture the author told to fill the page (Expand Image) keeps filling it, with its ends clipped rather than blank bands. - getFitScaleForQuarterTurn and removeCropping are gone. The inline transform now carries the turn and the mirrors only. - The element the turn reaches is not always the background: Rotate Right also turns the picture inside an element whose outline comicaljs draws. Such an element keeps its own size and place, and turns about its own centre. - Undo of a turn puts the element's size and place back as well. - Expand Image works on a turned picture, and the mark that shows which side of a picture is cropped counts the turn, so it marks two sides rather than four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
[Claude Opus 5 from Hatton's machine during preflight] Consulted Devin on this PR up to Devin re-read the whole PR after the change that makes Rotate right keep a crop. It raised no new Its five Investigate flags are the same five as before, each with its own thread above: four are CI is green. Greptile posted only a notice that its trial has ended, so it reviewed nothing. |
Both crop drags measured the picture's own layout box. That box is the right answer only while the picture is upright. A quarter turn of the picture is a CSS transform about the box's own centre, so what the element shows is a rectangle with the box's two dimensions swapped, in the same place. The two disagree by half the difference of the dimensions, which is enough to break both drags on a page background the author had turned: - Dragging the picture about inside the crop compared the box against the element, so the lower limit crossed the upper one and every mouse move gave the same position back. The picture did not follow the pointer at all. - Dragging a crop handle stopped the edge at the box's own edge, so the author could not bring back part of the picture that was there, or could drag a blank band in past the end of it. getShownContentRectangle now works out that rectangle from the box and the number of quarter turns, and clampCropPosition keeps a picture covering its element in that rectangle's frame and carries the result back into the box's frame, which is what we write. The four limits in continueSideDrag measure the same rectangle, and the side drag records the picture's size at the start of the drag, which it needs to do so. getCroppedSides, which puts the marks on the handles, now shares the one piece of arithmetic. For a picture that is upright, or turned a half turn, the rectangle is the box, so every number these produce is the number the old code produced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ctrl+R is Bloom's right-justify shortcut. Rotate right shared it, on the grounds that the two can never apply at once, and the shortcut now goes back to right justify alone. Rotate right stays on the picture menu, which is where it started. So the menu item no longer shows a shortcut, the translator note for it no longer names one, and the two helpers the key handler needed are gone. The guard that binds the document key handlers once stays. Binding one more copy of every one of those handlers for each canvas element added to the page is wrong on its own account, whether or not a command in them counts its calls. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
[Claude Opus 5 from Hatton's machine during preflight] Consulted Devin on this PR up to Devin re-read the whole PR at this commit and raised nothing new. It now marks all three of its Two of the three Bugs concerned the Ctrl+R shortcut. That shortcut is gone as of this commit, so |
Four fixes for a canvas element the user has turned. A crop drag no longer walks the element across the page. The rotation correction is applied once after the ordinary arithmetic instead of being read back out of the element, which fed each correction into the next. A turned element no longer creeps up the page on every page load. The check that keeps elements inside the page now reads the element's own laid-out position rather than the upright box the browser reports around a turned element. The rotation handle moves to the element's other edge only past three eighths of a turn. At a quarter turn the handle points out to the side, where the panel of controls does not reach it, so switching there moved the handle for no gain, at the angle a user stops at most often. A turned video keeps its play, pause and replay buttons. The element's own transform makes it a stacking context, so the z-index its contents use to get above the comicaljs canvas no longer reaches past the element; the canvas covers the element and takes the pointer, and the browser gives the element no :hover. Bloom now marks the turned element the pointer is really inside, using the same rotation-aware hit test that a click uses, and the CSS accepts that mark in place of :hover. A click on one of the three buttons is found by looking through everything under the pointer, limited to the element the user pressed on, so a button that something else covers is never clicked. The mark is cleared when the element goes upright, and stripped before the page is saved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
| Filename | Overview |
|---|---|
| src/BloomBrowserUI/bookEdit/js/imageContentTransform.ts | Adds parsing, persistence, flipping, and quarter-turn layout calculations for image content, including background-image geometry. |
| src/BloomBrowserUI/bookEdit/js/canvasElementManager/canvasElementRotation.ts | Adds canonical canvas-element rotation, coordinate conversion, hit testing, cursor selection, and rotation eligibility. |
| src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementHandleDragInteractions.ts | Makes crop and resize interactions rotation-aware and adds rotation-handle dragging; one new nullable traversal violates the frontend undefined convention. |
| src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementPointerInteractions.ts | Adds rotation-aware selection, hover tracking, dragging, and forwarding of video-control clicks. |
| src/BloomBrowserUI/bookEdit/js/canvasElementManager/CanvasElementManager.ts | Integrates rotate, flip, reset, undo, and transformed-background sizing behavior into canvas commands. |
| src/BloomBrowserUI/bookEdit/js/ImageUndoManager.ts | Extends image undo records to restore image transforms, canvas rotation, crop values, and element geometry. |
| src/BloomBrowserUI/bookEdit/toolbox/canvas/canvasControlRegistry.ts | Adds Rotate right and Flip menu commands, expands Reset Image, and reorganizes image controls. |
| src/BloomBrowserUI/bookEdit/css/editMode.less | Styles the rotation handle and rotation-aware tooltips and video hover states. |
| DistFiles/localization/en/BloomMediumPriority.xlf | Adds intentionally untranslated-in-development English entries for the new rotation and flip controls. |
Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/mas..." | Re-trigger Greptile
|
[Claude Opus 5 from Hatton's machine during preflight] Consulted Devin on this PR up to Devin re-read the whole PR at this commit and raised nothing new. It marks all three of its
Nothing Devin says touches the newest work in this commit: the rotation knob moving to the other |
1 participant
Problem
A picture that arrives on its side stays on its side. Bloom could crop a picture and move it,
but it could not turn it, mirror it, or set an item on a page at an angle. An author whose
camera saved a photograph sideways had to leave Bloom, turn the file in another program, and
put it back.
Fix
picture turns as a box; a background picture, which fills its page area and cannot turn as a
box, has the picture inside it turned and its area reshaped to match. The result is what the
author would have had if the picture had arrived already turned: upright, at the same framing,
with any crop kept. A picture told to fill the page still fills it, ends clipped rather than
blank bands. Neither path acts on an empty placeholder.
whichever way the picture and its box have been turned.
edge, snapping to every 45 degrees within 14 degrees of one, with CTRL to turn snapping off.
Past 135 degrees the knob hangs from the item's other edge, because the top edge then points
down the screen and the knob would sit behind the panel of controls; it does not move mid-drag.
An item whose outline comicaljs draws (a speech bubble, caption, rectangle or ellipse) gets no
handle, because that outline stays upright.
item's own coordinates, and the item is shifted once so that the edge opposite the dragged one
stays where it looks. A turned item also stays put across page loads and through a whole crop
drag, because every check now reads the item's own laid-out box rather than the upright box the
browser reports around it. A click picks the right item, which comicaljs alone cannot do. The
control frame turns with the item: each handle shows the cursor for the direction it really
moves, each tooltip stays level, and the crop marks count both turns, so they mark the two
sides that are really hidden.
so the z-index its contents use to clear the comicaljs canvas no longer reaches past the item.
The canvas then covers it and takes the pointer, and the browser gives it no
:hover, which iswhy a turned video lost its play button. Bloom now marks the turned item the pointer is really
inside, using the same rotation-aware hit test a click uses, and the CSS accepts that mark in
place of
:hover. A click on the play, pause or replay button is found among everything underthe pointer, limited to the item pressed on, so a covered button is never clicked. The mark goes
when the item is upright again, and before the page is saved.
Image now clears the crop, the picture's turn and mirror, and a transparency chosen by hand.
It does not straighten a turned box, because that angle belongs to the box as its size and
position do; the handle and Undo are the way back. The picture menu is now three groups: which
picture is in the box, how it sits in the box, and the rest.
Every angle and mirror lives in the item's own inline
transform, saved in the book HTML, so itappears in BloomPlayer and in PDF output, where no Bloom JavaScript runs.
Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16741
Devin review
This change is