Skip to content

Commit da92bd6

Browse files
zachelnetDeepSeek V4
andcommitted
fix(textbox-rotation): address PR review feedback
- Hoist ROTATE_CURSOR_MAP to module-level constant (avoid per-render allocs). - Use local string draft state for rotation number input so '-' and intermediate values can be typed (commit on blur/Enter). - Add unit tests for overlay_sprite_with_rotation (0°/90°/180°) and rotate_sprite_expand_top_left (0°/90°). - Add UI test for rotation plus button dispatching updateNode. - Add data-testid attributes on rotation +/- buttons. Co-authored-by: DeepSeek V4 <deepseek@v4.ai>
1 parent 501a075 commit da92bd6

4 files changed

Lines changed: 125 additions & 9 deletions

File tree

crates/koharu-app/src/renderer.rs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1472,4 +1472,83 @@ mod tests {
14721472
assert_eq!(transform.x, 150.0);
14731473
assert_eq!(transform.y, 125.0);
14741474
}
1475+
1476+
// ------------------------------------------------------------------
1477+
// Rotation helpers
1478+
// ------------------------------------------------------------------
1479+
1480+
fn make_test_sprite(w: u32, h: u32) -> RgbaImage {
1481+
let mut img = RgbaImage::new(w, h);
1482+
// Paint a solid red pixel so we can verify the output is not empty.
1483+
img.put_pixel(0, 0, Rgba([255, 0, 0, 255]));
1484+
img
1485+
}
1486+
1487+
fn make_transform(x: f32, y: f32, rotation_deg: f32) -> Transform {
1488+
Transform {
1489+
x,
1490+
y,
1491+
width: 100.0,
1492+
height: 50.0,
1493+
rotation_deg,
1494+
}
1495+
}
1496+
1497+
#[test]
1498+
fn overlay_sprite_no_rotation_is_identity() {
1499+
let mut canvas = RgbaImage::new(300, 300);
1500+
let sprite = make_test_sprite(20, 20);
1501+
let transform = make_transform(50.0, 50.0, 0.0);
1502+
overlay_sprite_with_rotation(&mut canvas, &sprite, &transform, false);
1503+
// An unrotated sprite at (50, 50) should leave a red pixel there.
1504+
assert_eq!(canvas.get_pixel(50, 50), &Rgba([255, 0, 0, 255]));
1505+
}
1506+
1507+
#[test]
1508+
fn overlay_sprite_90deg_rotates() {
1509+
let mut canvas = RgbaImage::new(600, 600);
1510+
// A large sprite in the center should stay visible after any rotation.
1511+
let sprite = make_test_sprite(100, 100);
1512+
let transform = make_transform(250.0, 250.0, 90.0);
1513+
// The function must not panic.
1514+
overlay_sprite_with_rotation(&mut canvas, &sprite, &transform, false);
1515+
}
1516+
1517+
#[test]
1518+
fn overlay_sprite_180deg_rotates() {
1519+
let mut canvas = RgbaImage::new(600, 600);
1520+
let sprite = make_test_sprite(100, 100);
1521+
let transform = make_transform(250.0, 250.0, 180.0);
1522+
overlay_sprite_with_rotation(&mut canvas, &sprite, &transform, false);
1523+
// Must not panic.
1524+
}
1525+
1526+
#[test]
1527+
fn rotate_sprite_expand_zero_angle_is_identity() {
1528+
let src = make_test_sprite(20, 10);
1529+
let (rotated, min_x, min_y) = rotate_sprite_expand_top_left(&src, 0.0);
1530+
assert_eq!(rotated.width(), src.width());
1531+
assert_eq!(rotated.height(), src.height());
1532+
assert_eq!(min_x, 0.0);
1533+
assert_eq!(min_y, 0.0);
1534+
// The red pixel at (0,0) should be preserved.
1535+
assert_eq!(rotated.get_pixel(0, 0), &Rgba([255, 0, 0, 255]));
1536+
}
1537+
1538+
#[test]
1539+
fn rotate_sprite_expand_90deg_swaps_dimensions() {
1540+
let src = make_test_sprite(40, 20);
1541+
let (rotated, _min_x, _min_y) = rotate_sprite_expand_top_left(&src, std::f32::consts::FRAC_PI_2);
1542+
// A 90° rotation swaps width and height, plus bilinear rounding = roughly 20×40.
1543+
assert!(
1544+
rotated.width() >= 20 && rotated.width() <= 22,
1545+
"expected ~20-22 wide, got {}",
1546+
rotated.width()
1547+
);
1548+
assert!(
1549+
rotated.height() >= 39 && rotated.height() <= 41,
1550+
"expected ~39-41 tall, got {}",
1551+
rotated.height()
1552+
);
1553+
}
14751554
}

ui/components/canvas/TextBlockLayer.tsx

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,16 @@ type BoxGeometry = {
2525
}
2626

