Skip to content

Commit 43527ef

Browse files
authored
Fix #270: Resolve django_form limitations for multi-value and multi-select fields (#298)
Improved `django_form` submission handling to support multi-value form fields (e.g., `MultipleChoiceField`, `MultiValueField`) by fixing a client-side data serialization issue.
1 parent 27c4c57 commit 43527ef

8 files changed

Lines changed: 378 additions & 43 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ Don't forget to remove deprecated code on each major release!
4646
- Replaced `web.module_from_file`/`web.export` with `reactjs.component_from_file`.
4747
- Replaced `reactpy.backend.types` and `reactpy.core.types` imports with `reactpy.types`.
4848
- Renamed `Location.pathname` to `Location.path` and `Location.search` to `Location.query_string`.
49+
- Improved `django_form` submission handling to support multi-value form fields (e.g., `MultipleChoiceField`, `MultiValueField`) by fixing a client-side data serialization issue.
4950

5051
### Removed
5152

‎src/js/src/components.ts‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,31 @@ export class DjangoForm extends React.Component<DjangoFormProps> {
1010
event.preventDefault();
1111
const formData = new FormData(form);
1212

13-
// Convert the FormData object to a plain object
14-
const formObject = Object.fromEntries(formData.entries());
13+
// Accumulate duplicate keys into arrays to support multi-select fields
14+
// (e.g. MultipleChoiceField). Object.fromEntries would silently drop
15+
// duplicate entries, keeping only the last value per key.
16+
const formObject: Record<
17+
string,
18+
FormDataEntryValue | FormDataEntryValue[]
19+
> = {};
20+
for (const [key, value] of formData.entries()) {
21+
if (Object.prototype.hasOwnProperty.call(formObject, key)) {
22+
const existing = formObject[key];
23+
if (Array.isArray(existing)) {
24+
existing.push(value);
25+
} else {
26+
formObject[key] = [existing, value];
27+
}
28+
} else {
29+
formObject[key] = value;
30+
}
31+
}
1532

1633
onSubmitCallback(formObject);
1734
};
1835

