-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A4A: Format currency #100135
base: trunk
Are you sure you want to change the base?
A4A: Format currency #100135
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
" For example, if your client's store generates %(maxAmount)s in TPV per year, your revenue share for that year would be %(amount)s.", | ||
{ | ||
args: { | ||
maxAmount: formatCurrency( 1000000, 'USD' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting! We should probably add a "compact" notation to formatCurrency
. Let's stick with $1M
and I can follow up on this separately.
cc @sirbrillig / @Automattic/i18n We can just allow formatCurrency
to accept numberFormatOptions
in the same way numberFormat
does, so someone can just tailor the rendering a little better. Or otherwise we can create a numberFormatCompactCurrency
abstraction. I'll create an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I think it's better to format this the same way. I did a workaround: bd67c53, a quick hack to get this right. Let me know if this works until we have a function for it. Or we could skip the formatting for this particular section for both $1M and $500.


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is perfect. Thanks! Good use of the currency object parts. Glad to see there is a workaround with the existing APIs :-)
@@ -14,27 +15,39 @@ const BudgetSelector = ( { setBudget, budgetLowerRange }: Props ) => { | |||
value: '0', | |||
}, | |||
{ | |||
label: '$500', | |||
label: formatCurrency( 500, 'USD', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -47,11 +48,16 @@ const MigrationOfferV3 = ( { isExpanded, onToggleView }: Props ) => { | |||
<h3 className="a4a-migration-offer-v3__title"> | |||
<span> | |||
{ translate( | |||
'{{b}}Limited time offer:{{/b}} Migrate your sites to Pressable or WordPress.com and earn up to $10,000!*', | |||
'{{b}}Limited time offer:{{/b}} Migrate your sites to Pressable or WordPress.com and earn up to %(maxCommission)s*', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~491 bytes added 📈 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome. There were quite a few after all 👍
Thanks for spotting all of these cases. I left some comments, and I'll follow up myself on one of them. Thanks for bringing it up.
@@ -245,7 +246,13 @@ export default function PressablePlanSection( { | |||
|
|||
<span className="pressable-plan-section__details-footnote"> | |||
{ translate( | |||
`*If you exceed your plan's storage or traffic limits, you will be charged $0.50 per GB and $8 per 10K visits per month.` | |||
`*If you exceed your plan's storage or traffic limits, you will be charged %(storageCharge)s per GB and %(trafficCharge)s per 10K visits per month.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
per 10K visits
I think let's also pass 10K
through numberFormatCompact
- it should render as 10 χιλ.
in EL/Greek.
" For example, if your client's store generates %(maxAmount)s in TPV per year, your revenue share for that year would be %(amount)s.", | ||
{ | ||
args: { | ||
maxAmount: formatCurrency( 1000000, 'USD' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting! We should probably add a "compact" notation to formatCurrency
. Let's stick with $1M
and I can follow up on this separately.
cc @sirbrillig / @Automattic/i18n We can just allow formatCurrency
to accept numberFormatOptions
in the same way numberFormat
does, so someone can just tailor the rendering a little better. Or otherwise we can create a numberFormatCompactCurrency
abstraction. I'll create an issue.
e08e599
to
5be7de0
Compare
Closes https://github.com/Automattic/i18n-issues/issues/921
Related to #
Proposed Changes
Why are these changes being made?
Testing Instructions
Pre-merge Checklist