-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Feat: Add Billable Toggle UI #27172
Feat: Add Billable Toggle UI #27172
Changes from 13 commits
a0cf731
7d2954f
df03d58
8e62499
f1ace02
a084124
bd88a13
230c5e3
f91c45d
007833c
5eead90
99f286a
1f3f062
fdaa194
7b3a137
caf0cf6
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 |
---|---|---|
|
@@ -5,6 +5,7 @@ import {format} from 'date-fns'; | |
import _ from 'underscore'; | ||
import {View} from 'react-native'; | ||
import lodashGet from 'lodash/get'; | ||
import Text from './Text'; | ||
import styles from '../styles/styles'; | ||
import * as ReportUtils from '../libs/ReportUtils'; | ||
import * as OptionsListUtils from '../libs/OptionsListUtils'; | ||
|
@@ -30,6 +31,7 @@ import Image from './Image'; | |
import useLocalize from '../hooks/useLocalize'; | ||
import * as ReceiptUtils from '../libs/ReceiptUtils'; | ||
import categoryPropTypes from './categoryPropTypes'; | ||
import Switch from './Switch'; | ||
import tagPropTypes from './tagPropTypes'; | ||
import ConfirmedRoute from './ConfirmedRoute'; | ||
import transactionPropTypes from './transactionPropTypes'; | ||
|
@@ -530,6 +532,16 @@ function MoneyRequestConfirmationList(props) { | |
disabled={didConfirm || props.isReadOnly} | ||
/> | ||
)} | ||
{canUseTags && !lodashGet(props.policy, 'disabledFields.defaultBillable', true) && ( | ||
<View style={[styles.flexRow, styles.mb4, styles.justifyContentBetween, styles.alignItemsCenter, styles.ml5, styles.mr8]}> | ||
<Text color={!props.iouIsBillable ? themeColors.textSupporting : undefined}>{translate('common.billable')}</Text> | ||
<Switch | ||
accessibilityLabel={translate('common.billable')} | ||
isOn={props.iouIsBillable} | ||
onToggle={props.onToggleBillable} | ||
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. It appears this was never added to the propType definitions. Can you please add it? 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. In progress here: #29327 Thanks for flagging!
Comment on lines
+538
to
+541
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. Here we missed to disable switch its readonly, caused #43756 |
||
/> | ||
</View> | ||
)} | ||
</> | ||
)} | ||
</OptionsSelector> | ||
|
@@ -561,5 +573,8 @@ export default compose( | |
transaction: { | ||
key: ({transactionID}) => `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, | ||
}, | ||
policy: { | ||
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY}${policyID}`, | ||
}, | ||
}), | ||
)(MoneyRequestConfirmationList); |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -77,6 +77,7 @@ function MoneyRequestConfirmPage(props) { | |||||||||
if (policyExpenseChat) { | ||||||||||
Policy.openDraftWorkspaceRequest(policyExpenseChat.policyID); | ||||||||||
} | ||||||||||
if (typeof props.iou.defaultBillable !== 'boolean') IOU.setMoneyRequestDefaultBillable(lodashGet(props.policy, 'defaultBillable', false)); | ||||||||||
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.
Suggested change
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. +1 to the code style request, and also please add a comment explaining why we're doing this |
||||||||||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||||||||||
}, []); | ||||||||||
|
||||||||||
|
@@ -135,6 +136,7 @@ function MoneyRequestConfirmPage(props) { | |||||||||
trimmedComment, | ||||||||||
receipt, | ||||||||||
props.iou.category, | ||||||||||
props.iou.defaultBillable, | ||||||||||
); | ||||||||||
}, | ||||||||||
[ | ||||||||||
|
@@ -146,6 +148,7 @@ function MoneyRequestConfirmPage(props) { | |||||||||
props.currentUserPersonalDetails.login, | ||||||||||
props.currentUserPersonalDetails.accountID, | ||||||||||
props.iou.category, | ||||||||||
props.iou.defaultBillable, | ||||||||||
], | ||||||||||
); | ||||||||||
|
||||||||||
|
@@ -283,6 +286,8 @@ function MoneyRequestConfirmPage(props) { | |||||||||
iouAmount={props.iou.amount} | ||||||||||
iouComment={props.iou.comment} | ||||||||||
iouCurrencyCode={props.iou.currency} | ||||||||||
iouIsBillable={props.iou.defaultBillable} | ||||||||||
onToggleBillable={IOU.setMoneyRequestDefaultBillable} | ||||||||||
iouCategory={props.iou.category} | ||||||||||
iouTag={props.iou.tag} | ||||||||||
onConfirm={createTransaction} | ||||||||||
|
@@ -352,4 +357,9 @@ export default compose( | |||||||||
key: `${ONYXKEYS.COLLECTION.SELECTED_TAB}${CONST.TAB.RECEIPT_TAB_ID}`, | ||||||||||
}, | ||||||||||
}), | ||||||||||
withOnyx({ | ||||||||||
policy: { | ||||||||||
key: ({report}) => `${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`, | ||||||||||
}, | ||||||||||
}), | ||||||||||
Comment on lines
+363
to
+367
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. This caused a regression #28182. The report object may be undefined and accessing the policyID on an undefined crashes the app. |
||||||||||
)(MoneyRequestConfirmPage); |
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.
To confirm: billable toggle is only for expense request (workspace) right? Not applicable to IOU request (DM, group)?
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.
As I understand yes
@amyevans to be sure
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.
Yes, that's correct.
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.
Yes, correct. An IOU request doesn't have an associated policy ID so the field is never going to show for one, which is intended.
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.
Whoops I was too slow. Thanks Puneet 😄
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.
Also to confirm that this uses same
newDotTags
beta permission?