OLMIS-8177: Show required tooltip on invalid table inputs - #84
Conversation
|
The tooltip carried an English literal, so it stayed English in every locale. Read the shared form message instead, which is the same key and the same wording the AngularJS forms use for an empty required input.
The component keeps its inputs two levels below the cell, out of reach of the rule that marks a single input control, so an invalid quantity showed the cell icon but never outlined the input the way every other invalid table input does. Marking them here also makes the packs unit, which renders two inputs and therefore loses the single-control class, look like the doses unit.
| {numeric && <NumericInput value={value} onChange={onChange} onBlur={onBlur} disabled={disabled} {...props} />} | ||
| {!numeric && <Input value={value} onChange={onChange} onBlur={onBlur} disabled={disabled} {...props} />} | ||
| </div> | ||
| <WebTooltip shouldDisplayTooltip={!valid} tooltipContent={formatMessage('openlmisForm.required')}> |
There was a problem hiding this comment.
Could we derive the tooltip message from the validation result instead of always showing required? validateRow is generic and could fail for other reasons as well.
There was a problem hiding this comment.
Fair point, and I would like to do it, but there is a catch outside this file.
validateRow is read as a boolean in three places: here, table.jsx validatePage() (valid = validateRow(row); if (!valid) return false;) and table-by-categories.jsx. If it starts returning a message, a non-empty string is truthy, so if (!valid) never fires and all three tables stop marking invalid pages. It is also supplied from three places: order-create-tab.jsx here and two BUQ screens in referencedata-ui.
The good news is that both producers already build the list of errors and then throw it away: requisition does !validateOrderItem(row).length, BUQ does getErrors(row).length === 0. So the clean contract is "return the errors, empty means valid", which is five mechanical edits and two of them delete a wrapper.
The catch is that BUQ returns codes (VERIFIED_CONSUMPTION, FORECASTED_DEMAND, REMARK) rather than text, so showing errors[0] would put a code in their tooltip. Doing it properly needs a code to message mapping in referencedata-ui, which means a fourth PR outside this ticket.
Two things worth knowing either way. validateOrderItem's other branch ("cannot be negative") cannot be reached from the UI, because NumericInput defaults to allowNegative = false with a ^\d+$ regex, so today the tooltip never lies. And those messages are hardcoded English, so whichever way we go they need message keys, otherwise the tooltip stops being translated.
Which do you prefer: the contract change with a fourth PR in referencedata-ui, or keeping validateRow boolean here, adding an optional message prop with the shared required message as fallback, and filing the contract change as a refactor ticket?
There was a problem hiding this comment.
Agreed. The tooltip doesn't currently mislead, since NumericInput prevents negative values and required is the only reachable validation failure.
Let's go with Option 2: keep validateRow returning a boolean and add an optional message prop, falling back to openlmisForm.required. The broader return-errors contract can be handled in a separate refactor ticket, including updating all three consumers and introducing message keys for the error texts. We can discuss ticket itself on next daily meeting.
I'm approving this PR, but I'd appreciate it if you could add the optional message prop before merging.
The tooltip falls back to the shared required message, which is the only validation failure the table cells can reach today, but a consumer that knows why its row is invalid can now say so.
The sibling packs/doses component already resolves its messages that way, and it keeps the cell working with mocks that stub only get, which is what the new react spec for that component does. get also stays quiet when a locale has no translation for the key, instead of logging an error per render.
|
3 participants
React table input cells show a required-field tooltip when invalid, instead of a red mark with no tooltip.
The text comes from the shared form message, the same key and wording the AngularJS forms use for an empty required input, so it gets translated instead of sitting in the component as an English literal.
It also marks the packs/doses quantity inputs when their cell is invalid. The component keeps its inputs two levels below the cell, out of reach of the rule that marks a single input control, so an invalid quantity showed the cell icon but never outlined the input the way every other invalid table input does. That also makes the packs unit, which renders two inputs and so loses the single-control class, look like the doses unit.
Checked against the rebuilt stylesheet and then in the running app: invalid quantity controls in both units are outlined in the danger colour and valid cells are untouched. Unit suite 920 of 920 green.