Skip to content
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

Design clean-up and consistency across expense reports and requests #26603

Merged
merged 28 commits into from
Sep 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
840c7be
remove caret of report/money preview
rezkiy37 Aug 30, 2023
06be8ff
hide marchant for iou preview
rezkiy37 Aug 30, 2023
1f4d975
improve showing description and merchant
rezkiy37 Aug 30, 2023
f321566
improve description hiding
rezkiy37 Aug 30, 2023
7cdcc72
fix twice merchant for distance request
rezkiy37 Aug 31, 2023
5e66460
partly fix remaining counter position
rezkiy37 Aug 31, 2023
88792f1
enable counter for normal cash requests
rezkiy37 Aug 31, 2023
8bcad7c
pass isWhisper to ReportPreview
rezkiy37 Aug 31, 2023
4c033c1
Merge branch 'main' of https://github.com/rezkiy37/Expensify into fix…
rezkiy37 Sep 1, 2023
2a8f0bc
Merge branch 'main' of https://github.com/rezkiy37/Expensify into fix…
rezkiy37 Sep 1, 2023
78a0104
tweak show merchant condition
rezkiy37 Sep 1, 2023
1b16e09
Revert "partly fix remaining counter position"
rezkiy37 Sep 1, 2023
aaac5b5
fix usage of whisper var
rezkiy37 Sep 1, 2023
7daf4aa
Merge branch 'main' of https://github.com/rezkiy37/Expensify into fix…
rezkiy37 Sep 2, 2023
ec2d986
fix whisper effect of MoneyRequestPreview
rezkiy37 Sep 2, 2023
a8f5cec
change order of arguments
rezkiy37 Sep 2, 2023
3d3950a
Merge branch 'main' into vit-26503theft
mountiny Sep 3, 2023
ed07c38
Improve the shouldShowMerchant logic
mountiny Sep 3, 2023
8c53c60
Add bottom rounded corners
mountiny Sep 3, 2023
8449727
Merge branch 'main' into vit-26503theft
mountiny Sep 3, 2023
aebd0b1
Remove the email from the saastr and sbe policy names
mountiny Sep 3, 2023
a95e0cd
Check if the first part of the workspace name is email
mountiny Sep 3, 2023
849560b
Merge branch 'main' into vit-26503theft
mountiny Sep 3, 2023
9807490
Update the logic for showing a merchant
mountiny Sep 3, 2023
3dddf92
Merge branch 'main' into vit-26503theft
mountiny Sep 3, 2023
392d011
Fix styles
mountiny Sep 3, 2023
de9dacc
Update outdated comment
mountiny Sep 3, 2023
05d63ca
Clean up
mountiny Sep 3, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,7 @@ const CONST = {
TRANSACTION: {
DEFAULT_MERCHANT: 'Request',
UNKNOWN_MERCHANT: 'Unknown Merchant',
PARTIAL_TRANSACTION_MERCHANT: '(none)',
TYPE: {
CUSTOM_UNIT: 'customUnit',
},
Expand Down
6 changes: 6 additions & 0 deletions src/components/ReportActionItem/MoneyRequestAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ const propTypes = {

network: networkPropTypes.isRequired,

/** Whether a message is a whisper */
isWhisper: PropTypes.bool,

/** Styles to be assigned to Container */
// eslint-disable-next-line react/forbid-prop-types
style: PropTypes.arrayOf(PropTypes.object),
Expand All @@ -71,6 +74,7 @@ const defaultProps = {
reportActions: {},
isHovered: false,
style: [],
isWhisper: false,
};

function MoneyRequestAction({
Expand All @@ -86,6 +90,7 @@ function MoneyRequestAction({
isHovered,
network,
style,
isWhisper,
}) {
const {translate} = useLocalize();
const isSplitBillAction = lodashGet(action, 'originalMessage.type', '') === CONST.IOU.REPORT_ACTION_TYPE.SPLIT;
Expand Down Expand Up @@ -137,6 +142,7 @@ function MoneyRequestAction({
onPreviewPressed={onMoneyRequestPreviewPressed}
containerStyles={[styles.cursorPointer, isHovered ? styles.reportPreviewBoxHoverBorder : undefined, ...style]}
isHovered={isHovered}
isWhisper={isWhisper}
/>
);
}
Expand Down
21 changes: 12 additions & 9 deletions src/components/ReportActionItem/MoneyRequestPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ import * as CurrencyUtils from '../../libs/CurrencyUtils';
import * as IOUUtils from '../../libs/IOUUtils';
import * as ReportUtils from '../../libs/ReportUtils';
import * as TransactionUtils from '../../libs/TransactionUtils';
import * as StyleUtils from '../../styles/StyleUtils';
import getButtonState from '../../libs/getButtonState';
import refPropTypes from '../refPropTypes';
import PressableWithFeedback from '../Pressable/PressableWithoutFeedback';
import * as ReceiptUtils from '../../libs/ReceiptUtils';
Expand Down Expand Up @@ -111,6 +109,9 @@ const propTypes = {
*/
shouldShowPendingConversionMessage: PropTypes.bool,

/** Whether a message is a whisper */
isWhisper: PropTypes.bool,

...withLocalizePropTypes,
};

Expand All @@ -129,6 +130,7 @@ const defaultProps = {
},
transaction: {},
shouldShowPendingConversionMessage: false,
isWhisper: false,
};

function MoneyRequestPreview(props) {
Expand Down Expand Up @@ -156,6 +158,11 @@ function MoneyRequestPreview(props) {
const hasFieldErrors = TransactionUtils.hasMissingSmartscanFields(props.transaction);
const isDistanceRequest = TransactionUtils.isDistanceRequest(props.transaction);

// Show the merchant for IOUs and expenses only if they are custom or not related to scanning smartscan
const shouldShowMerchant =
!_.isEmpty(requestMerchant) && !props.isBillSplit && requestMerchant !== CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT && requestMerchant !== CONST.TRANSACTION.DEFAULT_MERCHANT;
const shouldShowDescription = !_.isEmpty(description) && !shouldShowMerchant;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have checked if the receipt was not being scanned here. Not doing so caused #31104.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, thank you for letting me know. AFAIK, the bug already fixed.


const getSettledMessage = () => {
switch (lodashGet(props.action, 'originalMessage.paymentType', '')) {
case CONST.IOU.PAYMENT_TYPE.PAYPAL_ME:
Expand Down Expand Up @@ -220,7 +227,7 @@ function MoneyRequestPreview(props) {
errorRowStyles={[styles.mbn1]}
needsOffscreenAlphaCompositing
>
<View style={[styles.moneyRequestPreviewBox, isScanning ? styles.reportPreviewBoxHoverBorder : undefined, ...props.containerStyles]}>
<View style={[styles.moneyRequestPreviewBox, isScanning || props.isWhisper ? styles.reportPreviewBoxHoverBorder : undefined, ...props.containerStyles]}>
{hasReceipt && (
<ReportActionItemImages
images={[ReceiptUtils.getThumbnailAndImageURIs(props.transaction.receipt.source, props.transaction.filename)]}
Expand Down Expand Up @@ -249,10 +256,6 @@ function MoneyRequestPreview(props) {
fill={colors.red}
/>
)}
<Icon
fill={StyleUtils.getIconFillColor(getButtonState(props.isHovered))}
src={Expensicons.ArrowRight}
/>
</View>
<View style={[styles.flexRow]}>
<View style={[styles.flex1, styles.flexRow, styles.alignItemsCenter]}>
Expand All @@ -278,7 +281,7 @@ function MoneyRequestPreview(props) {
</View>
)}
</View>
{!props.isBillSplit && !_.isEmpty(requestMerchant) && (
{shouldShowMerchant && (
<View style={[styles.flexRow]}>
<Text style={[styles.textLabelSupporting, styles.mb1, styles.lh20, styles.breakWord]}>{requestMerchant}</Text>
</View>
Expand All @@ -288,7 +291,7 @@ function MoneyRequestPreview(props) {
{!isCurrentUserManager && props.shouldShowPendingConversionMessage && (
<Text style={[styles.textLabel, styles.colorMuted, styles.mt1]}>{props.translate('iou.pendingConversionMessage')}</Text>
)}
{!_.isEmpty(description) && <Text style={[styles.mt1, styles.colorMuted]}>{description}</Text>}
{shouldShowDescription && <Text style={[styles.mt1, styles.colorMuted]}>{description}</Text>}
</View>
{props.isBillSplit && !_.isEmpty(participantAccountIDs) && (
<Text style={[styles.textLabel, styles.colorMuted, styles.ml1]}>
Expand Down
14 changes: 6 additions & 8 deletions src/components/ReportActionItem/ReportPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ import ROUTES from '../../ROUTES';
import SettlementButton from '../SettlementButton';
import * as IOU from '../../libs/actions/IOU';
import refPropTypes from '../refPropTypes';
import * as StyleUtils from '../../styles/StyleUtils';
import getButtonState from '../../libs/getButtonState';
import PressableWithoutFeedback from '../Pressable/PressableWithoutFeedback';
import themeColors from '../../styles/themes/default';
import reportPropTypes from '../../pages/reportPropTypes';
Expand Down Expand Up @@ -89,6 +87,9 @@ const propTypes = {
/** Callback for updating context menu active state, used for showing context menu */
checkIfContextMenuActive: PropTypes.func,

/** Whether a message is a whisper */
isWhisper: PropTypes.bool,

...withLocalizePropTypes,
};

Expand All @@ -101,6 +102,7 @@ const defaultProps = {
session: {
accountID: null,
},
isWhisper: false,
};

function ReportPreview(props) {
Expand Down Expand Up @@ -176,7 +178,7 @@ function ReportPreview(props) {
accessibilityRole="button"
accessibilityLabel={props.translate('iou.viewDetails')}
>
<View style={[styles.reportPreviewBox, props.isHovered || isScanning ? styles.reportPreviewBoxHoverBorder : undefined]}>
<View style={[styles.reportPreviewBox, props.isHovered || isScanning || props.isWhisper ? styles.reportPreviewBoxHoverBorder : undefined]}>
{hasReceipts && (
<ReportActionItemImages
images={_.map(lastThreeTransactionsWithReceipts, ({receipt, filename}) => ReceiptUtils.getThumbnailAndImageURIs(receipt.source, filename))}
Expand All @@ -196,10 +198,6 @@ function ReportPreview(props) {
fill={colors.red}
/>
)}
<Icon
fill={StyleUtils.getIconFillColor(getButtonState(props.isHovered))}
src={Expensicons.ArrowRight}
/>
</View>
<View style={styles.flexRow}>
<View style={[styles.flex1, styles.flexRow, styles.alignItemsCenter]}>
Expand All @@ -214,7 +212,7 @@ function ReportPreview(props) {
)}
</View>
</View>
{hasReceipts && !isScanning && (
{!isScanning && (numberOfRequests > 1 || hasReceipts) && (
<View style={styles.flexRow}>
<View style={[styles.flex1, styles.flexRow, styles.alignItemsCenter]}>
<Text style={[styles.textLabelSupporting, styles.mb1, styles.lh20]}>{previewSubtitle || moneyRequestComment}</Text>
Expand Down
10 changes: 9 additions & 1 deletion src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,15 @@ function getPolicyName(report, returnEmptyIfNotFound = false, policy = undefined

// Public rooms send back the policy name with the reportSummary,
// since they can also be accessed by people who aren't in the workspace
return lodashGet(finalPolicy, 'name') || report.policyName || report.oldPolicyName || noPolicyFound;
const policyName = lodashGet(finalPolicy, 'name') || report.policyName || report.oldPolicyName || noPolicyFound;

// The SBE and SAASTR policies have the user name in its name, however, we do not want to show that
if (lodashGet(finalPolicy, 'owner') === CONST.EMAIL.SBE || lodashGet(finalPolicy, 'owner') === CONST.EMAIL.SAASTR) {
const policyNameParts = policyName.split(' ');
if (!Str.isValidEmail(policyNameParts[0])) return policyName;
mountiny marked this conversation as resolved.
Show resolved Hide resolved
return policyNameParts.length > 1 ? policyNameParts.slice(1).join(' ') : policyName;
}
return policyName;
}

/**
Expand Down
7 changes: 5 additions & 2 deletions src/pages/home/report/ReportActionItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,11 @@ function ReportActionItem(props) {
/**
* Get the content of ReportActionItem
* @param {Boolean} hovered whether the ReportActionItem is hovered
* @param {Boolean} isWhisper whether the report action is a whisper
* @param {Boolean} hasErrors whether the report action has any errors
* @returns {Object} child component(s)
*/
const renderItemContent = (hovered = false, hasErrors = false) => {
const renderItemContent = (hovered = false, isWhisper = false, hasErrors = false) => {
let children;
const originalMessage = lodashGet(props.action, 'originalMessage', {});

Expand All @@ -273,6 +274,7 @@ function ReportActionItem(props) {
contextMenuAnchor={popoverAnchorRef}
checkIfContextMenuActive={toggleContextMenuFromActiveReportAction}
style={props.displayAsGroup ? [] : [styles.mt2]}
isWhisper={isWhisper}
/>
);
} else if (props.action.actionName === CONST.REPORT.ACTIONS.TYPE.REPORTPREVIEW) {
Expand All @@ -286,6 +288,7 @@ function ReportActionItem(props) {
isHovered={hovered}
contextMenuAnchor={popoverAnchorRef}
checkIfContextMenuActive={toggleContextMenuFromActiveReportAction}
isWhisper={isWhisper}
/>
);
} else if (
Expand Down Expand Up @@ -447,7 +450,7 @@ function ReportActionItem(props) {
* @returns {Object} report action item
*/
const renderReportActionItem = (hovered, isWhisper, hasErrors) => {
const content = renderItemContent(hovered || isContextMenuActive, hasErrors);
const content = renderItemContent(hovered || isContextMenuActive, isWhisper, hasErrors);

if (props.draftMessage) {
return <ReportActionItemDraft>{content}</ReportActionItemDraft>;
Expand Down
2 changes: 2 additions & 0 deletions src/styles/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -3782,6 +3782,8 @@ const styles = {
borderColor: themeColors.cardBG,
borderTopLeftRadius: variables.componentBorderRadiusLarge,
borderTopRightRadius: variables.componentBorderRadiusLarge,
borderBottomLeftRadius: variables.componentBorderRadiusLarge,
borderBottomRightRadius: variables.componentBorderRadiusLarge,
overflow: 'hidden',
height: 200,
},
Expand Down