Add inactive pill to cardlist, show frozen card #82961
Conversation
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/de.ts b/src/languages/de.ts
index 11e31ae8..0e7d47d1 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -2120,7 +2120,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: 'Inactive',
+ cardInactive: 'Inaktiv',
assignedCards: 'Zugewiesene Karten',
assignedCardsDescription: 'Transaktionen von diesen Karten werden automatisch synchronisiert.',
expensifyCard: 'Expensify Card',
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 9fc53305..ab135485 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -2126,7 +2126,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: 'Inactive',
+ cardInactive: 'Inactif',
assignedCards: 'Cartes assignées',
assignedCardsDescription: 'Les transactions de ces cartes se synchronisent automatiquement.',
expensifyCard: 'Carte Expensify',
diff --git a/src/languages/it.ts b/src/languages/it.ts
index 0f48192f..3d5ee2b2 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -2116,7 +2116,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: 'Inactive',
+ cardInactive: 'Inattivo',
assignedCards: 'Carte assegnate',
assignedCardsDescription: 'Le transazioni di queste carte si sincronizzano automaticamente.',
expensifyCard: 'Carta Expensify',
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index fb193cb9..e1b8a429 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -2107,7 +2107,7 @@ const translations: TranslationDeepObject<typeof en> = {
enableWallet: 'ウォレットを有効にする',
addBankAccountToSendAndReceive: '支払いや入金を行うには銀行口座を追加してください。',
addDebitOrCreditCard: 'デビットカードまたはクレジットカードを追加',
- cardInactive: 'Inactive',
+ cardInactive: '非アクティブ',
assignedCards: '割り当て済みカード',
assignedCardsDescription: 'これらのカードからの取引は自動的に同期されます。',
expensifyCard: 'Expensify カード',
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index c20ac420..0e27ba43 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -2115,7 +2115,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: 'Inactive',
+ cardInactive: 'Inactief',
assignedCards: 'Toegewezen kaarten',
assignedCardsDescription: 'Transacties van deze kaarten worden automatisch gesynchroniseerd.',
expensifyCard: 'Expensify Card',
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index e3ef492b..e3fcd548 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -2114,7 +2114,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: 'Inactive',
+ cardInactive: 'Nieaktywne',
assignedCards: 'Przypisane karty',
assignedCardsDescription: 'Transakcje z tych kart synchronizują się automatycznie.',
expensifyCard: 'Karta Expensify',
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index 09828397..aeda591b 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -2111,7 +2111,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: 'Inactive',
+ cardInactive: 'Inativo',
assignedCards: 'Cartões atribuídos',
assignedCardsDescription: 'As transações desses cartões são sincronizadas automaticamente.',
expensifyCard: 'Cartão Expensify',
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 1814e379..6db1cac5 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -2078,7 +2078,7 @@ const translations: TranslationDeepObject<typeof en> = {
enableWallet: '启用钱包',
addBankAccountToSendAndReceive: '添加银行账户以进行或接收付款。',
addDebitOrCreditCard: '添加借记卡或信用卡',
- cardInactive: 'Inactive',
+ cardInactive: '未激活',
assignedCards: '已分配的卡片',
assignedCardsDescription: '这些银行卡的交易会自动同步。',
expensifyCard: 'Expensify 卡',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
This comment was marked as resolved.
This comment was marked as resolved.
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
grgia
changed the title
# Conflicts: # src/pages/settings/Wallet/PaymentMethodListItem.tsx
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
grgia
changed the title
melvin-bot
bot
requested review from
aimane-chnaif and
shawnborton
and removed request for
a team
|
@shawnborton @aimane-chnaif One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
melvin-bot
bot
requested review from
heyjennahay
and removed request for
a team
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| iconWidth: variables.cardIconWidth, | ||
| iconHeight: variables.cardIconHeight, | ||
| isMethodActive: activePaymentMethodID === card.cardID, | ||
| isSuspended: card.state === CONST.EXPENSIFY_CARD.STATE.STATE_SUSPENDED && !card.nameValuePairs?.frozen, |
There was a problem hiding this comment.
❌ 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.
| /** A description text to show under the title */ | ||
| description?: string; | ||
|
|
||
| /** Optional component to render before the description text (e.g. a badge pill) */ |
There was a problem hiding this comment.
❌ 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.
|
Any recent screenshots available for review? |
3 participants
HELD ON #82210 to integrate the new button component
Explanation of Change
Fixed Issues
$ #82808
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari