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

Add default currency to ProductPrice component story #8049

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from
Open

Add default currency to ProductPrice component story #8049

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c7f0df2
Add default currency to `ProductPrice` component story
sunyatasattva Dec 29, 2022
07f3230
Invert options mapping for currency control
sunyatasattva Jan 17, 2023
75c1283
Merge branch 'trunk' into fix/8048-storybook-productprice-error
sunyatasattva Jan 17, 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
3 changes: 2 additions & 1 deletion assets/js/base/components/product-price/stories/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import type { Story, Meta } from '@storybook/react';
import { currencyControl } from '@woocommerce/storybook-controls';
import { currencies, currencyControl } from '@woocommerce/storybook-controls';

/**
* Internal dependencies
Expand All @@ -23,6 +23,7 @@ export default {
},
args: {
align: 'left',
currency: currencies.EUR,
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for working on this! I realized that the whole object is being passed as the default value, so the Currency control seems not to be working properly. I mean, we can change the value, but the Selector does not keep the value selected.

Screen Capture on 2022-12-29 at 16-53-35

One suggestion to solve this is to invert the logic on storybook/custom-controls/currency.ts:

export const currencyControl = {
	control: 'select',
	options: Object.keys( currencies ), // from - options: currencies
	mapping: currencies, // from - mapping: Object.keys( currencies )
};

The second example of the Dealing with Complex Values on this documentation they use the mapping property to hold the "complex" object and the options property to contain the keys for that object.

After that it's possible to edit this default value to be 'EUR' or 'USD' (the currencies object keys)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason I had missed this. Thanks @thealexandrelara ! I have implemented your proposed change, could you please review it once more?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, thank you @sunyatasattva for solving this, I just tested and now the selector is working 🎉 . The only minor issue that I saw is that when the page is first loaded, no option is selected by default:

image

It seems that the expected value for the currency property is now the currencies object key (i.e.: EUR, USD):

args: {
		align: 'left',
		currency: 'EUR'
        ...
};

Doing that will trigger TypeScript warnings on the file assets/js/base/components/product-price/index.tsx (line 194), because the currency property is expecting one of these types instead of a string:

currency?: Currency | Record< CurrencyCode, never >;

In this case, I think we have two options:

  1. Update line 194 on the file above; however, in this case we would be editing the "production code" just to make it work with Storybook;
  2. The second option is to extend ProductPriceStoryProps on file assets/js/base/components/product-price/stories/index.tsx and override the currency property with the CurrencyCode type, something like this:
import { CurrencyCode } from '@woocommerce/types';

[...]

interface ProductPriceStoryProps extends Omit< ProductPriceProps, 'currency' > {
	currency: CurrencyCode;
}

export default {
	title: 'WooCommerce Blocks/@base-components/ProductPrice',
	component: ProductPrice,
	argTypes: {
		align: {
			control: { type: 'radio' },
			options: ALLOWED_ALIGN_VALUES,
		},
		currency: currencyControl,
	},
	args: {
		align: 'left',
		currency: 'EUR',
		format: '<price/>',
		price: 3000,
	},
} as Meta< ProductPriceStoryProps >;

These are just suggestions, maybe you know a better way to solve this

format: '<price/>',
price: 3000,
},
Expand Down