jrief / django-angular

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

Fixes #297 #301

Closed craftycorvid closed 7 years ago

craftycorvid commented 7 years ago

Here's another possible solution for #297 that won't skip the get_context method of the current widget class. This seem cleaner than the previous solution and should avoid any potential infinite loops.

jrief commented 7 years ago

Why don't you just override update_widget_rendering_context() in the corresponding field class?

craftycorvid commented 7 years ago

Because widgets don't always need corresponding field classes. In my use case I am creating a widget that works with ModelMultipleChoiceField, but I have no need to modify the ModelMultipleChoiceField.

More importantly, the current implementation is relying on an implementation detail. It just so happens that Django currently does not override get_context on any of their field classes. If, say, TextInput needed a get_context() method then the current implementation would break.

jrief commented 7 years ago

OK, got it. I will look for a working solution.

craftycorvid commented 7 years ago

Actually, I misspoke. Looking over django/forms/widgets.py, plenty of Widgets are overriding get_context. For example, the get_context method on Select is skipped by the current implementation.

Is there something you don't like about my pull request? It does solve the issue and I have not seen it break anything in my testing thus far.