angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.32k stars 6.73k forks source link

Add 'compareWith' option to mat-radio-group just like mat-select #10495

Open CurtisDS opened 6 years ago

CurtisDS commented 6 years ago

Bug, feature request, or proposal:

Feature request: There is an issue with the mat-radio-group. If you use an object as the value for the form control the corresponding radio button is not checked. It only uses reference based comparison for checking which button should be highlighted.

I would like the ability to specify a comparison method to use while the mat-radio-group is deciding which item is selected. Just like how you can already specify a 'compareWith' method for a mat-select control.

What is the expected behavior?

If you use basic values it works as expected. Example here. In this case the value is set to 1.1 and the 'one,one' button is selected.

What is the current behavior?

However when you use an array, it does not work. Example here. In this case the value is set to [1,1] and none of the buttons are selected. Even though the form control's value is clearly shown to be [1,1].

What is the use-case or motivation for changing an existing behavior?

I am currently trying to dynamically generate a form based on some data given back from an API. The api gives me choices and a selected value to show by default.

{ tag: 'radio button', value: '[0,1]', choices: [ { tag: 'option 1', value: [0,0] }, { tag: 'option 2', value: [0,1] } ] }

In the spec for mat-radio-group and mat-radio-button the value is of type any. So it should support any type of value I want to use.

The current mat-select control already possesses this feature and I would like to see it paralleled in the radio button control. Here is an example of the select control with the 'compareWith' attribute. As you can see it selects the "one,one" value by default.

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Angular: 5.2.6 @angular/material: 5.2.3 typescript: 2.4.2

julianobrasil commented 6 years ago

This is not material fault. That's how javascript works: you're comparing different objects. If you change your code to the following, it'll work:

seasons = [
  { value : [1,0], tag : 'one,zero' },
  { value : [2,0], tag : 'two,zero' },
  { value : [3,0], tag : 'three,zero' },
  { value : [1,1], tag : 'one,one' },
  { value : [2,1], tag : 'two,one' },
  { value : [3,1], tag : 'three,one' }
];

selectedValue  = this.seasons[3].value; // same array, not a 'similar' one
favoriteSeason: FormControl = new FormControl(this.selectedValue);

you were doing selectedValue=[1,1]; in your example code. To javascript (and almost every programming language), [1,1] == [1,1] is false: they are similar arrays, but not equal arrays (the array comparison actually compares references, not values).

CurtisDS commented 6 years ago

you were doing selectedValue=[1,1]; in your example code. To javascript (and almost every programming language), [1,1] == [1,1] is false: they are similar arrays, but not equal arrays (the array comparison actually compares references, not values).

Yes I understand that is how it works. I brought it up in the end bit of my post there. I consider it a bug to not use a value based comparison on objects for setting the selected item but I would be willing to change my wording and make it a feature request instead if that is more appropriate. Asking for the ability to provide my own comparison method to the radio group. So that I can have a values based comparison, instead of a reference based one.

julianobrasil commented 6 years ago

Well, other components have something similar to *ngFor's trackBy. I've never thought of mat-radio-group as a component similar to mat-select, but now I'm reconsidering my position. In the sense of this use case IMO, it would be nice to have such a feature, but someone from the material team will surely give you a more accurate info about it.

CurtisDS commented 6 years ago

Yes, I didn't realize select already had this feature. Thank you for that, it makes it a lot easier to communicate what I would like. I have edited my post.

CurtisDS commented 6 years ago

I found a semi work around. By using the 'checked' attribute of the radio button and setting it equal to the result of the same compare function written for the select. Here is that example. However there is still a hiccup where if you reset the value of the form control, while the form control is already set to the default value, it will uncheck all the radio buttons. So ill have to add more code to block the reset function from resetting the value if its already equal to the default value.

I still think implementing the same compareWith function is the better solution, especially for consistency. But I guess this is an ok workaround for now.

keatkeat87 commented 6 years ago

sometime, we change the select to radio group is just because of user experience. user want to see all selection all the time. so i think compare With is a necessary feature.

cvaliere commented 5 years ago

in case someone wants a directive to do that:

import { AfterContentInit, ContentChildren, Directive, EventEmitter, Input, OnChanges, Output, QueryList, SimpleChanges } from '@angular/core';
import { MatRadioButton } from '@angular/material';
import { AutoUnsubscribe } from '@bemyeye/auto-unsubscribe';
import { BehaviorSubject } from 'rxjs';
import { takeUntil } from 'rxjs/operators';

/**
 * Add 'compareWith' to mat-radio-group just like mat-select, because angular team didn't implement it
 * see https://github.com/angular/components/issues/10495
 */
@Directive({
  selector: 'mat-radio-group[bmeCompareWith]',
})
export class RadioGroupCompareWithDirective<T> extends AutoUnsubscribe implements AfterContentInit, OnChanges {

  @Input() bmeCompareWith: (o1: T, o2: T) => boolean;
  @Input() ngModel: T;

  @Output() ngModelChange = new EventEmitter<T>();

  @ContentChildren(MatRadioButton, { descendants: true }) radioButtons: QueryList<MatRadioButton>; // List of descendant RadioButtons

  ngOnChangesModel = new BehaviorSubject<T>(null);

