Bug: radio-group breaks when changing options after initial render of directive

[ntziolis]

Use case:

• We have an observable that returns a list of options
  • The list of options changes based what the user has selected in previous form inputs
• The first time the options are rendered everything works as expected
  • Options are rendered
  • And only single option can be selected)
• The second time the options are rendered the directive breaks
  • Options are properly rendered
  • But now all options can be selected at the same time

Looking at the source of the directive this is likely due to only iterating through the options after initial render of the directive

Likely solution: The QueryList has a property changes which is an observable that captures any subsequent changes to the list of radioButtons

https://github.com/IgniteUI/igniteui-angular/blob/fa8c887b72fde85bb9ccb4a5e8b309b0083151ef/projects/igniteui-angular/src/lib/directives/radio/radio-group.directive.ts#L377

Please note:

• We have also tried if removing async form the html
• Instead we saved the new options array into a normal non obs variable
• But the directive still breaks
• So this is not an observable based issue it will happen with declarative programming as well

[ntziolis]

We have found a (very ugly) workaround to be able to keep using the radio button group, by re-rending the radio group fresh each time.

How:

• We created a structural directive based on ngIf
• The main difference to ngIf is:
  • every time there is new data
  • it clears the view container
  • afterwards it creates an embedded view with the templateRef

This directive renderIf can be used like ngIf (without the else part).

import {
  Directive,
  Input,
  OnInit,
  TemplateRef,
  ViewContainerRef,
} from '@angular/core';
import { UntilDestroy, untilDestroyed } from '@ngneat/until-destroy';
import { ReplaySubject, tap } from 'rxjs';

@UntilDestroy()
@Directive({
  selector: '[renderIf]',
})
export class RenderIfDirective<T = unknown> implements OnInit {
  private _context: RenderIfContext<T> = new RenderIfContext<T>();
  private _data$ = new ReplaySubject<T | null>();

  constructor(
    private _viewContainer: ViewContainerRef,
    private templateRef: TemplateRef<RenderIfContext<T>>
  ) {}

  @Input()
  set renderIf(data: T | null) {
    this._context.$implicit = this._context.renderIf = data as T;
    this._data$.next(data);
  }

  ngOnInit(): void {
    this._data$
      .asObservable()
      .pipe(
        untilDestroyed(this),
        tap((data) => {
          this._viewContainer.clear();

          if (data) {
            this._viewContainer.createEmbeddedView(this.templateRef, this._context);
          }
        })
      )
      .subscribe();
  }

  // taken from  https://github.com/angular/angular/blob/main/packages/common/src/directives/ng_if.ts
  // the below is required to recieve typing in the template inside the *renderIf tags

  // eslint-disable-next-line @typescript-eslint/member-ordering
  static ngTemplateGuard_renderIf: 'binding';

  // eslint-disable-next-line @typescript-eslint/member-ordering
  static ngTemplateContextGuard<T>(
    dir: RenderIfDirective<T>,
    ctx: unknown
  ): ctx is RenderIfContext<Exclude<T, null>> {
    return true;
  }
}

export class RenderIfContext<T = unknown> {
  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
  public $implicit: T = null!;
  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
  public renderIf: T = null!;
}

The radio group now works as expected, which is no surprise since its a fresh "instance" of the component each time.

Obviously this is a terrible way to circumvent this bug and the core problem should be fixed instead.

Did you have time to review yet? Or even better do you have a timeline for a fix?

Let me know if I can be of any assitance.

[simeonoff]

@ntziolis Thanks for reporting this. Would you be so kind to provide a running sample of the issue in Stackblitz/Codesandbox?

[ntziolis]

Sure thing, do you guys have a base repo setup with igx components already properly installed that I can use?

[simeonoff]

Here - you can use this sample as a base.

[ntziolis]

Sry somehow I missed the notification of this thread.

Here you go: https://stackblitz.com/edit/angular-ypuifx-yc5qxg?file=src/app/radio-styling-sample/radio-styling-sample.component.ts

I deliberately didn't make use of Observables as the issue is purely related to changing the options, no matter if via observable or declaratively.

Here is what you will be able to observe:

• When the initial options load everything works as expected, only one radio can be selected at a time
• Once the options change (after 5 sec) you can now select all options at the same time

[baleeds]

We've also been experiencing this issue for some time now. I think I first noticed something similar over a year ago. Our workaround is to remove the radio button group from the DOM and add it back every time the options change. It does mean that we see unnecessary blinking.

[ntziolis]

@baleeds the removing and adding to the Dom is exactly what we are doing as well.

The directive above does it in a way that we can easily identify where we had todo it and remove it (by simply reicht the directive) once this gets fixed.

[baleeds]

@ntziolis Our policy is to avoid using library components directly where possible, so we have our own radio button component wrapping the igx-radio-group. Our wrapping component uses a setter for the available options where it hides the igx-radio-group and sets an immediate timeout to bring it back.

Similar thought processes and advantages 💯

[baleeds]

@baleeds the removing and adding to the Dom is exactly what we are doing as well.

The directive above does it in a way that we can easily identify where we had todo it and remove it (by simply reicht the directive) once this gets fixed.

That directive seems generally very useful. It reminds me of the behavior of React's key which I have been missing in Angular.

[simeonoff]

@ntziolis I've updated your sample to bind the radio instances to ngModel. After binding everything works as expected.

[ntziolis]

@simeonoff I have had a look at your updated sample, here is what I found:

• While your workaround fixes the "multiple radios can be selected at once" issue
• It does not result in the correct behavior of the formControl
  • The valueChanges observable on the formControl only returns values for options that were rendered initially
  • I have extended your sample with a the valueChanges being rendered underneath the group:
    https://stackblitz.com/edit/angular-ypuifx-4ceqka?file=src/app/radio-styling-sample/radio-styling-sample.component.ts
• Please note we need to use reactive forms due to corp policy

As I described in my initial bug report:

• The core of the issue is not using the observable on the query list to receive any updates to the options inside the group.
• Using the observable would also eliminate the need for setTimeout that is used in the view init of the group.

[ntziolis]

Hi guys, any update?

[ChronosSF]

@ntziolis , sorry forgot to change the label! It's been in development since last week but it requires some refactoring so it might take a few more days. Hopefully, we can release it with the weekly patch next Monday.