feat(button-toggle): general component cleanup and support value input in multiple mode #9191

crisbeto · 2018-01-02T13:42:27Z

Reworks the MatButtonToggleGroup component to remove the need for the MatButtonToggleGroupMultiple component and to avoid having to implement features in two places.
Reworks the MatButtonToggleGroup to use the SelectionModel for managing its state.
Switches all of the async button toggle tests to run in fakeAsync.

As a side-effect of the above-mentioned changes, the toggle group in multiple mode now supports the same inputs as the one in single-selection mode (value input/output, name input, change event etc.).

Note: this is along the same lines as #2773, however #2773 got left behind for too long and it would need a lot more work to fit in our current setup.

Fixes #9058.

jelbourn · 2018-01-02T22:41:36Z

src/lib/button-toggle/button-toggle.ts

  }
+  set selected(selected: MatButtonToggle|MatButtonToggle[]|null) {


We usually have spaces between the unioned types

jelbourn · 2018-01-02T22:58:37Z

src/lib/button-toggle/button-toggle.ts

  }
-})
-export class MatButtonToggleGroupMultiple extends _MatButtonToggleGroupMixinBase


This would be a major change since it totally removes the symbol; people may have been using @ViewChild etc. to grab hold of these. Can we leave the symbol in as an alias for the combined one?

crisbeto · 2018-01-03T11:14:20Z

Addressed the feedback @jelbourn.

tinayuangao · 2018-01-09T23:56:34Z

src/lib/button-toggle/button-toggle.ts

+  private _vertical = false;
+  private _multiple = false;
+  private _selectionModel: SelectionModel<MatButtonToggle>;
+  private _tempValue: any;


Add some comments about how this variable is used?

tinayuangao · 2018-01-10T00:27:19Z

src/lib/button-toggle/button-toggle.ts

