search

formly-js / angular-formly

JavaScript powered forms for AngularJS
http://docs.angular-formly.com
MIT License
2.23k stars 405 forks source link

feat(FormlyFormController): Exposes FormlyFormController so it can be used outside of formly-form #670

Closed pcardune closed 8 years ago

pcardune commented 8 years ago

What

This change simply moves FormlyFormController into a separate file and registers it with the $controller service. There is no change in functionality.

Why

For the formly plugin I am writing, I want to reuse the logic in the FormlyFormController with a different directive. This change makes the controller available to third party code via the $controller service. See an example of the usage here: https://github.com/Shoobx/plato-form/commit/aa609b49532af036274351acc014060ca8ee5d8c#diff-61cd5ccb0f47d71f982481c6bb4a2fa6R40

How

The diff is fairly self explanatory.

For issue #663

Checklist:

GitCop commented 8 years ago

There were the following issues with your Pull Request

Guidelines are available at https://github.com/formly-js/angular-formly/blob/master/CONTRIBUTING.md

This message was auto-generated by https://gitcop.com

codecov-io commented 8 years ago

Current coverage is 95.87%

Merging #670 into master will not affect coverage as of 25c8908

@@            master    #670   diff @@
======================================
  Files           16      17     +1
  Stmts         1163    1164     +1
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit           1115    1116     +1
  Partial          0       0       
  Missed          48      48

Review entire Coverage Diff as of 25c8908

Uncovered Suggestions

  1. +0.34% via ...alidationMessages.js#25...28
  2. +0.26% via ...s/formlyUsability.js#18...20
  3. +0.26% via src/test.utils.js#38...40
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

kentcdodds commented 8 years ago

I think this looks good. Could you update the commit message so the subject isn't so long?

I'll let someone else merge this once you've done that. Thanks!

BarryThePenguin commented 8 years ago

Awesome! I've been thinking of doing the same thing for a while now... If there's any tests in formly-form.test.js that don't have a compileAndDigest in them, would be good to seperate them into a different file too

BarryThePenguin commented 8 years ago

Maybe even just a controller test file that is empty for now?

pcardune commented 8 years ago

I managed to extract out the tests for the fieldTransform config option. A lot of the other tests look a bit more tricky to extract.

kentcdodds commented 8 years ago

LGTM! Thanks!