This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Add default currency to ProductPrice component story
#8049
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sunyatasattva
added
skip-changelog
woocommercebot
requested review from
a team and
thealexandrelara
and removed request for
a team
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
|
Size Change: 0 B Total Size: 1.08 MB ℹ️ View Unchanged
|
| @@ -23,6 +23,7 @@ export default { | |||
| }, | |||
| args: { | |||
| align: 'left', | |||
| currency: currencies.EUR, | |||
There was a problem hiding this comment.
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.
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)
There was a problem hiding this comment.
For some reason I had missed this. Thanks @thealexandrelara ! I have implemented your proposed change, could you please review it once more?
There was a problem hiding this comment.
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:
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:
- 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;
- The second option is to extend
ProductPriceStoryPropson fileassets/js/base/components/product-price/stories/index.tsxand override thecurrencyproperty with theCurrencyCodetype, 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
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
sunyatasattva
removed
the
status: stale
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
sunyatasattva
removed
the
status: stale
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
sunyatasattva
removed
the
status: stale
1 task
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes the
ProductPricecomponent story erroring out. The problem was that no default currency was provided and, despite of what the type definition says, acurrencyis actually required. There is a problem in the type definition, but this will be addressed in another PR. See this conversation.Fixes #8048
User Facing Testing
npm run storybook.