Skip to content
This repository has been archived by the owner on May 20, 2023. It is now read-only.

radio-group ngModel radio element not selected after change #61

Open
chalin opened this issue Mar 14, 2017 · 6 comments
Open

radio-group ngModel radio element not selected after change #61

chalin opened this issue Mar 14, 2017 · 6 comments

Comments

@chalin
Copy link
Contributor

chalin commented Mar 14, 2017

The simple app from this repo runs as expected initially and displays this radio group with the currentHero selected:

screen shot 2017-03-14 at 10 12 04 am

This is an excerpt from the template used to generate the view:

    <material-radio-group [(ngModel)]="currentHero">
        <material-radio *ngFor="let h of heroes" [value]="h">
            {{h.name}} ({{h.id}})
        </material-radio>
    </material-radio-group>

But if you click "Reset Heroes", the current hero is no longer selected in the radio group:

screen shot 2017-03-14 at 10 12 17 am

The reset action code is:

  void resetHeroes() {
    heroes = Hero.mockHeroes.map((hero) => new Hero.copy(hero)).toList();
    currentHero = heroes[0];
  }

The full source for this demo (generated from a stagehand Angular web app base), is here.
This app uses 3.0.0-alpha+1 version of Angular and the latest ACX.

This issue was originally discovered while working on the AngularDart 3.x sample for the Template Syntax page.

cc @kwalrath @filiph

@chalin
Copy link
Contributor Author

chalin commented Mar 14, 2017

@TedSander - oops, the code used to illustrate the issue wasn't using the proper version of ng and ACX. It is now.

@chalin
Copy link
Contributor Author

chalin commented Mar 14, 2017

@TedSander - I've added a vanilla ngDart version of the radio group to show that it works there but not in ACX. This is what the app looks like after a reset:

screen shot 2017-03-14 at 11 22 54 am

Notice that the vanilla ngDart radio group has Mr. Nice selected, but the ACX radio group has no selection.

This is the ngDart template HTML:

<p>Same functionality in vanilla AngularDart:</p>
<div *ngFor="let h of heroes">
    <input type="radio" name="heroes"
           [value]="h" [checked]="currentHero == h"
           (click)="currentHero = h">
    {{h.name}}
</div>

@nshahan
Copy link
Contributor

nshahan commented Mar 14, 2017

Do you need to recreate the list of heros every time? I tried moving that code to ngOnInit and that works:

class AppComponent implements OnInit {
  List<Hero> heroes;
  Hero currentHero;

  void ngOnInit() {
    heroes = Hero.mockHeroes.map((hero) => new Hero.copy(hero)).toList();
    resetHeroes();
  }

  void resetHeroes() {
    currentHero = heroes[0];
  }
}

@chalin
Copy link
Contributor Author

chalin commented Mar 14, 2017

Yes, that is part of the original demo.

In fact, it is the first part of a section illustrating the use of ngFors trackby feature.

@TedSander
Copy link
Contributor

TedSander commented Mar 15, 2017

I think I have a solution but I won't be able to implement it for awhile (going on vacation.)
A workaround is to set the value on the next turn:

void resetHeroes() {
  heroes = Hero.mockHeroes.map((hero) => new Hero.copy(hero)).toList();
  Timer.run(() {currentHero = heroes[0];});
}

Another option to to set the checked value on the radio itself like you did with the plain value above.

Longer explanation:
The code is here:
https://github.com/dart-lang/angular2_components/blob/master/lib/src/components/material_radio/material_radio_group.dart#L199

If there are no children previous this works as it saves the value to check on any children that may come later, but since there are already children it checks the current children for the value. In this case it actually finds the value, but it will be destroyed/re-created. Even if it doesn't find the value the value will be dropped as invalid.

Separate note: one should be careful about recreating lists without using tracked by. Throwing away the views associated with those list items and recreating them is usually not what the developer wants, and it is unperformant. We've had quite a few bugs with engineers returning a different list and them expecting the view wouldn't change as much as it did.

@chalin
Copy link
Contributor Author

chalin commented Mar 15, 2017

Thanks. Looking forward to the complete solution.

FYI, Timer.run(() {currentHero = heroes[0];}); doesn't work. I've updated the sample code in case you'd like to try it out yourself.

Separate note: one should be careful about recreating lists without using tracked by.

Right. This code is atypical since it is meant to illustrate why users would want to use trackBy.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants