fix: consistently coerce boolean and number properties to the correct type #7283

amcdnl · 2017-09-24T16:39:35Z

Addresses #6370

rafaelss95 · 2017-09-24T16:44:26Z

src/lib/slide-toggle/slide-toggle.ts

@@ -125,6 +125,7 @@ export class MdSlideToggle extends _MdSlideToggleMixinBase implements OnDestroy,
  @Input()
  get checked(): boolean { return this._checked; }
  set checked(value) {
+    value = coerceBooleanProperty(value);
    this._checked = !!value;


this._checked = coerceBooleanProperty(value);?

Yep, it should be that

coerceBooleanProperty already returns a boolean, you don't need !! :)

mmalerba · 2017-10-01T02:11:47Z

src/lib/datepicker/datepicker.ts

+  get touchUi() {
+    return this._touchUi;
+  }
+  set touchUi(value: any) {


I think its best to leave the types as boolean that way when people @ViewChild this and try to programmatically set it they at least know that they should be passing a boolean (and then behind the scenes we coerce it anyways it just in case it came from a template binding).

I just followed the pattern that disabled had right below it. I'll update both.

yeah I think there's a lot that use any, but we can use the expected type for new ones we add and maybe eventually fix all the any ones

mmalerba · 2017-10-01T02:17:26Z

src/lib/tabs/tab-group.ts

@@ -111,7 +111,7 @@ export class MatTabGroup extends _MatTabGroupMixinBase implements AfterContentIn

  /** The index of the active tab. */
  @Input()
-  set selectedIndex(value: number | null) { this._indexToSelect = value; }
+  set selectedIndex(value: number | null) { this._indexToSelect = coerceNumberProperty(value); }


it looks like the author was intentionally trying to allow null here in which case I think you want coerceNumberProperty(value, null)

The coerceNumberProperty signature does not support null as the second parameter. Should we extend it to support that?

Hmm, in a lot of cases people really do just want number. What if we did this?

export function coerceNumberProperty(value: any): number; export function coerceNumberProperty<D>(value: any, fallback: D): number | D; export function coerceNumberProperty(value: any, fallbackValue = 0) { ... }

It would allow people to get just a number in 1-arg case and number|whatever in 2-arg case. @jelbourn WDYT?

That seems good to me

Works well.

jelbourn · 2017-09-26T20:25:33Z

src/lib/menu/menu-directive.ts

-  @Input() overlapTrigger = this._defaultOptions.overlapTrigger;
+  @Input()
+  set overlapTrigger(value: any) {
+    this._defaultOptions.overlapTrigger = coerceBooleanProperty(value);


I don't think this one is quite right. The current behavior is to default the value to this._defaultOptions.overlapTrigger; the default shouldn't be changed.

jelbourn · 2017-10-11T21:21:49Z

src/lib/datepicker/datepicker.ts

@@ -224,7 +231,7 @@ export class MatDatepicker<D> implements OnDestroy {
    }
  }

-  ngOnDestroy() {
+  ngOnDestroy(): void {


Don't need to add types to lifecycle methods

jelbourn · 2017-10-11T21:22:18Z

src/lib/expansion/expansion-panel.ts

  /** Whether the toggle indicator should be hidden. */
-  @Input() hideToggle: boolean = false;
+  @Input()
+  get hideToggle(): any {


any -> boolean

…erce-6360 # Conflicts: # src/lib/expansion/expansion-panel.ts

jelbourn

LGTM

jelbourn · 2017-10-26T17:39:52Z

@amcdnl can you change the PR title to

fix: consistently coerce boolean and number properties to the correct type

?

amcdnl · 2017-10-30T15:07:14Z

done :)

josephperrott · 2017-11-09T18:34:03Z

src/cdk/accordion/accordion-item.ts

@@ -41,8 +42,10 @@ export class CdkAccordionItem implements OnDestroy {

  /** Whether the AccordionItem is expanded. */
  @Input()
-  get expanded(): boolean { return this._expanded; }
-  set expanded(expanded: boolean) {
+  get expanded(): any { return this._expanded; }


Shouldn't this getter now return boolean?

angular-automatic-lock-bot · 2019-09-07T17:02:35Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

chore(audit): audit boolean inputs angular#6370

ec833b4

amcdnl requested a review from jelbourn September 24, 2017 16:39

amcdnl requested review from andrewseguin, crisbeto, devversion, josephperrott, kara and mmalerba as code owners September 24, 2017 16:39

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 24, 2017

rafaelss95 reviewed Sep 24, 2017

View reviewed changes

jelbourn mentioned this pull request Sep 26, 2017

feat(menu): coercing overlapTrigger attribute #4882

Closed

amcdnl added 3 commits September 30, 2017 11:52

Merge branch 'master' into coerce-6360

fe0f358

chore(nit): nit feedback

be2fefd

chore(nit): remove !!

f89ae6f

mmalerba reviewed Oct 1, 2017

View reviewed changes

jelbourn reviewed Oct 1, 2017

View reviewed changes

chore(nit): update per feedback

682c2fd

jelbourn reviewed Oct 12, 2017

View reviewed changes

amcdnl added 2 commits October 16, 2017 10:52

Merge branch 'master' of https://github.com/angular/material2 into co…

dd67f88

…erce-6360 # Conflicts: # src/lib/expansion/expansion-panel.ts

chore(nit): address feedback

6095c65

jelbourn approved these changes Oct 26, 2017

View reviewed changes

jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Oct 26, 2017

amcdnl changed the title ~~chore(audit): audit boolean inputs #6370~~ fix: consistently coerce boolean and number properties to the correct type Oct 30, 2017

josephperrott reviewed Nov 9, 2017

View reviewed changes

josephperrott merged commit 3ca801a into angular:master Nov 10, 2017

amcdnl deleted the coerce-6360 branch November 19, 2017 19:13

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: consistently coerce boolean and number properties to the correct type #7283

fix: consistently coerce boolean and number properties to the correct type #7283

amcdnl commented Sep 24, 2017

rafaelss95 Sep 24, 2017

jelbourn Sep 26, 2017

amcdnl Sep 30, 2017

rafaelss95 Sep 30, 2017

amcdnl Sep 30, 2017

mmalerba Oct 1, 2017

amcdnl Oct 1, 2017

mmalerba Oct 1, 2017

mmalerba Oct 1, 2017

amcdnl Oct 1, 2017 •

edited

Loading

mmalerba Oct 1, 2017

jelbourn Oct 11, 2017

amcdnl Oct 16, 2017

jelbourn Sep 26, 2017

jelbourn Oct 11, 2017

jelbourn Oct 11, 2017

jelbourn left a comment

jelbourn commented Oct 26, 2017 •

edited

Loading

amcdnl commented Oct 30, 2017

josephperrott Nov 9, 2017

angular-automatic-lock-bot bot commented Sep 7, 2019

fix: consistently coerce boolean and number properties to the correct type #7283

fix: consistently coerce boolean and number properties to the correct type #7283

Conversation

amcdnl commented Sep 24, 2017

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

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

Choose a reason for hiding this comment

amcdnl Oct 1, 2017 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jelbourn left a comment

Choose a reason for hiding this comment

jelbourn commented Oct 26, 2017 • edited Loading

amcdnl commented Oct 30, 2017

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 7, 2019

amcdnl Oct 1, 2017 •

edited

Loading

jelbourn commented Oct 26, 2017 •

edited

Loading