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): fix type error #283

Merged
merged 1 commit into from
Sep 15, 2017

Conversation

create-share
Copy link
Contributor

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?

Issue Number: N/A
#282

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[ ] No

Other information

@coveralls
Copy link

Coverage Status

Coverage remained the same at 40.722% when pulling e9c415b on deart1mer:fix-type-error-of-autosize into 46ae81a on NG-ZORRO:master.

@vthinkxie
Copy link
Member

duplicated #271 and it is not a bug but a hack.

@create-share
Copy link
Contributor Author

@vthinkxie Got it.
But I think it's hard to accept an error when running npm run start to develop.
It could be handled like disabled attribute in form element. If it it an attribute, no matter what the value is true. If it is set to a type of boolean or AutoSizeType as an input value, match the boolean | AutoSizeType type.

Such like this:

  set nzAutosize(value: string | boolean | AutoSizeType) {
    if (typeof value === 'string') {
      this._autosize = true;
    } else {
      this._autosize = <boolean | AutoSizeType>value;
    }

    if (this._autosize) {
      this._render.setAttribute(this.nativeElement, 'autosize', '');
    } else {
      this._render.removeAttribute(this.nativeElement, 'autosize');
    }
  }

Is that a good way?

@vthinkxie
Copy link
Member

vthinkxie commented Sep 13, 2017

@deart1mer yes, but it is strange that there is no error when I run npm run start, and CI is ok.
You can update your pr as your newest code if it can fix your environment.

@create-share
Copy link
Contributor Author

@vthinkxie OK :-)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 40.722% when pulling 92fe1c9 on deart1mer:fix-type-error-of-autosize into 46ae81a on NG-ZORRO:master.

@vthinkxie vthinkxie merged commit cd84e47 into NG-ZORRO:master Sep 15, 2017
@standbyme
Copy link
Contributor

😄 I thought I was the only one who ran into this bug before this PR....

@LinboLen
Copy link
Contributor

btw, I think autosize is ambiguity.

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.

5 participants