diff --git a/packages/reflex-components-core/news/+foreach-form-submit.bugfix.md b/packages/reflex-components-core/news/+foreach-form-submit.bugfix.md new file mode 100644 index 00000000000..62ada3c5069 --- /dev/null +++ b/packages/reflex-components-core/news/+foreach-form-submit.bugfix.md @@ -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. diff --git a/packages/reflex-components-core/src/reflex_components_core/el/elements/forms.py b/packages/reflex-components-core/src/reflex_components_core/el/elements/forms.py index 6fd8855a222..600ee0cf856 100644 --- a/packages/reflex-components-core/src/reflex_components_core/el/elements/forms.py +++ b/packages/reflex-components-core/src/reflex_components_core/el/elements/forms.py @@ -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, @@ -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. @@ -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}); @@ -77,7 +75,7 @@ def _handle_submit_js_template( if ({reset_on_submit}) {{ $form.reset() }} - }}) + }} """ @@ -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 diff --git a/pyi_hashes.json b/pyi_hashes.json index e5a9a57582d..1afa70a95cb 100644 --- a/pyi_hashes.json +++ b/pyi_hashes.json @@ -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", diff --git a/tests/integration/test_form_submit.py b/tests/integration/test_form_submit.py index 261cfbb7389..6d4e955a11f 100644 --- a/tests/integration/test_form_submit.py +++ b/tests/integration/test_form_submit.py @@ -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"] @@ -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 @@ -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", ) diff --git a/tests/integration/tests_playwright/test_form_foreach.py b/tests/integration/tests_playwright/test_form_foreach.py new file mode 100644 index 00000000000..87f8859f101 --- /dev/null +++ b/tests/integration/tests_playwright/test_form_foreach.py @@ -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("") diff --git a/tests/units/compiler/test_memoize_plugin.py b/tests/units/compiler/test_memoize_plugin.py index ffea8c7ca2d..9faf2276e5f 100644 --- a/tests/units/compiler/test_memoize_plugin.py +++ b/tests/units/compiler/test_memoize_plugin.py @@ -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 diff --git a/tests/units/components/forms/test_form.py b/tests/units/components/forms/test_form.py index e1b8bd79dae..bed9242823c 100644 --- a/tests/units/components/forms/test_form.py +++ b/tests/units/components/forms/test_form.py @@ -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(): @@ -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 @@ -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 @@ -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(): @@ -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): @@ -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]) @@ -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"]) }));' diff --git a/tests/units/reflex_components_internal/blocks/test_demo_form.py b/tests/units/reflex_components_internal/blocks/test_demo_form.py index a2a6ffbb760..b327164210b 100644 --- a/tests/units/reflex_components_internal/blocks/test_demo_form.py +++ b/tests/units/reflex_components_internal/blocks/test_demo_form.py @@ -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: diff --git a/tests/units/reflex_components_internal/blocks/test_intro_form.py b/tests/units/reflex_components_internal/blocks/test_intro_form.py index 0ee1eb3268a..b3519ce4e48 100644 --- a/tests/units/reflex_components_internal/blocks/test_intro_form.py +++ b/tests/units/reflex_components_internal/blocks/test_intro_form.py @@ -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"')