Refactor/uniform scale 2d transform#30
Merged
Merged
Conversation
QR, Aztec, and DataMatrix all share the same commitTransform shape: clamp(min, max, round(prop * min(sx, sy))) on a single integer module- size prop. The bodies were near-duplicates; Aztec was missing the call entirely (canvas resize had no effect — users had to edit the props panel). Closes that gap as a side effect of consolidating. Add a factory helper, parametrised by prop name and range, and route all three registries through it.
Two small ergonomics improvements to the helper added in the previous commit: - Default P to Record<K, number> in the helper signature so callers no longer have to repeat the prop literal twice as a type argument and again as the runtime arg. K is inferred from the runtime arg, P is contextually narrowed by the registry slot. Type safety is preserved (typos still fail via parameter contravariance against the registry's AztecProps/QrCodeProps/DataMatrixProps). - Extract per-registry MIN/MAX constants for the magnification / dimension range. The same range was repeated three times per code (helper call + NumberInput min + max) — now one source of truth per registry file.
…m invariant Two tests motivated by the aztec resize regression: 1. Direct unit test for commitUniformScaleTransform — clamp on both ends, integer rounding, min(sx,sy) selection. The helper's other siblings in transformHelpers.ts have no direct tests either, but this is the only one with non-trivial logic worth pinning down. 2. Smoke test in registry.test.ts: every code-2d type must declare a commitTransform handler. This is the invariant aztec violated silently — without it, a canvas drag-resize is a no-op. Catches the same shape of regression for any future 2D code.
- transformHelpers.test.ts: drop the explicit type args at the helper call to actually exercise the inference path the previous commit introduced. The unused Sample type stays as the props shape. - qrcode/aztec/datamatrix: shorten the interface field comments to the semantic meaning. The range was duplicated as 'range FOO_MIN..MAX' pointing at file-private constants — readers consulting the props type from outside don't see those, and inside the file the constants sit right above. Plain 'module size in dots' is enough.
There was a problem hiding this comment.
Code Review
This pull request refactors 2D barcode components to use a new commitUniformScaleTransform helper for consistent scaling and introduces constants for property limits. It also adds unit tests for the transform helper and a registry test to ensure all 2D code types implement a transformation handler. A review comment points out that the maximum error correction level for Aztec codes should be increased to 300 to support Aztec Runes.
EC_LEVEL_MAX was inherited as 232 from the previously hardcoded value; that excludes Aztec Rune (ecLevel=300), which the AztecProps comment documents as valid. The NumberInput's single-max constraint cannot express the discontinuous domain (0, 1-99, 101-104, 201-232, 300), so use the highest valid value as the upper bound. Closes the pre-existing gap surfaced by extracting the constant.
Strip the domain restatement (already in AztecProps), the regression reference (commit-message material), and keep only the non-obvious constraint: NumberInput's single-max can't model the discontinuous domain, so we pick the highest valid value.
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
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.