Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 20 additions & 3 deletions src/components/MenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,9 @@ type MenuItemBaseProps = ForwardedFSClassProps &
/** A description text to show under the title */
description?: string;

/** Optional component to render before the description text (e.g. a badge pill) */
Copy link
Contributor

Choose a reason for hiding this comment

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

❌ CLEAN-REACT-PATTERNS-1 (docs)

Adding a new optional ReactNode prop (descriptionAddon) to MenuItem is a Case 2 violation -- its sole purpose is conditional rendering ({!!descriptionAddon && (...)}) and gating another block ({!descriptionAddon && !!description && (...)}). This continues the configuration-heavy pattern of MenuItem rather than moving toward composition.

MenuItem already has a large prop interface with many optional content/slot props (rightComponent, furtherDetailsComponent, etc.). Each new slot prop increases the component's surface area and makes it harder to refactor toward composition in the future.

Consider rendering the badge inline at the call site in PaymentMethodListItem rather than threading it through MenuItem as a new prop. Alternatively, if MenuItem needs to support this pattern, a compound component approach (e.g., <MenuItem.DescriptionAddon>) would keep the component's prop interface stable.


Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

descriptionAddon?: ReactNode;

/** Text to show below menu item. This text is not interactive */
helperText?: string;

Expand Down Expand Up @@ -497,6 +500,7 @@ function MenuItem({
furtherDetailsStyle,
furtherDetailsComponent,
description,
descriptionAddon,
helperText,
helperTextStyle,
errorText,
Expand Down Expand Up @@ -955,16 +959,29 @@ function MenuItem({
)}
</View>
)}
{!!description && !shouldShowDescriptionOnTop && (
{!shouldShowDescriptionOnTop && !descriptionAddon && !!description && (
<Text
style={descriptionTextStyles}
style={[descriptionTextStyles]}
numberOfLines={numberOfLinesDescription}
>
{description}
</Text>
)}
{!shouldShowDescriptionOnTop && !!descriptionAddon && (
<View style={[styles.flexRow, styles.alignItemsCenter, styles.gap2, styles.ml3, styles.mt1]}>
{descriptionAddon}
{!!description && (
<Text
style={[descriptionTextStyles, styles.flexShrink1, styles.ml0]}
numberOfLines={numberOfLinesDescription}
>
{description}
</Text>
)}
</View>
)}
{!!furtherDetails && (
<View style={[styles.flexRow, styles.mt1, styles.alignItemsCenter]}>
<View style={[styles.flexRow, styles.alignItemsCenter]}>
{!!furtherDetailsIcon && (
<Icon
src={furtherDetailsIcon}
Expand Down
1 change: 1 addition & 0 deletions src/languages/de.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2262,6 +2262,7 @@ const translations: TranslationDeepObject<typeof en> = {
enableWallet: 'Wallet aktivieren',
addBankAccountToSendAndReceive: 'Füge ein Bankkonto hinzu, um Zahlungen zu senden oder zu empfangen.',
addDebitOrCreditCard: 'Debit- oder Kreditkarte hinzufügen',
cardInactive: 'Inaktiv',
assignedCards: 'Zugewiesene Karten',
assignedCardsDescription: 'Transaktionen von diesen Karten werden automatisch synchronisiert.',
expensifyCard: 'Expensify Karte',
Expand Down
1 change: 1 addition & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2310,6 +2310,7 @@ const translations = {
enableWallet: 'Enable wallet',
addBankAccountToSendAndReceive: 'Add a bank account to make or receive payments.',
addDebitOrCreditCard: 'Add debit or credit card',
cardInactive: 'Inactive',
assignedCards: 'Assigned cards',
assignedCardsDescription: 'Transactions from these cards sync automatically.',
expensifyCard: 'Expensify Card',
Expand Down
1 change: 1 addition & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2135,6 +2135,7 @@ const translations: TranslationDeepObject<typeof en> = {
enableWallet: 'Habilitar billetera',
addBankAccountToSendAndReceive: 'Añade una cuenta bancaria para hacer o recibir pagos.',
addDebitOrCreditCard: 'Añadir tarjeta de débito o crédito',
cardInactive: 'Inactiva',
assignedCards: 'Tarjetas asignadas',
assignedCardsDescription: 'Las transacciones de estas tarjetas se sincronizan automáticamente.',
expensifyCard: 'Tarjeta Expensify',
Expand Down
1 change: 1 addition & 0 deletions src/languages/fr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2268,6 +2268,7 @@ const translations: TranslationDeepObject<typeof en> = {
enableWallet: 'Activer le portefeuille',
addBankAccountToSendAndReceive: 'Ajoutez un compte bancaire pour effectuer ou recevoir des paiements.',
addDebitOrCreditCard: 'Ajouter une carte de débit ou de crédit',
cardInactive: 'Inactif',
assignedCards: 'Cartes assignées',
assignedCardsDescription: 'Les transactions de ces cartes se synchronisent automatiquement.',
expensifyCard: 'Carte Expensify',
Expand Down
1 change: 1 addition & 0 deletions src/languages/it.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2258,6 +2258,7 @@ const translations: TranslationDeepObject<typeof en> = {
enableWallet: 'Abilita portafoglio',
addBankAccountToSendAndReceive: 'Aggiungi un conto bancario per effettuare o ricevere pagamenti.',
addDebitOrCreditCard: 'Aggiungi carta di debito o di credito',
cardInactive: 'Inattivo',
assignedCards: 'Carte assegnate',
assignedCardsDescription: 'Le transazioni di queste carte si sincronizzano automaticamente.',
expensifyCard: 'Carta Expensify',
Expand Down
1 change: 1 addition & 0 deletions src/languages/ja.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2245,6 +2245,7 @@ const translations: TranslationDeepObject<typeof en> = {
enableWallet: 'ウォレットを有効にする',
addBankAccountToSendAndReceive: '支払いや入金を行うには銀行口座を追加してください。',
addDebitOrCreditCard: 'デビットカードまたはクレジットカードを追加',
cardInactive: '非アクティブ',
assignedCards: '割り当て済みカード',
assignedCardsDescription: 'これらのカードからの取引は自動的に同期されます。',
expensifyCard: 'Expensify カード',
Expand Down
1 change: 1 addition & 0 deletions src/languages/nl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2256,6 +2256,7 @@ const translations: TranslationDeepObject<typeof en> = {
enableWallet: 'Portemonnee inschakelen',
addBankAccountToSendAndReceive: 'Voeg een bankrekening toe om betalingen te doen of te ontvangen.',
addDebitOrCreditCard: 'Debet- of creditcard toevoegen',
cardInactive: 'Inactief',
assignedCards: 'Toegewezen kaarten',
assignedCardsDescription: 'Transacties van deze kaarten worden automatisch gesynchroniseerd.',
expensifyCard: 'Expensify Kaart',
Expand Down
1 change: 1 addition & 0 deletions src/languages/pl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2256,6 +2256,7 @@ const translations: TranslationDeepObject<typeof en> = {
enableWallet: 'Włącz portfel',
addBankAccountToSendAndReceive: 'Dodaj konto bankowe, aby wysyłać lub odbierać płatności.',
addDebitOrCreditCard: 'Dodaj kartę debetową lub kredytową',
cardInactive: 'Nieaktywne',
assignedCards: 'Przypisane karty',
assignedCardsDescription: 'Transakcje z tych kart synchronizują się automatycznie.',
expensifyCard: 'Karta Expensify',
Expand Down
1 change: 1 addition & 0 deletions src/languages/pt-BR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2252,6 +2252,7 @@ const translations: TranslationDeepObject<typeof en> = {
enableWallet: 'Ativar carteira',
addBankAccountToSendAndReceive: 'Adicione uma conta bancária para fazer ou receber pagamentos.',
addDebitOrCreditCard: 'Adicionar cartão de débito ou crédito',
cardInactive: 'Inativo',
assignedCards: 'Cartões atribuídos',
assignedCardsDescription: 'As transações desses cartões são sincronizadas automaticamente.',
expensifyCard: 'Cartão Expensify',
Expand Down
1 change: 1 addition & 0 deletions src/languages/zh-hans.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2213,6 +2213,7 @@ const translations: TranslationDeepObject<typeof en> = {
enableWallet: '启用钱包',
addBankAccountToSendAndReceive: '添加银行账户以进行或接收付款。',
addDebitOrCreditCard: '添加借记卡或信用卡',
cardInactive: '未激活',
assignedCards: '已分配的卡片',
assignedCardsDescription: '这些银行卡的交易会自动同步。',
expensifyCard: 'Expensify 卡',
Expand Down
2 changes: 2 additions & 0 deletions src/pages/settings/Wallet/PaymentMethodList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ function PaymentMethodList({
iconWidth: variables.cardIconWidth,
iconHeight: variables.cardIconHeight,
isMethodActive: activePaymentMethodID === card.cardID,
isSuspended: card.state === CONST.EXPENSIFY_CARD.STATE.STATE_SUSPENDED && !card.nameValuePairs?.frozen,
Copy link
Contributor

Choose a reason for hiding this comment

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

❌ CONSISTENCY-3 (docs)

The expression card.state === CONST.EXPENSIFY_CARD.STATE.STATE_SUSPENDED && !card.nameValuePairs?.frozen is duplicated here and again at line 376. This is the logical inverse of the existing isCardFrozen utility in CardUtils.ts (which checks state === STATE_SUSPENDED && frozen != null). Duplicating this logic in two places increases maintenance risk -- if the definition of "suspended but not frozen" changes, both sites must be updated.

Extract a utility function in src/libs/CardUtils.ts alongside isCardFrozen, for example:

function isCardInactive(card?: OnyxEntry<Card>): boolean {
    return card?.state === CONST.EXPENSIFY_CARD.STATE.STATE_SUSPENDED && !card?.nameValuePairs?.frozen;
}

Then use it in both locations:

isSuspended: isCardInactive(card),

Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

onPress: cardOnPress,
});
continue;
Expand Down Expand Up @@ -372,6 +373,7 @@ function PaymentMethodList({
iconStyles: [styles.cardIcon],
iconWidth: variables.cardIconWidth,
iconHeight: variables.cardIconHeight,
isSuspended: card.state === CONST.EXPENSIFY_CARD.STATE.STATE_SUSPENDED && !card.nameValuePairs?.frozen,
isCardFrozen: isCardFrozen(card),
});
}
Expand Down
36 changes: 25 additions & 11 deletions src/pages/settings/Wallet/PaymentMethodListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, {useMemo, useRef} from 'react';
import type {GestureResponderEvent, StyleProp, ViewStyle} from 'react-native';
import {View} from 'react-native';
import type {ValueOf} from 'type-fest';
import Badge from '@components/Badge';
import Icon from '@components/Icon';
import MenuItem from '@components/MenuItem';
import OfflineWithFeedback from '@components/OfflineWithFeedback';
Expand Down Expand Up @@ -42,6 +43,7 @@ type PaymentMethodItem = PaymentMethod & {
errors?: Errors;
iconRight?: IconAsset;
isMethodActive?: boolean;
isSuspended?: boolean;
cardID?: number;
plaidUrl?: string;
onThreeDotsMenuPress?: (e: GestureResponderEvent | KeyboardEvent | undefined) => void;
Expand Down Expand Up @@ -127,21 +129,32 @@ function PaymentMethodListItem({item, shouldShowDefaultBadge, threeDotsMenuItems
if (isInSetupState) {
return translate('common.actionRequired');
}
if (item.isCardFrozen) {
return translate('cardPage.frozen');
}
return shouldShowDefaultBadge ? translate('paymentMethodList.defaultPaymentMethod') : undefined;
}, [isInSetupState, item.isCardFrozen, shouldShowDefaultBadge, translate]);
}, [isInSetupState, shouldShowDefaultBadge, translate]);

const badgeIcon = useMemo(() => {
if (isInSetupState) {
return icons.DotIndicator;
}
const descriptionAddon = useMemo(() => {
if (item.isCardFrozen) {
return icons.FreezeCard;
return (
<Badge
text={translate('cardPage.frozen')}
icon={icons.FreezeCard}
isCondensed
badgeStyles={[styles.ml0]}
iconStyles={[styles.mr1]}
/>
);
}
if (item.isSuspended) {
return (
<Badge
text={translate('walletPage.cardInactive')}
isCondensed
badgeStyles={[styles.ml0]}
/>
);
}
return undefined;
}, [icons.DotIndicator, icons.FreezeCard, isInSetupState, item.isCardFrozen]);
}, [item.isCardFrozen, item.isSuspended, icons.FreezeCard, styles, translate]);

return (
<OfflineWithFeedback
Expand All @@ -155,6 +168,7 @@ function PaymentMethodListItem({item, shouldShowDefaultBadge, threeDotsMenuItems
onPress={handleRowPress}
title={item.title}
description={item.description}
descriptionAddon={descriptionAddon}
icon={item.icon}
plaidUrl={item.plaidUrl}
disabled={item.disabled}
Expand All @@ -165,7 +179,7 @@ function PaymentMethodListItem({item, shouldShowDefaultBadge, threeDotsMenuItems
iconStyles={item.iconStyles}
iconFill={item.iconFill}
badgeText={badgeText}
badgeIcon={badgeIcon}
badgeIcon={isInSetupState ? icons.DotIndicator : undefined}
isBadgeSuccess={isInSetupState ? true : undefined}
wrapperStyle={[styles.paymentMethod, listItemStyle]}
iconRight={isInSetupState ? undefined : item.iconRight}
Expand Down
Loading