Skip to content

Commit

Permalink
Fix currency negative sign position on JS rendered amounts (#7935)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tpaksu
tpaksu authored Dec 20, 2023
1 parent 21ee31d commit 6f59377
Show file tree
Hide file tree
Showing 18 changed files with 78 additions and 62 deletions.
4 changes: 4 additions & 0 deletions changelog/fix-6700-remove-currency-sign-modification-code
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fix

Fix currency negative sign position on JS rendered amounts
26 changes: 13 additions & 13 deletions client/payment-details/summary/test/__snapshots__/index.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ exports[`PaymentDetailsSummary capture notification and fraud buttons renders ca
<p>
Fees:
$-0.70
-$0.70
</p>
<p>
Expand Down Expand Up @@ -343,7 +343,7 @@ exports[`PaymentDetailsSummary capture notification and fraud buttons renders th
<p>
Fees:
$-0.70
-$0.70
</p>
<p>
Expand Down Expand Up @@ -654,7 +654,7 @@ exports[`PaymentDetailsSummary correctly renders a charge 1`] = `
<p>
Fees:
$-0.70
-$0.70
</p>
<p>
Expand Down Expand Up @@ -914,7 +914,7 @@ exports[`PaymentDetailsSummary order missing notice does not render notice if or
<p>
Fees:
$-0.70
-$0.70
</p>
<p>
Expand Down Expand Up @@ -1174,7 +1174,7 @@ exports[`PaymentDetailsSummary order missing notice renders notice if order miss
<p>
Fees:
$-0.70
-$0.70
</p>
<p>
Expand Down Expand Up @@ -1456,7 +1456,7 @@ exports[`PaymentDetailsSummary renders a charge with subscriptions 1`] = `
<p>
Fees:
$-0.70
-$0.70
</p>
<p>
Expand Down Expand Up @@ -1742,16 +1742,16 @@ exports[`PaymentDetailsSummary renders fully refunded information for a charge 1
>
<p>
Refunded:
$-20.00
-$20.00
</p>
<p>
Fees:
$-0.70
-$0.70
</p>
<p>
Net:
$-0.70
-$0.70
</p>
</div>
</div>
Expand Down Expand Up @@ -2238,11 +2238,11 @@ exports[`PaymentDetailsSummary renders partially refunded information for a char
>
<p>
Refunded:
$-12.00
-$12.00
</p>
<p>
Fees:
$-0.70
-$0.70
</p>
<p>
Expand Down Expand Up @@ -2502,7 +2502,7 @@ exports[`PaymentDetailsSummary renders the Tap to Pay channel from metadata 1`]
<p>
Fees:
$-0.70
-$0.70
</p>
<p>
Expand Down Expand Up @@ -2762,7 +2762,7 @@ exports[`PaymentDetailsSummary renders the information of a dispute-reversal cha
<p>
Fees:
$-0.70
-$0.70
</p>
<p>
Expand Down
2 changes: 1 addition & 1 deletion client/payment-details/summary/test/index.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ describe( 'PaymentDetailsSummary', () => {
} );

const container = renderCharge( charge );
screen.getByText( /Refunded: \$-20.00/i );
screen.getByText( /Refunded: -\$20.00/i );
expect( container ).toMatchSnapshot();
} );

Expand Down
2 changes: 1 addition & 1 deletion client/payment-details/timeline/test/__snapshots__/index.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1494,7 +1494,7 @@ exports[`PaymentDetailsTimeline renders subscription fee correctly 1`] = `
>
<span />
<span>
Fee (3.9% + $0.30): $-0.34
Fee (3.9% + $0.30): -$0.34
</span>
<span>
<ul
Expand Down
12 changes: 6 additions & 6 deletions client/payment-details/timeline/test/__snapshots__/map-events.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Array [
Object {
"body": Array [
"1.00 EUR → 1.19944 USD: $21.59 USD",
"Fee (2.9% + $0.30): $-0.62",
"Fee (2.9% + $0.30): -$0.62",
undefined,
"Net deposit: $20.97 USD",
],
Expand Down Expand Up @@ -389,7 +389,7 @@ Array [
Object {
"body": Array [
undefined,
"Fee (1.95% + $0.15): $-3.50",
"Fee (1.95% + $0.15): -$3.50",
<ul
className="fee-breakdown-list"
>
Expand All @@ -412,7 +412,7 @@ Array [
Variable fee: -4.9%
</li>
<li>
Fixed fee: £-0.20
Fixed fee: -£0.20
</li>
</ul>
</li>
Expand Down Expand Up @@ -455,7 +455,7 @@ Array [
Object {
"body": Array [
undefined,
"Base fee (1.95% + $0.15): $-3.50",
"Base fee (1.95% + $0.15): -$3.50",
undefined,
"Net deposit: $59.50 USD",
],
Expand Down Expand Up @@ -695,7 +695,7 @@ Array [
Object {
"body": Array [
undefined,
"Fee (2.6% + $0.20): $-0.61",
"Fee (2.6% + $0.20): -$0.61",
<ul
className="fee-breakdown-list"
>
Expand Down Expand Up @@ -745,7 +745,7 @@ Array [
Object {
"body": Array [
undefined,
"Fee (1.95% + $0.15): $-3.50",
"Fee (1.95% + $0.15): -$3.50",
undefined,
"Net deposit: $59.50 USD",
],
Expand Down
32 changes: 16 additions & 16 deletions client/transactions/list/test/__snapshots__/index.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ exports[`Transactions list renders correctly when can filter by several currenci
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=pi_mock&transaction_id=txn_j23jda9JJa&transaction_type=refund"
tabindex="-1"
>
$-0.50
-$0.50
</a>
</td>
<td
Expand Down Expand Up @@ -723,7 +723,7 @@ exports[`Transactions list renders correctly when can filter by several currenci
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=pi_mock&transaction_id=txn_oa9kaKaa8&transaction_type=charge"
tabindex="-1"
>
$-0.50
-$0.50
</a>
</td>
<td
Expand Down Expand Up @@ -871,7 +871,7 @@ exports[`Transactions list renders correctly when can filter by several currenci
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=ch_rskkmpe46yu&transaction_id=txn_mmtr89gjh5&transaction_type=charge"
tabindex="-1"
>
$-0.75
-$0.75
</a>
</td>
<td
Expand Down Expand Up @@ -1531,7 +1531,7 @@ exports[`Transactions list renders correctly when filtered by currency 1`] = `
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=pi_mock&transaction_id=txn_j23jda9JJa&transaction_type=refund"
tabindex="-1"
>
$-0.50
-$0.50
</a>
</td>
<td
Expand Down Expand Up @@ -1712,7 +1712,7 @@ exports[`Transactions list renders correctly when filtered by currency 1`] = `
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=pi_mock&transaction_id=txn_oa9kaKaa8&transaction_type=charge"
tabindex="-1"
>
$-0.50
-$0.50
</a>
</td>
<td
Expand Down Expand Up @@ -1860,7 +1860,7 @@ exports[`Transactions list renders correctly when filtered by currency 1`] = `
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=ch_rskkmpe46yu&transaction_id=txn_mmtr89gjh5&transaction_type=charge"
tabindex="-1"
>
$-0.75
-$0.75
</a>
</td>
<td
Expand Down Expand Up @@ -2524,7 +2524,7 @@ exports[`Transactions list renders correctly when filtered by deposit 1`] = `
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=pi_mock&transaction_id=txn_oa9kaKaa8&transaction_type=charge"
tabindex="-1"
>
$-0.50
-$0.50
</a>
</td>
<td
Expand Down Expand Up @@ -3210,7 +3210,7 @@ exports[`Transactions list subscription column renders correctly 1`] = `
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=pi_mock&transaction_id=txn_j23jda9JJa&transaction_type=refund"
tabindex="-1"
>
$-0.50
-$0.50
</a>
</td>
<td
Expand Down Expand Up @@ -3401,7 +3401,7 @@ exports[`Transactions list subscription column renders correctly 1`] = `
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=pi_mock&transaction_id=txn_oa9kaKaa8&transaction_type=charge"
tabindex="-1"
>
$-0.50
-$0.50
</a>
</td>
<td
Expand Down Expand Up @@ -3552,7 +3552,7 @@ exports[`Transactions list subscription column renders correctly 1`] = `
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=ch_rskkmpe46yu&transaction_id=txn_mmtr89gjh5&transaction_type=charge"
tabindex="-1"
>
$-0.75
-$0.75
</a>
</td>
<td
Expand Down Expand Up @@ -4257,7 +4257,7 @@ exports[`Transactions list when not filtered by deposit renders correctly 1`] =
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=pi_mock&transaction_id=txn_j23jda9JJa&transaction_type=refund"
tabindex="-1"
>
$-0.50
-$0.50
</a>
</td>
<td
Expand Down Expand Up @@ -4438,7 +4438,7 @@ exports[`Transactions list when not filtered by deposit renders correctly 1`] =
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=pi_mock&transaction_id=txn_oa9kaKaa8&transaction_type=charge"
tabindex="-1"
>
$-0.50
-$0.50
</a>
</td>
<td
Expand Down Expand Up @@ -4586,7 +4586,7 @@ exports[`Transactions list when not filtered by deposit renders correctly 1`] =
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=ch_rskkmpe46yu&transaction_id=txn_mmtr89gjh5&transaction_type=charge"
tabindex="-1"
>
$-0.75
-$0.75
</a>
</td>
<td
Expand Down Expand Up @@ -5288,7 +5288,7 @@ exports[`Transactions list when not filtered by deposit renders table summary on
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=pi_mock&transaction_id=txn_j23jda9JJa&transaction_type=refund"
tabindex="-1"
>
$-0.50
-$0.50
</a>
</td>
<td
Expand Down Expand Up @@ -5469,7 +5469,7 @@ exports[`Transactions list when not filtered by deposit renders table summary on
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=pi_mock&transaction_id=txn_oa9kaKaa8&transaction_type=charge"
tabindex="-1"
>
$-0.50
-$0.50
</a>
</td>
<td
Expand Down Expand Up @@ -5617,7 +5617,7 @@ exports[`Transactions list when not filtered by deposit renders table summary on
href="admin.php?page=wc-admin&path=%2Fpayments%2Ftransactions%2Fdetails&id=ch_rskkmpe46yu&transaction_id=txn_mmtr89gjh5&transaction_type=charge"
tabindex="-1"
>
$-0.75
-$0.75
</a>
</td>
<td
Expand Down
32 changes: 26 additions & 6 deletions client/utils/currency/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,19 +119,39 @@ export const formatCurrency = (
amount /= 100;
}

const isNegative = amount < 0;
const positiveAmount = isNegative ? -1 * amount : amount;
const prefix = isNegative ? '-' : '';
const currency = getCurrency( currencyCode, baseCurrencyCode );

if ( currency === null ) {
return composeFallbackCurrency( amount, currencyCode, isZeroDecimal );
return (
prefix +
composeFallbackCurrency(
positiveAmount,
currencyCode,
isZeroDecimal
)
);
}

try {
return typeof currency.formatAmount === 'function'
? htmlDecode( currency.formatAmount( amount ) )
: htmlDecode( currency.formatCurrency( amount ) );
return (
prefix +
( typeof currency.formatAmount === 'function'
? htmlDecode( currency.formatAmount( positiveAmount ) )
: htmlDecode( currency.formatCurrency( positiveAmount ) ) )
);
} catch ( err ) {
return htmlDecode(
composeFallbackCurrency( amount, currencyCode, isZeroDecimal )
return (
prefix +
htmlDecode(
composeFallbackCurrency(
positiveAmount,
currencyCode,
isZeroDecimal
)
)
);
}
};
Expand Down
10 changes: 1 addition & 9 deletions includes/class-wc-payments-utils.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -829,15 +829,7 @@ public static function format_currency( float $amount, string $currency ): strin
)
);

if ( $amount >= 0 ) {
return $formatted;
}

// Handle the subtle display difference for the negative amount between PHP wc_price `-$0.74` vs JavaScript formatCurrency `$-0.74` for the same input.
// Remove the minus sign, and then move it right before the number.
$formatted = str_replace( '-', '', $formatted );

return preg_replace( '/([0-9,\.]+)/', '-$1', $formatted );
return $formatted;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/fixtures/captured-payments/discount.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"discount": {
"label": "Discount",
"variable": "Variable fee: -4.9%",
"fixed": "Fixed fee: $-0.30"
"fixed": "Fixed fee: -$0.30"
}
},
"netString": "Net deposit: $105.48 USD"
Expand Down
Loading

0 comments on commit 6f59377

Please sign in to comment.