search

jrief / django-angular

Let AngularJS play well with Django
http://django-angular.awesto.com/
MIT License
1.23k stars 294 forks source link

Renamed djangular to djng #239

Closed jrief closed 8 years ago

jrief commented 8 years ago

As announced in 2014, djangular will be renamed to djng in version 0.8.0.

Please rebase your forks and help me to test this refactored release.

adrienbrunet commented 8 years ago

This is some good announcement. =)

The demo with Angular messages does not work. (static files missing) I had no time yet to test it entirely but on my machine, angular message does not work, a problem seems to come from NgMessagesFieldErrorList .... ng-show="{2}.$submitted, {2} is replaced by an empty string.

Maybe I've done something wrong with my dependencies but Combined validation also shows some parse syntax error as well. I'll have another look as soon as I can.

Cheers

jrief commented 8 years ago

What about removing NgMessagesMixin completely and use ng-messages in normals forms? I my opinion it doesn't make much sense to support two different form rendering engines. After all, in django-angular-0.8 we can presume at least AngularJS version 1.3.

Does the NgMessagesMixin provide any functionality not available in NgFormBaseMixin?

adrienbrunet commented 8 years ago

Would make a lot of sens !

jrief commented 8 years ago

I just had a look at the form rendering code. If I change this from

<ul class="djng-field-errors" ng-show="U3Vic2NyaWJlRm9ybQ['first_name'].$dirty">
  <li ng-show="U3Vic2NyaWJlRm9ybQ['first_name'].$error.required" class="invalid">This field is required.</li>
  <li ng-show="U3Vic2NyaWJlRm9ybQ['first_name'].$error.minlength" class="invalid">Ensure this value has at least 3 characters</li>
  ...
</ul>

to say 

<ul class="djng-field-errors" ng-messages="U3Vic2NyaWJlRm9ybQ['first_name'].$error">
  <li ng-message="required" class="invalid">This field is required.</li>
  <li ng-message="minlength" class="invalid">Ensure this value has at least 3 characters</li>
  ...
</ul>

what do we gain? Sure if you have to write that HTML by hand its much nicer because its less HTML code, but since its generated code, who cares?

Something what disturbs me with ngMessages is, that I haven't seen any ngMessage-way of adding a green tick or a red cross. Can anybody show me an example where ngMessage can do more than what its possible by using ng-show, ng-hide together with .$error, .$pristine and .$dirty.

BTW, the djangular-messages code was broken even before version 0.8. Since I didn't write that code, and since the former maintainer somehow lost interrest, and since I don't use it in my projects I think the best solution is to remove it from the project.

How do others feel about this?

jrief commented 8 years ago

Is there anything ngMessages can do, what normal directives can't do?

@jkosir @adrienbrunet what do you think about removing support for ngMessages? Having non-working code is far from being ideal.

adrienbrunet commented 8 years ago

Yep, you're absolutely right. This is far from ideal. I plan to fix that this week end, I'm pretty sure this is almost nothing and I'd like to see what can we do with that. It seems more elegant but as you said, it's generated code, who cares. I'm not sure what the possiblities are. So far, I guess it does not had any value to our existing system.

adrienbrunet commented 8 years ago

@jamesbrobb said:

It adds a better flow for dealing with server errors, using a 'rejected' validator to display a returned field error. Plus there's also a significant reduction in watchers. I have 3 forms with a total of 9 fields and have seen a reduction of 40.

Maybe should we have a look at this watcher reduction...

jkosir commented 8 years ago

@jrief While ng-messages is nice and code looks better I agree it doesn't make much difference when the code is automatically generated.

Not sure how that affects number of watchers. While angular has performance issues with a lot of watchers I suppose you wouldn't usually have big enough forms to cause any problems with that.

I also agree to delete it, if nobody is interested in maintaining it.

adrienbrunet commented 8 years ago

Now that ng-message is removed, I tested the latest release (on the master branch) with my applications using it. It all works like a charm (after updating djangular --> djng where I was using it..)

Don't you think changing the name could allow us to promote the library to a version 1.0? This release is not major but it breaks for sure existing apps.

Could we consider merging PR#242 (update build grunt + fix for #241 ) before releasing the latest version?

jrief commented 8 years ago

I actually had no time yet to test it. Feel free to merge it. As of today I can't work on it anyway. Am 29.02.2016 23:28 schrieb "Adrien Brunet" notifications@github.com:

Now that ng-message is removed, I tested the latest release (on the master branch) with my applications using it. It all works like a charm (after updating djangular --> djng where I was using it..)

Don't you think changing the name could allow us to promote the library to a version 1.0? This release is not major but it breaks for sure existing apps.

Could we consider merging PR#242 (update build grunt + fix for #241 https://github.com/jrief/django-angular/issues/241 ) before releasing the latest version?

— Reply to this email directly or view it on GitHub https://github.com/jrief/django-angular/issues/239#issuecomment-190428877 .

adrienbrunet commented 8 years ago

merged.

jrief commented 8 years ago

@adrienbrunet @jkosir do you really consider django-angular as stable? well, then lets jump to version 1.0 :)

jkosir commented 8 years ago

About time yeah :D :+1:

adrienbrunet commented 8 years ago

I have been using for over 1 year now with production site. Only two are still going (business is business). If they survive their beta, I'll share with you the links =)

jrief commented 8 years ago

Just released an intermediate 0.8 in case someone complains. Will probably jump to 1.0 directly instead 0f 0.8.1.

jrief commented 8 years ago

Closing