Opened 8 years ago
Last modified 21 months ago
#27487 new Cleanup/optimization
ModelAdmin.formfield_overrides on ManyToManyField isn't compatible with CheckboxSelectMultiple
Reported by: | nrogers64 | Owned by: | |
---|---|---|---|
Component: | contrib.admin | Version: | 1.10 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
The following ModelAdmin
code updates the widget, as expected:
formfield_overrides = { ManyToManyField: {'widget': CheckboxSelectMultiple}, }
However, it has two problems:
The first problem is that it provides the little green plus sign to add another object on the fly, but that functionality doesn't actually work properly with the CheckboxSelectMultiple
widget (after adding another object, you have to refresh the page in order to see it). So I would think that this widget shouldn't provide the little green plus sign.
The second problem might not be valid because of what I'm doing in my project. Currently, the <form>
in change_form.html has novalidate
hard-coded in:
I have overridden that in my project, removing the novalidate
attribute. I understand that my doing that might make this second problem invalid. The second problem is that the user has to check each of the checkboxes in order to submit the form.
Attachments (2)
Change History (12)
comment:1 by , 8 years ago
Component: | Uncategorized → contrib.admin |
---|---|
Summary: | Using formfield_overrides with CheckboxSelectMultiple has two problems → ModelAdmin.formfield_overrides on ManyToManyField isn't compatible with CheckboxSelectMultiple |
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
comment:2 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 8 years ago
To fix this ticket, i've opted for a similar solution to the formset behavior.
For the case of ChoiceFieldRenderer used on CheckboxSelectMultiple and RadioSelect widgets, an empty item will be added as first item of the list. This item could be used as a template for create a new element of the list(<ul></ul>). Respecting all the html customizations made by the developer.
The RelatedObjectLookups.js file is changed in order to handle the result given by the admin related object popup. I added some JavaScript's tests to validate the implementation of dismissAddRelatedObjectPopup
method for all cases.
Similar to the formset, I added an style class empty-choice
to keep hidden the empty item choice.
comment:5 by , 8 years ago
Patch needs improvement: | set |
---|
Thanks for the patch, but I'm not sure this approach will work. You can add the CSS for the admin, but that choice will appear everyone else Django renders forms.
By the way, your patch seems inverted -- all additions are removals and vice versa.
Also, we're hoping to merge template-based widget rendering for 1.11 so it would be nice if a patch is based on that branch. ChoiceFieldRenderer
, for example, is removed.
by , 8 years ago
Attachment: | #27487.diff added |
---|
Fixed add related object in admin popup for checkbox/radio
comment:6 by , 8 years ago
Thanks for your time... I can work to improve the patch, maybe adding an attribute with which you can change the default behavior in order to avoid render always the empty choice.
But, if ChoiceFieldRenderer
will be removed, then I have not option. I hope be more useful in the future.
To avoid this situation i'll try to share my idea on django-users mailing list
or #django IRC channel
before implement it.
On the other hand, you are totally true, I made git diff
in the inverse direction, I fixed that.
Thanks!
comment:7 by , 8 years ago
Patch needs improvement: | unset |
---|
Going back to the point about this ticket, and following the suggestion of that comment https://code.djangoproject.com/ticket/27487#comment:5, I implemented a new solution for this problem, this time using template-base widget rendering
recently merged to master.
This problem occur when you override the defaults Select
and SelectMultiple
widgets in the admin by RadioSelect
or CheckboxSelectMultiple
respectively, and you need to add a new object of the relationship. Both widgets are rendered as UL
HTML list, and there are not JavaScript
support for that in RelatedObjectLookups.js
. Every time when you add a new object of the relationship, you need to refresh the page to update the list with the added object.
I added support for these widgets in RelatedObjectLookups.js
. To fix that problem I added two attributes to the UL
list in multiple_input.html
, both attributes will have "true" or "false" as possibles values and are the following:
data-wrap-label
: Is "true" if the inputs of the list are wrapped into a label.data-multiple
: Is "true" if is a multiple select.
With that information I can handle the behavior of to add a new object of the relationship using RadioSelect
or CheckboxSelectMultiple
as widget.
To be sure of the correct operation, I added JavaScript
and Selenium
tests, the rest of the tests runs fine.
I will add a new patch, but if necessary I can make a Pull Request.
by , 8 years ago
Attachment: | ticket_27487.diff added |
---|
New patch with the corrections related to template-based widget rendering.
comment:8 by , 8 years ago
As noted on the PR, I'm not sure about adding the data attributes for all Django forms to solve this admin issue. Maybe you could summarize the problem and solution to the DevelopersMailingList to see how others feel.
comment:9 by , 8 years ago
Patch needs improvement: | set |
---|
comment:10 by , 21 months ago
Owner: | removed |
---|---|
Status: | assigned → new |
I'm not sure how much work will be required to make the green + work. It might be more feasible to remove it.
As for the second issue, I didn't try to reproduce it but I'm not sure what could be happening since
CheckboxSelectMultiple
shouldn't render with the required attribute.