Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Show decimals in mini cart regular price would not have decimals #5125

Closed
wants to merge 19 commits into from
Closed

Show decimals in mini cart regular price would not have decimals #5125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
0886998
Show decimals in mini cart even when price is 0
nielslange Nov 11, 2021
54b8a5c
Show decimals in mini cart for 0 price if number of decimals > 0
nielslange Nov 11, 2021
2b6da25
Display decimals when selected by merchant
nielslange Nov 12, 2021
1bdf9fc
Update unit test cases
nielslange Nov 12, 2021
b800f2f
Correct unit test cases
nielslange Nov 12, 2021
f96a26f
Show decimals for 0 prices if expected by the merchant
nielslange Nov 12, 2021
c69a023
Increase code readability
nielslange Nov 12, 2021
b10a5eb
Prevent duplicate decimals when price contains decimals
nielslange Nov 12, 2021
3c5b4fa
Create functions getDecimalValue() and getIntegerValue()
nielslange Nov 12, 2021
8024da9
Add additional unit test cases
nielslange Nov 12, 2021
d1bcedd
Rephrase wording of unit test cases
nielslange Nov 12, 2021
76895a3
Add return types to the introduces functions
nielslange Nov 12, 2021
0f7cab1
Merge branch 'trunk' into fix/5019-show-decimals-in-mini-cart-for-0-p…
nielslange Nov 23, 2021
c23108a
Update function documentation
nielslange Nov 23, 2021
bdabb9b
Remove obsolete test case
nielslange Nov 23, 2021
9737c5f
Update test case description
nielslange Nov 23, 2021
42ae4ed
Revert #5188 in benefit of #5125
nielslange Nov 23, 2021
9ba94db
Ensure that currency.decimalSeparator is not empty before using it
nielslange Nov 23, 2021
f6f29cb
Update function documentation
nielslange Nov 23, 2021
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
96 changes: 59 additions & 37 deletions packages/prices/utils/price.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import type {
SymbolPosition,
} from '@woocommerce/types';

type decimalType = string | RegExpMatchArray | null;

