Update so it works with Reactive forms #27

GenZodd · 2017-06-13T20:25:13Z

Address issue #22

Tested this locally with the code base that triggered the creation of this issue. Works for template and reactive. Locally the tests will not run but they also do not run with the main GIT master branch so seems to be a local issue. Please review and provide any feedback.

GenZodd · 2017-06-21T18:44:51Z

@SethDavenport Not sure if you are the right person to bug but what needs to happen to get this reviewed and brought in?

SethDavenport · 2017-06-21T21:20:35Z

Sorry for the delay on this - I actually haven't been super involved with angular-redux/form - I'm mostly the store and example-app guy. Unfortunately /form needs a new maintainer, and possibly a re-think, as you may have noticed.

My personal take is that we never intended to support the formBuilder stuff; in practice template-driven forms are much more declarative and therefore preferable.

@danielfigueiredo @clbond any thoughts on this change? It seems reasonable to me, but you have more experience with complex forms in Angular than I do.

SethDavenport · 2017-06-21T21:21:42Z

source/connect-base.ts

+    }
+    else{
+        formElement = this.form.control;
+      }


formatting :)

SethDavenport · 2017-06-21T21:22:02Z

README.md

@@ -203,6 +207,15 @@ the `path` property on our first `<select>` element, it would look like this:
 From there, `@angular-redux/form` is able to take that path and extract the value for
 that element from the Redux state.

+#### Reactive Forms
+The value in "connect" attribute is the value that will show up in the Redux store. The formGrup value is the name of hte object in your code that represents the form group.
+


There are few typos here

SethDavenport · 2017-06-21T21:22:32Z

source/module.ts

    ConnectArray,
  ],
  exports: [
-    Connect,
+    ConnectTemplateDirective,
+    ConnectReactiveDirective,


This represents a breaking API change - is there any way we can avoid this?

I don't see how, as the source of the form has now changed between the two. Maybe we can keep the template class named "connect" so that class stays the same for people plugged in for template driven forms? Then, at least to me, the name of the class is less declarative on what it does but may be an option.

No need for a breaking change.

Connect and

ReactiveConnect
are fine, too.

In @angular/forms its similar:

FormsModule

ReactiveFormsModule

yup I'm cool with that suggestion.

Updated PR based on this conversation to align.

SethDavenport · 2017-06-28T02:57:50Z

Give @angular-redux/form@6.5.0 a whirl and let me know.

ghost · 2017-11-06T17:24:32Z

Still have never gotten this to work. :/ Not once have I even seen a form show up in my state. Are the docs up to date?

todd.babbitt added 2 commits June 13, 2017 14:10

updated to work with reactive and template driven forms.

1a964bf

added a little documentation

c1454a6

SethDavenport reviewed Jun 21, 2017

View reviewed changes

todd.babbitt added 2 commits June 22, 2017 10:07

updated based on PR feedback

d61c0d0

updated based on PR feedback.

713664c

SethDavenport merged commit 6a75993 into angular-redux:master Jun 28, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update so it works with Reactive forms #27

Update so it works with Reactive forms #27

Update so it works with Reactive forms #27

Update so it works with Reactive forms #27

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment