fix(SDK-1264): fix gross-up modal default value, loading state, and alignment - #2718
fix(SDK-1264): fix gross-up modal default value, loading state, and alignment#2718krisxcrash wants to merge 3 commits into
Conversation
…lignment Three separate issues in the same modal: - netPay defaulted to 0, which NumberInput rendered as a visible "$0.00" before the user touched the field. Reuses the coerceNaN preprocessor already established for this in the deduction form schema so the field starts blank. - The Calculate button swapped its own label to "Calculating..." with no fixed width, shifting its neighbors. Switched to Button's built-in isLoading, matching how the footer's Apply button already handles this. - The button drifted out of alignment with the input whenever a validation error grew the input's height, because the button's align-self tracked the tallest sibling rather than the input itself. Visually hides the input's own label and adds one shared label above the row instead, so both boxes start flush at the same top edge regardless of what renders below.
| <div aria-hidden="true"> | ||
| <FieldCaption isRequired>{t('netPayLabel')}</FieldCaption> | ||
| </div> | ||
| <Flex flexDirection="row" gap={8} alignItems="flex-start"> | ||
| <NumberInputField | ||
| name="netPay" | ||
| label={t('netPayLabel')} | ||
| shouldVisuallyHideLabel | ||
| format="currency" | ||
| errorMessage={t('validations.netPay')} | ||
| min={0} | ||
| isRequired | ||
| /> |
There was a problem hiding this comment.
I don't understand what this one is doing, if we are just rendering the FieldCaption above NumberInputField wouldn't we just be able to use the component as is without needing to hide the label?
There was a problem hiding this comment.
Good question! The tricky part is that NumberInputField's built-in label only wraps the input itself, not the button next to it. So if I left that label visible and just dropped a caption above both, we'd get "Net amount" said twice, and the input box would still start lower than the button box (which is the exact misalignment this PR is trying to fix, just moved around). Hiding the input's own label and using one shared caption above the whole row is what lets both boxes line up at the top. Not the prettiest, but I couldn't find a way to get the alignment right without it. Happy to add a short comment in the code so this isn't a headscratcher for the next person.
serikjensen
left a comment
serikjensen
left a comment
There was a problem hiding this comment.
Thinking through this some more, we actually can't take this approach since it would break partner adapters.
We need to surface this with design to figure out a way forward. Technically these fields should just be marked as optional since they aren't required for submission
Serik flagged that defaulting netPay to NaN and coercing it back to 0 at validation time isn't safe for partner-supplied component adapters. Pulling that piece out so the button-width and alignment fixes here aren't blocked; the blank-default behavior needs a design conversation about making the field genuinely optional before we try again.
|
Good catch, thank you. I pulled the netPay default-value piece back out and just pushed that — the loading-state and alignment fixes are still here since they're not affected by this. Filed SDK-1309 to come back to the blank-by-default part once we've talked to design about whether net amount should actually be optional. This one should be good for another look. |
2 participants
Summary
Three separate, previously-reported issues in the gross-up (net amount) calculator modal:
netPaydefaulted to0, whichNumberInputrendered as a visible$0.00before the user touched the field. Reuses thecoerceNaNpreprocessor already established for this exact purpose in the deduction form schema, so the field renders blank until a value is entered."Calculating..."with no fixed width, visibly growing and shifting its neighbors. Switched toButton's built-inisLoading(same pattern the footer's Apply button already uses one screen element away), which keeps the label's width fixed and overlays a spinner instead.align-selfhack) didn't hold up, because the button'salign-self: flex-endtracked whichever sibling was tallest — fine with no error, but drifting once a validation error grew the input's height. Root-caused instead: visually hides the input's own label (shouldVisuallyHideLabel, keeps the accessible name) and renders one shared label above the whole row, so the input and button boxes now start flush at the same top edge regardless of what renders below either of them.Jira: SDK-1264
Test plan
GrossUpModal.test.tsxpasses (7/7)eslint/tsc --noEmitcleanaria-hidden; the real accessible name is the visually-hidden field label)