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.

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

add format parser #53

Merged
merged 1 commit into from
Mar 10, 2017
Merged

Conversation

Jiramew
Copy link
Contributor

@Jiramew Jiramew commented Mar 6, 2017

Add parser to extract value from format.
Default parser will be:

string.replace(/[^\w\.-]*/g, '') 

fix ant-design/ant-design#5178

Thanks so much
Jiramew

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 93.249% when pulling 7a80304 on Jiramew:feature/format_parser into cd1f2f8 on react-component:master.

src/mixin.js Outdated
if (this.props.parser) {
input = this.props.parser(input);
} else {
input = input.replace(/[^\w\.-]*/g, '');
Copy link
Member

Choose a reason for hiding this comment

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

maybe set defaultProps.parser is better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks so much~ (^^)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 93.644% when pulling 0c6ee4d on Jiramew:feature/format_parser into cd1f2f8 on react-component:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 93.644% when pulling e88e192 on Jiramew:feature/format_parser into cd1f2f8 on react-component:master.

README.md Outdated
<tr>
<td>parser</td>
<td>Function</td>
<td></td>
Copy link
Member

Choose a reason for hiding this comment

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

parser default value should be show in document

@paranoidjk
Copy link
Member

always, it's better to use rebase -i to merge your multiple sommit before git push 😄

Copy link
Member

@afc163 afc163 left a comment

Choose a reason for hiding this comment

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

I think a parser may make thing complex and would be confused with formatter.

I will check it later.

using defaultProps for parser

README add default parser
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 93.644% when pulling 3ad82a0 on Jiramew:feature/format_parser into cd1f2f8 on react-component:master.

@afc163
Copy link
Member

afc163 commented Mar 7, 2017

Maybe renamed parse to unformatter would be more clear?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 93.644% when pulling 7cf58bc on Jiramew:feature/format_parser into cd1f2f8 on react-component:master.

@afc163
Copy link
Member

afc163 commented Mar 7, 2017

Since formatter and parser basicly must be defined at same time. How about this one?

function formatter(value, direction) { // direction will be 'format' or 'parse'
  if (direction === 'parse') {
    return value.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ','); 
  } else {
    return `$ ${this.numberWithCommas(value)} boeing737`; 
  }
}

@paranoidjk
Copy link
Member

maybe like this

props.transform = {
 formatter: value => value, // default maybe
 parse: value => value
}

@afc163 afc163 merged commit e9b11ba into react-component:master Mar 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants