-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UISACQCOMP-218: add "Amount must be a positive number" validation for "Set exchange rate" field. #815
UISACQCOMP-218: add "Amount must be a positive number" validation for "Set exchange rate" field. #815
Changes from 8 commits
b604627
731b753
e6c31bb
bfcf518
ea94317
c34d15f
972efe2
0b7cce5
c787856
bcc4a6c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,16 +19,16 @@ import { | |
Row, | ||
} from '@folio/stripes/components'; | ||
|
||
import { FILED_NAMES } from './constants'; | ||
import { FieldCurrency } from '../Currency'; | ||
import CurrentExchangeRate from './CurrentExchangeRate'; | ||
import { TextField } from '../Fields'; | ||
import { TooltippedControl } from '../TooltippedControl'; | ||
import { | ||
IfFieldVisible, | ||
VisibilityControl, | ||
} from '../VisibilityControl'; | ||
import CurrentExchangeRate from './CurrentExchangeRate'; | ||
import { FILED_NAMES } from './constants'; | ||
import { validateRequired } from '../utils'; | ||
import { validateRequiredPositiveAmount } from '../utils'; | ||
|
||
const CurrencyExchangeRateFields = ({ | ||
currencyFieldName, | ||
|
@@ -118,9 +118,9 @@ const CurrencyExchangeRateFields = ({ | |
<TooltippedControl | ||
controlComponent={Checkbox} | ||
checked={isExchangeRateEnabled} | ||
data-testid="use-set-exhange-rate" | ||
data-testid="use-set-exchange-rate" | ||
readOnly={isSetUseExangeRateDisabled} | ||
id="use-set-exhange-rate" | ||
id="use-set-exchange-rate" | ||
label={<FormattedMessage id="stripes-acq-components.useSetExchangeRate" />} | ||
onChange={enableExchangeRate} | ||
vertical | ||
|
@@ -146,7 +146,7 @@ const CurrencyExchangeRateFields = ({ | |
readOnly={!isExchangeRateEnabled} | ||
tooltipText={tooltipTextExchangeRate} | ||
required={isExchangeRateRequired} | ||
validate={isExchangeRateRequired ? validateRequired : undefined} | ||
validate={isExchangeRateRequired ? validateRequiredPositiveAmount : undefined} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it expected to skip "positive amount" validation if the field is not required? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct. If the user wants to set a custom exchange rate then the required checkbox should be checked There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds strange There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when field is not required you just need to validate when value is populated, introduce another validation method (if doesn't exist) validatePositiveAmount There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Screen.Recording.2024-09-27.at.19.50.42.mp4 |
||
key={isExchangeRateRequired ? 1 : 0} | ||
isNonInteractive={isSetExchangeRateNonIntaractive ?? isNonInteractive} | ||
/> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same