+   * @param isUserInput Whether the change was a result of a user interaction.
+   */
+  _syncButtonToggle(toggle: MatButtonToggle, select: boolean, isUserInput = false) {
+    if (!this.multiple && this.selected && !toggle.checked) {


Why do we check !toggle.checked here? add some comments?

jelbourn · 2018-01-10T01:59:38Z

src/lib/button-toggle/button-toggle.ts

+/**
+ * @deprecated Use `MatButtonToggleGroup` instead.
+ */
+export class MatButtonToggleGroupMultiple {}


Add a unit test that queries for this?

crisbeto · 2018-01-10T18:39:02Z

Addressed the feedback @tinayuangao @jelbourn.

jelbourn

LGTM

tinayuangao

LGTM

jelbourn · 2018-01-22T20:26:12Z

@crisbeto could you rebase this?

crisbeto · 2018-01-22T20:39:38Z

Rebased.

Yeradon · 2018-02-02T14:09:09Z

Any updates on this one? @crisbeto @jelbourn @tinayuangao

rafaelss95 · 2018-02-19T23:50:58Z

src/lib/button-toggle/button-toggle.ts


  /** Whether the toggle group is vertical. */
  @Input()
  get vertical(): boolean { return this._vertical; }
-  set vertical(value: boolean) { this._vertical = coerceBooleanProperty(value); }
-  private _vertical: boolean = false;
+  set vertical(value) {


I think you have removed the type annotation here and in other places by accident, no?

rafaelss95 · 2018-03-01T01:14:39Z

@crisbeto can you add closes #8791 here, please?

tinayuangao · 2018-03-12T18:36:54Z

src/lib/button-toggle/button-toggle.ts

+  ngAfterContentInit() {
+    // If there was an attempt to assign a value before init, use it to set the
+    // initial selection, otherwise check the `checked` state of the toggles.
+    if (this._tempValue) {


We need this._tempValue !== null here to accept falsy values.

…t in multiple mode * Reworks the `MatButtonToggleGroup` component to remove the need for the `MatButtonToggleGroupMultiple` component and to avoid having to implement features in two places. * Reworks the `MatButtonToggleGroup` to use the `SelectionModel` for managing its state. * Switches all of the `async` button toggle tests to run in `fakeAsync`. As a side-effect of the above-mentioned changes, the toggle group in multiple mode now supports the same inputs as the one in single-selection mode (`value` input/output, `name` input, `change` event etc.). Note: this is along the same lines as angular#2773, however angular#2773 got left behind for too long and it would need a lot more work to fit in our current setup. Fixes angular#9058.

rhutchison · 2018-03-26T00:25:37Z

Thank you! This should resolve #1039

angular-automatic-lock-bot · 2019-09-08T13:02:03Z

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.}

crisbeto added the pr: needs review label Jan 2, 2018

crisbeto assigned jelbourn and tinayuangao Jan 2, 2018

crisbeto requested a review from tinayuangao as a code owner January 2, 2018 13:42

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 2, 2018

crisbeto mentioned this pull request Jan 2, 2018

refactor(button-toggle): switch to SelectionModel #2773

Closed

jelbourn reviewed Jan 2, 2018

View reviewed changes

crisbeto mentioned this pull request Jan 3, 2018

chore(radio): change declaration of MatRadioChange class #9201

Merged

crisbeto force-pushed the button-toggle-model-again branch from ce873ae to 8af2daf Compare January 3, 2018 11:14

tinayuangao reviewed Jan 10, 2018

View reviewed changes

jelbourn reviewed Jan 10, 2018

View reviewed changes

crisbeto force-pushed the button-toggle-model-again branch from 8af2daf to 2a27883 Compare January 10, 2018 18:38

jelbourn added the target: minor This PR is targeted for the next minor release label Jan 10, 2018

jelbourn approved these changes Jan 10, 2018

View reviewed changes

jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Jan 10, 2018

tinayuangao approved these changes Jan 10, 2018

View reviewed changes

crisbeto force-pushed the button-toggle-model-again branch from 2a27883 to 3cacceb Compare January 22, 2018 20:39

crisbeto force-pushed the button-toggle-model-again branch from 3cacceb to 4619d4b Compare January 26, 2018 19:22

crisbeto force-pushed the button-toggle-model-again branch from 4619d4b to e7f04df Compare February 3, 2018 17:15

crisbeto force-pushed the button-toggle-model-again branch from e7f04df to 81d63df Compare February 19, 2018 21:39

rafaelss95 reviewed Feb 19, 2018

View reviewed changes

crisbeto force-pushed the button-toggle-model-again branch 2 times, most recently from 7158d0e to 6ff097d Compare February 25, 2018 20:23

tinayuangao reviewed Mar 12, 2018

View reviewed changes

crisbeto force-pushed the button-toggle-model-again branch from 6ff097d to c5da822 Compare March 12, 2018 18:37

crisbeto force-pushed the button-toggle-model-again branch from c5da822 to 79fcf6c Compare March 12, 2018 18:37

mmalerba merged commit a403bac into angular:master Mar 13, 2018

This was referenced Mar 27, 2018

[MatButtonToggle] allow property type to be an @Input #10585

Closed

[button-toggle-group] bound to NgModel throws error if multiple selection is used #1039

Closed

This was referenced Mar 31, 2018

fix(button-toggle): dirty on load when used with ngModel #4898

Closed

md-button-toggle-group starts off as dirty #4888

Closed

mmalerba mentioned this pull request Apr 3, 2018

Regression in MatButtonToggleGroup: Value not applied on buttons model update #10690

Closed

tinayuangao mentioned this pull request Apr 16, 2018

[Feature request] "mat-button-toggle-group" with "multiple=true/false" #10867

Closed

angular-automatic-lock-bot bot unassigned tinayuangao Sep 8, 2019

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

		}
		set selected(selected: MatButtonToggle\|MatButtonToggle[]\|null) {

feat(button-toggle): general component cleanup and support value input in multiple mode #9191

feat(button-toggle): general component cleanup and support value input in multiple mode #9191

Uh oh!

Conversation

crisbeto commented Jan 2, 2018

Uh oh!

jelbourn Jan 2, 2018

Choose a reason for hiding this comment

Uh oh!

jelbourn Jan 2, 2018

Choose a reason for hiding this comment

Uh oh!

crisbeto commented Jan 3, 2018

Uh oh!

tinayuangao Jan 9, 2018

Choose a reason for hiding this comment

Uh oh!

tinayuangao Jan 10, 2018

Choose a reason for hiding this comment

Uh oh!

jelbourn Jan 10, 2018

Choose a reason for hiding this comment

Uh oh!

crisbeto commented Jan 10, 2018

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

tinayuangao left a comment

Choose a reason for hiding this comment

Uh oh!

jelbourn commented Jan 22, 2018

Uh oh!

crisbeto commented Jan 22, 2018

Uh oh!

Yeradon commented Feb 2, 2018

Uh oh!

rafaelss95 Feb 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rafaelss95 commented Mar 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tinayuangao Mar 12, 2018

Choose a reason for hiding this comment

Uh oh!

crisbeto Mar 12, 2018

Choose a reason for hiding this comment

Uh oh!

rhutchison commented Mar 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

rafaelss95 Feb 19, 2018 •

edited

Loading

rafaelss95 commented Mar 1, 2018 •

edited

Loading

rhutchison commented Mar 26, 2018 •

edited

Loading