Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Radix 3.0 #3159

Merged
merged 15 commits into from
Apr 25, 2024
Merged

Radix 3.0 #3159

merged 15 commits into from
Apr 25, 2024

Conversation

Lendemor
Copy link
Collaborator

Upgrade to 3.0 version of Radix UI to include new components and some new features.

Lendemor and others added 7 commits April 24, 2024 16:21
* add spinner from radix 3.0

* fix pyi

* fix TextField.Input being removed

* add cb_group, cb_cards, radio_card, skeleton, segmented_control, progress, data_list
picklelo and others added 2 commits April 24, 2024 23:36
Co-authored-by: Masen Furer <m_github@0x26.net>
Co-authored-by: Masen Furer <m_github@0x26.net>
masenf
masenf previously approved these changes Apr 25, 2024
avoid element not interactable error in github actions
@picklelo picklelo merged commit ac36bfc into main Apr 25, 2024
46 checks passed
@picklelo picklelo mentioned this pull request Apr 26, 2024
18 tasks
@benedikt-bartscher
Copy link
Contributor

@Lendemor could this PR have broken class_name prop for rx.input? I just discovered all class names are missing on my inputs since i updated reflex to main (after this PR was merged)

@abulvenz
Copy link
Contributor

@Lendemor could this PR have broken class_name prop for rx.input? I just discovered all class names are missing on my inputs since i updated reflex to main (after this PR was merged)

@benedikt-bartscher You are right. I have added a css class to test_input.py. Now the test fails.

diff --git a/integration/test_input.py b/integration/test_input.py
index a57219b9..783cb784 100644
--- a/integration/test_input.py
+++ b/integration/test_input.py
@@ -27,6 +27,7 @@ def FullyControlledInput():
                 id="debounce_input_input",
                 on_change=State.set_text,  # type: ignore
                 value=State.text,
+                class_name="first-input",
             ),
             rx.input(value=State.text, id="value_input", is_read_only=True),
             rx.input(on_change=State.set_text, id="on_change_input"),  # type: ignore
@@ -81,6 +82,10 @@ async def test_fully_controlled_input(fully_controlled_input: AppHarness):
     token_input = driver.find_element(By.ID, "token")
     assert token_input
 
+    assert driver.find_element(
+        By.CLASS_NAME, "first-input"
+    ).is_displayed(), "Expecting input to be found and visible."
+
     # wait for the backend connection to send the token
     token = fully_controlled_input.poll_for_value(token_input)
     assert token

@Lendemor
Copy link
Collaborator Author

Lendemor commented Apr 29, 2024

@benedikt-bartscher @abulvenz
This might be a bug in Radix itself, we will investigate more.

Python / Reflex :

rx.input(value=State.x, class_name="test-class"),

Compiled React :

<RadixThemesTextField.Root
  className={`test-class`}
  value={state__state.x}
/>

Compiled HTML in browser:

<input spellcheck="false" class="rt-reset rt-TextFieldInput" value="Hello World 2">

@Lendemor
Copy link
Collaborator Author

May have read the html a bit too fast, the test-classdoes show up, but on the encapsulating div

<div data-accent-color="" class="rt-TextFieldRoot rt-r-size-2 rt-variant-surface test-class">
    <input spellcheck="false" class="rt-reset rt-TextFieldInput" value="Hello World 2">
</div>

@Lendemor
Copy link
Collaborator Author

@abulvenz If I add a slightly different one, then it doesn't fail.

diff --git a/integration/test_input.py b/integration/test_input.py
index a57219b9..3438f55c 100644
--- a/integration/test_input.py
+++ b/integration/test_input.py
@@ -28,6 +28,7 @@ def FullyControlledInput():
                 on_change=State.set_text,  # type: ignore
                 value=State.text,
             ),
+            rx.input(placeholder="class name test", class_name="test_class"),
             rx.input(value=State.text, id="value_input", is_read_only=True),
             rx.input(on_change=State.set_text, id="on_change_input"),  # type: ignore
             rx.el.input(
@@ -81,6 +82,10 @@ async def test_fully_controlled_input(fully_controlled_input: AppHarness):
     token_input = driver.find_element(By.ID, "token")
     assert token_input

+    assert driver.find_element(
+        By.CLASS_NAME, "test_class"
+    ).is_displayed(), "Expecting input to be found and visible."
+
     # wait for the backend connection to send the token
     token = fully_controlled_input.poll_for_value(token_input)
     assert token

Will keep investigating.

@Lendemor
Copy link
Collaborator Author

@benedikt-bartscher Did some testing on my side, all the class names should be on the encapsulating div.

rx.input(
    value=InputState.value,
    on_change=InputState.set_value,
    class_name="test-class w-screen",
)

Using the above code, I have an input that properly take up the width of 100vw from the class name.

<div data-accent-color="" class="rt-TextFieldRoot rt-r-size-2 rt-variant-surface  test-class w-screen">
    <input spellcheck="false" type="text" class="rt-reset rt-TextFieldInput" value="">
</div>

Feel free to open an actual issue with some repro code if some problem persist, but I think it is working fine as it is now.

@abulvenz
The unit test must have failed because you used is_displayed() on a debounced input.
I think when debounced, the actual input is "hidden" and replaced by the debounced one, hence failing the test.
That's also why the unit test work when testing on a non debounced input, since it is actually displayed and not "hidden".

@abulvenz
Copy link
Contributor

@Lendemor Ah, you are right. If I ask for it in the browser also the hidden input shows up
grafik
Are the inputs debounced by their ID or are all inputs debounced by default now? I remember that we had to use rx.debounced_input(rx.input(...)) once upon a time.

@benedikt-bartscher
Copy link
Contributor

@Lendemor thanks for investigating, seems like there is no bug in reflex, just a change introduced by radix 3.0

@abulvenz rx.input is auto-wrapped in debounce if you pass both, value and on_change props, see https://github.com/reflex-dev/reflex/blob/main/reflex/components/radix/themes/components/text_field.py#L82

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants