Squash some bugs with multi choice and boolean fields

Author: Archmonger

src/reactpy_django/forms/components.py

@@ -14,9 +14,10 @@
     convert_html_props_to_reactjs,
     convert_textarea_children_to_prop,
     ensure_input_elements_are_controlled,
+    intercept_anchor_links,
     set_value_prop_on_select_element,
 )
-from reactpy_django.forms.utils import convert_boolean_fields, convert_choice_fields
+from reactpy_django.forms.utils import convert_boolean_fields, convert_multiple_choice_fields
 
 if TYPE_CHECKING:
     from collections.abc import Sequence
@@ -79,7 +80,7 @@ def on_submit(_event):
         last_changed.set_current(timezone.now())
 
     def on_submit_callback(new_data: dict[str, Any]):
-        convert_choice_fields(new_data, initialized_form)
+        convert_multiple_choice_fields(new_data, initialized_form)
         convert_boolean_fields(new_data, initialized_form)
 
         # TODO: ReactPy's use_state hook really should be de-duplicating this by itself. Needs upstream fix.
@@ -95,6 +96,7 @@ async def on_change(_event):
         convert_textarea_children_to_prop,
         set_value_prop_on_select_element,
         ensure_input_elements_are_controlled(on_change),
+        intercept_anchor_links,
         strict=False,
     )
 

src/reactpy_django/forms/transforms.py

@@ -9,12 +9,9 @@
 
 # TODO: Move all this logic to `reactpy.utils._mutate_vdom()` and remove this file.
 
-UNSUPPORTED_PROPS = {"children", "ref", "aria-*", "data-*"}
-
 
 def convert_html_props_to_reactjs(vdom_tree: VdomDict) -> VdomDict:
     """Transformation that standardizes the prop names to be used in the component."""
-
     if not isinstance(vdom_tree, dict):
         return vdom_tree
 
@@ -38,69 +35,113 @@ def convert_textarea_children_to_prop(vdom_tree: VdomDict) -> VdomDict:
         text_content = vdom_tree.pop("children")
         text_content = "".join([child for child in text_content if isinstance(child, str)])
         default_value = vdom_tree["attributes"].pop("defaultValue", "")
-        vdom_tree["attributes"]["value"] = text_content or default_value
+        vdom_tree["attributes"]["defaultValue"] = text_content or default_value
 
     for child in vdom_tree.get("children", []):
         convert_textarea_children_to_prop(child)
 
     return vdom_tree
 
 
-def _find_selected_options(vdom_tree: VdomDict, mutation: Callable) -> list[VdomDict]:
-    """Recursively iterate through the tree of dictionaries to find an <option> with the 'selected' prop."""
-    selected_options = []
-
+def set_value_prop_on_select_element(vdom_tree: VdomDict) -> VdomDict:
+    """Use the `value` prop on <select> instead of setting `selected` on <option>."""
     if not isinstance(vdom_tree, dict):
-        return selected_options
+        return vdom_tree
 
-    if vdom_tree["tagName"] == "option" and "attributes" in vdom_tree and "selected" in vdom_tree["attributes"]:
-        mutation(vdom_tree)
-        selected_options.append(vdom_tree)
+    # If the current tag is <select>, remove 'selected' prop from any <option> children and
+    # instead set the 'value' prop on the <select> tag.
+    if vdom_tree["tagName"] == "select" and "children" in vdom_tree:
+        selected_options = _find_selected_options(vdom_tree)
+        multiple_choice = vdom_tree["attributes"]["multiple"] = bool(vdom_tree["attributes"].get("multiple"))
+        if selected_options and not multiple_choice:
+            vdom_tree["attributes"]["defaultValue"] = selected_options[0]
+        if selected_options and multiple_choice:
+            vdom_tree["attributes"]["defaultValue"] = selected_options
 
     for child in vdom_tree.get("children", []):
-        selected_options.extend(_find_selected_options(child, mutation))
+        set_value_prop_on_select_element(child)
 
-    return selected_options
+    return vdom_tree
 
 
-def set_value_prop_on_select_element(vdom_tree: VdomDict) -> VdomDict:
-    """Use the `value` prop on <select> instead of setting `selected` on <option>."""
+def ensure_input_elements_are_controlled(event_func: Callable | None = None) -> Callable:
+    """Adds an onChange handler on form <input> elements, since ReactJS doesn't like uncontrolled inputs."""
+
+    def mutation(vdom_tree: VdomDict) -> VdomDict:
+        """Adds an onChange event handler to all input elements."""
+        if not isinstance(vdom_tree, dict):
+            return vdom_tree
+
+        vdom_tree.setdefault("eventHandlers", {})
+        if vdom_tree["tagName"] in {"input", "textarea"}:
+            if "onChange" in vdom_tree["eventHandlers"]:
+                pass
+            elif isinstance(event_func, EventHandler):
+                vdom_tree["eventHandlers"]["onChange"] = event_func
+            else:
+                vdom_tree["eventHandlers"]["onChange"] = EventHandler(
+                    to_event_handler_function(event_func or _do_nothing_event)
+                )
+
+        if "children" in vdom_tree:
+            for child in vdom_tree["children"]:
+                mutation(child)
+
+        return vdom_tree
+
+    return mutation
+
 
+def intercept_anchor_links(vdom_tree: VdomDict) -> VdomDict:
+    """Intercepts anchor links and prevents the default behavior.
+    This allows ReactPy-Router to handle the navigation instead of the browser."""
     if not isinstance(vdom_tree, dict):
         return vdom_tree
 
-    # If the current tag is <select>, remove 'selected' prop from any <option> children and
-    # instead set the 'value' prop on the <select> tag.
-    # TODO: Fix this, is broken
-    if vdom_tree["tagName"] == "select" and "children" in vdom_tree:
+    if vdom_tree["tagName"] == "a":
         vdom_tree.setdefault("eventHandlers", {})
-        vdom_tree["eventHandlers"]["onChange"] = EventHandler(to_event_handler_function(do_nothing_event))
-        selected_options = _find_selected_options(vdom_tree, lambda option: option["attributes"].pop("selected"))
-        multiple_choice = vdom_tree["attributes"].get("multiple")
-        if selected_options and not multiple_choice:
-            vdom_tree["attributes"]["value"] = selected_options[0]["children"][0]
-        if selected_options and multiple_choice:
-            vdom_tree["attributes"]["value"] = [option["children"][0] for option in selected_options]
+        vdom_tree["eventHandlers"]["onClick"] = EventHandler(
+            to_event_handler_function(_do_nothing_event), prevent_default=True
+        )
 
     for child in vdom_tree.get("children", []):
-        set_value_prop_on_select_element(child)
+        intercept_anchor_links(child)
 
     return vdom_tree
 
 
+def _find_selected_options(vdom_tree: VdomDict) -> list[str]:
+    """Recursively iterate through the tree of dictionaries to find an <option> with the 'selected' prop."""
+    if not isinstance(vdom_tree, dict):
+        return []
+
+    selected_options = []
+    if vdom_tree["tagName"] == "option" and "attributes" in vdom_tree:
+        value = vdom_tree["attributes"].setdefault("value", vdom_tree["children"][0])
+
+        if "selected" in vdom_tree["attributes"]:
+            vdom_tree["attributes"].pop("selected")
+            selected_options.append(value)
+
+    for child in vdom_tree.get("children", []):
+        selected_options.extend(_find_selected_options(child))
+
+    return selected_options
+
+
 def _normalize_prop_name(prop_name: str) -> str:
     """Standardizes the prop name to be used in the component."""
     return REACT_PROP_SUBSTITUTIONS.get(prop_name, prop_name)
 
 
-def react_props_set(string: str) -> set[str]:
+def _react_props_set(string: str) -> set[str]:
     """Extracts the props from a string of React props."""
     lines = string.strip().split("\n")
     props = set()
 
     for line in lines:
         parts = line.split(":", maxsplit=1)
-        if len(parts) == 2 and parts[0] not in UNSUPPORTED_PROPS:
+        if len(parts) == 2 and parts[0] not in {"children", "ref", "aria-*", "data-*"}:
             key, value = parts
             key = key.strip()
             value = value.strip()
