Repository navigation
refactor(canvas): split KonvaObject god component - #36
Conversation
KonvaObject.tsx had grown to 718 lines bundling four component bodies (LineObject, ImageObject, BarcodeObject dispatch, KonvaObjectInner) plus duplicated FT-baseline + 15-dot rotation math. Three changes: - Pure helpers in textPositionTransforms.ts (objectToDisplay / displayToObject) with a round-trip property test that locks the inverse property across all rotation × positionType combinations. Kills the duplicated math at the render path and the drag-end handler. - LineObject and ImageObject move to their own files and share the renderer Props contract via konvaObjectProps.ts. - The dispatcher narrows obj before passing it down, so each per-type renderer takes the narrowed LabelObject variant directly and the runtime cast disappears. KonvaObject.tsx is now 339 lines, focused on dispatch + the shape/ text body in KonvaObjectInner.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the KonvaObject component by extracting LineObject and ImageObject into separate files and introducing a shared KonvaObjectProps interface. It also centralizes coordinate transformation logic for text and serial objects into a new textPositionTransforms utility, complete with unit tests. Feedback suggests implementing grid snapping and optimizing drag performance in ImageObject, addressing a potential division by zero, and replacing a magic number used for font height calculations with a named constant.
…rops Three small cleanups on top of the KonvaObject split: - The 1.3 ZPL-fontHeight to CSS-fontSize ratio is now a named export (ZPL_FONT_HEIGHT_TO_CSS_RATIO) used by textPositionTransforms and the text/serial render paths in KonvaObjectInner. Same number, no longer a magic literal sprinkled across files. - BarcodeObject was redeclaring the renderer Props interface structurally identical to KonvaObjectProps. Drop the duplicate and import the shared one. - konvaObjectProps.ts docstring updated: LineObject/ImageObject now receive the narrowed obj from the dispatcher (no internal cast) while BarcodeObject and KonvaObjectInner still narrow internally.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the canvas rendering system by modularizing LineObject and ImageObject into standalone components and introducing a dedicated utility for ZPL coordinate transformations. Review feedback highlights a performance bottleneck in ImageObject where onChange is called during every mouse movement, and identifies missing snapping logic in both the new ImageObject and the refactored LineObject drag handlers.
Image drag fired onChange on every onDragMove, hammering the store with one update per mouse pixel and inconsistent with the shape/text/serial renderers in KonvaObjectInner. Split into a snap-during-drag handler and a single commit on drag end. The behavior now matches the rest of the canvas: visual feedback while dragging, one store update when the user releases.
LineObject's whole-line onDragMove only updated dragDelta with the raw cursor delta — the endpoint handles in the same component snap, and the rest of the canvas snaps too, so the inconsistency was visible. Mirror the endpoint idiom: round-trip pixel → dot → snap → pixel, then update both Konva position and the React state. ImageObject divided cached.height by cached.width to compute aspect ratio. A 0-width image would yield NaN. Guard with width > 0; falls back to a square placeholder.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the Konva rendering logic by extracting LineObject and ImageObject into dedicated components and centralizing text position transformations into a new utility file with accompanying tests. It also introduces a shared KonvaObjectProps interface to standardize prop types across different object renderers. Feedback was provided regarding the snapping logic in LineObject, which currently snaps movement deltas rather than absolute positions, potentially causing alignment issues for off-grid objects.
Snapping the drag delta meant an off-grid line (e.g. x=11 imported into a snap=8 grid) would stay off-grid forever — drag delta of 8 yields x=19 instead of grid-aligned x=16. Snap the absolute start position instead and derive the delta from the snap result, mirroring the endpoint-handle drag in this same file and the shape/text/image drag elsewhere.
1 participant
KonvaObject.tsx had grown to 718 lines bundling four component bodies (LineObject, ImageObject, BarcodeObject dispatch, KonvaObjectInner) plus duplicated FT-baseline + 15-dot rotation math. Three changes:
KonvaObject.tsx is now 339 lines, focused on dispatch + the shape/ text body in KonvaObjectInner.