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

fix(module:input-number): fix display value after formatter changed #1371

Merged
merged 1 commit into from
May 1, 2018

Conversation

suraciii
Copy link
Contributor

@suraciii suraciii commented Apr 25, 2018

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Application (the showcase website) / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

After nzFormatter changed, the text on element will not apply the new formatter, unless changes value or unfocus.

What is the new behavior?

Text updates immediately if formatter changed.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@vthinkxie vthinkxie self-requested a review April 25, 2018 06:48
Copy link
Member

@vthinkxie vthinkxie left a comment

Choose a reason for hiding this comment

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

please update your code to fix ci.

@suraciii suraciii force-pushed the fix-input-number-formatter branch from 451c787 to b213870 Compare April 25, 2018 07:41
@codecov
Copy link

codecov bot commented Apr 25, 2018

Codecov Report

Merging #1371 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1371      +/-   ##
==========================================
+ Coverage   97.02%   97.07%   +0.04%     
==========================================
  Files         196      196              
  Lines        8339     8402      +63     
  Branches     1102     1136      +34     
==========================================
+ Hits         8091     8156      +65     
  Misses         31       31              
+ Partials      217      215       -2
Impacted Files Coverage Δ
...mponents/input-number/nz-input-number.component.ts 100% <100%> (+1.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20ae3c1...94fa7a0. Read the comment docs.

@vthinkxie
Copy link
Member

@csyszf please add test for the change, thanks.

@suraciii suraciii force-pushed the fix-input-number-formatter branch from b213870 to 2fb9347 Compare April 28, 2018 05:03
@suraciii
Copy link
Contributor Author

@vthinkxie test added.

@Input()
set nzFormatter(v: (value: number) => string | number) {
this._formatter = v;
this.writeValue(Number(this.actualValue));
Copy link
Member

@vthinkxie vthinkxie Apr 28, 2018

Choose a reason for hiding this comment

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

should be this.getCurrentValidValue(this.actualValue)

@suraciii suraciii force-pushed the fix-input-number-formatter branch from 2fb9347 to 94fa7a0 Compare April 29, 2018 04:20
@suraciii
Copy link
Contributor Author

@vthinkxie
If formatter is configured like value => `${value} %` and default value not been set, the display text will be "undefined %"
should it be considered a bug?

@vthinkxie
Copy link
Member

@csyszf maybe not I think.

@vthinkxie vthinkxie merged commit 179c1e2 into NG-ZORRO:master May 1, 2018
hsuanxyz pushed a commit to hsuanxyz/ng-zorro-antd that referenced this pull request Aug 5, 2020
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.

2 participants