Sample amount/units polish #1893
Conversation
packages/components/src/internal/components/domainproperties/samples/SampleTypeDesigner.tsx
Show resolved
Hide resolved
packages/components/src/internal/components/domainproperties/DomainForm.tsx
Show resolved
Hide resolved
| .filter(vd => vd && vd.display !== undefined) | ||
| .reduce((v, vd, i) => v + (i > 0 ? ', ' : '') + vd.display, ''); | ||
|
|
||
| const showMenu = (showLookup || !!col.inputRenderer) && (NON_NEGATIVE_NUMBER_CONCEPT_URI !== col?.conceptURI); |
There was a problem hiding this comment.
what would be the use case here? I wouldn't expect any number fields to showMenu? Is it because the amount field now has an inputRenderer? seems like this could use a comment
| const ColumnInputRenderer = resolveInputRenderer(col); | ||
| const ColumnInputRenderer = | ||
| col.conceptURI === NON_NEGATIVE_NUMBER_CONCEPT_URI | ||
| ? null /* Use standard forminput for media amount/unit */ |
There was a problem hiding this comment.
based on the comment, it sounds like the media case is the one that uses NON_NEGATIVE_NUMBER_CONCEPT_URI is that right? I would have expected that conceptURI to be used for all amounts/units
There was a problem hiding this comment.
I left media to continue to use separate amount / unit inputs because batch has 2 amount columns, so maybe it should be considered a special case: Recipe Amount and Recipe Actual Amount. It's possible to use the new component for it but would take some extra wiring for DetailDisplay and custom labels for the amount/unit combo so I'm not sure it's worth it.
packages/components/src/internal/components/forms/input/AmountUnitInput.tsx
Outdated
Show resolved
Hide resolved
packages/components/src/internal/components/forms/input/AmountUnitInput.tsx
Outdated
Show resolved
Hide resolved
packages/components/src/internal/components/forms/input/AmountUnitInput.tsx
Show resolved
Hide resolved
| .filter(vd => vd && vd.display !== undefined) | ||
| .reduce((v, vd, i) => v + (i > 0 ? ', ' : '') + vd.display, ''); | ||
|
|
||
| const showMenu = (showLookup || !!col.inputRenderer) && (NON_NEGATIVE_NUMBER_CONCEPT_URI !== col?.conceptURI /*storedamount has inputRenderer but shouldn't show menu*/); |
There was a problem hiding this comment.
Thanks for adding the comment, that helps. But as mentioned in the other PR, this still seems odd that we are tying together the NON_NEGATIVE_NUMBER_CONCEPT_URI and the new stored amount input renderer. Those seem like different parts/pieces.
There was a problem hiding this comment.
agree it's a bit odd. But it's also odd that we always show the menu dropdown if there is an inputRenderer. I'll leave as is for now, maybe will come back to it when we get to the rest of the amount/units polish.
2 participants
Rationale
Related Pull Requests
Changes
getValidatedEditableGridValueto check for negative amount valuesAmountUnitInputto show amount/units input side-by-side on bulk add/edit