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

7.0.0 Feature Planning #30

Open
SethDavenport opened this issue Jun 28, 2017 · 23 comments
Open

7.0.0 Feature Planning #30

SethDavenport opened this issue Jun 28, 2017 · 23 comments

Comments

@SethDavenport
Copy link
Member

@angular-redux/form needs a revamp. Since we've started working on a 7.0.0 release roadmap for /[email protected] (https://github.com/angular-redux/store/projects/2) this seems like a good opportunity to re-work /form in the same timeframe.

@SethDavenport
Copy link
Member Author

I'd like to get some input from the community about how you'd like to see redux+forms+ angular working together.

CC: @clbond @danielfigueiredo @renvrant @GenZodd @e-schultz @frederikaalund

@SethDavenport
Copy link
Member Author

For now, feel free to dump ideas in here. Since it's a major version, breaking changes or even a complete rewrite are possibly in scope.

@frederikaalund
Copy link

frederikaalund commented Jun 28, 2017

I'd like to see bidirectional data flow between redux and the forms.

I'd like to point your attention to my own go at an angular-redux/form directive that implements bidirectional data flow.

This is nice to have when some service/other component is updating the store and the form needs to instantly reflect the updates.

Also, if two forms are connected to the same store, changes made to any one form will instantly apply to the other. This strengthens the visual cue that these two forms are indeed representing the same data. Each form may represent said data differently (e.g., a slider and a number input).

@olaf89
Copy link

olaf89 commented Jun 28, 2017

I would like to have access to actions which are dispatched, to handle them in other reducers.
I would like to have possibility to dispatch actions handled by forms myself.
I would like to use template driven forms, with not much api overhead.
I would like to be able to mount store myself.

@GenZodd
Copy link
Contributor

GenZodd commented Jun 28, 2017

Currently, on form submission the form values are flushed from the store. I would like to be able to control this as when validating a form this can cause the store data to be cleared even though the fields still hold the values and the user is still trying to submit. This feeds into the bidirectional flow.

I would like to be able to use template forms
I would like to be able to use reactive forms

@smithad15
Copy link
Member

  • Splitting up the module definition to allow for the directives to be included in multiple modules without re-providing the forms store
  • Getting the test suite working again and CI added
  • Possibly reworking the build chain to better reflect the Angular Package Format (could be useful across all of angular-redux actually)

@SethDavenport
Copy link
Member Author

Definitely agree on the Angular package format thing, across all three packages. Feel like taking a stab?

@smithad15
Copy link
Member

Could do. The trick, as usual, will be finding the time :)

@rdinicut
Copy link

I think it will be very nice if the form will work with substores.

@jeffinmotion
Copy link

I'd love to see support for form validation. If that would be deemed too out-of-scope for the project, then more guidance on/recipes for.

@smithad15
Copy link
Member

Validation is on my roadmap too. I’ve got a solution that I’m piloting on a project right now that I want to fold back into this library

@jeffinmotion
Copy link

That's great! I'm in a position where I'm basically about to roll my own validation w/ this lib for a project, so if there's even a chance you could share your in-progress solution, it would help. Knowing there's validation being thought out for this, I'll likely want to implement it in the future when it's released officially. Starting from roughly the same place would certainly make that easier. 😄

@ghost
Copy link

ghost commented Aug 22, 2017

Does this mean that this package is going to be maintained? (Not meant as snark, genuinely asking)

@smithad15
Copy link
Member

@KSuttle-cng As best we can considering the usual concerns of work / life / OSS balance 😄

@JPeer264
Copy link

JPeer264 commented Aug 28, 2017

It would be a nice feature that radio-buttons or checkboxes which are preselected (maybe also inputs with given value attributes) will get stored into the store.

A mighty startup action would be really nice, where it parses the form, checks if something is predefined and stores it into the redux store. @@angular-redux/form/FORM_INIT would be a good naming of this action.