1936
if (form) {
2037
form.addEventListener("submit", onSubmitEvent);
21-
// Store cleanup function in instance
2238
(this as any)._cleanup = () => {
2339
form.removeEventListener("submit", onSubmitEvent);
2440
};

‎src/reactpy_django/forms/components.py‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@
33
from asyncio import iscoroutinefunction
44
from logging import getLogger
55
from pathlib import Path
6-
from typing import TYPE_CHECKING, Any, Callable, Union, cast
6+
from typing import TYPE_CHECKING, Any, Callable, cast
77
from uuid import uuid4
88

99
from django.forms import Form, ModelForm
1010
from reactpy import component, hooks, html, utils
11-
from reactpy.core.events import event
1211
from reactpy.reactjs import component_from_file
1312

1413
from reactpy_django.forms.transforms import (
@@ -57,7 +56,8 @@ def _django_form(
5756
top_children_count = hooks.use_ref(len(top_children))
5857
bottom_children_count = hooks.use_ref(len(bottom_children))
5958
submitted_data, set_submitted_data = hooks.use_state({} or None)
60-
rendered_form, set_rendered_form = hooks.use_state(cast("Union[str, None]", None))
59+
rendered_form, set_rendered_form = hooks.use_state(cast("str | None", None))
60+
render_count, set_render_count = hooks.use_state(0)
6161

6262
# Initialize the form with the provided data
6363
validate_form_args(top_children, top_children_count, bottom_children, bottom_children_count, form)
@@ -93,6 +93,7 @@ async def render_form():
9393
await ensure_async(initialized_form.save)()
9494
set_submitted_data(None)
9595

96+
set_render_count(render_count + 1)
9697
set_rendered_form(
9798
await ensure_async(initialized_form.render)(form_template or config.REACTPY_DEFAULT_FORM_TEMPLATE)
9899
)
@@ -124,10 +125,15 @@ async def _on_change(_event):
124125
if not rendered_form:
125126
return None
126127

127-
form_props = {
128+
# Note: `key` is intentionally left stable (does not include `render_count`) so the
129+
# client-side `DjangoForm` component is not torn down and re-mounted on every render.
130+
# The `DjangoForm` registers a native `submit` listener that calls `preventDefault()`
131+
# and forwards the submitted FormData via `onSubmitCallback`; keeping that component
132+
# (and its listener) alive across re-renders guarantees the browser never navigates
133+
# away natively, while still letting each submission reach the server.
134+
form_props: dict[str, Any] = {
128135
"id": f"reactpy-{uuid}",
129-
# Intercept the form submission to prevent the browser from navigating
130-
"onSubmit": event(lambda _: None, prevent_default=True),
136+
"key": f"reactpy-{uuid}",
131137
}
132138
if on_change:
133139
form_props["onChange"] = _on_change

‎src/reactpy_django/forms/transforms.py‎

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -33,17 +33,32 @@ def convert_textarea_children_to_prop(vdom_tree: VdomDict) -> VdomDict:
3333

3434

3535
def set_value_prop_on_select_element(vdom_tree: VdomDict) -> VdomDict:
36-
"""Use the `value` prop on <select> instead of setting `selected` on <option>."""
37-
# If the current tag is <select>, remove 'selected' prop from any <option> children and
38-
# instead set the 'value' prop on the <select> tag.
36+
"""Set the correct selection props on a <select> and its <option> children.
37+
38+
ReactPy's built-in ``RequiredTransforms.select_element_to_reactjs`` (which runs during
39+
``string_to_reactpy``, before this transform) removes the ``selected`` attribute from
40+
each ``<option>`` and records the selected value(s) as ``defaultValue`` on the parent
41+
``<select>`` element. That works with React, but Preact does **not** apply a ``<select>``
42+
element's ``defaultValue`` prop to its ``<option>`` children. As a result the selection is
43+
lost when a form remounts/``<option>`` nodes are re-created (e.g. after a submission).
44+
45+
To preserve selection for Preact, this transform reads the ``defaultValue`` already set on
46+
the ``<select>`` and sets the ``selected`` prop directly on each matching ``<option>``
47+
element, which Preact applies as the option's DOM ``selected`` property.
48+
"""
3949
if vdom_tree["tagName"] == "select" and "children" in vdom_tree:
4050
vdom_tree.setdefault("attributes", {})
41-
selected_options = _find_selected_options(vdom_tree)
42-
multiple_choice = vdom_tree["attributes"]["multiple"] = bool(vdom_tree["attributes"].get("multiple"))
43-
if selected_options and not multiple_choice:
44-
vdom_tree["attributes"]["defaultValue"] = selected_options[0]
45-
if selected_options and multiple_choice:
46-
vdom_tree["attributes"]["defaultValue"] = selected_options
51+
attributes = vdom_tree["attributes"]
52+
attributes["multiple"] = bool(attributes.get("multiple"))
53+
54+
# Preact ignores a <select>'s `defaultValue`, so propagate the selected value(s)
55+
# (already stored on the <select> by reactpy's builtin transform) onto the
56+
# matching <option> elements as `selected`.
57+
selected_values = attributes.get("defaultValue")
58+
if isinstance(selected_values, str):
59+
selected_values = [selected_values]
60+
if selected_values:
61+
_set_selected_on_options(vdom_tree, set(selected_values))
4762

4863
return vdom_tree
4964

@@ -87,24 +102,25 @@ def infer_key_from_attributes(vdom_tree: VdomDict) -> VdomDict:
87102
return vdom_tree
88103

89104

90-
def _find_selected_options(vdom_node: Any) -> list[str]:
91-
"""Recursively iterate through the tree to find all <option> tags with the 'selected' prop.
92-
Removes the 'selected' prop and returns a list of the 'value' prop of each selected <option>."""
105+
def _set_selected_on_options(vdom_node: Any, selected_values: set[str]) -> None:
106+
"""Recursively mark the matching <option> elements as selected.
107+
108+
We set the ``selected`` prop on the <option> element. Preact applies ``selected``
109+
as a DOM property on ``HTMLOptionElement``, which updates the option's ``selected``
110+
state on every re-render. This is required because Preact does not apply a
111+
<select>'s ``defaultValue`` prop to its <option> children, so selection is otherwise
112+
lost when a form remounts (e.g. on submit).
113+
"""
93114
if not isinstance(vdom_node, dict):
94-
return []
115+
return
95116

96-
selected_options = []
97117
if vdom_node["tagName"] == "option" and "attributes" in vdom_node:
98-
value = vdom_node["attributes"].setdefault("value", vdom_node["children"][0])
99-
100-
if "selected" in vdom_node["attributes"]:
101-
vdom_node["attributes"].pop("selected")
102-
selected_options.append(value)
118+
value = vdom_node["attributes"].get("value")
119+
if value in selected_values:
120+
vdom_node["attributes"]["selected"] = True
103121

104122
for child in vdom_node.get("children", []):
105-
selected_options.extend(_find_selected_options(child))
106-
107-
return selected_options
123+
_set_selected_on_options(child, selected_values)
108124

109125

110126
def _normalize_prop_name(prop_name: str) -> str:

‎src/reactpy_django/forms/utils.py‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,14 @@
22

33
from typing import TYPE_CHECKING, Any
44

5-
from django.forms import BooleanField, Form, ModelForm, ModelMultipleChoiceField, MultipleChoiceField, NullBooleanField
5+
from django.forms import (
6+
BooleanField,
7+
Form,
8+
ModelForm,
9+
ModelMultipleChoiceField,
10+
MultipleChoiceField,
11+
NullBooleanField,
12+
)
613

714
if TYPE_CHECKING:
815
from collections.abc import Sequence
@@ -11,11 +18,28 @@
1118

1219

1320
def convert_form_fields(data: dict[str, Any], initialized_form: Form | ModelForm) -> None:
21+
"""Convert submitted form data into the format expected by Django fields.
22+
23+
This handles the mismatch between browser FormData serialisation and
24+
Django's field-level expectations:
25+
26+
* ``MultipleChoiceField`` / ``ModelMultipleChoiceField`` – always stored as a
27+
list. When no option is selected the key may be absent or ``None``; we
28+
normalise to an empty list.
29+
* ``BooleanField`` (non-null) – represented by the browser as "key present
30+
= checked". We convert to ``True``/``False``.
31+
* All other fields (including ``MultiValueField``, ``SplitDateTimeField``)
32+
are passed through unchanged. Their sub-widgets already produce unique
33+
``_0``, ``_1``, … keys so no re-shaping is needed.
34+
"""
1435
for field_name, field in initialized_form.fields.items():
1536
value = data.get(field_name)
1637

17-
if isinstance(field, (MultipleChoiceField, ModelMultipleChoiceField)) and value is not None:
18-
data[field_name] = value if isinstance(value, list) else [value]
38+
if isinstance(field, (MultipleChoiceField, ModelMultipleChoiceField)):
39+
if value is None:
40+
data[field_name] = []
41+
elif not isinstance(value, list):
42+
data[field_name] = [value]
1943

2044
elif isinstance(field, BooleanField) and not isinstance(field, NullBooleanField):
2145
data[field_name] = field_name in data
@@ -28,7 +52,6 @@ def validate_form_args(
2852
bottom_children_count: Ref[int],
2953
form: type[Form | ModelForm],
3054
) -> None:
31-
# Validate the provided arguments
3255
if len(top_children) != top_children_count.current or len(bottom_children) != bottom_children_count.current:
3356
msg = "Dynamically changing the number of top or bottom children is not allowed."
3457
raise ValueError(msg)

‎tests/test_app/tests/test_components.py‎

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -809,6 +809,24 @@ def test_form_basic(self):
809809
# Make sure no errors remain
810810
assert len(self.page.query_selector_all(".errorlist")) == 0
811811

812+
# Verify multi-select field values survived the round-trip
813+
# After successful submission, the re-rendered form should have
814+
# the same options selected, proving the FormData duplicate-key fix worked.
815+
# NOTE: `input_value()` on a multi-select returns only the first selected
816+
# value as a string, so we read the selected <option> values directly.
817+
def _selected_values(selector: str) -> list[str]:
818+
return self.page.locator(f"{selector} option:checked").evaluate_all("els => els.map(e => e.value)")
819+
820+
assert sorted(_selected_values("#id_multiple_choice_field")) == ["2", "3"]
821+
assert sorted(_selected_values("#id_typed_multiple_choice_field")) == ["1", "2"]
822+
823+
# Verify model multi-select field values survived the round-trip
824+
model_choice_selected = _selected_values("#id_model_multiple_choice_field")
825+
assert sorted(model_choice_selected) == sorted([
826+
model_choice_field_values[1],
827+
model_choice_field_values[2],
828+
])
829+
812830
@navigate_to_page("/form/bootstrap/")
813831
def test_form_bootstrap(self):
814832
try:
@@ -880,6 +898,28 @@ def test_form_orm_model(self):
880898
finally:
881899
os.environ.pop("DJANGO_ALLOW_ASYNC_UNSAFE")
882900

901+
def _retry_until_true(self, selector: str) -> None:
902+
"""Poll for `#success[data-value='true']` after submitting the filled form.
903+
904+
Filling the `char_field` triggers the form's `onChange` handler, which causes an
905+
async server re-render that reconciles (and can transiently remount) the submit
906+
button. A single Playwright click on `input[type=submit]` may therefore land on
907+
the form element instead of the button, swallowing the `submit` event and losing
908+
the valid submission. This helper re-fills the field and re-clicks submit until
909+
the `on_success` callback actually takes effect, making the interaction
910+
deterministic instead of racing the reconciliation.
911+
"""
912+
for _ in range(3):
913+
self.page.wait_for_selector("#id_char_field").type("test", delay=DELAY)
914+
self.page.wait_for_selector("input[type=submit]").click(delay=DELAY)
915+
try:
916+
self.page.wait_for_selector(selector, timeout=5000)
917+
return
918+
except PlaywrightTimeoutError:
919+
continue
920+
# Let the final wait raise the real failure if it still never succeeds.
921+
self.page.wait_for_selector(selector)
922+
883923
@navigate_to_page("/form/sync_event/")
884924
def test_form_sync_events(self):
885925
self.page.wait_for_selector("form")
@@ -899,12 +939,12 @@ def test_form_sync_events(self):
899939
self.page.wait_for_selector("#receive_data[data-value='true']")
900940
self.page.wait_for_selector("#change[data-value='false']")
901941

902-
# Fill out the form and re-submit
903-
self.page.wait_for_selector("#id_char_field").type("test", delay=DELAY)
904-
self.page.wait_for_selector("input[type=submit]").click(delay=DELAY)
942+
# Fill out the form and re-submit. The `onChange`-triggered async re-render can
943+
# transiently remount the submit button, so retry (re-filling + re-clicking) until
944+
# the `on_success` callback visibly takes effect rather than racing the reconcile.
945+
self._retry_until_true("#success[data-value='true']")
905946

906947
# Form should have been successfully submitted
907-
self.page.wait_for_selector("#success[data-value='true']")
908948
self.page.wait_for_selector("#error[data-value='true']")
909949
self.page.wait_for_selector("#receive_data[data-value='true']")
910950
self.page.wait_for_selector("#change[data-value='true']")
@@ -928,12 +968,12 @@ def test_form_async_events(self):
928968
self.page.wait_for_selector("#receive_data[data-value='true']")
929969
self.page.wait_for_selector("#change[data-value='false']")
930970

931-
# Fill out the form and re-submit
932-
self.page.wait_for_selector("#id_char_field").type("test", delay=DELAY)
933-
self.page.wait_for_selector("input[type=submit]").click(delay=DELAY)
971+
# Fill out the form and re-submit. The `onChange`-triggered async re-render can
972+
# transiently remount the submit button, so retry (re-filling + re-clicking) until
973+
# the `on_success` callback visibly takes effect rather than racing the reconcile.
974+
self._retry_until_true("#success[data-value='true']")
934975

935976
# Form should have been successfully submitted
936-
self.page.wait_for_selector("#success[data-value='true']")
937977
self.page.wait_for_selector("#error[data-value='true']")
938978
self.page.wait_for_selector("#receive_data[data-value='true']")
939979
self.page.wait_for_selector("#change[data-value='true']")

0 commit comments

Comments
 (0)