Add MultiWidget support: fix FieldGroup data handling and renderer CSS classes - #269
Add MultiWidget support: fix FieldGroup data handling and renderer CSS classes#269Spawin wants to merge 4 commits into
Conversation
|
Instead of receiving a huge pull request, I would prefer a reproducable description of a problem you're trying to solve. This would give me the opportunity to look for a solution which fits into django-formset's mindset. |
Thank you for the feedback @jrief. This PR addresses bug #61, for which I had already opened PR #148 as a first attempt. Let me describe the problem properly. Problem descriptionWhen using a Django form with a 1. JavaScript crash on page load
This only happens when the sub-widgets inherit from 2. Framework CSS classes not applied to sub-widgets
Minimal reproducible exampleTo reproduce both issues, I added a test form and registered it at In short: from django import forms
from formset.forms import Form
class PhoneWidget(forms.MultiWidget):
def __init__(self, attrs=None):
super().__init__(
widgets=[forms.TextInput, forms.TextInput, forms.TextInput],
attrs=attrs,
)
def decompress(self, value):
return value.split('-') if value else ['', '', '']
class PhoneField(forms.MultiValueField):
widget = PhoneWidget()
def __init__(self, **kwargs):
super().__init__(
fields=(forms.CharField(), forms.CharField(), forms.CharField()),
**kwargs,
)
def compress(self, data_list):
return '-'.join(data_list)
class MultiWidgetForm(Form):
phone = PhoneField()Loading this form in any framework produces the JS crash. The sub-widget inputs also render without Expected behavior
I have a working implementation ready if you would like to review it once the design direction is confirmed. Happy to provide any additional information. |
|
Sorry, didn't have time yet to have a look at this pull request. |
Summary
Django's
MultiWidgetrenders sub-widgets as separate HTML elements with distinctnameattributes (e.g.phone_numbers_0,phone_numbers_1, …). Two independent layers were broken for any form usingMultiWidget:FieldGroupcrashed on initialization and submitted wrong dataform-control,uk-input) to MultiWidget sub-widgetsCommit 1 — Frontend
client/django-formset/DjangoFormset.tsRoot causes
assertUniqueName()threw when a FieldGroup containedInput-based sub-widgets with different names — the original code required all elements to share one name.The constructor discarded
Inputelements when aSelectwas also present in the same group:this.fieldElements = [selectElement]overwrote the already-collected inputs. This meant theTextInputvalue was silently lost in any mixed widget (TextInput + Select).aggregateValues()submitted data under the wrong key — everything ended up underprice_0instead of separateprice_0/price_1keys, breaking Django'sMultiWidget.value_from_datadict().Server error messages were never displayed —
this.namewas set toprice_0while Django returns field errors keyed byprice.Changes
Constructor (~line 126) — stop Select from overwriting Input elements
assertUniqueName()— allow differing names; derive base Django field nameNo longer throws when sub-widgets carry different names. When multiple distinct non-checkbox/radio names are detected (
seen.size > 1), strips the_Nsuffix to return the base name (phone_numbers,price), so server error mapping andsetFieldValuelookups work correctly.aggregateValue()— multi-element fallback for non-checkbox/radio fieldsNew
elsebranch collects{ name, value }pairs per sub-widget, serving as both the value capture and the detection signal used byaggregateValues().aggregateValues()— expand MultiWidget into separate keysDetects
{ name, value }object arrays and emits each sub-widget under its own key, matching Django's expectedfieldname_Nformat:Known limitation: MultiWidgets whose sub-widgets are all
ChoiceWidget-based silently drop every select after the first due to.at(0)in the constructor. This pre-existing edge case is out of scope.Commit 2 — Backend renderer + test fixtures + unit tests
formset/renderers/default.py— fix CSS classes on MultiWidget sub-widgetsMultiWidgetrenders viadjango/forms/widgets/multiwidget.html, which iterates sub-widgets using{% include widget.template_name %}. Django's{% include %}bypasses the form renderer'srender()method entirely, so_context_modifiers(which apply framework CSS classes) were never invoked for sub-widget templates.A single addition to
default.pyfixes all frameworks at once:Because
selfis the concrete renderer instance,self._context_modifiersresolves to the framework-specific mapping at runtime — no changes tobootstrap.py,bulma.py,tailwind.py, oruikit.py.form-controlform-selectinputuk-inputuk-selectformset-text-inputformset-selecttestapp/forms/multiwidget.py— test fixture (no third-party dependency)Two custom fields cover the two distinct failure scenarios:
PhoneField(3 ×TextInput) — exercises theassertUniqueNamecrash and multi-input data aggregation.PriceField(TextInput+Select, viaPriceWidget) — explicitly documents and exercises the constructor overwrite: before the fix, theSelectsilently discarded the already-collectedTextInput, making the amount value unrecoverable.testapp/tests/test_multiwidget.py— 17 unit tests (all passing)CSS rendering (11 tests): renders
PhoneWidgetandPriceWidgetdirectly with each framework renderer and asserts the correct CSS class on each sub-widget element. Covers Bootstrap, Bulma, UIkit, Tailwind, and the default renderer.Form validation (6 tests): submits data in the
fieldname_Nformat thataggregateValues()now emits, and asserts valid data passes,compress()returns the expected value, and missing or invalid sub-widget data fails with the correct field error.testapp/views.pyRegisters
MultiWidgetFormat themultiwidgetendpoint.