Skip to content
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.

Sign up for GitHub

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

Plans: add support for client-side currency formatting #6046

Merged
merged 18 commits into from
Jun 17, 2016
Merged

Plans: add support for client-side currency formatting #6046

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
763e2ce
Plans: add currency-formatter, simplify data needs of plan-features-h…
gwwar Jun 14, 2016
4649fce
Plans: add tests for plan-features-header prices
gwwar Jun 14, 2016
6489445
Plans: add currency examples for plan-features
gwwar Jun 14, 2016
6627cf9
Plans: override currency formatter thousands separator
gwwar Jun 15, 2016
7f770d7
Plans: export class, and use identity as translate fallback
gwwar Jun 15, 2016
cbba94c
Plans: split out price component from plans feature header
gwwar Jun 15, 2016
3d7e4b2
Plans: remove currency-formatter lib, simplify price component
gwwar Jun 15, 2016
33b2463
Plans: update test to use inlined data
gwwar Jun 15, 2016
7d99dde
Plans: multi-line destructure
gwwar Jun 15, 2016
03f0765
Plans: remove credits
gwwar Jun 15, 2016
15da89c
Plans: tweak readme
gwwar Jun 15, 2016
cc26f9e
Plans: fix ternary spacing
gwwar Jun 15, 2016
d39f5bb
Plans: remove unneeded export
gwwar Jun 16, 2016
3b5a6b6
Plans: fix getPlanRawPrice when price is zero
gwwar Jun 16, 2016
515c222
Plans: rename currency-formatter variables and update signature
gwwar Jun 16, 2016
77cbba5
Plans: remove translate default
gwwar Jun 16, 2016
9bdcc45
Plans: update plan selector
gwwar Jun 16, 2016
96155c8
Plans: rename lib from currency-formatter to format-currency
gwwar Jun 16, 2016
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
7 changes: 6 additions & 1 deletion client/state/plans/selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* External Dependencies
*/
import get from 'lodash/get';

/**
* Internal Dependencies
*/
Expand Down Expand Up @@ -43,7 +48,7 @@ export const getPlan = createSelector(
*/
export function getPlanRawPrice( state, productId, isMonthly = false ) {
const plan = getPlan( state, productId );
if ( ! plan || ! ( plan.raw_price || plan.raw_price === 0 ) ) {
if ( get( plan, 'raw_price', -1 ) < 0 ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

To be honest, this looks rather strange and complicated. I liked your previous solution better. It was more readable. But that's just my opinion and this is a minor detail. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think an existential operator would read better, but it basically boils down to fetch plan.raw_price if possible, otherwise default to -1.

https://lodash.com/docs#get

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. My point is that a programmer who didn't read the lodash docs or is not super familiar with get won't be able to parse this line quickly. Or to phrase it differently: I did not understand it at first sight, only after going through the lodash docs. :)

return null;
}
return isMonthly ? parseFloat( ( plan.raw_price / 12 ).toFixed( 2 ) ) : plan.raw_price;
Expand Down