/**
* Get currency prefix.
*/
Expand Down Expand Up @@ -66,8 +68,6 @@ const siteCurrencySettings: Currency = {

/**
* Gets currency information in normalized format from an API response or the server.
*
* If no currency was provided, or currency_code is empty, the default store currency will be used.
*/
export const getCurrencyFromPriceResponse = (
// Currency data object, for example an API response containing currency formatting data.
Expand All @@ -76,7 +76,7 @@ export const getCurrencyFromPriceResponse = (
| Record< string, never >
| CartShippingPackageShippingRate
): Currency => {
if ( ! currencyData?.currency_code ) {
Copy link
Member

Choose a reason for hiding this comment

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

This prevents invalid (default state) currencyData taking precedence over the default siteCurrencySettings.

if ( ! currencyData || typeof currencyData !== 'object' ) {
return siteCurrencySettings;
}

Expand Down Expand Up @@ -115,26 +115,48 @@ export const getCurrency = (
};
};

const applyThousandSeparator = (
numberString: string,
thousandSeparator: string
/**
* Get the integer value from the minor unit value.
*
* @param {number} priceInt The price in minor unit value, e.g. 100 for $1.00.
* @param {string} thousandSeparator The thousand separator.
* @param {number} minorUnit The number of decimals to display.
* @return {string} The extracted integer value.
*/
export const getIntegerValue = (
priceInt: number,
thousandSeparator: string,
minorUnit: number
): string => {
return numberString.replace( /\B(?=(\d{3})+(?!\d))/g, thousandSeparator );
return Math.floor( priceInt / 10 ** minorUnit )
.toString()
.replace( /\B(?=(\d{3})+(?!\d))/g, thousandSeparator );
};

const splitDecimal = (
numberString: string
): {
beforeDecimal: string;
afterDecimal: string;
} => {
const parts = numberString.split( '.' );
const beforeDecimal = parts[ 0 ];
const afterDecimal = parts[ 1 ] || '';
return {
beforeDecimal,
afterDecimal,
};
/**
* Get the decimal value from the minor unit value.
*
* @param {number} priceInt The price in minor unit value, e.g. 100 for $1.00.
* @param {number} minorUnit The number of decimals to display.
* @return {string} The extracted decimal value.
*/
export const getDecimalValue = (
Copy link
Member

Choose a reason for hiding this comment

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

These functions seem to be parsing the decimal value, and formatting it, is that correct? The splitDecimal I added seemed more easy to understand. Can we not do the formatting part (adding 0's etc) later using a separate function? cc @senadir

priceInt: number,
minorUnit: number
): string | undefined => {
if ( minorUnit === 0 ) return '';

const decimalValue: decimalType = priceInt
.toString()
.match( new RegExp( `[0-9]{${ minorUnit }}$` ) );

if ( decimalValue === null && minorUnit > 0 ) {
return '0'.repeat( minorUnit );
}

if ( Array.isArray( decimalValue ) ) {
return decimalValue[ 0 ];
}
};

/**
Expand All @@ -159,26 +181,26 @@ export const formatPrice = (

const currency: Currency = getCurrency( currencyData );

const {
minorUnit,
prefix,
suffix,
decimalSeparator,
thousandSeparator,
} = currency;

const formattedPrice: number = priceInt / 10 ** minorUnit;
const integerValue: string = getIntegerValue(
priceInt,
currency.thousandSeparator,
currency.minorUnit
);

const { beforeDecimal, afterDecimal } = splitDecimal(
formattedPrice.toString()
const decimalValue: string | undefined = getDecimalValue(
priceInt,
currency.minorUnit
);

const formattedValue = `${ prefix }${ applyThousandSeparator(
beforeDecimal,
thousandSeparator
) }${
afterDecimal ? `${ decimalSeparator }${ afterDecimal }` : ''
}${ suffix }`;
const formattedPrice: string =
currency.minorUnit > 0 &&
currency.decimalSeparator !== '' &&
decimalValue !== 'undefined'
? integerValue + currency.decimalSeparator + decimalValue
nielslange marked this conversation as resolved.
Show resolved Hide resolved
: integerValue;

const formattedValue: string =
currency.prefix + formattedPrice + currency.suffix;

// This uses a textarea to magically decode HTML currency symbols.
const txt = document.createElement( 'textarea' );
Expand Down
108 changes: 81 additions & 27 deletions packages/prices/utils/test/price.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,50 +1,104 @@
/**
* Internal dependencies
*/
import { formatPrice, getCurrency } from '../price';
import {
getIntegerValue,
getDecimalValue,
formatPrice,
getCurrency,
} from '../price';

describe( 'formatPrice', () => {
describe( 'The getIntegerValue() function', () => {
test.each`
value | prefix | suffix | expected
${ 1000 } | ${ '€' } | ${ '' } | ${ '€10' }
${ 1000 } | ${ '' } | ${ '€' } | ${ '10€' }
${ 1000 } | ${ '' } | ${ '$' } | ${ '10$' }
${ '1000' } | ${ '€' } | ${ '' } | ${ '€10' }
${ 0 } | ${ '€' } | ${ '' } | ${ '€0' }
${ '' } | ${ '€' } | ${ '' } | ${ '' }
${ null } | ${ '€' } | ${ '' } | ${ '' }
${ undefined } | ${ '€' } | ${ '' } | ${ '' }
${ 100000 } | ${ '€' } | ${ '' } | ${ '€1,000' }
${ 1000000 } | ${ '€' } | ${ '' } | ${ '€10,000' }
${ 1000000000 } | ${ '€' } | ${ '' } | ${ '€10,000,000' }
value | thousandSeparator | minorUnit | expected
${ 1 } | ${ ',' } | ${ 2 } | ${ '0' }
${ 12 } | ${ ',' } | ${ 2 } | ${ '0' }
${ 123 } | ${ ',' } | ${ 2 } | ${ '1' }
${ 1000 } | ${ ',' } | ${ 2 } | ${ '10' }
${ 1234 } | ${ ',' } | ${ 2 } | ${ '12' }
${ 12345 } | ${ ',' } | ${ 2 } | ${ '123' }
${ 123456 } | ${ ',' } | ${ 2 } | ${ '1,234' }
${ 1234567 } | ${ ',' } | ${ 2 } | ${ '12,345' }
${ 12345678 } | ${ ',' } | ${ 2 } | ${ '123,456' }
${ 123456789 } | ${ ',' } | ${ 2 } | ${ '1,234,567' }
${ 123456789 } | ${ ',' } | ${ 3 } | ${ '123,456' }
${ 123456789 } | ${ ',' } | ${ 4 } | ${ '12,345' }
${ 123456789 } | ${ ',' } | ${ 5 } | ${ '1,234' }
${ 1234567890 } | ${ ',' } | ${ 0 } | ${ '1,234,567,890' }
${ 1234567890 } | ${ '.' } | ${ 0 } | ${ '1.234.567.890' }
`(
'correctly formats price given "$value", "$prefix" prefix, and "$suffix" suffix as "$expected"',
( { value, prefix, suffix, expected } ) => {
const formattedPrice = formatPrice(
"correctly returns the integer value given {thousandSeparator: '$thousandSeparator', minorUnit: '$minorUnit', value: '$value'}",
( { value, thousandSeparator, minorUnit, expected } ) => {
const formattedPrice = getIntegerValue(
value,
getCurrency( { prefix, suffix } )
thousandSeparator,
minorUnit
);

expect( formattedPrice ).toEqual( expected );
}
);
} );

describe( 'The getDecimalValue() function', () => {
test.each`
value | minorUnit | expected
${ 0 } | ${ 0 } | ${ '' }
${ 0 } | ${ 1 } | ${ '0' }
${ 0 } | ${ 2 } | ${ '00' }
${ 0 } | ${ 3 } | ${ '000' }
${ 123456789 } | ${ 0 } | ${ '' }
${ 123456789 } | ${ 1 } | ${ '9' }
${ 123456789 } | ${ 2 } | ${ '89' }
${ 123456789 } | ${ 3 } | ${ '789' }
${ 123456789 } | ${ 4 } | ${ '6789' }
${ 123456789 } | ${ 5 } | ${ '56789' }
${ 123456789 } | ${ 6 } | ${ '456789' }
`(
"correctly returns decimal value given {minorUnit: '$minorUnit' , value:'$value' }",
( { value, minorUnit, expected } ) => {
const formattedPrice = getDecimalValue( value, minorUnit );

expect( formattedPrice ).toEqual( expected );
}
);
} );

describe( 'The formatPrice() function', () => {
test.each`
value | prefix | decimalSeparator | thousandSeparator | expected
${ 1000000099 } | ${ '$' } | ${ '.' } | ${ ',' } | ${ '$10,000,000.99' }
${ 1000000099 } | ${ '$' } | ${ ',' } | ${ '.' } | ${ '$10.000.000,99' }
value | prefix | suffix | thousandSeparator | decimalSeparator | minorUnit | expected
${ 1000 } | ${ '€' } | ${ '' } | ${ '.' } | ${ ',' } | ${ 2 } | ${ '€10,00' }
${ 2305 } | ${ '€' } | ${ '' } | ${ '.' } | ${ ',' } | ${ 2 } | ${ '€23,05' }
${ 12345 } | ${ '' } | ${ ' €' } | ${ '.' } | ${ ',' } | ${ 3 } | ${ '12,345 €' }
${ 1000 } | ${ '' } | ${ '$' } | ${ ',' } | ${ '.' } | ${ 2 } | ${ '10.00$' }
${ '1000' } | ${ '€' } | ${ '' } | ${ '.' } | ${ ',' } | ${ 2 } | ${ '€10,00' }
${ 0 } | ${ '€' } | ${ '' } | ${ '.' } | ${ ',' } | ${ 2 } | ${ '€0,00' }
${ 0 } | ${ '€' } | ${ '' } | ${ '.' } | ${ ',' } | ${ 0 } | ${ '€0' }
${ 12345678 } | ${ 'Rp ' } | ${ '' } | ${ '.' } | ${ ',' } | ${ 0 } | ${ 'Rp 12.345.678' }
${ 12345678000 } | ${ '€ ' } | ${ '' } | ${ '.' } | ${ ',' } | ${ 3 } | ${ '€ 12.345.678,000' }
${ '' } | ${ '€' } | ${ '' } | ${ '.' } | ${ ',' } | ${ 2 } | ${ '' }
${ null } | ${ '€' } | ${ '' } | ${ '.' } | ${ ',' } | ${ 2 } | ${ '' }
${ undefined } | ${ '€' } | ${ '' } | ${ '.' } | ${ ',' } | ${ 2 } | ${ '' }
`(
'correctly formats price given "$value", "$prefix" prefix, "$decimalSeparator" decimal separator, "$thousandSeparator" thousand separator as "$expected"',
"correctly returns decimal value given { thousandSeparator: '$thousandSeparator', decimalSeparator: '$decimalSeparator', minorUnit: '$minorUnit', prefix: '$prefix', suffix: '$suffix', value: '$value' }",
( {
value,
prefix,
decimalSeparator,
suffix,
thousandSeparator,
decimalSeparator,
minorUnit,
expected,
} ) => {
const formattedPrice = formatPrice(
value,
getCurrency( { prefix, decimalSeparator, thousandSeparator } )
getCurrency( {
prefix,
suffix,
thousandSeparator,
decimalSeparator,
minorUnit,
} )
);

expect( formattedPrice ).toEqual( expected );
Expand All @@ -53,13 +107,13 @@ describe( 'formatPrice', () => {

test.each`
value | expected
${ 1000 } | ${ '$10' }
${ 0 } | ${ '$0' }
${ 1000 } | ${ '$10.00' }
${ 0 } | ${ '$0.00' }
${ '' } | ${ '' }
${ null } | ${ '' }
${ undefined } | ${ '' }
`(
'correctly formats price given "$value" only as "$expected"',
Copy link
Member

Choose a reason for hiding this comment

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

Having $expected was useful when viewing the output of the tests.

"correctly formats price given { value: '$value' }",
( { value, expected } ) => {
const formattedPrice = formatPrice( value );

Expand Down