Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<select /> for number with *ngFor number #23

Closed
philjones88 opened this issue Dec 11, 2017 · 24 comments
Closed

<select /> for number with *ngFor number #23

philjones88 opened this issue Dec 11, 2017 · 24 comments

Comments

@philjones88
Copy link
Contributor

Hey,

Can't seem to get this working.

Lets say you have the model:

export class Template {
   barId: number
}

and an array of Bar options:

export class Bar {
   id: number;
   label: string;
}

and you try to combine them in a form like:

template$: Observable<FormGroupState<Template>>;
bars$: Observable<Bar[]>;
<select [ngrxFormControlState]="(template$ | async)?.controls?.barId">
    <option *ngFor="let b of (bars$ | async)" [ngValue]="b.id">{{b.label}}</option>
</select>

I cannot get it to select the saved value.

Any ideas?

@MrWolfZ
Copy link
Owner

MrWolfZ commented Dec 11, 2017

Use [value] instead of [ngValue]. While developing this library I found a solution that makes the ngValue from Angular unnecessary.

@philjones88
Copy link
Contributor Author

Hmm. Still no luck.

image

I kinda feel like it's doing something wrong, value and the reflected value don't match (the reflected value is correct)

@MrWolfZ
Copy link
Owner

MrWolfZ commented Dec 11, 2017

Indeed, those two values are different on purpose. The value is set inside the form view adapter to the index of the option to track it internally. The ng-reflect-value is your own value. What is the value you receive in the barId property of the form value?

@philjones88
Copy link
Contributor Author

in this case barId is set to 2 (of type number)

and bars looks like:

[ { "id": 1, "label": "Admin" }, { "id": 3, "label": "Operator" }, { "id": 5, "label": "Foo" }, { "id": 2, "label": "User" } ]

@MrWolfZ
Copy link
Owner

MrWolfZ commented Dec 11, 2017

But what is selected in the view? "User"?

@philjones88
Copy link
Contributor Author