2727
/** Rotate a CSS resize-cursor name by the given degrees (snapped to 45° steps). */
28+
const ROTATE_CURSOR_MAP: Record<string, readonly string[]> = {
29+
'ns-resize': ['ns-resize', 'nesw-resize', 'ew-resize', 'nwse-resize', 'ns-resize', 'nesw-resize', 'ew-resize', 'nwse-resize'],
30+
'ew-resize': ['ew-resize', 'nwse-resize', 'ns-resize', 'nesw-resize', 'ew-resize', 'nwse-resize', 'ns-resize', 'nesw-resize'],
31+
'nwse-resize': ['nwse-resize', 'ns-resize', 'nesw-resize', 'ew-resize', 'nwse-resize', 'ns-resize', 'nesw-resize', 'ew-resize'],
32+
'nesw-resize': ['nesw-resize', 'ew-resize', 'nwse-resize', 'ns-resize', 'nesw-resize', 'ew-resize', 'nwse-resize', 'ns-resize'],
33+
}
34+
2835
const rotateCursor = (cursor: string, deg: number): string => {
2936
const steps = Math.round(((deg % 360) + 360) % 360 / 45) % 8
30-
const map: Record<string, string[]> = {
31-
'ns-resize': ['ns-resize', 'nesw-resize', 'ew-resize', 'nwse-resize', 'ns-resize', 'nesw-resize', 'ew-resize', 'nwse-resize'],
32-
'ew-resize': ['ew-resize', 'nwse-resize', 'ns-resize', 'nesw-resize', 'ew-resize', 'nwse-resize', 'ns-resize', 'nesw-resize'],
33-
'nwse-resize': ['nwse-resize', 'ns-resize', 'nesw-resize', 'ew-resize', 'nwse-resize', 'ns-resize', 'nesw-resize', 'ew-resize'],
34-
'nesw-resize': ['nesw-resize', 'ew-resize', 'nwse-resize', 'ns-resize', 'nesw-resize', 'ew-resize', 'nwse-resize', 'ns-resize'],
35-
}
36-
return map[cursor]?.[steps] ?? cursor
37+
return ROTATE_CURSOR_MAP[cursor]?.[steps] ?? cursor
3738
}
3839

3940
/**

ui/components/panels/RenderControlsPanel.tsx

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,16 @@ export function RenderControlsPanel() {
364364
queueAutoRender(page.id)
365365
}
366366

367+
// Local draft state for the rotation number input so that intermediate
368+
// values like "-" (needed to type negative angles) are not discarded by
369+
// the controlled-input cycle.
370+
const [rotationDraft, setRotationDraft] = useState<string | null>(null)
371+
372+
// Sync the draft when selected node or its rotation changes externally.
373+
useEffect(() => {
374+
setRotationDraft(null)
375+
}, [selectedNode?.id, selectedNode?.transform.rotationDeg])
376+
367377
const effectItems: {
368378
key: 'italic' | 'bold'
369379
label: string
@@ -799,6 +809,7 @@ export function RenderControlsPanel() {
799809
variant='ghost'
800810
size='icon-sm'
801811
className='size-7 shrink-0 rounded-r-none border-r'
812+
data-testid='render-rotation-minus'
802813
disabled={!selectedNode}
803814
onClick={() =>
804815
void updateSelectedRotation((selectedNode?.transform.rotationDeg ?? 0) - 1)
@@ -836,12 +847,22 @@ export function RenderControlsPanel() {
836847
className='h-7 w-14 min-w-0 [appearance:textfield] rounded-none border-0 px-1 text-center text-xs shadow-none focus-visible:ring-0 [&::-webkit-inner-spin-button]:appearance-none [&::-webkit-outer-spin-button]:appearance-none'
837848
data-testid='render-rotation-input'
838849
disabled={!selectedNode}
839-
value={selectedNode?.transform.rotationDeg ?? 0}
850+
value={rotationDraft ?? String(selectedNode?.transform.rotationDeg ?? 0)}
840851
onChange={(event) => {
841-
const parsed = Number.parseFloat(event.target.value)
852+
setRotationDraft(event.target.value)
853+
}}
854+
onBlur={() => {
855+
if (rotationDraft == null) return
856+
const parsed = Number.parseFloat(rotationDraft)
857+
setRotationDraft(null)
842858
if (!Number.isFinite(parsed)) return
843859
void updateSelectedRotation(parsed)
844860
}}
861+
onKeyDown={(event) => {
862+
if (event.key === 'Enter') {
863+
(event.target as HTMLInputElement).blur()
864+
}
865+
}}
845866
/>
846867
<span className='flex h-7 w-5 items-center justify-center text-[10px] text-muted-foreground'>
847868
°
@@ -851,6 +872,7 @@ export function RenderControlsPanel() {
851872
variant='ghost'
852873
size='icon-sm'
853874
className='size-7 shrink-0 rounded-l-none border-l'
875+
data-testid='render-rotation-plus'
854876
disabled={!selectedNode}
855877
onClick={() =>
856878
void updateSelectedRotation((selectedNode?.transform.rotationDeg ?? 0) + 1)

ui/tests/components/RenderControlsPanel.test.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,4 +199,18 @@ describe('RenderControlsPanel Font Assignment', () => {
199199
expect(op.updateNode.id).toBe('t1')
200200
expect(op.updateNode.patch.data.text.style.color).toEqual([0, 0, 0, 255])
201201
})
202+
203+
it('changing rotation via the + button dispatches updateNode with rotationDeg updated', async () => {
204+
renderWithQuery(<RenderControlsPanel />)
205+
useSelectionStore.getState().select('t1', false)
206+
207+
const plusBtn = await screen.findByTestId('render-rotation-plus')
208+
await userEvent.click(plusBtn)
209+
210+
await waitFor(() => expect(sceneActions.applyOp).toHaveBeenCalled())
211+
const op = (sceneActions.applyOp as any).mock.calls[0][0]
212+
expect(op.updateNode.id).toBe('t1')
213+
expect(op.updateNode.patch.transform.rotationDeg).toBe(1)
214+
expect(sceneActions.queueAutoRender).toHaveBeenCalled()
215+
})
202216
})

0 commit comments

Comments
 (0)