refactor: code cleanup pass#27
Merged
Merged
Conversation
text.tsx and serial.tsx were the last two registries duplicating the rotation-select markup inline (label + select with 4 hardcoded options). Every other rotation-using registry already routes through the shared RotationSelect component, which centralises the t.registry.text.rotation* locale-key reuse so the cross-block access lives in one well-named place instead of being spread across two more files.
Adds a non-reactive sibling to useCurrentObjects so event handlers and callbacks no longer have to spell out `currentObjects(useLabelStore.getState())` every time. Replaces the verbose form in useGlobalShortcuts, useCanvasLasso, useKonvaTransformer, and the one-shot site in LabelCanvas. The remaining LabelCanvas call sites that share a state snapshot across multiple reads keep using `currentObjects(state)` directly, which preserves the snapshot-consistency intent.
…number ObjectTypeDefinition's P generic already defaults to `object`, so the Record value type can simply be `ObjectTypeDefinition` without any explicit `any` and the eslint-disable comment that came with it. The bare `< 10` guard in useKonvaTransformer.boundBoxFunc becomes MIN_RESIZE_BOX_PX with a comment explaining why we ignore newer boxes that small (the user flicked past the object — keep the previous box rather than collapsing it).
text.tsx and serial.tsx had identical label+number-input markup for their fontHeight/fontWidth pairs. Extracts a NumberInput component into components/Properties/ that wraps the labelCls/inputCls layout and applies clampMin when a min is set, so registry panels stop hand-rolling the same boilerplate. Other registries can migrate their bare `<input type="number">` usages to this primitive incrementally — out of scope for this commit, which only consolidates the duplicated pair.
![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 introduces a getCurrentObjects helper for non-reactive state access and refactors several components to use it. It also centralizes property inputs by creating a new NumberInput component and utilizing RotationSelect in the serial and text registries. A critical bug was identified in LabelCanvas.tsx where the removal of a variable used in subsequent logic would cause a ReferenceError.
CI surfaced two issues from the cleanup pass that local `tsc --noEmit` missed because it ran against the looser root config: 1) Dropping the explicit `<any>` from ObjectTypeDefinition triggers function-parameter contravariance under tsconfig.app.json strict — ObjectTypeDefinition<EllipseProps>.toZPL is not assignable to one that takes `object`. Restore the `<any>` with a comment explaining why the eslint-disable is necessary. 2) The handleStageDragMove cleanup that switched to getCurrentObjects().find(...) accidentally dropped the `objs` binding still used a few lines later. Re-introduce the local.
Sweeps the bare `<input type="number">` patterns in barcode1d, code128, code39, ean13, qrcode, datamatrix, aztec, pdf417, micropdf417, codablock to the shared NumberInput component. Each migration removes the hand-rolled label+input wrapper and the duplicate `Number(e.target.value)` plumbing, picking up clampMin sanitisation where a `min` is set.
…erial extras
Sweeps the remaining number inputs onto the shared NumberInput
primitive: box/ellipse/circle/line shape registries (which were already
on clampMin inline) plus gs1databar moduleWidth, text blockWidth/
blockLines/blockLineSpacing, and serial increment. Image widthDots
keeps its custom step={8} handler; gs1databar segments keeps its even-
snapping logic. The simple 1D barcodes (ean8, upca, upce, code93, etc.)
already route through barcode1d so they pick up the change for free.
Owner
Author
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new NumberInput component to centralize number input logic, including clamping and sanitization, and refactors various registry components to use it. It also adds a non-reactive getCurrentObjects helper to the label store and a MIN_RESIZE_BOX_PX constant to the Konva transformer hook. Feedback was provided regarding the NumberInput component, specifically that its onChange handler should enforce the max constraint and guard against NaN values to prevent invalid state updates.
Gemini surfaced two corner cases in the NumberInput change handler: `max` was only an HTML hint, so the state could exceed it via paste/ keyboard, and a non-numeric paste with no `min` set would push NaN into the model. Tightens the handler so neither leaks into the store.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.