fix: address code review findings from crop/text commit

- CropOverlay: register move/up handlers dynamically on drag start (single
  handler instead of 8x per-handle), remove on drag end. Override destroy()
  to call _cleanup() preventing keyboard listener leaks.
- Text format toolbar: re-measure text bounds (w/h) after fontSize/fontFamily
  changes, update spatial index and transform box.
- TextEditor: add clearText() method; tools.ts uses it instead of fragile
  document.querySelector('textarea').
- SceneManager._updateItem: apply crop mask on remote sync for image items.
- useCanvasSetup: stop TextEditor on unmount to prevent orphaned textarea.
- Double-click zoom: use item.data dimensions instead of getBounds() (which
  includes shadow offset).
This commit is contained in:
Hiren Kangad
2026-03-11 17:28:52 +05:30
parent 58da934ec1
commit 01f22c9d43
6 changed files with 72 additions and 21 deletions
+39 -14
View File
@@ -10,7 +10,6 @@ import { Container, Graphics, FederatedPointerEvent } from 'pixi.js';
import type { Viewport } from 'pixi-viewport'; import type { Viewport } from 'pixi-viewport';
import type { SceneItem } from './SceneManager'; import type { SceneItem } from './SceneManager';
import type { ImageObject, CropRect } from './scene-format'; import type { ImageObject, CropRect } from './scene-format';
import { ImageSprite } from './sprites/ImageSprite';
const HANDLE_SIZE = 8; const HANDLE_SIZE = 8;
const HANDLE_FILL = 0xffffff; const HANDLE_FILL = 0xffffff;
@@ -32,6 +31,9 @@ export class CropOverlay extends Container {
private _onConfirm: ((item: SceneItem, crop: CropRect) => void) | null = null; private _onConfirm: ((item: SceneItem, crop: CropRect) => void) | null = null;
private _onCancel: (() => void) | null = null; private _onCancel: (() => void) | null = null;
private _keyHandler: ((e: KeyboardEvent) => void) | null = null; private _keyHandler: ((e: KeyboardEvent) => void) | null = null;
// Bound references for dynamic event registration
private _boundMove: ((e: FederatedPointerEvent) => void) | null = null;
private _boundUp: (() => void) | null = null;
constructor(viewport: Viewport) { constructor(viewport: Viewport) {
super(); super();
@@ -50,7 +52,7 @@ export class CropOverlay extends Container {
this._border.eventMode = 'none'; this._border.eventMode = 'none';
this.addChild(this._border); this.addChild(this._border);
// Create handles // Create handles — only pointerdown on each handle
const ids: HandleId[] = ['tl', 'tc', 'tr', 'ml', 'mr', 'bl', 'bc', 'br']; const ids: HandleId[] = ['tl', 'tc', 'tr', 'ml', 'mr', 'bl', 'bc', 'br'];
const cursors: Record<HandleId, string> = { const cursors: Record<HandleId, string> = {
tl: 'nwse-resize', tr: 'nesw-resize', bl: 'nesw-resize', br: 'nwse-resize', tl: 'nwse-resize', tr: 'nesw-resize', bl: 'nesw-resize', br: 'nwse-resize',
@@ -61,9 +63,6 @@ export class CropOverlay extends Container {
h.eventMode = 'static'; h.eventMode = 'static';
h.cursor = cursors[id]; h.cursor = cursors[id];
h.on('pointerdown', (e: FederatedPointerEvent) => this._onDown(e, id)); h.on('pointerdown', (e: FederatedPointerEvent) => this._onDown(e, id));
h.on('globalpointermove', (e: FederatedPointerEvent) => this._onMove(e));
h.on('pointerup', () => this._onUp());
h.on('pointerupoutside', () => this._onUp());
this._handles.set(id, h); this._handles.set(id, h);
this.addChild(h); this.addChild(h);
} }
@@ -94,7 +93,6 @@ export class CropOverlay extends Container {
if (!this._item) return; if (!this._item) return;
const item = this._item; const item = this._item;
const crop = { ...this._crop }; const crop = { ...this._crop };
// If crop is full image, remove it
const isFullImage = crop.x < 0.001 && crop.y < 0.001 && crop.w > 0.999 && crop.h > 0.999; const isFullImage = crop.x < 0.001 && crop.y < 0.001 && crop.w > 0.999 && crop.h > 0.999;
this._cleanup(); this._cleanup();
this._onConfirm?.(item, isFullImage ? { x: 0, y: 0, w: 1, h: 1 } : crop); this._onConfirm?.(item, isFullImage ? { x: 0, y: 0, w: 1, h: 1 } : crop);
@@ -108,7 +106,9 @@ export class CropOverlay extends Container {
get isActive(): boolean { return this._item !== null; } get isActive(): boolean { return this._item !== null; }
/** Clean up all state and listeners. Safe to call multiple times. */
private _cleanup(): void { private _cleanup(): void {
this._removeDragListeners();
this._item = null; this._item = null;
this._drag = null; this._drag = null;
this.visible = false; this.visible = false;
@@ -118,6 +118,12 @@ export class CropOverlay extends Container {
} }
} }
/** Override destroy to ensure cleanup runs. */
override destroy(options?: any): void {
this._cleanup();
super.destroy(options);
}
private _draw(): void { private _draw(): void {
if (!this._item) return; if (!this._item) return;
const data = this._item.data; const data = this._item.data;
@@ -135,26 +141,21 @@ export class CropOverlay extends Container {
const cw = this._crop.w * iw; const cw = this._crop.w * iw;
const ch = this._crop.h * ih; const ch = this._crop.h * ih;
// Dim: draw full image rect, then cut out the crop area using inverted mask approach // Dim: draw 4 rectangles around the crop area
// Simpler: draw 4 dim rectangles around the crop area
this._dim.clear(); this._dim.clear();
// Top
if (cy > iy) { if (cy > iy) {
this._dim.rect(ix, iy, iw, cy - iy); this._dim.rect(ix, iy, iw, cy - iy);
this._dim.fill({ color: 0x000000, alpha: DIM_ALPHA }); this._dim.fill({ color: 0x000000, alpha: DIM_ALPHA });
} }
// Bottom
const cropBottom = cy + ch; const cropBottom = cy + ch;
if (cropBottom < iy + ih) { if (cropBottom < iy + ih) {
this._dim.rect(ix, cropBottom, iw, iy + ih - cropBottom); this._dim.rect(ix, cropBottom, iw, iy + ih - cropBottom);
this._dim.fill({ color: 0x000000, alpha: DIM_ALPHA }); this._dim.fill({ color: 0x000000, alpha: DIM_ALPHA });
} }
// Left
if (cx > ix) { if (cx > ix) {
this._dim.rect(ix, cy, cx - ix, ch); this._dim.rect(ix, cy, cx - ix, ch);
this._dim.fill({ color: 0x000000, alpha: DIM_ALPHA }); this._dim.fill({ color: 0x000000, alpha: DIM_ALPHA });
} }
// Right
const cropRight = cx + cw; const cropRight = cx + cw;
if (cropRight < ix + iw) { if (cropRight < ix + iw) {
this._dim.rect(cropRight, cy, ix + iw - cropRight, ch); this._dim.rect(cropRight, cy, ix + iw - cropRight, ch);
@@ -201,11 +202,35 @@ export class CropOverlay extends Container {
} }
} }
// -- Handle drag -- // -- Handle drag (single handler, registered dynamically) --
private _onDown(e: FederatedPointerEvent, id: HandleId): void { private _onDown(e: FederatedPointerEvent, id: HandleId): void {
e.stopPropagation(); e.stopPropagation();
this._drag = { handleId: id, startCrop: { ...this._crop } }; this._drag = { handleId: id, startCrop: { ...this._crop } };
// Register move/up on the stage (single handler, not per-handle)
this._removeDragListeners();
this._boundMove = (ev: FederatedPointerEvent) => this._onMove(ev);
this._boundUp = () => this._onUp();
const stage = this._viewport.parent;
if (stage) {
stage.on('globalpointermove', this._boundMove);
stage.on('pointerup', this._boundUp);
stage.on('pointerupoutside', this._boundUp);
}
}
private _removeDragListeners(): void {
const stage = this._viewport.parent;
if (stage) {
if (this._boundMove) stage.off('globalpointermove', this._boundMove);
if (this._boundUp) {
stage.off('pointerup', this._boundUp);
stage.off('pointerupoutside', this._boundUp);
}
}
this._boundMove = null;
this._boundUp = null;
} }
private _onMove(e: FederatedPointerEvent): void { private _onMove(e: FederatedPointerEvent): void {
@@ -216,7 +241,6 @@ export class CropOverlay extends Container {
const ih = data.h * data.sy; const ih = data.h * data.sy;
const world = this._viewport.toWorld(e.global.x, e.global.y); const world = this._viewport.toWorld(e.global.x, e.global.y);
// Convert world position to normalized image coordinates
const nx = (world.x - data.x) / iw; const nx = (world.x - data.x) / iw;
const ny = (world.y - data.y) / ih; const ny = (world.y - data.y) / ih;
@@ -266,5 +290,6 @@ export class CropOverlay extends Container {
private _onUp(): void { private _onUp(): void {
this._drag = null; this._drag = null;
this._removeDragListeners();
} }
} }
+4
View File
@@ -421,6 +421,10 @@ export class SceneManager {
if (data.type === 'group' && obj instanceof FrameSprite) { if (data.type === 'group' && obj instanceof FrameSprite) {
obj.updateFromData(data as GroupObject); obj.updateFromData(data as GroupObject);
} }
if (data.type === 'image' && obj instanceof ImageSprite) {
const imgData = data as ImageObject;
obj.applyCrop(imgData.crop);
}
// Update stored data // Update stored data
item.data = { ...data }; item.data = { ...data };
+8
View File
@@ -24,6 +24,14 @@ export class TextEditor {
return this._textarea !== null; return this._textarea !== null;
} }
/** Clear the current textarea content (for new text items). */
clearText(): void {
if (this._textarea) {
this._textarea.value = '';
this._textarea.dispatchEvent(new Event('input')); // trigger autoSize
}
}
/** /**
* Open an inline textarea over the given text item. * Open an inline textarea over the given text item.
*/ */
+1 -2
View File
@@ -130,8 +130,7 @@ export function activateTool(
ctx.onChange(); ctx.onChange();
}); });
// Clear placeholder so user types from scratch // Clear placeholder so user types from scratch
const ta = document.querySelector('textarea'); ctx.textEditor!.clearText();
if (ta) { ta.value = ''; }
}, 50); }, 50);
} }
+7 -5
View File
@@ -132,15 +132,16 @@ export function useCanvasSetup(deps: CanvasSetupDeps) {
// Double-click image: zoom-to-fit (PureRef-style focus) // Double-click image: zoom-to-fit (PureRef-style focus)
selection.onDoubleClickImage = (item) => { selection.onDoubleClickImage = (item) => {
const bounds = item.displayObject.getBounds();
const padding = 80; // screen pixels of padding around the image const padding = 80; // screen pixels of padding around the image
const screenW = viewport.screenWidth; const screenW = viewport.screenWidth;
const screenH = viewport.screenHeight; const screenH = viewport.screenHeight;
const scaleX = (screenW - padding * 2) / bounds.width; const bw = item.data.w * Math.abs(item.data.sx);
const scaleY = (screenH - padding * 2) / bounds.height; const bh = item.data.h * Math.abs(item.data.sy);
const scaleX = (screenW - padding * 2) / bw;
const scaleY = (screenH - padding * 2) / bh;
const targetScale = Math.min(scaleX, scaleY, 3); // cap at 3x const targetScale = Math.min(scaleX, scaleY, 3); // cap at 3x
const cx = item.data.x + (item.data.w * item.data.sx) / 2; const cx = item.data.x + bw / 2;
const cy = item.data.y + (item.data.h * item.data.sy) / 2; const cy = item.data.y + bh / 2;
viewport.animate({ viewport.animate({
time: 300, time: 300,
position: { x: cx, y: cy }, position: { x: cx, y: cy },
@@ -442,6 +443,7 @@ export function useCanvasSetup(deps: CanvasSetupDeps) {
laserCleanupRef.current = null; laserCleanupRef.current = null;
dropCleanupRef.current?.(); dropCleanupRef.current?.();
pasteCleanupRef.current?.(); pasteCleanupRef.current?.();
textEditorRef.current?.stopEditing(false);
if (cropOverlayRef.current) { if (cropOverlayRef.current) {
cropOverlayRef.current.destroy(); cropOverlayRef.current.destroy();
cropOverlayRef.current = null; cropOverlayRef.current = null;
+13
View File
@@ -700,20 +700,33 @@ export default function Editor({ isPublicView }: EditorProps) {
fill={textToolbar.fill} fill={textToolbar.fill}
position={textToolbar.items.length >= 2 ? 'below' : 'above'} position={textToolbar.items.length >= 2 ? 'below' : 'above'}
onFontSizeChange={(size) => { onFontSizeChange={(size) => {
const scene = canvasRef.current?.getScene();
for (const item of textToolbar.items) { for (const item of textToolbar.items) {
(item.data as TextObject).fontSize = size; (item.data as TextObject).fontSize = size;
const s = (item.displayObject as any)?.style; const s = (item.displayObject as any)?.style;
if (s) s.fontSize = size; if (s) s.fontSize = size;
// Re-measure text bounds
const bounds = item.displayObject.getLocalBounds();
item.data.w = bounds.width;
item.data.h = bounds.height;
if (scene) scene.updateSpatialEntry(item);
} }
selectionRef.current?.transformBox.update(textToolbar.items);
onCanvasChange(textToolbar.items.map(i => i.id)); onCanvasChange(textToolbar.items.map(i => i.id));
updateOverlays(); updateOverlays();
}} }}
onFontFamilyChange={(family) => { onFontFamilyChange={(family) => {
const scene = canvasRef.current?.getScene();
for (const item of textToolbar.items) { for (const item of textToolbar.items) {
(item.data as TextObject).fontFamily = family; (item.data as TextObject).fontFamily = family;
const s = (item.displayObject as any)?.style; const s = (item.displayObject as any)?.style;
if (s) s.fontFamily = family; if (s) s.fontFamily = family;
const bounds = item.displayObject.getLocalBounds();
item.data.w = bounds.width;
item.data.h = bounds.height;
if (scene) scene.updateSpatialEntry(item);
} }
selectionRef.current?.transformBox.update(textToolbar.items);
onCanvasChange(textToolbar.items.map(i => i.id)); onCanvasChange(textToolbar.items.map(i => i.id));
updateOverlays(); updateOverlays();
}} }}