Skip to content
This repository was archived by the owner on Apr 30, 2018. It is now read-only.

Watch multiCheckbox model value to support async options #33

Merged
merged 2 commits into from
Jun 29, 2015

Conversation

m0t0r
Copy link
Contributor

@m0t0r m0t0r commented Jun 29, 2015

Hi, @kentcdodds. I've added changes to watch model value so that support async options values (related to #32 ). Please, take a look. I didn't find a better approach. For performance improvement I use for loop over angular.forEach which is not the most performant solution.
As a last thing I added documentation for the multiCheckbox form field. I'll add an example for this field once the PR gets merged.

Thank you

@kentcdodds
Copy link
Member

Sounds good. I don't think there's really a concern for performance where you swapped the for loop for the angular.forEach loop, but meh. Just making sure, this covers your use case right? You tested this with your stuff and it works?

@m0t0r
Copy link
Contributor Author

m0t0r commented Jun 29, 2015

Yes, I've created a demo example locally and it works as expected

@m0t0r
Copy link
Contributor Author

m0t0r commented Jun 29, 2015

Unit tests are not established here, so I didn't create test cases

@kentcdodds
Copy link
Member

Yeah, this library still needs some love for sure. I'll merge this and create a release. Perfect PR. Thanks.

kentcdodds pushed a commit that referenced this pull request Jun 29, 2015
Watch multiCheckbox model value to support async options
@kentcdodds kentcdodds merged commit 134dbe0 into formly-js:master Jun 29, 2015
@m0t0r
Copy link
Contributor Author

m0t0r commented Jun 29, 2015

Cool, thank you 👍

@kentcdodds
Copy link
Member

Done: npm install [email protected]

@m0t0r
Copy link
Contributor Author

m0t0r commented Jun 30, 2015

@kentcdodds can you publish changes to bower, please ? I cannot get 4.4.0 on bower

@kentcdodds
Copy link
Member

Whoops! I forgot that my npm run release is different in this repo and I forgot a step. It should be available now!

@m0t0r
Copy link
Contributor Author

m0t0r commented Jun 30, 2015

@kentcdodds I am afraid it will not work, there was no new release created. I still get previous version.

@kentcdodds
Copy link
Member

Ah! :-( I didn't notice my console totally borked on me. Try again, I just pushed it :-( Sorry @m0t0r!

@m0t0r
Copy link
Contributor Author

m0t0r commented Jun 30, 2015

No problem. It works now, thanks for support 👍

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

Successfully merging this pull request may close these issues.

2 participants