fix(ui): default measurement value-type by measurement type (#604) - #815
Open
skearnes wants to merge 1 commit into
Open
fix(ui): default measurement value-type by measurement type (#604)#815skearnes wants to merge 1 commit into
skearnes wants to merge 1 commit into
Conversation
A single global "%" default was applied to every measurement's value, which is wrong for non-percentage types (e.g. a Selectivity ratio preselected as %). Replace it with a per-measurement-type default: Yield/Purity → %, Amount → Mass, and everything else (Selectivity, Area, Counts, Intensity, Custom, …) → Number. Only affects measurements with no value yet; saved values keep their stored type. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
🤖 Pulumi Neo didn't review this pull request: no Pulumi preview ran for it. Agentic reviews require a preview of the affected stacks (for example from your CI's |
|
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.



Closes #604.
Problem
The Product Measurements value field always preselected the
%value-type for every measurement type — so e.g. a Selectivity measurement defaulted to%, which is wrong for a ratio.Cause
MeasurementValueControlused a single globaldefaultValueType = Percentwith no per-type branching.Fix
Pick the default value-type from the measurement's own
type(MeasurementValueControl.tsx):%MassNumberOnly applies when no value has been entered yet; saved measurements keep their stored type.
Tests
Parametrized tests assert the preselected value-type per measurement type (Selectivity→Number, Yield/Purity→%, Amount→Mass, Area→Number); existing numeric-render/view-only tests retained (mock updated for the new
getValuesread).tsc -b+ lint green.🤖 Generated with Claude Code
Greptile Summary
This PR fixes the Product Measurements UI always pre-selecting
%as the value-type regardless of measurement kind. A per-type lookup map (YIELD/PURITY→ Percent,AMOUNT→ Mass, everything else → Number) now drives the default, applied only when no value has yet been stored.measurementTypeToDefaultValueTypeandbuildDefaultMeasurementValue, reading the siblingtypefield viaformMethods.getValues(), replacing the former hard-codedPercentdefault.getValuesstub and a parametrizedit.eachblock asserting the correct pre-selection for Selectivity, Yield, Purity, Amount, and Area; fieldnamecorrected from"measurement"to"value"to match the actual form model.Confidence Score: 5/5
Safe to merge — the change is narrowly scoped to the unset-value path, existing saved measurements are unaffected, and the new lookup map covers all types listed in
valueCompatibleTypes.The fix is small, well-bounded, and fully exercised by the updated parametrized tests. The
getValues()call reads live form state (the parent re-renders on every dropdown change in Mantine'suseForm, so the snapshot is always fresh on remount). No regressions introduced to saved data or view-only mode.No files require special attention.
Important Files Changed
buildDefaultMeasurementValueusinggetValues()?.type— logic is sound, types are safe, and only applies when no value has been entered.makeFormMethodsfactory withgetValuesstub, correctsnamefrom"measurement"to"value", and adds parametrized per-measurement-type default assertions. Coverage is good.Reviews (1): Last reviewed commit: "fix(ui): default measurement value-type ..." | Re-trigger Greptile