  ngAfterContentInit() { // Use ngAfterContentInit to make sure that radioButtons is initialized
    this.ngOnChangesModel.pipe(
      takeUntil(this.componentDestroyed)
    ).subscribe(() => {
      const foundRadioButton = this.radioButtons.toArray().find(radioButton => { // Find a radio button whose value compares to ngModel
        return this.bmeCompareWith(radioButton.value, this.ngModel);
      });
      if (foundRadioButton) { // If radio button is found
        if (this.ngModel !== foundRadioButton.value) { // But its value is not already the ngModel
          this.ngModelChange.emit(foundRadioButton.value); // Then emit the new value
        }
      }
    });
  }

  ngOnChanges(changes: SimpleChanges): void {
    if (changes.ngModel) {
      this.ngOnChangesModel.next(changes.ngModel.currentValue);
    }
  }

}
cvaliere commented 5 years ago

with the class AutoUnsubscribe, which is useful too :)

import { OnDestroy } from '@angular/core';
import { Subject } from 'rxjs';

/**
 * Extend this class in order to autounsubscribe all subscriptions of a component onDestroy
 * Usage: replace XXX.subscribe() with XXX.takeUntil(this.componentDestroyed).subscribe()
 * See https://stackoverflow.com/questions/38008334/angular-rxjs-when-should-i-unsubscribe-from-subscription
 */
export class AutoUnsubscribe implements OnDestroy {
  protected componentDestroyed = new Subject<void>();

  constructor() {

    // wrap the ngOnDestroy to be an Observable. and set free from calling super() on ngOnDestroy.
    const _$ = this.ngOnDestroy.bind(this);
    this.ngOnDestroy = () => {
      this.componentDestroyed.next();
      this.componentDestroyed.complete();
      _$();
    };
  }

  ngOnDestroy() {
    // placeholder of ngOnDestroy. no need to do super() call of extended class.
  }
}
Diegovictorbr commented 5 years ago

Any updates on this? It is really necessary in edition forms when you want to set an object as a value for a radio group.

bradtaniguchi commented 4 years ago

I was able to fix this by using a similar approach as @cvalire, except in my case our overall formGroup (and thus formControl) is an observable. Basically re-applying the radioButtons value any-time the value changes, and using the compareWith to identify the correct option works as the radioButtons value is the same reference as before, regardless of what happens with our formGroup/formControl value.

CurtisDS commented 4 years ago

I ended up extending FormControl and making a RadioFormControl which stores a list and whenever you set the value of the control it looks up the value in the list using the lists object as a value instead... keeping the original object reference.

https://stackblitz.com/edit/mat-radio-group-with-object-as-value

But having official support for a compareWith function like with mat-select would still be preferable.

SC-JC commented 4 years ago

Any update on this, seems like a rather basic core feature for any value accessor where you can pass objects as values.

MikaStark commented 3 years ago

Not supported since version 5 and only P4...

BenLune commented 2 years ago

+1 here. I will deal without it as everyone here.

BenLune commented 2 years ago

Thank you @cvaliere for your workaround! I had the same need but with FormControl. I adapted your code with the FormControl as a SCAM directive.

import {
  AfterContentInit,
  ChangeDetectorRef,
  ContentChildren,
  Directive,
  Input,
  NgModule,
  OnDestroy,
  QueryList,
} from '@angular/core';
import { startWith, Subscription } from 'rxjs';
import { CommonModule } from '@angular/common';
import { MatRadioButton } from '@angular/material/radio';
import { FormControl } from '@angular/forms';

/**
 * Add 'compareWith' to mat-radio-group just like mat-select, because angular team didn't implement it
 * see https://github.com/angular/components/issues/10495
 */
@Directive({
  // eslint-disable-next-line @angular-eslint/directive-selector
  selector: 'mat-radio-group[rbCompareWith]',
})
export class RadioGroupCompareWithDirective<T>
  implements AfterContentInit, OnDestroy
{
  @Input() rbCompareWith!: (o1: T, o2: T) => boolean;
  @Input() formControl!: FormControl;

  @ContentChildren(MatRadioButton, { descendants: true })
  radioButtons!: QueryList<MatRadioButton>; // List of descendant RadioButtons

  formControlSub!: Subscription;

  constructor(private cdr: ChangeDetectorRef) {}

  ngAfterContentInit() {
    // Use ngAfterContentInit to make sure that radioButtons is initialized
    this.formControlSub = this.formControl.valueChanges
      .pipe(startWith(this.formControl.value))
      .subscribe((value) => {
        const foundRadioButton = this.radioButtons
          .toArray()
          .find((radioButton) => {
            // Find a radio button whose value compares to ngModel
            return this.rbCompareWith(radioButton.value, value);
          });
        if (foundRadioButton) {
          foundRadioButton.checked = true;
          this.cdr.detectChanges();
        }
      });
  }

  ngOnDestroy(): void {
    if (this.formControlSub) {
      this.formControlSub.unsubscribe();
    }
  }
}

@NgModule({
  imports: [CommonModule],
  declarations: [RadioGroupCompareWithDirective],
  exports: [RadioGroupCompareWithDirective],
})
export class RadioGroupCompareWithDirectiveModule {}
haskelcurry commented 2 years ago

Any updates on this?

adam-marshall commented 1 year ago

this is still a problem in 2023

note: although it is mentioned in the history above, to add context, it was added as an Angular feature request for their radio control value accessor https://github.com/angular/angular/issues/28486