Skip to content

Commit

Permalink
update ('none') display
Browse files Browse the repository at this point in the history
  • Loading branch information
@waterim
waterim committed Dec 12, 2023
1 parent 3a81ea4 commit c583248
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 19 deletions.
11 changes: 6 additions & 5 deletions src/components/MoneyRequestConfirmationList.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,8 @@ function MoneyRequestConfirmationList(props) {
return (props.hasSmartScanFailed && TransactionUtils.hasMissingSmartscanFields(transaction)) || (didConfirmSplit && TransactionUtils.areRequiredFieldsEmpty(transaction));
}, [props.isEditingSplitBill, props.hasSmartScanFailed, transaction, didConfirmSplit]);

const isIOUMerchantPresent = props.isPolicyExpenseChat && !props.isScanRequest && !props.iouMerchant;
const isMerchantEmpty = !props.iouMerchant || props.iouMerchant === CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT || props.iouMerchant === CONST.TRANSACTION.DEFAULT_MERCHANT;
const shouldDisplayMerchantError = props.isPolicyExpenseChat && !props.isScanRequest && isMerchantEmpty;

useEffect(() => {
if (shouldDisplayFieldError && props.hasSmartScanFailed) {
Expand Down Expand Up @@ -498,7 +499,7 @@ function MoneyRequestConfirmationList(props) {
}

const shouldShowSettlementButton = props.iouType === CONST.IOU.TYPE.SEND;
const shouldDisableButton = selectedParticipants.length === 0 || isIOUMerchantPresent;
const shouldDisableButton = selectedParticipants.length === 0 || shouldDisplayMerchantError;

This comment has been minimized.

Sign in to view

Copy link
@jayeshmangwani

jayeshmangwani Mar 18, 2024

Contributor

@waterim @mountiny coming from the issue #38130, just wanted to confirm why we added shouldDisplayMerchantError before accepting any proposal,
why we added the shouldDisplayMerchantError to the shouldDisableButton ?
shouldDisableButton value determine the form button should the disable or not, IMO we should not be disable or block user from press the button in this case.

This comment has been minimized.

Sign in to view

Copy link
@mountiny

mountiny Mar 18, 2024

Contributor

This must have been oversight.

from how our forms should work, we should not disable them based on the errors, you can basically always submit, but we should you the errors when you submit and cannot proceed

This comment has been minimized.

Sign in to view

Copy link
@jayeshmangwani

jayeshmangwani Mar 18, 2024

Contributor

Thanks for the help

const button = shouldShowSettlementButton ? (
<SettlementButton
Expand Down Expand Up @@ -549,7 +550,7 @@ function MoneyRequestConfirmationList(props) {
props.iouCurrencyCode,
props.policyID,
selectedParticipants.length,
isIOUMerchantPresent,
shouldDisplayMerchantError,
confirm,
splitOrRequestOptions,
formError,
Expand Down Expand Up @@ -679,7 +680,7 @@ function MoneyRequestConfirmationList(props) {
{shouldShowMerchant && (
<MenuItemWithTopDescription
shouldShowRightIcon={!props.isReadOnly}
title={props.iouMerchant}
title={isMerchantEmpty ? '' : props.iouMerchant}
description={translate('common.merchant')}
style={[styles.moneyRequestMenuItem]}
titleStyle={styles.flex1}
Expand All @@ -693,7 +694,7 @@ function MoneyRequestConfirmationList(props) {
disabled={didConfirm}
interactive={!props.isReadOnly}
brickRoadIndicator={shouldDisplayFieldError && TransactionUtils.isMerchantMissing(transaction) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
error={isIOUMerchantPresent || (shouldDisplayFieldError && TransactionUtils.isMerchantMissing(transaction)) ? translate('common.error.enterMerchant') : ''}
error={shouldDisplayMerchantError || (shouldDisplayFieldError && TransactionUtils.isMerchantMissing(transaction)) ? translate('common.error.enterMerchant') : ''}
/>
)}
{shouldShowCategories && (
Expand Down
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/MoneyRequestView.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ function MoneyRequestView({report, parentReport, policyCategories, shouldShowHor
<OfflineWithFeedback pendingAction={lodashGet(transaction, 'pendingFields.merchant') || lodashGet(transaction, 'pendingAction')}>
<MenuItemWithTopDescription
description={translate('common.merchant')}
title={transactionMerchant}
title={isEmptyMerchant ? '' : transactionMerchant}
interactive={canEdit}
shouldShowRightIcon={canEdit}
titleStyle={styles.flex1}
Expand Down
2 changes: 1 addition & 1 deletion src/libs/actions/IOU.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ function resetMoneyRequestInfo(id = '') {
currency: lodashGet(currentUserPersonalDetails, 'localCurrencyCode', CONST.CURRENCY.USD),
comment: '',
participants: [],
merchant: '',
merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT,
category: '',
tag: '',
created,
Expand Down
27 changes: 16 additions & 11 deletions src/pages/EditRequestMerchantPage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,27 @@ const propTypes = {

/** Callback to fire when the Save button is pressed */
onSubmit: PropTypes.func.isRequired,

/** Boolean to enable validation */
isPolicyExpenseChat: PropTypes.bool.isRequired,
};

function EditRequestMerchantPage({defaultMerchant, onSubmit}) {
function EditRequestMerchantPage({defaultMerchant, onSubmit, isPolicyExpenseChat}) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const merchantInputRef = useRef(null);
const isEmptyMerchant = defaultMerchant === '' || defaultMerchant === CONST.TRANSACTION.UNKNOWN_MERCHANT || defaultMerchant === CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT;

const validate = useCallback((value) => {
const errors = {};

if (_.isEmpty(value.merchant)) {
errors.merchant = 'common.error.fieldRequired';
}

return errors;
}, []);
const validate = useCallback(
(value) => {
const errors = {};
if (_.isEmpty(value.merchant) && isPolicyExpenseChat) {
errors.merchant = 'common.error.fieldRequired';
}
return errors;
},
[isPolicyExpenseChat],
);

return (
<ScreenWrapper
Expand All @@ -56,7 +61,7 @@ function EditRequestMerchantPage({defaultMerchant, onSubmit}) {
InputComponent={TextInput}
inputID="merchant"
name="merchant"
defaultValue={defaultMerchant}
defaultValue={isEmptyMerchant ? '' : defaultMerchant}
label={translate('common.merchant')}
accessibilityLabel={translate('common.merchant')}
role={CONST.ACCESSIBILITY_ROLE.TEXT}
Expand Down
5 changes: 5 additions & 0 deletions src/pages/EditRequestPage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,17 @@ function EditRequestPage({report, route, parentReport, policyCategories, policyT
return (
<EditRequestMerchantPage
defaultMerchant={transactionMerchant}
isPolicyExpenseChat={isPolicyExpenseChat}
onSubmit={(transactionChanges) => {
// In case the merchant hasn't been changed, do not make the API request.
if (transactionChanges.merchant.trim() === transactionMerchant) {
Navigation.dismissModal();
return;
}
if (transactionChanges.merchant.trim() === '') {
editMoneyRequest({merchant: CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT});
return;
}
editMoneyRequest({merchant: transactionChanges.merchant.trim()});
}}
/>
Expand Down
4 changes: 3 additions & 1 deletion src/pages/iou/MoneyRequestMerchantPage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,13 @@ const defaultProps = {
};

function MoneyRequestMerchantPage({iou, route}) {
console.log('iou :>> ', iou);

Check failure on line 51 in src/pages/iou/MoneyRequestMerchantPage.js

View workflow job for this annotation

GitHub Actions / lint 

Unexpected console statement
const styles = useThemeStyles();
const {translate} = useLocalize();
const {inputCallbackRef} = useAutoFocusInput();
const iouType = lodashGet(route, 'params.iouType', '');
const reportID = lodashGet(route, 'params.reportID', '');
const isEmptyMerchant = iou.merchant === '' || iou.merchant === CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT || iou.merchant === CONST.TRANSACTION.DEFAULT_MERCHANT;

useEffect(() => {
const moneyRequestId = `${iouType}${reportID}`;
Expand Down Expand Up @@ -114,7 +116,7 @@ function MoneyRequestMerchantPage({iou, route}) {
InputComponent={TextInput}
inputID="moneyRequestMerchant"
name="moneyRequestMerchant"
defaultValue={iou.merchant}
defaultValue={isEmptyMerchant ? '' : iou.merchant}
maxLength={CONST.MERCHANT_NAME_MAX_LENGTH}
label={translate('common.merchant')}
accessibilityLabel={translate('common.merchant')}
Expand Down

0 comments on commit c583248

Please sign in to comment.