@@ -130,35 +171,7 @@ def _add_on_change_event(event_func, vdom_tree: VdomDict) -> VdomDict:
     return vdom_tree
 
 
-def ensure_input_elements_are_controlled(event_func: Callable | None = None) -> Callable:
-    """Adds an onChange handler on form <input> elements, since ReactJS doesn't like uncontrolled inputs."""
-
-    def mutation(vdom_tree: VdomDict) -> VdomDict:
-        """Adds an onChange event handler to all input elements."""
-        if not isinstance(vdom_tree, dict):
-            return vdom_tree
-
-        vdom_tree.setdefault("eventHandlers", {})
-        if vdom_tree["tagName"] in {"input", "textarea"}:
-            if "onChange" in vdom_tree["eventHandlers"]:
-                pass
-            elif isinstance(event_func, EventHandler):
-                vdom_tree["eventHandlers"]["onChange"] = event_func
-            else:
-                vdom_tree["eventHandlers"]["onChange"] = EventHandler(
-                    to_event_handler_function(event_func or do_nothing_event)
-                )
-
-        if "children" in vdom_tree:
-            for child in vdom_tree["children"]:
-                mutation(child)
-
-        return vdom_tree
-
-    return mutation
-
-
-def do_nothing_event(*args, **kwargs):
+def _do_nothing_event(*args, **kwargs):
     pass
 
 
@@ -495,7 +508,7 @@ def do_nothing_event(*args, **kwargs):
 type: a string. Says whether the script is a classic script, ES module, or import map.
 """
 
-KNOWN_REACT_PROPS = react_props_set(
+KNOWN_REACT_PROPS = _react_props_set(
     SPECIAL_PROPS
     + STANDARD_PROPS
     + FORM_PROPS

src/reactpy_django/forms/utils.py

@@ -1,37 +1,24 @@
 from typing import Any
 
-from django.forms import BooleanField, ChoiceField, Form, MultipleChoiceField
+from django.forms import BooleanField, Form, MultipleChoiceField, NullBooleanField
 
 
-def convert_choice_fields(data: dict[str, Any], initialized_form: Form) -> None:
-    choice_field_map = {
-        field_name: {choice_value: choice_key for choice_key, choice_value in field.choices}
-        for field_name, field in initialized_form.fields.items()
-        if isinstance(field, ChoiceField)
-    }
+def convert_multiple_choice_fields(data: dict[str, Any], initialized_form: Form) -> None:
     multi_choice_fields = {
         field_name for field_name, field in initialized_form.fields.items() if isinstance(field, MultipleChoiceField)
     }
 
-    # Choice fields submit their values as text, but Django choice keys are not always equal to their values.
-    # Due to this, we need to convert the text into keys that Django would be happy with
-    for choice_field_name, choice_map in choice_field_map.items():
-        if choice_field_name in data:
-            submitted_value = data[choice_field_name]
-            if isinstance(submitted_value, list):
-                data[choice_field_name] = [
-                    choice_map.get(submitted_value_item, submitted_value_item)
-                    for submitted_value_item in submitted_value
-                ]
-            elif choice_field_name in multi_choice_fields:
-                data[choice_field_name] = [choice_map.get(submitted_value, submitted_value)]
-            else:
-                data[choice_field_name] = choice_map.get(submitted_value, submitted_value)
+    # Convert multiple choice field text into a list of values
+    for choice_field_name in multi_choice_fields:
+        if choice_field_name in data and not isinstance(data[choice_field_name], list):
+            data[choice_field_name] = [data[choice_field_name]]
 
 
 def convert_boolean_fields(data: dict[str, Any], initialized_form: Form) -> None:
     boolean_fields = {
-        field_name for field_name, field in initialized_form.fields.items() if isinstance(field, BooleanField)
+        field_name
+        for field_name, field in initialized_form.fields.items()
+        if isinstance(field, BooleanField) and not isinstance(field, NullBooleanField)
     }
 
     # Convert boolean field text into actual booleans