Skip to content

Commit

Permalink
Use the default widget height for non-stacked checkbox & toggle widge…
Browse files Browse the repository at this point in the history
…ts (streamlit#8835)

## Describe your changes

This PR changes the min-height of single checkbox and toggle widgets to
2.5rem (default for widgets). This allows better vertical alignment. If
checkboxes / toggles are stacked, we will apply a smaller height of
1.5rem (current default) to have them look visually closer together.

## Testing Plan

- Added e2e tests and updated snapshots.

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
  • Loading branch information
lukasmasuch authored Jun 13, 2024
1 parent 90c4d78 commit d7caf46
Show file tree
Hide file tree
Showing 275 changed files with 192 additions and 106 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
9 changes: 4 additions & 5 deletions e2e_playwright/multipage_apps_v2/mpa_v2_basics_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
wait_for_app_loaded,
wait_for_app_run,
)
from e2e_playwright.shared.app_utils import click_checkbox


def main_heading(app: Page):
Expand All @@ -33,16 +34,14 @@ def page_heading(app: Page):
def check_field(
app: Page, *, hide_sidebarnav=False, dynamic_pages=False, add_sidebar_elements=False
):
checkboxes = app.get_by_test_id("stCheckbox")

if hide_sidebarnav:
checkboxes.nth(0).click(delay=50)
click_checkbox(app, "Hide sidebar")

if dynamic_pages:
checkboxes.nth(1).click(delay=50)
click_checkbox(app, "Change navigation dynamically")

if add_sidebar_elements:
checkboxes.nth(2).click(delay=50)
click_checkbox(app, "Show sidebar elements")


expected_page_order = [
Expand Down
62 changes: 44 additions & 18 deletions e2e_playwright/shared/app_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from e2e_playwright.conftest import wait_for_app_run


def get_checkbox(locator: Locator, label: str | Pattern[str]) -> Locator:
def get_checkbox(locator: Locator | Page, label: str | Pattern[str]) -> Locator:
"""Get a checkbox widget with the given label.
Parameters
Expand Down Expand Up @@ -93,7 +93,9 @@ def get_button(locator: Locator | Page, label: str | Pattern[str]) -> Locator:
return element


def get_form_submit_button(locator: Locator, label: str | Pattern[str]) -> Locator:
def get_form_submit_button(
locator: Locator | Page, label: str | Pattern[str]
) -> Locator:
"""Get a form submit button with the given label.
Parameters
Expand All @@ -117,7 +119,7 @@ def get_form_submit_button(locator: Locator, label: str | Pattern[str]) -> Locat
return element


def get_expander(locator: Locator, label: str | Pattern[str]) -> Locator:
def get_expander(locator: Locator | Page, label: str | Pattern[str]) -> Locator:
"""Get a expander container with the given label.
Parameters
Expand All @@ -141,7 +143,9 @@ def get_expander(locator: Locator, label: str | Pattern[str]) -> Locator:
return element


def get_markdown(locator: Locator, text_inside_markdown: str | Pattern[str]) -> Locator:
def get_markdown(
locator: Locator | Page, text_inside_markdown: str | Pattern[str]
) -> Locator:
"""Get a markdown element with the given text inside.
Parameters
Expand Down Expand Up @@ -261,11 +265,11 @@ def expect_exception(
expect(exception_el).to_be_visible()


def click_button(
def click_checkbox(
page: Page,
label: str | Pattern[str],
) -> None:
"""Click a button with the given label
"""Click a checkbox with the given label
and wait for the app to run.
Parameters
Expand All @@ -277,16 +281,36 @@ def click_button(
label : str or Pattern[str]
The label of the button to click.
"""
button_element = get_button(page, label)
button_element.click()
checkbox_element = get_checkbox(page, label)
# Click the checkbox label to be more reliable
checkbox_element.locator("label").click()
wait_for_app_run(page)


def click_form_button(
def click_toggle(
page: Page,
label: str | Pattern[str],
) -> None:
"""Click a form submit button with the given label
"""Click a toggle with the given label
and wait for the app to run.
Parameters
----------
page : Page
The page to click the toggle on.
label : str or Pattern[str]
The label of the toggle to click.
"""
click_checkbox(page, label)


def click_button(
page: Page,
label: str | Pattern[str],
) -> None:
"""Click a button with the given label
and wait for the app to run.
Parameters
Expand All @@ -298,34 +322,36 @@ def click_form_button(
label : str or Pattern[str]
The label of the button to click.
"""
button_element = get_form_submit_button(page, label)
button_element = get_button(page, label)
button_element.click()
wait_for_app_run(page)


def click_checkbox(
def click_form_button(
page: Page,
label: str | Pattern[str],
) -> None:
"""Click a checkbox with the given label
"""Click a form submit button with the given label
and wait for the app to run.
Parameters
----------
page : Page
The page to click the checkbox on.
The page to click the button on.
label : str or Pattern[str]
The label of the checkbox to click.
The label of the button to click.
"""
checkbox_element = get_checkbox(page, label)
checkbox_element.click()
button_element = get_form_submit_button(page, label)
button_element.click()
wait_for_app_run(page)


def expect_help_tooltip(
app: Page, element_with_help_tooltip: Locator, tooltip_text: str | Pattern[str]
app: Locator | Page,
element_with_help_tooltip: Locator,
tooltip_text: str | Pattern[str],
):
"""Expect a tooltip to be displayed when hovering over the help symbol of an element.
Expand Down
23 changes: 8 additions & 15 deletions e2e_playwright/st_audio_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
import pytest
from playwright.sync_api import Page, expect

from e2e_playwright.conftest import wait_for_app_run, wait_until
from e2e_playwright.conftest import wait_until
from e2e_playwright.shared.app_utils import click_button, click_checkbox


def test_audio_has_correct_properties(app: Page):
Expand Down Expand Up @@ -74,9 +75,8 @@ def test_audio_autoplay(app: Page):
expect(audio_element).to_have_js_property("paused", True)
expect(audio_element).to_have_js_property("autoplay", False)

checkbox_element = app.get_by_test_id("stCheckbox")
autoplay_checkbox = checkbox_element.nth(0)
autoplay_checkbox.click()
click_checkbox(app, "Autoplay")

# To prevent flakiness, we wait for the audio to load and start playing
wait_until(app, lambda: audio_element.evaluate("el => el.readyState") == 4)
expect(audio_element).to_have_js_property("autoplay", True)
Expand All @@ -94,22 +94,15 @@ def test_audio_remount_no_autoplay(app: Page):
expect(audio_element).to_have_js_property("paused", True)
expect(audio_element).to_have_js_property("autoplay", False)

checkbox_element = app.get_by_test_id("stCheckbox")
autoplay_checkbox = checkbox_element.nth(0)
autoplay_checkbox.click()
click_checkbox(app, "Autoplay")

# To prevent flakiness, we wait for the audio to load and start playing
wait_until(app, lambda: audio_element.evaluate("el => el.readyState") == 4)
expect(audio_element).to_have_js_property("autoplay", True)
expect(audio_element).to_have_js_property("paused", False)

autoplay_checkbox.click()

button_element = app.get_by_test_id("stButton").locator("button").first
expect(app.get_by_test_id("stMarkdownContainer").nth(1)).to_have_text(
"Create some elements to unmount component"
)
button_element.click()
wait_for_app_run(app)
click_checkbox(app, "Autoplay")
click_button(app, "Create some elements to unmount component")

expect(audio_element).to_have_js_property("autoplay", False)
expect(audio_element).to_have_js_property("paused", True)
8 changes: 4 additions & 4 deletions e2e_playwright/st_button_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from playwright.sync_api import Page, expect

from e2e_playwright.conftest import ImageCompareFunction
from e2e_playwright.shared.app_utils import click_button, click_checkbox


def test_button_widget_rendering(
Expand Down Expand Up @@ -84,14 +85,13 @@ def test_click_increment_count(app: Page):


def test_reset_on_other_widget_change(app: Page):
button_element = app.get_by_test_id("stButton").locator("button").first
checkbox_element = app.get_by_test_id("stCheckbox")
button_element.click()
click_button(app, "button 1")
expect(app.get_by_test_id("stMarkdown").nth(0)).to_have_text("value: True")
expect(app.get_by_test_id("stMarkdown").nth(1)).to_have_text(
"value from state: True"
)
checkbox_element.click()

click_checkbox(app, "reset button return value")
expect(app.get_by_test_id("stMarkdown").nth(0)).to_have_text("value: False")
expect(app.get_by_test_id("stMarkdown").nth(1)).to_have_text(
"value from state: False"
Expand Down
6 changes: 6 additions & 0 deletions e2e_playwright/st_checkbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,9 @@ def on_change():

i8 = st.checkbox("checkbox 8 (label collapsed)", label_visibility="collapsed")
st.write("checkbox 8 - value:", i8)

with st.expander("Grouped checkboxes", expanded=True):
st.checkbox("checkbox group - 1")
st.checkbox("checkbox group - 2")
st.checkbox("checkbox group - 3")
st.text("A non-checkbox element")
32 changes: 27 additions & 5 deletions e2e_playwright/st_checkbox_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,26 @@
from playwright.sync_api import Page, expect

from e2e_playwright.conftest import ImageCompareFunction, wait_for_app_run
from e2e_playwright.shared.app_utils import get_expander

CHECKBOX_ELEMENTS = 11


def test_checkbox_widget_display(
themed_app: Page, assert_snapshot: ImageCompareFunction
):
"""Test that st.checkbox renders correctly."""
checkbox_elements = themed_app.get_by_test_id("stCheckbox")
expect(checkbox_elements).to_have_count(8)
expect(checkbox_elements).to_have_count(CHECKBOX_ELEMENTS)

for i, element in enumerate(checkbox_elements.all()):
assert_snapshot(element, name=f"st_checkbox-{i}")
assert_snapshot(checkbox_elements.nth(0), name="st_checkbox-true")
assert_snapshot(checkbox_elements.nth(1), name="st_checkbox-false")
assert_snapshot(checkbox_elements.nth(2), name="st_checkbox-long_label")
assert_snapshot(checkbox_elements.nth(3), name="st_checkbox-callback")
assert_snapshot(checkbox_elements.nth(4), name="st_checkbox-false_disabled")
assert_snapshot(checkbox_elements.nth(5), name="st_checkbox-true_disabled")
assert_snapshot(checkbox_elements.nth(6), name="st_checkbox-hidden_label")
assert_snapshot(checkbox_elements.nth(7), name="st_checkbox-collapsed_label")


def test_checkbox_initial_values(app: Page):
Expand All @@ -53,7 +62,7 @@ def test_checkbox_initial_values(app: Page):
def test_checkbox_values_on_click(app: Page):
"""Test that st.checkbox updates values correctly when user clicks."""
checkbox_elements = app.get_by_test_id("stCheckbox")
expect(checkbox_elements).to_have_count(8)
expect(checkbox_elements).to_have_count(CHECKBOX_ELEMENTS)

for checkbox_element in checkbox_elements.all():
# Not sure if this is needed, but somehow it is slightly
Expand All @@ -63,7 +72,7 @@ def test_checkbox_values_on_click(app: Page):
# So, maybe thats the reason why it fails to click it.
# But this is just a guess.
checkbox_element.scroll_into_view_if_needed()
checkbox_element.click(delay=50)
checkbox_element.locator("label").click(delay=50, force=True)
wait_for_app_run(app)

markdown_elements = app.get_by_test_id("stMarkdown")
Expand All @@ -81,3 +90,16 @@ def test_checkbox_values_on_click(app: Page):

for markdown_element, expected_text in zip(markdown_elements.all(), expected):
expect(markdown_element).to_have_text(expected_text, use_inner_text=True)


def test_grouped_checkboxes_height(app: Page, assert_snapshot: ImageCompareFunction):
"""Test that grouped checkboxes have the correct height."""

expander_details = get_expander(app, "Grouped checkboxes").get_by_test_id(
"stExpanderDetails"
)
expect(expander_details.get_by_test_id("stCheckbox")).to_have_count(3)
assert_snapshot(expander_details, name="st_checkbox-grouped_styling")
expect(expander_details.get_by_test_id("stCheckbox").nth(0)).to_have_css(
"height", "24px"
)
14 changes: 9 additions & 5 deletions e2e_playwright/st_container_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from playwright.sync_api import Page, expect

from e2e_playwright.conftest import ImageCompareFunction, wait_for_app_run
from e2e_playwright.shared.app_utils import click_button, click_checkbox, get_checkbox


def test_permits_multiple_out_of_order_elements(app: Page):
Expand All @@ -29,14 +30,17 @@ def test_permits_multiple_out_of_order_elements(app: Page):

def test_persists_widget_state_across_reruns(app: Page):
"""Test that st.container persists widget state across reruns."""
checkbox_widget = app.get_by_test_id("stCheckbox").first
checkbox_widget.click()

click_checkbox(app, "Step 1: Check me")

expect(app.locator("h1").first).to_have_text("Checked!")

app.get_by_test_id("stButton").first.locator("button").click()
click_button(app, "Step 2: Press me")

expect(app.locator("h2").first).to_have_text("Pressed!")
expect(checkbox_widget.locator("input")).to_have_attribute("aria-checked", "true")
expect(get_checkbox(app, "Step 1: Check me").locator("input")).to_have_attribute(
"aria-checked", "true"
)


def test_renders_container_with_border(
Expand Down Expand Up @@ -85,7 +89,7 @@ def test_correctly_handles_first_chat_message(
behaviour change when adding the first chat message ."""

# Click button to add a chat message to the empty container:
app.get_by_test_id("stButton").nth(2).locator("button").click()
click_button(app, "Add message")

wait_for_app_run(app)

Expand Down
7 changes: 3 additions & 4 deletions e2e_playwright/st_experimental_fragment_basics_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from playwright.sync_api import Page, expect

from e2e_playwright.conftest import wait_for_app_run
from e2e_playwright.shared.app_utils import click_button, click_checkbox


def get_uuids(app: Page):
Expand All @@ -41,8 +42,7 @@ def expect_only_fragment_uuid_changed(
def test_button_in_fragment(app: Page):
old_text_in_fragment, old_text_outside_fragment = get_uuids(app)

app.get_by_test_id("stButton").locator("button").click()
wait_for_app_run(app)
click_button(app, "a button")

expect_only_fragment_uuid_changed(
app, old_text_in_fragment, old_text_outside_fragment
Expand Down Expand Up @@ -76,8 +76,7 @@ def test_chat_input_in_fragment(app: Page):
def test_checkbox_in_fragment(app: Page):
old_text_in_fragment, old_text_outside_fragment = get_uuids(app)

app.get_by_test_id("stCheckbox").click()
wait_for_app_run(app)
click_checkbox(app, "a checkbox")

expect_only_fragment_uuid_changed(
app, old_text_in_fragment, old_text_outside_fragment
Expand Down
Loading

0 comments on commit d7caf46

Please sign in to comment.