EDIT:
In this FORM_INIT it would be cool, if checkboxes and stuff would get set in the store correctly if the checked attribute is set.

@GenZodd
Copy link
Contributor

GenZodd commented Sep 2, 2017

I was working with module this week and ran into an issue with dynamically added form controls. When you do something like this:

let checkboxArray = new FormArray([
            new FormControl({name: 'false', checked: false}),
            new FormControl({name: 'false', checked: false}),
            new FormControl({name: 'true', checked: true})]);
this.myFormGroup = this.formBuilder.group({
            myValues: checkboxArray
        });

The store creates a model like this:

{ myFormGroup: { 
        myValues: {
                   0 : {name: 'true', checked: false}
            ......
}}}

However, when you check the checkbox the 0 store values gets updated to 0: true (so it does not maintain the object shape of the object bound to the control).

Here is the HTML binding:

<div class="checkbox" *ngFor="let item of this.myFormGroup.controls['myValues'].controls; let i = index">
            <label>
                <input 
                    type="checkbox"
                    [id]="i"
                    [checked]="item.value.checked"
                    [formControl]="item"/>
                    {{item.value.name}}
            </label>
        </div>

Would be great if 7.0 could handle dynamic forms fields like this.

@kuhnroyal
Copy link
Contributor

kuhnroyal commented Sep 4, 2017

@GenZodd In your example the form control returns the value true or false after you check it. Your original value is lost in that moment, checkbox is not meant to handle any object model, only boolean values. You can probably write a custom CheckboxControlValueAccessor and then it should work.

@kuhnroyal
Copy link
Contributor

I have done some work today that addresses a part of the issues here. I opened #42 for discussion and further ideas.

@GenZodd
Copy link
Contributor

GenZodd commented Sep 5, 2017

Ya, I guess that is my point, and maybe I did not explain it well, is that check boxes and forms don't play well together because there is really no way to understand what "thing" was checked. There is a record in the store of 0: true but it is hard to map back to what "thing" was in spot 0. Especially, when you are dynamically creating a list of checkboxes.

@JPeer264
Copy link

JPeer264 commented Sep 8, 2017

Thinking about the project scaffolding (for usage and contributing)

Contributors point of view:
It would be nice to have a good naming of the files. Like currently form-store is mixed with actions and the store. So it might be good to have files like form-reducer, form-types and form-actions, where everything is in this file which belongs in there.

Users point of view:
It would be easier to import types, reducers and so on if you want to manually trigger any state:

import { FormTypes } from '@angular-redux/form' 

Refactoring thoughts

Probably the functions in form-store: getState and subscribe are not really important, because here you access the entire store, which is not a real part of this library and gives a little overhead.
In my opinion, I wouldn't dispatch the method valueChanged directly, as this is should be made by the user in an action, maybe like following:

// form-actions.ts
import { Injectable } from '@angular/core';
import { NgForm } from '@angular/forms';
import { FORM_CHANGED } from 'form-types';

@Injectable()
export class FormActions {
  valueChanged<T>(path: string[], form: NgForm, value: T) {
    return {
      type: FORM_CHANGED,
      payload: {
        path,
        form,
        valid: form.valid === true,
        value
      }
    };
  }
}

So in connect-base.ts, where you use it, you can dispatch the action by using the NgReduxs dispatch.

@ghost
Copy link

ghost commented Oct 4, 2017

Re: validation: Would that be leveraging redux-observable as in: https://github.com/angular-redux/store/blob/master/articles/epics.md ?

@ghost
Copy link

ghost commented Nov 1, 2017

Are there considerations for leveraging: changeDetection: ChangeDetectionStrategy.OnPush? https://angular-2-training-book.rangle.io/handout/change-detection/how_change_detection_works.html

@maplion
Copy link

maplion commented Apr 16, 2018

@SethDavenport Are you still working on this project?

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

No branches or pull requests

10 participants