From 3de01d193704503ea8c65b1d61ba1c756135e2a3 Mon Sep 17 00:00:00 2001 From: Elijah Date: Wed, 11 Dec 2024 11:25:37 +0000 Subject: [PATCH 01/10] Use empty string for None values in `rx.input` and `rx.el.input` --- reflex/components/el/elements/forms.py | 19 +++++++++++++++++++ .../radix/themes/components/text_field.py | 7 +++++++ 2 files changed, 26 insertions(+) diff --git a/reflex/components/el/elements/forms.py b/reflex/components/el/elements/forms.py index a82d6bcddc..4d4becbd10 100644 --- a/reflex/components/el/elements/forms.py +++ b/reflex/components/el/elements/forms.py @@ -7,6 +7,7 @@ from jinja2 import Environment +from reflex.components.core.cond import cond from reflex.components.el.element import Element from reflex.components.tags.tag import Tag from reflex.constants import Dirs, EventTriggers @@ -384,6 +385,24 @@ class Input(BaseHTML): # Fired when a key is released on_key_up: EventHandler[key_event] + @classmethod + def create(cls, *children, **props): + """Create an Input component. + + Args: + *children: The children of the component. + **props: The properties of the component. + + Returns: + The component. + """ + value = props.get("value") + + # React expects an empty string(instead of null) for uncontrolled inputs. + if value is not None: + props["value"] = cond(value, value, "") + return super().create(*children, **props) + class Label(BaseHTML): """Display the label element.""" diff --git a/reflex/components/radix/themes/components/text_field.py b/reflex/components/radix/themes/components/text_field.py index 3dabe09366..5819b89344 100644 --- a/reflex/components/radix/themes/components/text_field.py +++ b/reflex/components/radix/themes/components/text_field.py @@ -6,6 +6,7 @@ from reflex.components.component import Component, ComponentNamespace from reflex.components.core.breakpoints import Responsive +from reflex.components.core.cond import cond from reflex.components.core.debounce import DebounceInput from reflex.components.el import elements from reflex.event import EventHandler, input_event, key_event @@ -96,6 +97,12 @@ def create(cls, *children, **props) -> Component: Returns: The component. """ + value = props.get("value") + + # React expects an empty string(instead of null) for uncontrolled inputs. + if value is not None: + props["value"] = cond(value, value, "") + component = super().create(*children, **props) if props.get("value") is not None and props.get("on_change") is not None: # create a debounced input if the user requests full control to avoid typing jank From 302b810ed7ef9c0c5742847dbd6524cc3cf01855 Mon Sep 17 00:00:00 2001 From: Elijah Date: Wed, 11 Dec 2024 11:49:17 +0000 Subject: [PATCH 02/10] fix tests --- tests/units/components/core/test_debounce.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/units/components/core/test_debounce.py b/tests/units/components/core/test_debounce.py index 29d3bcddc4..a355b99285 100644 --- a/tests/units/components/core/test_debounce.py +++ b/tests/units/components/core/test_debounce.py @@ -60,7 +60,9 @@ def test_render_child_props(): assert "css" in tag.props and isinstance(tag.props["css"], rx.vars.Var) for prop in ["foo", "bar", "baz", "quuc"]: assert prop in str(tag.props["css"]) - assert tag.props["value"].equals(LiteralVar.create("real")) + assert tag.props["value"].equals( + rx.cond(real_var := LiteralVar.create("real"), real_var, "") + ) assert len(tag.props["onChange"].events) == 1 assert tag.props["onChange"].events[0].handler == S.on_change assert tag.contents == "" From 6b2028769e102aa2d71598c1c2fb438f743f803b Mon Sep 17 00:00:00 2001 From: Elijah Date: Wed, 11 Dec 2024 11:53:58 +0000 Subject: [PATCH 03/10] fix pyi scripts --- reflex/components/el/elements/forms.pyi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/reflex/components/el/elements/forms.pyi b/reflex/components/el/elements/forms.pyi index e2d6593382..df63891dd9 100644 --- a/reflex/components/el/elements/forms.pyi +++ b/reflex/components/el/elements/forms.pyi @@ -512,7 +512,7 @@ class Input(BaseHTML): on_unmount: Optional[EventType[[], BASE_STATE]] = None, **props, ) -> "Input": - """Create the component. + """Create an Input component. Args: *children: The children of the component. @@ -576,7 +576,7 @@ class Input(BaseHTML): class_name: The class name for the component. autofocus: Whether the component should take the focus once the page is loaded custom_attrs: custom attribute - **props: The props of the component. + **props: The properties of the component. Returns: The component. From 76034a1a0d8c1d25f521530a31b41c702dde9b06 Mon Sep 17 00:00:00 2001 From: Elijah Date: Wed, 11 Dec 2024 15:01:19 +0000 Subject: [PATCH 04/10] use nullish coalescing operator --- reflex/components/el/elements/forms.py | 3 +-- reflex/components/radix/themes/components/text_field.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/reflex/components/el/elements/forms.py b/reflex/components/el/elements/forms.py index 4d4becbd10..9c83684213 100644 --- a/reflex/components/el/elements/forms.py +++ b/reflex/components/el/elements/forms.py @@ -7,7 +7,6 @@ from jinja2 import Environment -from reflex.components.core.cond import cond from reflex.components.el.element import Element from reflex.components.tags.tag import Tag from reflex.constants import Dirs, EventTriggers @@ -400,7 +399,7 @@ def create(cls, *children, **props): # React expects an empty string(instead of null) for uncontrolled inputs. if value is not None: - props["value"] = cond(value, value, "") + props["value"] = Var(_js_expr=f"{value} ?? ''", _var_type=str) return super().create(*children, **props) diff --git a/reflex/components/radix/themes/components/text_field.py b/reflex/components/radix/themes/components/text_field.py index 5819b89344..191ae01527 100644 --- a/reflex/components/radix/themes/components/text_field.py +++ b/reflex/components/radix/themes/components/text_field.py @@ -6,7 +6,6 @@ from reflex.components.component import Component, ComponentNamespace from reflex.components.core.breakpoints import Responsive -from reflex.components.core.cond import cond from reflex.components.core.debounce import DebounceInput from reflex.components.el import elements from reflex.event import EventHandler, input_event, key_event @@ -101,7 +100,7 @@ def create(cls, *children, **props) -> Component: # React expects an empty string(instead of null) for uncontrolled inputs. if value is not None: - props["value"] = cond(value, value, "") + props["value"] = Var(_js_expr=f"{value} ?? ''", _var_type=str) component = super().create(*children, **props) if props.get("value") is not None and props.get("on_change") is not None: From 768e15a100b9b440c06dda46a0b7178001dac6d6 Mon Sep 17 00:00:00 2001 From: Elijah Date: Wed, 11 Dec 2024 15:13:07 +0000 Subject: [PATCH 05/10] fix unit tests --- tests/units/components/core/test_debounce.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/units/components/core/test_debounce.py b/tests/units/components/core/test_debounce.py index a355b99285..2c7d7b3579 100644 --- a/tests/units/components/core/test_debounce.py +++ b/tests/units/components/core/test_debounce.py @@ -61,7 +61,7 @@ def test_render_child_props(): for prop in ["foo", "bar", "baz", "quuc"]: assert prop in str(tag.props["css"]) assert tag.props["value"].equals( - rx.cond(real_var := LiteralVar.create("real"), real_var, "") + Var(_js_expr="real ?? ''", _var_type=str) ) assert len(tag.props["onChange"].events) == 1 assert tag.props["onChange"].events[0].handler == S.on_change From 1cddb8803050a87c73470457bd2f6d3175cbc594 Mon Sep 17 00:00:00 2001 From: Elijah Date: Wed, 11 Dec 2024 15:55:10 +0000 Subject: [PATCH 06/10] use ternary operator so old browsers and dynamic components tests pass --- reflex/components/el/elements/forms.py | 9 ++++++++- reflex/components/radix/themes/components/text_field.py | 9 ++++++++- tests/units/components/core/test_debounce.py | 8 +++++++- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/reflex/components/el/elements/forms.py b/reflex/components/el/elements/forms.py index 9c83684213..cfa1cb0aab 100644 --- a/reflex/components/el/elements/forms.py +++ b/reflex/components/el/elements/forms.py @@ -395,11 +395,18 @@ def create(cls, *children, **props): Returns: The component. """ + from reflex.vars.number import ternary_operation + value = props.get("value") # React expects an empty string(instead of null) for uncontrolled inputs. if value is not None: - props["value"] = Var(_js_expr=f"{value} ?? ''", _var_type=str) + props["value"] = ternary_operation( + ((value_var := Var.create(value)) != Var.create(None)) + & (value_var != Var(_js_expr="undefined")), + value, + "", + ) return super().create(*children, **props) diff --git a/reflex/components/radix/themes/components/text_field.py b/reflex/components/radix/themes/components/text_field.py index 191ae01527..af146d172a 100644 --- a/reflex/components/radix/themes/components/text_field.py +++ b/reflex/components/radix/themes/components/text_field.py @@ -10,6 +10,7 @@ from reflex.components.el import elements from reflex.event import EventHandler, input_event, key_event from reflex.vars.base import Var +from reflex.vars.number import ternary_operation from ..base import LiteralAccentColor, LiteralRadius, RadixThemesComponent @@ -100,7 +101,13 @@ def create(cls, *children, **props) -> Component: # React expects an empty string(instead of null) for uncontrolled inputs. if value is not None: - props["value"] = Var(_js_expr=f"{value} ?? ''", _var_type=str) + # props["value"] = Var(_js_expr=f"{value} ?? ''", _var_type=str) + props["value"] = ternary_operation( + ((value_var := Var.create(value)) != Var.create(None)) + & (value_var != Var(_js_expr="undefined")), + value, + "", + ) component = super().create(*children, **props) if props.get("value") is not None and props.get("on_change") is not None: diff --git a/tests/units/components/core/test_debounce.py b/tests/units/components/core/test_debounce.py index 2c7d7b3579..5da095c5b6 100644 --- a/tests/units/components/core/test_debounce.py +++ b/tests/units/components/core/test_debounce.py @@ -6,6 +6,7 @@ from reflex.components.core.debounce import DEFAULT_DEBOUNCE_TIMEOUT from reflex.state import BaseState from reflex.vars.base import LiteralVar, Var +from reflex.vars.number import ternary_operation def test_create_no_child(): @@ -60,8 +61,13 @@ def test_render_child_props(): assert "css" in tag.props and isinstance(tag.props["css"], rx.vars.Var) for prop in ["foo", "bar", "baz", "quuc"]: assert prop in str(tag.props["css"]) + value_var = Var.create("real") assert tag.props["value"].equals( - Var(_js_expr="real ?? ''", _var_type=str) + ternary_operation( + (value_var != Var.create(None)) & (value_var != Var(_js_expr="undefined")), + "real", + "", + ) ) assert len(tag.props["onChange"].events) == 1 assert tag.props["onChange"].events[0].handler == S.on_change From 31462ad14d5e3eb037fec13d644d9bfd8aade180 Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 12 Dec 2024 19:21:05 +0000 Subject: [PATCH 07/10] address comment on doing this only for optionals --- reflex/components/el/elements/forms.py | 7 +++++-- reflex/components/radix/themes/components/text_field.py | 8 +++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/reflex/components/el/elements/forms.py b/reflex/components/el/elements/forms.py index cfa1cb0aab..4c54cddf4b 100644 --- a/reflex/components/el/elements/forms.py +++ b/reflex/components/el/elements/forms.py @@ -18,6 +18,7 @@ prevent_default, ) from reflex.utils.imports import ImportDict +from reflex.utils.types import is_optional from reflex.vars import VarData from reflex.vars.base import LiteralVar, Var @@ -400,9 +401,11 @@ def create(cls, *children, **props): value = props.get("value") # React expects an empty string(instead of null) for uncontrolled inputs. - if value is not None: + if value is not None and is_optional( + (value_var := Var.create(value))._var_type + ): props["value"] = ternary_operation( - ((value_var := Var.create(value)) != Var.create(None)) + (value_var != Var.create(None)) & (value_var != Var(_js_expr="undefined")), value, "", diff --git a/reflex/components/radix/themes/components/text_field.py b/reflex/components/radix/themes/components/text_field.py index af146d172a..82c59fa2ce 100644 --- a/reflex/components/radix/themes/components/text_field.py +++ b/reflex/components/radix/themes/components/text_field.py @@ -9,6 +9,7 @@ from reflex.components.core.debounce import DebounceInput from reflex.components.el import elements from reflex.event import EventHandler, input_event, key_event +from reflex.utils.types import is_optional from reflex.vars.base import Var from reflex.vars.number import ternary_operation @@ -100,10 +101,11 @@ def create(cls, *children, **props) -> Component: value = props.get("value") # React expects an empty string(instead of null) for uncontrolled inputs. - if value is not None: - # props["value"] = Var(_js_expr=f"{value} ?? ''", _var_type=str) + if value is not None and is_optional( + (value_var := Var.create(value))._var_type + ): props["value"] = ternary_operation( - ((value_var := Var.create(value)) != Var.create(None)) + (value_var != Var.create(None)) & (value_var != Var(_js_expr="undefined")), value, "", From 437d421afeffa5b5757737464edc6db100ca2474 Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 12 Dec 2024 19:33:31 +0000 Subject: [PATCH 08/10] Fix tests --- tests/units/components/core/test_debounce.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/units/components/core/test_debounce.py b/tests/units/components/core/test_debounce.py index 5da095c5b6..29d3bcddc4 100644 --- a/tests/units/components/core/test_debounce.py +++ b/tests/units/components/core/test_debounce.py @@ -6,7 +6,6 @@ from reflex.components.core.debounce import DEFAULT_DEBOUNCE_TIMEOUT from reflex.state import BaseState from reflex.vars.base import LiteralVar, Var -from reflex.vars.number import ternary_operation def test_create_no_child(): @@ -61,14 +60,7 @@ def test_render_child_props(): assert "css" in tag.props and isinstance(tag.props["css"], rx.vars.Var) for prop in ["foo", "bar", "baz", "quuc"]: assert prop in str(tag.props["css"]) - value_var = Var.create("real") - assert tag.props["value"].equals( - ternary_operation( - (value_var != Var.create(None)) & (value_var != Var(_js_expr="undefined")), - "real", - "", - ) - ) + assert tag.props["value"].equals(LiteralVar.create("real")) assert len(tag.props["onChange"].events) == 1 assert tag.props["onChange"].events[0].handler == S.on_change assert tag.contents == "" From fb8e28c219fb108bde0f01ad11854aa65151b392 Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 12 Dec 2024 20:10:23 +0000 Subject: [PATCH 09/10] pyright! --- reflex/components/el/elements/forms.py | 4 ++-- reflex/components/radix/themes/components/text_field.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/reflex/components/el/elements/forms.py b/reflex/components/el/elements/forms.py index 4c54cddf4b..55f5ec99b3 100644 --- a/reflex/components/el/elements/forms.py +++ b/reflex/components/el/elements/forms.py @@ -405,10 +405,10 @@ def create(cls, *children, **props): (value_var := Var.create(value))._var_type ): props["value"] = ternary_operation( - (value_var != Var.create(None)) + (value_var != Var.create(None)) # pyright: ignore [reportGeneralTypeIssues] & (value_var != Var(_js_expr="undefined")), value, - "", + Var.create(""), ) return super().create(*children, **props) diff --git a/reflex/components/radix/themes/components/text_field.py b/reflex/components/radix/themes/components/text_field.py index 82c59fa2ce..1043554041 100644 --- a/reflex/components/radix/themes/components/text_field.py +++ b/reflex/components/radix/themes/components/text_field.py @@ -105,10 +105,10 @@ def create(cls, *children, **props) -> Component: (value_var := Var.create(value))._var_type ): props["value"] = ternary_operation( - (value_var != Var.create(None)) + (value_var != Var.create(None)) # pyright: ignore [reportGeneralTypeIssues] & (value_var != Var(_js_expr="undefined")), value, - "", + Var.create(""), ) component = super().create(*children, **props) From 7286ec5c5cf27067d62fb1303fed8f78f1995269 Mon Sep 17 00:00:00 2001 From: Elijah Date: Thu, 12 Dec 2024 22:54:27 +0000 Subject: [PATCH 10/10] fix comments --- reflex/components/el/elements/forms.py | 2 +- reflex/components/radix/themes/components/text_field.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/reflex/components/el/elements/forms.py b/reflex/components/el/elements/forms.py index 55f5ec99b3..7b893cf98f 100644 --- a/reflex/components/el/elements/forms.py +++ b/reflex/components/el/elements/forms.py @@ -400,7 +400,7 @@ def create(cls, *children, **props): value = props.get("value") - # React expects an empty string(instead of null) for uncontrolled inputs. + # React expects an empty string(instead of null) for controlled inputs. if value is not None and is_optional( (value_var := Var.create(value))._var_type ): diff --git a/reflex/components/radix/themes/components/text_field.py b/reflex/components/radix/themes/components/text_field.py index 1043554041..7e6dfe85c9 100644 --- a/reflex/components/radix/themes/components/text_field.py +++ b/reflex/components/radix/themes/components/text_field.py @@ -100,7 +100,7 @@ def create(cls, *children, **props) -> Component: """ value = props.get("value") - # React expects an empty string(instead of null) for uncontrolled inputs. + # React expects an empty string(instead of null) for controlled inputs. if value is not None and is_optional( (value_var := Var.create(value))._var_type ):