"Admin" :(

@philjones88
Copy link
Contributor Author

Interesting. I think it's something wrong with the *ngFor. If I hard code the options like:

<option [value]="1">Admin</option>
<option [value]="3">Operator</option>
<option [value]="5">Foo</option>
<option [value]="2">User</option>

it selects User properly.

@MrWolfZ
Copy link
Owner

MrWolfZ commented Dec 11, 2017

I'll try your example after work.

@MrWolfZ
Copy link
Owner

MrWolfZ commented Dec 11, 2017

I tried this locally by changing the simple form from the example app to use *ngFor with your example values and it worked just fine.

options$ = Observable.of([
  { 'id': 1, 'label': 'Admin' },
  { 'id': 3, 'label': 'Operator' },
  { 'id': 5, 'label': 'Foo' },
  { 'id': 2, 'label': 'User' },
]);
<select [ngrxFormControlState]="formState.controls.favoriteColor">
  <option [value]="null"></option>
  <option *ngFor="let o of (options$ | async)" [value]="o.id">{{o.label}}</option>
</select>

Can you please provide a reproduction repo? Or at least the full code for this, i.e. the full component and reducer?

@philjones88
Copy link
Contributor Author

philjones88 commented Dec 12, 2017

There is something funky going on and I'm not sure what.

It seems all the <select />'s in my project have a tendency to do this, which is break and not select a value from a list.

Interestingly, if I swap to [ngModel]="(template$ | async)?.controls?.barId it binds properly unlike [ngrxFormControlState]="(template$ | async)?.controls?.barId"

Here's the select:

<form #templateForm="ngForm" novalidate class="horizontal-form" [ngrxFormState]="(template$ | async)">
<select name="templateId" [ngrxFormControlState]="(template$ | async)?.controls?.barId">
    <option [value]="-1">Select...</option>
    <option *ngFor="let p of (meta$ | async)?.templates" [value]="p.id">{{p.label}}</option>
</select>
</form>

The actions:

import { Action } from '@ngrx/store';

import {
  TemplateMetaViewModel,
  TemplateViewModel
} from '../../models/Template';

export const FETCH = '[Template] Fetch';
export const FETCH_NEW = '[Template] Fetch New';
export const FETCH_COMPLETE = '[Template] Fetch Complete';
export const META = '[Template] Meta';
export const META_COMPLETE = '[Template] Meta Complete';

export class Fetch implements Action {
  public readonly type = FETCH;

  constructor(public id: number) { }
}

export class FetchNew implements Action {
  public readonly type = FETCH_NEW;
}

export class FetchComplete implements Action {
  public readonly type = FETCH_COMPLETE;

  constructor(public template: TemplateViewModel) { }
}

export class Meta implements Action {
  public readonly type = META;
}

export class MetaComplete implements Action {
  public readonly type = META_COMPLETE;

  constructor(public meta: TemplateMetaViewModel) { }
}

export type Actions =
  Fetch |
  FetchNew |
  FetchComplete |
  Meta |
  MetaComplete;

The reducer for TemplateVIewModel isn't anything special:

import {
  FormGroupState,
  FormArrayState,
  createFormGroupState,
  formGroupReducer,
  updateGroup,
  setValue
} from 'ngrx-forms';

import * as editActions from '../actions/edit';
import { TemplateViewModel } from '../../models/Template';

export function createState(model: TemplateViewModel) {
  return createFormGroupState<TemplateViewModel>(
    'template',
    model
  );
}

export const initialState = createState({
    id: -1,
    barId: -1
  });

export function reducer(state = initialState, action: editActions.Actions): FormGroupState<TemplateViewModel> {
  const tenant = formGroupReducer(state, action);

  switch (action.type) {

    case editActions.FETCH_COMPLETE:
      return createState(action.template);

    default:
      return state;
  }
}

and the reducer for TemplateMetaViewModel:

import * as editActions from '../actions/edit';
import {
  TemplateMetaViewModel
} from '../../models/Template';

export function reducer(state = new TemplateMetaViewModel(), action: editActions.Actions): TemplateMetaViewModel {
  switch (action.type) {

    case editActions.META_COMPLETE:
      return action.meta;

    default:
      return state;
  }
}

the effects is basically just a HTTP request to an api like:

  @Effect()
  public load$ = this.actions$
    .ofType(editActions.FETCH)
    .flatMap((action: editActions.Fetch) => {
      return this.templateService.getTemplate(action.id)
        .map((template) => new editActions.FetchComplete(template))
        .catch((error) => Observable.from([
          new editActions.FetchError(error),
          new layoutActions.ErrorToast(error)
        ]));
    })
    .catch((error) => Observable.from([
      new editActions.FetchError(error),
      new layoutActions.ErrorToast(error)
    ]))
    .repeat();

the component:

import { Component, OnInit, OnDestroy, } from '@angular/core';
import { Observable } from 'rxjs/Observable';
import { Store } from '@ngrx/store';
import { FormGroupState } from 'ngrx-forms';
import * as templateState from '../../reducers';
import * as editState from '../../reducers/edit';
import * as editActions from '../../actions/edit';
import {
  TemplateMetaViewModel,
  TemplateViewModel
} from '../../../models/Template';

@Component({
  selector: 'template-edit',
  templateUrl: './template.component.html'
})
export class EditComponent implements OnInit, OnDestroy {

  public template$: Observable<FormGroupState<TemplateViewModel>>;
  public meta$: Observable<TemplateMetaViewModel>;

  constructor(private store: Store<templateState.State>) { }

  public ngOnInit(): void {
    this.store.dispatch(new editActions.Meta());

    this.activatedRoute.params
      .subscribe((params) => {
        if (params['id'] && params['id'] !== 'new') {
          this.store.dispatch(new editActions.Fetch(params['id']));
        } else {
          this.store.dispatch(new editActions.FetchNew());
        }
      });

    this.tenant$ = this.store
      .select(templateState.getTemplate)
      .distinctUntilChanged();

    this.meta$ = this.store
      .select(templateState.getMeta)
      .distinctUntilChanged();
  }

  public ngOnDestroy(): void {
  }
}

With TemplateViewModel being:

export class TemplateViewModel {
   id = -1;
   barId = -1
}

With TemplateMetaViewModel being:

export class ListItemViewModel {
    id: any;
    label: any;
    value: any;
}

export class TemplateMetaViewModel {
   uxProfiles: ListItemViewModel[] = [];
}

@philjones88
Copy link
Contributor Author

philjones88 commented Dec 12, 2017

Also the relevant package versions

"dependencies": {
    "@angular/animations": "5.1.0",
    "@angular/common": "5.1.0",
    "@angular/compiler": "5.1.0",
    "@angular/core": "5.1.0",
    "@angular/forms": "5.1.0",
    "@angular/http": "5.1.0",
    "@angular/platform-browser": "5.1.0",
    "@angular/platform-browser-dynamic": "5.1.0",
    "@angular/router": "5.1.0",
    "@ngrx/effects": "4.1.1",
    "@ngrx/store": "4.1.1",
    ....
    "ngrx-forms": "2.0.1",
    ....
    "rxjs": "5.5.5",
    "zone.js": "0.8.18"
},
"devDependencies": {
    "@angular/cli": "1.6.0",
    "@angular/compiler-cli": "5.1.0",
    "@angular/language-service": "5.1.0",
    "@angular/platform-server": "5.1.0",
    "@ngrx/store-devtools": "4.1.1",
    ....
    "typescript": "2.6.2"
}

Edit:

I've tried downgrading to Angular 5.0.5 / RXJS 5.5.2 and the last few versions of typescript (2.5.x/2.4.x) and that didn't make any difference

@philjones88
Copy link
Contributor Author

Clutching at straws but I pulled out the form element to a seperate component so I could avoid the async pipe in bindings.

Didn't work but interestingly again the [ngModel] binding does.

With:

<input type="text" [ngrxFormControlState]="template.controls.barId">

<select name="barId" [ngModel]="template.value.barId">
  <option [value]="-1">Select...</option>
  <option *ngFor="let p of meta.templates" [ngValue]="p.id">{{p.label}}</option>
</select>

<select name="barId2" [ngrxFormControlState]="template.controls.barId">
  <option [value]="-1">Select...</option>
  <option *ngFor="let p of meta.templates" [value]="p.id">{{p.label}}</option>
</select>

You end up with:

image

So I'm not sure why NGRX-Forms is broken for me.

@MrWolfZ
Copy link
Owner

MrWolfZ commented Dec 12, 2017

I'll have another look later today but I have a social event so I can't guarantee I'll make it.

You could help me in tracking down this issue by debugging locally though. You should be able to find the source of ngrx-forms while running the app in e.g. Chrome. The file should be named something like "forms.es5.js" (note that Angular forms has the same filename, so pick the right one). In there find the function NgrxSelectViewAdapter and in there you'll find two interesting functions which you can debug with breakpoints. setViewValue is called whenever the value in the state changes. onChange is called whenever a new option is selected in the view. By debugging these functions you may be able to track down the issue.

My only theory right now is that somehow ngrx-forms and angular forms are interfering, but I don't know how that would be.

@philjones88
Copy link
Contributor Author

philjones88 commented Dec 12, 2017

Don't worry, I'm context switching to other tasks, this is an interesting problem...

I set these break points:

image

breakpoint 1: set is called with -1 then with 2 correctly

breakpoint 2:called with -1 then with 2 correctly, however selectedId is null when called with 2 and the function is never called again.

breakpoint 3: never hit

@MrWolfZ
Copy link
Owner

MrWolfZ commented Dec 12, 2017

Alright, it works like this: Each option registers itself with the parent select and gets assigned an ID. The select stores options in multiple mappings, one of which is value to ID. The getOptionId should find the index based on the value. Can you debug into it and see why it can't find the proper mapping? It seems like the options are not registering at all. Maybe it is indeed interference from angular forms.

In any case, at least I now know where to roughly search for the issue.

@philjones88
Copy link
Contributor Author

I think I can help you replicate it. I had a hunch it was down the "delay" caused by the options coming from a HTTP based observable.

If you do this:

    this.meta$ = Observable.of([
      { id: 1, label: 'foo' },
      { id: 2, label: 'SELECTED' }
    ]).delay(10000);

it fails to select the option, but this passes because it's fast?

    this.meta$ = Observable.of([
      { id: 1, label: 'foo' },
      { id: 2, label: 'SELECTED' }
    ]);

@philjones88
Copy link
Contributor Author

philjones88 commented Dec 12, 2017

Debugging the getOptionId it's simply empty (well, has 0: null) and it's never hit again when the meta$ observable resolves (using the above .delay(10000).

I can see the createOptionId and updateOptionValue get hit then the meta$ obserable resolves but the getOptionId is never hit.

@MrWolfZ
Copy link
Owner

MrWolfZ commented Dec 12, 2017

You are right. Whenever a new option registers itself I need to check whether the selected option changed and if so update the state.

I'll try to provide you with a fixed version to test it out.

@philjones88
Copy link
Contributor Author

Cool. Thanks for your help.

@MrWolfZ
Copy link
Owner

MrWolfZ commented Dec 12, 2017

Alright, I think I have the fix. Can you please install the version from this link and see if it fixes your issue?

Just extract the zip file into your project directory and then install it with

npm install ngrx-forms-2.1.0.tgz

This should override the version from npm.

@philjones88
Copy link
Contributor Author

👍 works correctly now! Many thanks, I'll await the 2.1.0 release :)

@MrWolfZ
Copy link
Owner

MrWolfZ commented Dec 13, 2017

It'll be 2.1.1 actually. I had to release 2.1.0 yesterday for another bugfix. I'll release the new version later today.

@MrWolfZ
Copy link
Owner

MrWolfZ commented Dec 13, 2017

I have just released version 2.1.1 which contains a fix for this issue.

@devakone
Copy link

devakone commented Feb 7, 2020

For anybody else running into this issue, I was still able to reproduce it with 6.x with code like this:

<select [ngrxFormControlState]="(template$ | async)?.controls?.barId">
    <option value="">Select an option</option>
    <option *ngFor="let option of (options$ | async)" [value]="option.id">{{option.label}}</option>
</select>

If you've paid attention, which I didn't and wasted enough hours on you'd notice that on the *ngFor i correctly binded to [value] but not on the initial empty option. That throws off ngrx-forms. To get this to work properly make sure to bind value on the "empty" option as well, as in:

<select [ngrxFormControlState]="(template$ | async)?.controls?.barId">
    <option [value]="">Select an option</option>
    <option *ngFor="let option of (options$ | async)" [value]="option.id">{{option.label}}</option>
</select>

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

No branches or pull requests

3 participants