Skip to content
This repository has been archived by the owner on Apr 15, 2019. It is now read-only.

Create <LiskAmount val={val} /> React component - Closes #501 #502

Merged
merged 3 commits into from
Jul 21, 2017

Conversation

slaweet
Copy link
Contributor

@slaweet slaweet commented Jul 21, 2017

Closes #501

@slaweet slaweet self-assigned this Jul 21, 2017
@slaweet slaweet requested review from reyraa and yasharAyari July 21, 2017 09:53
import { mount } from 'enzyme';
import LiskAmount from './';

describe('<LiskAmount />', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove HTML annotations


describe('<LiskAmount />', () => {
const normalizeNumber = 100000000;
it('should render "12932689.645" as "12,932,689.645"', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use a more descriptive title. I'd suggest:
should normalize "12932689.645" as "12,932,689.645"

Copy link
Contributor

@reyraa reyraa left a comment

Choose a reason for hiding this comment

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

Overall seems good. thank you.
There's just one point. There're requests for adding other currencies. to take the possibility of adding other currencies into account, I asked Yashar to keep it's normaliser (FormattedNumber) free of lisk name.
so we can use the same component for (for example) showing USD.
Please use a more global name for files and components

@slaweet slaweet merged commit 439b3d4 into development Jul 21, 2017
@slaweet slaweet deleted the 501-lisk-amout-component branch July 21, 2017 13:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants