[Django] #27487: Using formfield_overrides with CheckboxSelectMultiple has two problems
Django
-----------------------------------------+------------------------
Reporter: nrogers64 | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27487>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
CheckboxSelectMultiple
--------------------------------------+------------------------------------
Reporter: nrogers64 | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
* component: Uncategorized => contrib.admin
Comment:
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`
[https://github.com/django/django/blob/ae660cde5a426f1a843b5bfdf8aadc12a54fa143/django/forms/widgets.py#L811-L818
shouldn't render with the required attribute].
--
Ticket URL: <https://code.djangoproject.com/ticket/27487#comment:1>
Django
CheckboxSelectMultiple
Reporter: nrogers64 | Owner: Adonys
Type: | Alea Boffill
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adonys Alea Boffill):
* owner: nobody => Adonys Alea Boffill
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/27487#comment:2>
Django
CheckboxSelectMultiple
-------------------------------------+-------------------------------------
Reporter: nrogers64 | Owner: Adonys
Type: | Alea Boffill
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adonys Alea Boffill):
* Attachment "#27487.diff" added.
Fixed add related object in admin popup for checkbox/radio
Django
CheckboxSelectMultiple
-------------------------------------+-------------------------------------
Reporter: nrogers64 | Owner: Adonys
Type: | Alea Boffill
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adonys Alea Boffill):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27487#comment:3>
Django
CheckboxSelectMultiple
-------------------------------------+-------------------------------------
Reporter: nrogers64 | Owner: Adonys
Type: | Alea Boffill
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Adonys Alea Boffill):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27487#comment:4>
Django
CheckboxSelectMultiple
-------------------------------------+-------------------------------------
Reporter: nrogers64 | Owner: Adonys
Type: | Alea Boffill
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
Comment:
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 [https://github.com/django/django/pull/6498
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27487#comment:5>
Django
CheckboxSelectMultiple
-------------------------------------+-------------------------------------
Reporter: nrogers64 | Owner: Adonys
Type: | Alea Boffill
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Adonys Alea Boffill):
* Attachment "#27487.diff" added.
Fixed add related object in admin popup for checkbox/radio
--
Django
CheckboxSelectMultiple
-------------------------------------+-------------------------------------
Reporter: nrogers64 | Owner: Adonys
Type: | Alea Boffill
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Adonys Alea Boffill):
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!
--
Ticket URL: <https://code.djangoproject.com/ticket/27487#comment:6>
Django
CheckboxSelectMultiple
-------------------------------------+-------------------------------------
Reporter: nrogers64 | Owner: Adonys
Type: | Alea Boffill
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adonys Alea Boffill):
* needs_better_patch: 1 => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27487#comment:7>
Django
CheckboxSelectMultiple
-------------------------------------+-------------------------------------
Reporter: nrogers64 | Owner: Adonys
Type: | Alea Boffill
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Adonys Alea Boffill):
* Attachment "ticket_27487.diff" added.
New patch with the corrections related to template-based widget rendering.
Django
CheckboxSelectMultiple
-------------------------------------+-------------------------------------
Reporter: nrogers64 | Owner: Adonys
Type: | Alea Boffill
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
As noted on the [https://github.com/django/django/pull/7805 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27487#comment:8>
Django
CheckboxSelectMultiple
-------------------------------------+-------------------------------------
Reporter: nrogers64 | Owner: Adonys
Type: | Alea Boffill
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/27487#comment:9>
Django
CheckboxSelectMultiple
Reporter: nrogers64 | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 1.10
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* owner: Adonys Alea Boffill => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/27487#comment:10>