Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Keep form submit callbacks in their foreach row's scope, so handlers can receive the row's item and index together with the submitted fields.
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@


def _handle_submit_js_template(
handle_submit_unique_name: str,
form_data: str,
field_ref_mapping: str,
on_submit_event_chain: str,
Expand All @@ -57,7 +56,6 @@ def _handle_submit_js_template(
"""Generate handle submit JS using f-string formatting.

Args:
handle_submit_unique_name: Unique name for the handle submit function.
form_data: Name of the form data variable.
field_ref_mapping: JSON string of field reference mappings.
on_submit_event_chain: Event chain for the submit handler.
Expand All @@ -67,7 +65,7 @@ def _handle_submit_js_template(
JavaScript code for the form submit handler.
"""
return f"""
const handleSubmit_{handle_submit_unique_name} = useCallback((ev) => {{
(ev) => {{
const $form = ev.target
ev.preventDefault()
const {form_data} = getFormData($form, {field_ref_mapping});
Expand All @@ -77,7 +75,7 @@ def _handle_submit_js_template(
if ({reset_on_submit}) {{
$form.reset()
}}
}})
}}
"""


Expand Down Expand Up @@ -404,37 +402,28 @@ def add_imports(self) -> ImportDict:
The imports for the form component.
"""
return {
"react": "useCallback",
f"$/{Dirs.STATE_PATH}": ["getFormData", "getRefValue", "getRefValues"],
}

def add_hooks(self) -> list[str]:
"""Add hooks for the form.

Returns:
The hooks for the form.
"""
if EventTriggers.ON_SUBMIT not in self.event_triggers:
return []
return [
_handle_submit_js_template(
handle_submit_unique_name=str(self.handle_submit_unique_name),
form_data=str(FORM_DATA),
field_ref_mapping=str(LiteralVar.create(self._get_form_refs())),
on_submit_event_chain=str(
LiteralVar.create(self.event_triggers[EventTriggers.ON_SUBMIT])
),
reset_on_submit=str(self.reset_on_submit).lower(),
)
]

def _render(self) -> CommonTag:
render_tag = super()._render()
if EventTriggers.ON_SUBMIT in self.event_triggers:
on_submit = LiteralVar.create(self.event_triggers[EventTriggers.ON_SUBMIT])
form_refs = LiteralVar.create(self._get_form_refs())
render_tag = render_tag.add_props(**{
EventTriggers.ON_SUBMIT: Var(
_js_expr=f"handleSubmit_{self.handle_submit_unique_name}",
_js_expr=_handle_submit_js_template(
form_data=str(FORM_DATA),
field_ref_mapping=str(form_refs),
on_submit_event_chain=str(on_submit),
reset_on_submit=str(self.reset_on_submit).lower(),
),
_var_type=EventChain,
_var_data=VarData.merge(
on_submit._get_all_var_data(),
form_refs._get_all_var_data(),
self.reset_on_submit._get_all_var_data(),
),
)
})
return render_tag
Expand Down
2 changes: 1 addition & 1 deletion pyi_hashes.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"packages/reflex-components-core/src/reflex_components_core/el/element.pyi": "930b020d7c933ab0e29ba7971ba1abd2",
"packages/reflex-components-core/src/reflex_components_core/el/elements/__init__.pyi": "ded6fc0acc37c9260da5cfd5f4309079",
"packages/reflex-components-core/src/reflex_components_core/el/elements/base.pyi": "d075a1420b5a08cccd45aa58599ab3dc",
"packages/reflex-components-core/src/reflex_components_core/el/elements/forms.pyi": "5c80e8c9e49b6909e8b514699afdeb86",
"packages/reflex-components-core/src/reflex_components_core/el/elements/forms.pyi": "e2ede64744bd4284bf76eeba5eaf4994",
"packages/reflex-components-core/src/reflex_components_core/el/elements/inline.pyi": "c4085b9ee43847b6765cbf775341bc0b",
"packages/reflex-components-core/src/reflex_components_core/el/elements/media.pyi": "c2515454828415b78799f0ee0fb7a6af",
"packages/reflex-components-core/src/reflex_components_core/el/elements/metadata.pyi": "2d8d722c68f5a3082c0d3fa4883bb5c0",
Expand Down
32 changes: 32 additions & 0 deletions tests/integration/test_form_submit.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ def FormSubmitName(
class FormState(rx.State):
form_data: rx.Field[dict] = rx.field(default_factory=dict)
tags: rx.Field[list[str]] = rx.field(default_factory=list)
submitted_rows: rx.Field[list[dict[str, str | int]]] = rx.field(
default_factory=list
)
row_items: rx.Field[list[str]] = rx.field(
default_factory=lambda: ["alpha", "beta", "gamma"]
)
val: str = "foo"
options: list[str] = ["option1", "option2"]

Expand All @@ -124,6 +130,21 @@ def form_submit(self, form_data: dict):
def form_submit_all(self, form_data: rx.form.FormData[str, str]):
self.tags = form_data.getlist("tag")

@rx.event
def form_submit_row(self, item: str, index: int, form_data: dict[str, str]):
"""Record the submitted row and its form data.

Args:
item: The foreach item.
index: The row index.
form_data: The submitted fields.
"""
self.submitted_rows.append({
"item": item,
"index": index,
"value": form_data["row_value"],
})

app = rx.App()

@app.add_page
Expand Down Expand Up @@ -171,6 +192,17 @@ def index():
),
rx.text(FormState.form_data.to_string(), id="form-data"),
rx.text(FormState.tags.to_string(), id="tags"),
rx.foreach(
FormState.row_items,
lambda item, index: eval(form_component)(
rx.input(name="row_value"),
rx.button("Submit row", type="submit"),
on_submit=lambda data: FormState.form_submit_row(item, index, data),
reset_on_submit=True,
id=f"row-{index}",
),
),
rx.text(FormState.submitted_rows.to_string(), id="submitted-rows"),
rx.spacer(),
height="100vh",
)
Expand Down
62 changes: 62 additions & 0 deletions tests/integration/tests_playwright/test_form_foreach.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
"""Browser regression tests for submit handlers inside foreach rows."""

import functools
import json
from collections.abc import Generator

import pytest
from playwright.sync_api import Page, expect

from reflex.testing import AppHarness
from tests.integration.test_form_submit import FormSubmitName


@pytest.fixture(
scope="module", params=["rx.form.root", "rx.el.form"], ids=["radix", "html"]
)
def foreach_form_app(
request: pytest.FixtureRequest,
app_harness_env: type[AppHarness],
tmp_path_factory: pytest.TempPathFactory,
) -> Generator[AppHarness, None, None]:
"""Run the existing form app with native and Radix forms.

Args:
request: The selected form factory.
app_harness_env: Harness for development or production.
tmp_path_factory: Temporary application directory factory.

Yields:
The running form application.
"""
with app_harness_env.create(
root=tmp_path_factory.mktemp("foreach_form"),
app_source=functools.partial(FormSubmitName, form_component=request.param),
app_name="foreach_form_app",
) as harness:
yield harness


def test_each_row_submits_its_item_index_and_form_data(
foreach_form_app: AppHarness, page: Page
) -> None:
"""Submit rows out of order without losing their captures or reset behavior.

Args:
foreach_form_app: The running form application.
page: The browser page.
"""
assert foreach_form_app.frontend_url is not None
page.goto(foreach_form_app.frontend_url)
expected: list[dict[str, str | int]] = []
result = page.locator("#submitted-rows")
for index, item in ((1, "beta"), (0, "alpha"), (2, "gamma")):
form = page.locator(f"#row-{index}")
field = form.locator('input[name="row_value"]')
value = f"value-{index}"
field.fill(value)
form.get_by_role("button", name="Submit row", exact=True).click()
expect(result).to_contain_text(value)
expected.append({"item": item, "index": index, "value": value})
assert json.loads(result.inner_text()) == expected
expect(field).to_have_value("")
2 changes: 1 addition & 1 deletion tests/units/compiler/test_memoize_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2176,7 +2176,7 @@ def test_form_memo_preserves_control_refs(form_factory) -> None:
if isinstance(definition.component, Form)
]
assert len(forms) == 1
form_hooks = "\n".join(forms[0].add_hooks())
form_hooks = str(forms[0]._render().props["onSubmit"])
assert "getFormData($form, " in form_hooks
for field_id in ("plain_field", "debounced_field", "unset_field"):
assert f'getRefValue(refs["ref_{field_id}"])' in form_hooks
Expand Down
52 changes: 41 additions & 11 deletions tests/units/components/forms/test_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,45 @@
MEMOIZED_INPUT_ID = "memoized_input"


@pytest.mark.parametrize("form_factory", [HTMLForm.create, Form.create])
def test_submit_handler_captures_foreach_item_and_index(form_factory):
"""Submit handlers must retain both variables from their foreach row."""

class RowState(rx.State):
@rx.event
def submit(self, item: str, index: int, form_data: dict[str, str]):
"""Receive a row's submitted fields.

Args:
item: The foreach item.
index: The row index.
form_data: The submitted fields.
"""

rows = rx.foreach(
["first", "second"],
lambda item, index: form_factory(
rx.input(name="input"),
on_submit=lambda form_data: RowState.submit(item, index, form_data),
),
)
form = rows._render().render_component()
handler = str(form._render().props["onSubmit"])
assert "item_rx_state_" in handler
assert "index_rx_state_" in handler
assert "form_data" in handler


def test_render_on_submit():
"""Test that on_submit event chain is rendered as a separate function."""
"""Test that on_submit wraps the event chain with form-data collection."""
submit_it = Var(
_js_expr="submit_it",
_var_type=EventChain,
)
f = Form.create(on_submit=submit_it)
exp_submit_name = f"handleSubmit_{f.handle_submit_unique_name}" # pyright: ignore [reportAttributeAccessIssue]
assert f"onSubmit:{exp_submit_name}" in f.render()["props"]
handler = str(f._render().props["onSubmit"])
assert "getFormData($form, " in handler
assert "submit_it(...args)" in handler


def test_render_no_on_submit():
Expand Down Expand Up @@ -72,7 +102,7 @@ def on_submit(self, form_data: dict):
on_submit=FormState.on_submit,
id=FORM_ID,
)
submit_hook = form.add_hooks()[0]
submit_hook = str(form._render().props["onSubmit"])
assert "filter(([, value]) => value != null)" not in submit_hook
assert f"ref_{EMAIL_FIELD_ID}" in submit_hook
assert f"ref_{EMAIL_LABEL_ID}" not in submit_hook
Expand All @@ -90,17 +120,17 @@ class NativeInput(BaseHTML):

form = HTMLForm.create(NativeInput.create(id="native_input"))

assert "ref_native_input" in form.add_hooks()[0]
assert "ref_native_input" in str(form._render().props["onSubmit"])


def test_form_refs_follow_replaced_children():
"""Replacing form children must not retain refs from the previous subtree."""
form = HTMLForm.create(Input.create(id="original"))
assert 'getRefValue(refs["ref_original"])' in form.add_hooks()[0]
assert 'getRefValue(refs["ref_original"])' in str(form._render().props["onSubmit"])

form.children = [Input.create(id="replacement")]

hook = form.add_hooks()[0]
hook = str(form._render().props["onSubmit"])
assert 'getRefValue(refs["ref_replacement"])' in hook
assert 'getRefValue(refs["ref_original"])' not in hook

Expand All @@ -114,7 +144,7 @@ class CustomControl(rx.Component):

form = HTMLForm.create(CustomControl.create(id="custom_control"))

assert "ref_custom_control" in form.add_hooks()[0]
assert "ref_custom_control" in str(form._render().props["onSubmit"])


def test_form_refs_include_debounced_controls():
Expand All @@ -125,7 +155,7 @@ def test_form_refs_include_debounced_controls():
)
)

assert f"ref_{DEBOUNCED_INPUT_ID}" in form.add_hooks()[0]
assert f"ref_{DEBOUNCED_INPUT_ID}" in str(form._render().props["onSubmit"])


def test_form_refs_include_memoized_controls(monkeypatch):
Expand All @@ -137,7 +167,7 @@ class MemoizedInput(Input):
monkeypatch.setattr(MemoizedInput, "_wrapped_component_type", Input, raising=False)
form = HTMLForm.create(MemoizedInput.create(id=MEMOIZED_INPUT_ID))

assert f"ref_{MEMOIZED_INPUT_ID}" in form.add_hooks()[0]
assert f"ref_{MEMOIZED_INPUT_ID}" in str(form._render().props["onSubmit"])


@pytest.mark.parametrize("form_factory", [HTMLForm.create, Form.create])
Expand Down Expand Up @@ -360,7 +390,7 @@ def test_on_submit_collects_form_data_with_id_refs(form_factory):
Input.create(id="email_input", name="email"),
on_submit=Var(_js_expr="submit_it", _var_type=EventChain),
)
(hook,) = form.add_hooks()
hook = str(form._render().props["onSubmit"])
assert (
"const form_data = getFormData($form, "
'({ ["email_input"] : getRefValue(refs["ref_email_input"]) }));'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ def test_demo_form_dialog_renders_given_trigger() -> None:
def test_demo_form_appends_a_tuple_of_submit_events() -> None:
"""A tuple of extra submit events is appended like a list of them."""
form = demo_form(on_submit=(rx.console_log("first"), rx.console_log("second")))
# the form's submit handler is a hook that dispatches its events
hooks = "".join(str(hook) for hook in form._get_all_hooks())
rendered = str(form)

assert '"first"' in hooks
assert '"second"' in hooks
assert '"first"' in rendered
assert '"second"' in rendered
assert rendered.index('"first"') < rendered.index('"second"')


def test_demo_form_standalone_retains_its_layout() -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
def test_intro_form_appends_a_tuple_of_submit_events() -> None:
"""A tuple of extra submit events is appended like a list of them."""
form = intro_form(on_submit=(rx.console_log("first"), rx.console_log("second")))
# the form's submit handler is a hook that dispatches its events
hooks = "".join(str(hook) for hook in form._get_all_hooks())
rendered = str(form)

assert '"first"' in hooks
assert '"second"' in hooks
assert '"first"' in rendered
assert '"second"' in rendered
assert rendered.index('"first"') < rendered.index('"second"')