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.

Sign up for GitHub

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

Add “Reveal details” for the digital card #26778

Merged
merged 11 commits into from
Oct 4, 2023
Merged

Add “Reveal details” for the digital card #26778

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8adb129
cherry pick PR changes
JKobrynski Sep 26, 2023
8de0bfc
handle empty address
JKobrynski Sep 26, 2023
bf6e4ba
replace native .every with underscore method
JKobrynski Sep 26, 2023
9adb8f0
move card details transactions under cardPage
JKobrynski Sep 27, 2023
1e23be4
check for virtual card existence before displaying it
JKobrynski Sep 27, 2023
53f2b65
move getFormattedAddress to CardUtils
JKobrynski Sep 27, 2023
23379f1
Add PressableWithDisplayToggle to CardDetails
JKobrynski Oct 2, 2023
bb6023e
move getFormattedAddress to PersonalDetailsUtils
JKobrynski Oct 2, 2023
0fb71a0
apply wrapperStyle to PressableWithoutFeedback to prevent MenuItem mi…
JKobrynski Oct 2, 2023
ca8ec70
Align copy icon vertically
JKobrynski Oct 3, 2023
3a82c34
add comment to temporary mock
JKobrynski Oct 4, 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
21 changes: 18 additions & 3 deletions src/components/MenuItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import _ from 'underscore';
import React, {useEffect, useMemo} from 'react';
import {View} from 'react-native';
import ExpensiMark from 'expensify-common/lib/ExpensiMark';
import PressableWithFeedback from './Pressable/PressableWithFeedback';
import Text from './Text';
import styles from '../styles/styles';
import themeColors from '../styles/themes/default';
Expand Down Expand Up @@ -49,6 +50,8 @@ const defaultProps = {
iconHeight: undefined,
description: undefined,
iconRight: Expensicons.ArrowRight,
onIconRightPress: undefined,
iconRightAccessibilityLabel: undefined,
iconStyles: [],
iconFill: undefined,
secondaryIconFill: undefined,
Expand Down Expand Up @@ -77,6 +80,8 @@ const defaultProps = {
numberOfLinesTitle: 1,
shouldGreyOutWhenDisabled: true,
shouldRenderAsHTML: false,
rightComponent: undefined,
shouldShowRightComponent: false,
};

const MenuItem = React.forwardRef((props, ref) => {
Expand Down Expand Up @@ -131,6 +136,8 @@ const MenuItem = React.forwardRef((props, ref) => {
return '';
}, [props.title, props.shouldRenderAsHTML, props.shouldParseTitle, html]);

const hasPressableRightComponent = props.onIconRightPress || props.iconRight || (props.rightComponent && props.shouldShowRightComponent);

return (
<Hoverable>
{(isHovered) => (
Expand Down Expand Up @@ -295,7 +302,7 @@ const MenuItem = React.forwardRef((props, ref) => {
</View>
</View>
</View>
<View style={[styles.flexRow, styles.menuItemTextContainer, styles.pointerEventsNone]}>
<View style={[styles.flexRow, styles.menuItemTextContainer, !hasPressableRightComponent && styles.pointerEventsNone]}>
{Boolean(props.badgeText) && (
<Badge
text={props.badgeText}
Expand Down Expand Up @@ -333,13 +340,21 @@ const MenuItem = React.forwardRef((props, ref) => {
</View>
)}
{Boolean(props.shouldShowRightIcon) && (
<View style={[styles.popoverMenuIcon, styles.pointerEventsAuto, props.disabled && styles.cursorDisabled]}>
<PressableWithFeedback
style={[styles.popoverMenuIcon, styles.pointerEventsAuto, props.disabled && styles.cursorDisabled]}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON}
accessibilityLabel={props.iconRightAccessibilityLabel ? props.iconRightAccessibilityLabel : ''}
accessible={!props.onIconRightPress}
disabled={!props.onIconRightPress}
onPress={props.onIconRightPress}
>
<Icon
src={props.iconRight}
fill={StyleUtils.getIconFillColor(getButtonState(props.focused || isHovered, pressed, props.success, props.disabled, props.interactive))}
/>
</View>
</PressableWithFeedback>
)}
{props.shouldShowRightComponent && props.rightComponent}
{props.shouldShowSelectedState && <SelectCircle isChecked={props.isSelected} />}
</View>
</>
Expand Down
12 changes: 12 additions & 0 deletions src/components/menuItemPropTypes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ const propTypes = {
/** Overrides the icon for shouldShowRightIcon */
iconRight: PropTypes.elementType,

/** Function to fire when the right icon has been pressed */
onIconRightPress: PropTypes.func,

/** accessibilityLabel for the right icon when it's pressable */
iconRightAccessibilityLabel: PropTypes.string,

/** A description text to show under the title */
description: PropTypes.string,

Expand Down Expand Up @@ -147,6 +153,12 @@ const propTypes = {

/** Should render the content in HTML format */
shouldRenderAsHTML: PropTypes.bool,

/** Component to be displayed on the right */
rightComponent: PropTypes.node,

/** Should render component on the right */
shouldShowRightComponent: PropTypes.bool,
};

export default propTypes;
8 changes: 8 additions & 0 deletions src/languages/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,14 @@ export default {
availableSpend: 'Remaining spending power',
virtualCardNumber: 'Virtual card number',
physicalCardNumber: 'Physical card number',
cardDetails: {
JKobrynski marked this conversation as resolved.
Show resolved Hide resolved
cardNumber: 'Virtual card number',
expiration: 'Expiration',
cvv: 'CVV',
address: 'Address',
revealDetails: 'Reveal details',
copyCardNumber: 'Copy card number',
},
},
transferAmountPage: {
transfer: ({amount}: TransferParams) => `Transfer${amount ? ` ${amount}` : ''}`,
Expand Down
8 changes: 8 additions & 0 deletions src/languages/es.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -809,6 +809,14 @@ export default {
availableSpend: 'Capacidad de gasto restante',
virtualCardNumber: 'Número de la tarjeta virtual',
physicalCardNumber: 'Número de la tarjeta física',
cardDetails: {
cardNumber: 'Número de tarjeta virtual',
expiration: 'Expiración',
cvv: 'CVV',
address: 'Dirección',
revealDetails: 'Revelar detalles',
copyCardNumber: 'Copiar número de la tarjeta',
},
},
transferAmountPage: {
transfer: ({amount}: TransferParams) => `Transferir${amount ? ` ${amount}` : ''}`,
Expand Down
23 changes: 22 additions & 1 deletion src/libs/CardUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,25 @@ function maskCard(lastFour = ''): string {
return maskedString.replace(/(.{4})/g, '$1 ').trim();
}

export {getDomainCards, getCompanyCards, getMonthFromExpirationDateString, getYearFromExpirationDateString, maskCard};
/**
* Formats an address object into an easily readable string
*
* @returns - formatted address
*/
function getFormattedAddress(privatePersonalDetails: OnyxTypes.PrivatePersonalDetails): string | null {
const {address} = privatePersonalDetails;
const [street1, street2] = (address?.street ?? '').split('\n');
const addressItems = [street1, street2, address?.city, address?.state, address?.zip, address?.country];
Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't have Address Line 2 populated, it just shows a blank space instead of filtering it out

image

Could we just re-use getFormattedAddress? Seems like both methods are supposed to be doing the same thing, we could move it to addressUtils (or similar) and use one method for both pages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it a little bit and moved it to PersonalDetailsUtils, let me know what you think!

const areAllAddressItemsEmpty = addressItems.every((item) => !item);

if (areAllAddressItemsEmpty) {
return null;
}

const formatted = addressItems.join(', ');

// Remove the last comma of the address
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct me if I'm wrong, but I don't think this is needed. join() doesn't add a separator string after the last element, I don't see a difference when commenting out this line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure I understand, you are saying we don't need join()? If we remove it, it will be an array

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the implementation a lot, so I guess that comment might be outdated

return formatted.trim().replace(/,$/, '');
}

export {getDomainCards, getCompanyCards, getMonthFromExpirationDateString, getYearFromExpirationDateString, maskCard, getFormattedAddress};
42 changes: 34 additions & 8 deletions src/pages/settings/Wallet/ExpensifyCardPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import PropTypes from 'prop-types';
import React from 'react';
import React, {useState} from 'react';
import {ScrollView, View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
Expand All @@ -16,6 +16,8 @@ import * as CurrencyUtils from '../../../libs/CurrencyUtils';
import Navigation from '../../../libs/Navigation/Navigation';
import styles from '../../../styles/styles';
import * as CardUtils from '../../../libs/CardUtils';
import Button from '../../../components/Button';
import CardDetails from './WalletPage/CardDetails';

const propTypes = {
/* Onyx Props */
Expand Down Expand Up @@ -45,12 +47,18 @@ function ExpensifyCardPage({
const virtualCard = _.find(domainCards, (card) => card.isVirtual) || {};
const physicalCard = _.find(domainCards, (card) => !card.isVirtual) || {};

const [shouldShowCardDetails, setShouldShowCardDetails] = useState(false);

if (_.isEmpty(virtualCard) && _.isEmpty(physicalCard)) {
return <NotFoundPage />;
}

const formattedAvailableSpendAmount = CurrencyUtils.convertToDisplayString(physicalCard.availableSpend || virtualCard.availableSpend || 0);

const handleRevealDetails = () => {
setShouldShowCardDetails(true);
};

return (
<ScreenWrapper
includeSafeAreaPaddingBottom={false}
Expand All @@ -73,13 +81,31 @@ function ExpensifyCardPage({
interactive={false}
titleStyle={styles.newKansasLarge}
/>
{!_.isEmpty(physicalCard) && (
<MenuItemWithTopDescription
description={translate('cardPage.virtualCardNumber')}
title={CardUtils.maskCard(virtualCard.lastFourPAN)}
interactive={false}
titleStyle={styles.walletCardNumber}
/>
{!_.isEmpty(virtualCard) && (
<>
{shouldShowCardDetails ? (
<CardDetails
pan="1234123412341234"
expiration="11/02/2024"
cvv="321"
Comment on lines +89 to +91
Copy link
Contributor

Choose a reason for hiding this comment

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

looks great, but could you add a quick comment that these will be replaced in this issue: https://github.com/orgs/Expensify/projects/58?pane=issue&itemId=33286617

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

/>
) : (
<MenuItemWithTopDescription
description={translate('cardPage.virtualCardNumber')}
title={CardUtils.maskCard(virtualCard.lastFourPAN)}
interactive={false}
titleStyle={styles.walletCardNumber}
shouldShowRightComponent
rightComponent={
<Button
medium
text={translate('cardPage.cardDetails.revealDetails')}
onPress={handleRevealDetails}
/>
}
/>
)}
</>
)}
{!_.isEmpty(physicalCard) && (
<MenuItemWithTopDescription
Expand Down
97 changes: 97 additions & 0 deletions src/pages/settings/Wallet/WalletPage/CardDetails.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import React from 'react';
import PropTypes from 'prop-types';
import {withOnyx} from 'react-native-onyx';
import * as Expensicons from '../../../../components/Icon/Expensicons';
import MenuItemWithTopDescription from '../../../../components/MenuItemWithTopDescription';
import Clipboard from '../../../../libs/Clipboard';
import useLocalize from '../../../../hooks/useLocalize';
import usePrivatePersonalDetails from '../../../../hooks/usePrivatePersonalDetails';
import ONYXKEYS from '../../../../ONYXKEYS';
import * as CartUtils from '../../../../libs/CardUtils';

const propTypes = {
/** Card number */
pan: PropTypes.string,

/** Card expiration date */
expiration: PropTypes.string,

/** 3 digit code */
cvv: PropTypes.string,

/** User's private personal details */
privatePersonalDetails: PropTypes.shape({
/** User's home address */
address: PropTypes.shape({
street: PropTypes.string,
city: PropTypes.string,
state: PropTypes.string,
zip: PropTypes.string,
country: PropTypes.string,
}),
}),
};

const defaultProps = {
pan: '',
expiration: '',
cvv: '',
privatePersonalDetails: {
address: {
street: '',
street2: '',
city: '',
state: '',
zip: '',
country: '',
},
},
};

function CardDetails({pan, expiration, cvv, privatePersonalDetails}) {
usePrivatePersonalDetails();
const {translate} = useLocalize();

const handleCopyToClipboard = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use CopyTextToClipboard instead
It will also add a hover effect to the copy icon and a tooltip, so it's consistent with the rest of the app

Screen.Recording.2023-10-02.at.11.13.49.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately it won't work for this UI, because CopyTextToClipboard displays a text component that we don't need as it's already displayed by MenuItemWithTopDescription. However, I found out that this would work:

<MenuItemWithTopDescription
    ...
    shouldShowRightComponent
    rightComponent={
        <PressableWithDelayToggle
            tooltipText={translate('reportActionContextMenu.copyToClipboard')}
            tooltipTextChecked={translate('reportActionContextMenu.copied')}
            icon={Expensicons.Copy}
            onPress={handleCopyToClipboard}
        />
    }
    ...
/>

So basically, a part of CopyTextToClipboard that displays the icon, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, let's do that instead!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 😄

Clipboard.setString(pan);
};

return (
<>
<MenuItemWithTopDescription
description={translate('cardPage.cardDetails.cardNumber')}
title={pan}
iconRight={Expensicons.Copy}
shouldShowRightIcon
interactive={false}
onIconRightPress={handleCopyToClipboard}
iconRightAccessibilityLabel={translate('cardPage.cardDetails.copyCardNumber')}
/>
<MenuItemWithTopDescription
description={translate('cardPage.cardDetails.expiration')}
title={expiration}
interactive={false}
/>
<MenuItemWithTopDescription
description={translate('cardPage.cardDetails.cvv')}
title={cvv}
interactive={false}
/>
<MenuItemWithTopDescription
description={translate('cardPage.cardDetails.address')}
title={CartUtils.getFormattedAddress(privatePersonalDetails)}
interactive={false}
/>
</>
);
}

CardDetails.displayName = 'CardDetails';
CardDetails.propTypes = propTypes;
CardDetails.defaultProps = defaultProps;

export default withOnyx({
privatePersonalDetails: {
key: ONYXKEYS.PRIVATE_PERSONAL_DETAILS,
},
})(CardDetails);
Loading