feat(module:all): simplify boolean attributes usage #459

trotyl · 2017-10-20T05:02:20Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/NG-ZORRO/ng-zorro-antd/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] 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: closes #458, closes #648.

(There will be a different PR for #460)

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

trotyl · 2017-10-20T05:09:10Z

src/components/button/nz-button.component.spec.ts

      buttonDebugElement.nativeElement.click();
      expect(testComponent.isLoading).toBe(true);
-      setTimeout(_ => {


This is failing randomly in my local machine, fixed now. (Likely it wasn't working at all in sync function)

tick is fine

trotyl · 2017-10-20T05:35:22Z

src/components/button/nz-button.component.ts

@@ -40,7 +40,12 @@ export class NzButtonComponent implements AfterContentInit {

  @Input()
  set nzGhost(value: boolean) {
-    this._ghost = value;
+    const ghost = value as boolean | ''


This way it will not change the property type, user still get boolean value when access via API. (getter & setter must have same type)

trotyl · 2017-11-30T10:31:32Z

src/components/calendar/nz-calendar.component.ts

  @Input() nzMode = 'year';
  @Input() nzFullScreen = true;
  @Input() nzShowHeader = true;
  @Input() nzDisabledDate: Function;

  @Input()
-  get nzValue(): Date {


@Input() should always on setter rather than getter, same below.

trotyl · 2017-11-30T10:32:54Z

src/components/card/nz-card.component.ts

+  }
+
+  @Input()
+  @HostBinding('class.ant-card-no-hovering')


Theoretically @HostBinding() should on getter, but TypeScript requires all decorators on same part.

trotyl · 2017-11-30T10:38:30Z

src/components/dropdown/nz-dropdown-button.component.ts

@@ -67,15 +68,24 @@ import { NzDropDownComponent } from './nz-dropdown.component';
 })

 export class NzDropDownButtonComponent extends NzDropDownComponent implements OnInit, OnDestroy, AfterViewInit {
-  @Input() nzDisable = false;


Here is a breaking bug fix, using nzDisable instead of nzDisabled is just a typo, but should be mentioned in change log.

vthinkxie · 2017-11-30T16:12:50Z

should it be merged now or later?

trotyl · 2017-11-30T16:14:45Z

Later, still working on it... 😃

trotyl · 2017-12-01T02:40:09Z

src/components/spin/nz-spin.component.ts

  }

  @ViewChild('ref')
  _ref;

  @HostBinding('class.ant-spin-nested-loading')
-  private get isNested() {


@HostBinding() cannot use private since it's accessed by external class.

coveralls · 2017-12-01T09:41:07Z

Coverage decreased (-0.3%) to 41.451% when pulling 5773f28 on trotyl:boolean-attribute into 68163a4 on NG-ZORRO:master.

trotyl · 2017-12-01T09:48:47Z

Ready for review, likely it would require local checkout.

Coverage decrease is caused by extra getter/setter (which are functions and counted), could be ignored.

trotyl · 2017-12-01T10:37:33Z

src/components/menu/nz-submenu.component.ts

  get subItemSelected(): boolean {
-    return !!this.nzMenuComponent.menuItems.find(e => e.selected && e.nzSubMenuComponent === this);


It was accessing internal field originally, added type info for menuItems to allow type checking.

vthinkxie · 2017-12-04T09:18:47Z

src/showcase/nz-demo-button/nz-demo-button-loading.component.ts

-      this.isLoadingOne = false;
-    }, 5000);
-  };
-  loadTwo = (value) => {


will cause error when click Click me! button

will check it.

Thx, it's just by mistake.

coveralls · 2017-12-04T10:10:13Z

Coverage decreased (-0.3%) to 41.451% when pulling 1fbc608 on trotyl:boolean-attribute into 9073bc8 on NG-ZORRO:master.

coveralls · 2017-12-04T14:17:22Z

Coverage decreased (-0.3%) to 41.451% when pulling 0cd2d8b on trotyl:boolean-attribute into 5e341de on NG-ZORRO:master.

trotyl commented Oct 20, 2017

View reviewed changes

wilsoncook force-pushed the master branch from d10b7e0 to c16a512 Compare October 28, 2017 09:06

trotyl closed this Nov 30, 2017

trotyl reopened this Nov 30, 2017

trotyl commented Nov 30, 2017

View reviewed changes

NG-ZORRO deleted a comment from coveralls Nov 30, 2017

trotyl changed the title ~~[WIP] feat(module:TODO): simplify boolean attributes~~ [WIP] feat(module:all): simplify boolean attributes usage Nov 30, 2017

trotyl commented Dec 1, 2017

View reviewed changes

NG-ZORRO deleted a comment from coveralls Dec 1, 2017

trotyl changed the title ~~[WIP] feat(module:all): simplify boolean attributes usage~~ feat(module:all): simplify boolean attributes usage Dec 1, 2017

trotyl commented Dec 1, 2017

View reviewed changes

This was referenced Dec 2, 2017

ci: enable tslint check #661

Closed

perf: using pure setter with OnChange hook #647

Closed

vthinkxie reviewed Dec 4, 2017

View reviewed changes

feat(module:TODO): simplify boolean attributes

0cd2d8b

vthinkxie merged commit 08f10e4 into NG-ZORRO:master Dec 5, 2017

vthinkxie mentioned this pull request Dec 5, 2017

refactor(module:all): remove property for static host className #664

Merged

3 tasks

hsuanxyz pushed a commit to hsuanxyz/ng-zorro-antd that referenced this pull request Dec 6, 2017

feat(module:TODO): simplify boolean attributes (NG-ZORRO#459)

268d233

hsuanxyz pushed a commit to hsuanxyz/ng-zorro-antd that referenced this pull request Aug 5, 2020

feat(module:TODO): simplify boolean attributes (NG-ZORRO#459)

a03b1d7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(module:all): simplify boolean attributes usage #459

feat(module:all): simplify boolean attributes usage #459

trotyl commented Oct 20, 2017 •

edited

trotyl Oct 20, 2017 •

edited

tigercosmos Oct 27, 2017

trotyl Oct 20, 2017 •

edited

trotyl Nov 30, 2017

trotyl Nov 30, 2017

trotyl Nov 30, 2017

vthinkxie commented Nov 30, 2017

trotyl commented Nov 30, 2017

trotyl Dec 1, 2017

coveralls commented Dec 1, 2017

trotyl commented Dec 1, 2017 •

edited

trotyl Dec 1, 2017

vthinkxie Dec 4, 2017

trotyl Dec 4, 2017

trotyl Dec 4, 2017

coveralls commented Dec 4, 2017

coveralls commented Dec 4, 2017

		get subItemSelected(): boolean {
		return !!this.nzMenuComponent.menuItems.find(e => e.selected && e.nzSubMenuComponent === this);

feat(module:all): simplify boolean attributes usage #459

feat(module:all): simplify boolean attributes usage #459

Conversation

trotyl commented Oct 20, 2017 • edited

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

trotyl Oct 20, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trotyl Oct 20, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vthinkxie commented Nov 30, 2017

trotyl commented Nov 30, 2017

Choose a reason for hiding this comment

coveralls commented Dec 1, 2017

trotyl commented Dec 1, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Dec 4, 2017

coveralls commented Dec 4, 2017

trotyl commented Oct 20, 2017 •

edited

trotyl Oct 20, 2017 •

edited

trotyl Oct 20, 2017 •

edited

trotyl commented Dec 1, 2017 •

edited