From 22f0223fb1d290e130ecb0072fdc35d3d9346252 Mon Sep 17 00:00:00 2001 From: Carson Date: Thu, 24 Jul 2025 10:48:52 -0500 Subject: [PATCH 1/8] fix: Introduce a more ergonomic API for express.ui.insert_accordion_panel() --- shiny/express/ui/__init__.py | 5 ++- shiny/express/ui/_insert.py | 80 ++++++++++++++++++++++++++++++++++++ shiny/ui/_accordion.py | 10 ++--- 3 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 shiny/express/ui/_insert.py diff --git a/shiny/express/ui/__init__.py b/shiny/express/ui/__init__.py index 62a43de79..663d13e70 100644 --- a/shiny/express/ui/__init__.py +++ b/shiny/express/ui/__init__.py @@ -74,7 +74,6 @@ input_text, input_text_area, panel_title, - insert_accordion_panel, remove_accordion_panel, update_accordion, update_accordion_panel, @@ -161,6 +160,10 @@ hold, ) +from ._insert import ( + insert_accordion_panel, +) + __all__ = ( # Imports from htmltools "TagList", diff --git a/shiny/express/ui/_insert.py b/shiny/express/ui/_insert.py new file mode 100644 index 000000000..0852f4232 --- /dev/null +++ b/shiny/express/ui/_insert.py @@ -0,0 +1,80 @@ +""" +Shims for `ui.insert_*()`, `ui.update_*()`, etc. functions that lead to a more ergonomic +Express API. + +These functions tend to have one issue in common: if they were re-exported verbatim from +Core to Express, they would want to take RecallContextManager(s) as input, which leads +to a somewhat awkward API. That's because, you'd have to know to use something like +@ui.hold() pass the UI as a value without displaying it. +""" + +from typing import Literal, Optional + +from htmltools import TagAttrs, TagChild + +from ..._docstring import add_example +from ...session import Session +from ...types import MISSING, MISSING_TYPE + + +@add_example() +def insert_accordion_panel( + id: str, + panel_title: str, + *panel_contents: TagChild | TagAttrs, + panel_value: str | MISSING_TYPE | None = MISSING, + panel_icon: TagChild = None, + target: Optional[str] = None, + position: Literal["after", "before"] = "after", + session: Optional[Session] = None, +) -> None: + """ + Insert an accordion panel into an existing accordion. + + Parameters + ---------- + id + A string that matches an existing :func:`~shiny.ui.express.accordion`'s `id`. + panel_title + The title to appear in the panel header. + panel_contents + UI elements for the panel's body. Can also be a dict of tag attributes for the + body's HTML container. + panel_value + A character string that uniquely identifies this panel. If `MISSING`, the + `title` will be used. + panel_icon + A :class:`~htmltools.TagChild` which is positioned just before the `title`. + target + The `value` of an existing panel to insert next to. + position + Should `panel` be added before or after the target? When `target=None`, + `"after"` will append after the last panel and `"before"` will prepend before + the first panel. + session + A Shiny session object (the default should almost always be used). + + References + ---------- + [Bootstrap Accordion](https://getbootstrap.com/docs/5.3/components/accordion/) + + See Also + -------- + * :func:`~shiny.ui.express.accordion` + * :func:`~shiny.ui.express.accordion_panel` + * :func:`~shiny.ui.express.update_accordion` + """ + + from ...ui import accordion_panel, insert_accordion_panel + + panel = accordion_panel( + panel_title, *panel_contents, value=panel_value, icon=panel_icon + ) + + insert_accordion_panel( + id=id, + panel=panel, + target=target, + position=position, + session=session, + ) diff --git a/shiny/ui/_accordion.py b/shiny/ui/_accordion.py index 2201b5eba..5d4ca72b5 100644 --- a/shiny/ui/_accordion.py +++ b/shiny/ui/_accordion.py @@ -315,17 +315,17 @@ def accordion_panel( Parameters ---------- title - A title to appear in the :func:`~shiny.ui.accordion_panel`'s header. + A title to appear in the panel's header. *args - Contents to the accordion panel body. Or tag attributes that are supplied to the - returned :class:`~htmltools.Tag` object. + UI elements for the panel's body. Can also be a dict of tag attributes for the + body's HTML container. value A character string that uniquely identifies this panel. If `MISSING`, the `title` will be used. icon - A :class:`~htmltools.Tag` which is positioned just before the `title`. + A :class:`~htmltools.TagChild` which is positioned just before the `title`. **kwargs - Tag attributes to the `accordion-body` div Tag. + Tag attributes for the body's HTML container. Returns ------- From 266b66285d517b84986da3a38ad6bb696d6adcc8 Mon Sep 17 00:00:00 2001 From: Carson Date: Thu, 24 Jul 2025 11:11:20 -0500 Subject: [PATCH 2/8] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a8f10830..b39da0152 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `playwright.controller.InputActionButton` gains a `expect_icon()` method. As a result, the already existing `expect_label()` no longer includes the icon. (#2020) +### Changes + +* `express.ui.insert_accordion_panel()`'s function signature has changed to be more ergonomic. Now you can pass the `panel_title` and `panel_contents` directly instead of `ui.hold()`ing the `ui.accordion_panel()` context manager. (#2042) + ### Improvements * Improved the styling and readability of markdown tables rendered by `ui.Chat()` and `ui.MarkdownStream()`. (#1973) From 0c68c53ef3e8552b2862386f913006d4fcaead7c Mon Sep 17 00:00:00 2001 From: Carson Date: Thu, 24 Jul 2025 11:55:00 -0500 Subject: [PATCH 3/8] Be more graceful about backwards-compat --- shiny/express/ui/_insert.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/shiny/express/ui/_insert.py b/shiny/express/ui/_insert.py index 0852f4232..c3be98b03 100644 --- a/shiny/express/ui/_insert.py +++ b/shiny/express/ui/_insert.py @@ -65,11 +65,17 @@ def insert_accordion_panel( * :func:`~shiny.ui.express.update_accordion` """ - from ...ui import accordion_panel, insert_accordion_panel + from ...ui import AccordionPanel, accordion_panel, insert_accordion_panel - panel = accordion_panel( - panel_title, *panel_contents, value=panel_value, icon=panel_icon - ) + if isinstance(panel_title, AccordionPanel): + # If the user passed an AccordionPanel, we can just use it as is. + # This isn't recommended, but we support it for backwards compatibility + # with the old API. + panel = panel_title + else: + panel = accordion_panel( + panel_title, *panel_contents, value=panel_value, icon=panel_icon + ) insert_accordion_panel( id=id, From 86e125255cc41a82f7eeb3f482fe317e9d9a736b Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Fri, 25 Jul 2025 11:37:09 +0530 Subject: [PATCH 4/8] Add test for Shiny Express insert accordion panel behavior Introduces a new app and Playwright test to verify dynamic updating and insertion of accordion panels in Shiny Express UI. Tests cover initial panel rendering, updating panel content and title, and adding new panels via action buttons. --- .../app-express.py | 40 +++++++++++++++ .../test_app_express.py | 49 +++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 tests/playwright/shiny/bugs/1802-shiny-express-ui-accordion-panel-doesnt-work/app-express.py create mode 100644 tests/playwright/shiny/bugs/1802-shiny-express-ui-accordion-panel-doesnt-work/test_app_express.py diff --git a/tests/playwright/shiny/bugs/1802-shiny-express-ui-accordion-panel-doesnt-work/app-express.py b/tests/playwright/shiny/bugs/1802-shiny-express-ui-accordion-panel-doesnt-work/app-express.py new file mode 100644 index 000000000..296643fe9 --- /dev/null +++ b/tests/playwright/shiny/bugs/1802-shiny-express-ui-accordion-panel-doesnt-work/app-express.py @@ -0,0 +1,40 @@ +import datetime + +from shiny import reactive +from shiny.express import input, ui + +with ui.accordion(id="my_accordion"): + with ui.accordion_panel(title="About"): + "This is a simple Shiny app." + + with ui.accordion_panel(title="Panel 1"): + "Some initial content for Panel 1." + + with ui.accordion_panel(title="Panel 2", value="panel_2_val"): + "Some initial content for Panel 2." + +ui.input_action_button("update_button", "Update Panel 2") +ui.input_action_button("add_panel_button", "Add New Panel") + +panel_counter = reactive.value(3) + + +@reactive.effect +@reactive.event(input.update_button) +def _(): + new_content = f"Content updated at: {datetime.datetime.now().strftime('%H:%M:%S')}" + ui.update_accordion_panel( + "my_accordion", "panel_2_val", new_content, title="Panel 2 (Updated)" + ) + + +@reactive.effect +@reactive.event(input.add_panel_button) +def _(): + current_count = panel_counter.get() + panel_counter.set(current_count + 1) + ui.insert_accordion_panel( + "my_accordion", + f"Panel {current_count}", + f"This is dynamically added panel {current_count}, created at {datetime.datetime.now().strftime('%H:%M:%S')}", + ) diff --git a/tests/playwright/shiny/bugs/1802-shiny-express-ui-accordion-panel-doesnt-work/test_app_express.py b/tests/playwright/shiny/bugs/1802-shiny-express-ui-accordion-panel-doesnt-work/test_app_express.py new file mode 100644 index 000000000..b5eb120f9 --- /dev/null +++ b/tests/playwright/shiny/bugs/1802-shiny-express-ui-accordion-panel-doesnt-work/test_app_express.py @@ -0,0 +1,49 @@ +from playwright.sync_api import Page + +from shiny.playwright import controller +from shiny.pytest import create_app_fixture +from shiny.run import ShinyAppProc + +app = create_app_fixture(["app-express.py"]) + + +def test_accordion_and_buttons(page: Page, app: ShinyAppProc) -> None: + page.goto(app.url) + + accordion = controller.Accordion(page, "my_accordion") + update_button = controller.InputActionButton(page, "update_button") + add_panel_button = controller.InputActionButton(page, "add_panel_button") + + accordion.expect_panels(["About", "Panel 1", "panel_2_val"]) + + about_panel = accordion.accordion_panel("About") + about_panel.expect_label("About") + about_panel.expect_body("This is a simple Shiny app.") + + panel_1 = accordion.accordion_panel("Panel 1") + panel_1.expect_label("Panel 1") + panel_1.expect_body("Some initial content for Panel 1.") + + panel_2 = accordion.accordion_panel("panel_2_val") + panel_2.expect_label("Panel 2") + panel_2.expect_body("Some initial content for Panel 2.") + + update_button.expect_label("Update Panel 2") + update_button.click() + + panel_2.expect_label("Panel 2 (Updated)") + + add_panel_button.expect_label("Add New Panel") + add_panel_button.click() + + accordion.expect_panels(["About", "Panel 1", "panel_2_val", "Panel 3"]) + + panel_3 = accordion.accordion_panel("Panel 3") + panel_3.expect_label("Panel 3") + + add_panel_button.click() + + accordion.expect_panels(["About", "Panel 1", "panel_2_val", "Panel 3", "Panel 4"]) + + panel_4 = accordion.accordion_panel("Panel 4") + panel_4.expect_label("Panel 4") From bf58ee2a67a44dfe0a15397d731f37da61b2a8b3 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Fri, 25 Jul 2025 11:48:36 +0530 Subject: [PATCH 5/8] Refactor accordion panel creation in app-express.py Replaces the previous list-based accordion panel creation with context-managed blocks for improved readability and structure. Updates dynamic panel insertion to match the new format, specifying both the section title and narrative. --- .../insert_accordion_panel/app-express.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/shiny/api-examples/insert_accordion_panel/app-express.py b/shiny/api-examples/insert_accordion_panel/app-express.py index 1de832962..59e244313 100644 --- a/shiny/api-examples/insert_accordion_panel/app-express.py +++ b/shiny/api-examples/insert_accordion_panel/app-express.py @@ -1,7 +1,7 @@ import random -from shiny import reactive, ui -from shiny.express import input +from shiny import reactive +from shiny.express import input, ui def make_panel(letter): @@ -11,10 +11,18 @@ def make_panel(letter): ui.input_action_button("add_panel", "Add random panel", class_="mt-3 mb-3") -ui.accordion(*[make_panel(letter) for letter in "ABCDE"], id="acc", multiple=True) + +with ui.accordion(id="acc", multiple=True): + for letter in "ABCDE": + with ui.accordion_panel(f"Section {letter}"): + f"Some narrative for section {letter}" @reactive.effect @reactive.event(input.add_panel) def _(): - ui.insert_accordion_panel("acc", make_panel(str(random.randint(0, 10000)))) + ui.insert_accordion_panel( + "acc", + f"Section {random.randint(0, 10000)}", + f"Some narrative for section {random.randint(0, 10000)}", + ) From 47ccb7c074b3fa2e17f7834437cc66575161149f Mon Sep 17 00:00:00 2001 From: Carson Date: Fri, 25 Jul 2025 14:57:36 -0500 Subject: [PATCH 6/8] cleanup --- shiny/api-examples/insert_accordion_panel/app-express.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/shiny/api-examples/insert_accordion_panel/app-express.py b/shiny/api-examples/insert_accordion_panel/app-express.py index 59e244313..cac1f7ee5 100644 --- a/shiny/api-examples/insert_accordion_panel/app-express.py +++ b/shiny/api-examples/insert_accordion_panel/app-express.py @@ -3,13 +3,6 @@ from shiny import reactive from shiny.express import input, ui - -def make_panel(letter): - return ui.accordion_panel( - f"Section {letter}", f"Some narrative for section {letter}" - ) - - ui.input_action_button("add_panel", "Add random panel", class_="mt-3 mb-3") with ui.accordion(id="acc", multiple=True): From a96032fd97fcfe135e210089967366aa2d2db386 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Mon, 28 Jul 2025 21:39:54 +0530 Subject: [PATCH 7/8] Refactor accordion app to use shiny.express and simplify logic Refactors the accordion test app to use the new shiny.express API. --- .../shiny/components/accordion/app.py | 275 +++++++++--------- .../components/accordion/test_accordion.py | 2 +- 2 files changed, 141 insertions(+), 136 deletions(-) diff --git a/tests/playwright/shiny/components/accordion/app.py b/tests/playwright/shiny/components/accordion/app.py index 5ee4d2b7a..f9b230cab 100644 --- a/tests/playwright/shiny/components/accordion/app.py +++ b/tests/playwright/shiny/components/accordion/app.py @@ -1,142 +1,147 @@ from __future__ import annotations -from shiny import App, Inputs, Outputs, Session, reactive, render, req, ui - - -def make_panel(letter: str) -> ui.AccordionPanel: - return ui.accordion_panel( - f"Section {letter}", - f"Some narrative for section {letter}", - ) - - -items = [make_panel(letter) for letter in "ABCD"] - -accordion = ui.accordion(*items, id="acc") -app_ui = ui.page_fluid( - ui.tags.div( - ui.input_action_button("toggle_b", "Open/Close B"), - ui.input_action_button("open_all", "Open All"), - ui.input_action_button("close_all", "Close All"), - ui.input_action_button("alternate", "Alternate"), - ui.input_action_button("toggle_efg", "Add/Remove EFG"), - ui.input_action_button("toggle_updates", "Add/Remove Updates"), - class_="d-flex", - ), - ui.output_text_verbatim("acc_txt", placeholder=True), - accordion, -) - - -def server(input: Inputs, output: Outputs, session: Session) -> None: - @reactive.calc - def acc() -> list[str]: - acc_val: list[str] | None = input.acc() - if acc_val is None: - acc_val = [] - return acc_val - - @reactive.effect - def _(): - req(input.toggle_b()) +from shiny import reactive, render +from shiny.express import input, ui - with reactive.isolate(): - if "Section B" in acc(): - ui.update_accordion_panel("acc", "Section B", show=False) - else: - ui.update_accordion_panel("acc", "Section B", show=True) - - @reactive.effect - def _(): - req(input.open_all()) - ui.update_accordion("acc", show=True) - - @reactive.effect - def _(): - req(input.close_all()) - ui.update_accordion("acc", show=False) - - has_efg = False - has_alternate = True - has_updates = False - - @reactive.effect - def _(): - req(input.alternate()) - - sections = [ - "updated_section_a" if has_updates else "Section A", - "Section B", - "Section C", - "Section D", - ] - if has_efg: - sections.extend(["Section E", "Section F", "Section G"]) - - nonlocal has_alternate - val = int(has_alternate) - sections = [section for i, section in enumerate(sections) if i % 2 == val] - ui.update_accordion("acc", show=sections) - has_alternate = not has_alternate - - @reactive.effect - def _(): - req(input.toggle_efg()) - - nonlocal has_efg - if has_efg: - ui.remove_accordion_panel("acc", ["Section E", "Section F", "Section G"]) - else: - ui.insert_accordion_panel("acc", make_panel("E"), "Section D") - ui.insert_accordion_panel("acc", make_panel("F"), "Section E") - ui.insert_accordion_panel("acc", make_panel("G"), "Section F") - - has_efg = not has_efg - - @reactive.effect - def _(): - req(input.toggle_updates()) - - nonlocal has_updates - if has_updates: - ui.update_accordion_panel( - "acc", - "updated_section_a", - "Some narrative for section A", - title="Section A", - value="Section A", - icon="", - ) - else: - with reactive.isolate(): - # print(acc()) - if "Section A" not in acc(): - ui.notification_show("Opening Section A", duration=2) - ui.update_accordion_panel("acc", "Section A", show=True) - ui.update_accordion_panel( - "acc", - "Section A", - "Updated body", - value="updated_section_a", - title=ui.tags.h3("Updated title"), - icon=ui.tags.div( - "Look! An icon! -->", - ui.HTML( - """\ - - - - - """ - ), - ), - ) - has_updates = not has_updates +def make_panel_title(letter: str) -> str: + return f"Section {letter}" + + +def make_panel_content(letter: str) -> str: + return f"Some narrative for section {letter}" + + +with ui.tags.div(class_="d-flex"): + ui.input_action_button("toggle_b", "Open/Close B") + ui.input_action_button("open_all", "Open All") + ui.input_action_button("close_all", "Close All") + ui.input_action_button("alternate", "Alternate") + ui.input_action_button("toggle_efg", "Add/Remove EFG") + ui.input_action_button("toggle_updates", "Add/Remove Updates") + - @render.text - def acc_txt(): - return f"input.acc(): {input.acc()}" +@render.text +def acc_txt(): + return f"input.acc(): {input.acc()}" -app = App(app_ui, server) +with ui.accordion(id="acc"): + for letter in "ABCD": + with ui.accordion_panel(f"Section {letter}"): + f"Some narrative for section {letter}" + + +@reactive.calc +def acc() -> list[str]: + acc_val: list[str] | None = input.acc() + if acc_val is None: + acc_val = [] + return acc_val + + +@reactive.effect +@reactive.event(input.toggle_b) +def _(): + with reactive.isolate(): + if "Section B" in acc(): + ui.update_accordion_panel("acc", "Section B", show=False) + else: + ui.update_accordion_panel("acc", "Section B", show=True) + + +@reactive.effect +@reactive.event(input.open_all) +def _(): + ui.update_accordion("acc", show=True) + + +@reactive.effect +@reactive.event(input.close_all) +def _(): + ui.update_accordion("acc", show=False) + + +has_efg = False +has_alternate = True +has_updates = False + + +@reactive.effect +@reactive.event(input.alternate) +def _(): + sections = [ + "updated_section_a" if has_updates else "Section A", + "Section B", + "Section C", + "Section D", + ] + if has_efg: + sections.extend(["Section E", "Section F", "Section G"]) + + global has_alternate + val = int(has_alternate) + sections = [section for i, section in enumerate(sections) if i % 2 == val] + ui.update_accordion("acc", show=sections) + has_alternate = not has_alternate + + +@reactive.effect +@reactive.event(input.toggle_efg) +def _(): + global has_efg + if has_efg: + ui.remove_accordion_panel("acc", ["Section E", "Section F", "Section G"]) + else: + ui.insert_accordion_panel( + "acc", make_panel_title("E"), make_panel_content("E"), target="Section D" + ) + ui.insert_accordion_panel( + "acc", make_panel_title("F"), make_panel_content("F"), target="Section E" + ) + ui.insert_accordion_panel( + "acc", make_panel_title("G"), make_panel_content("G"), target="Section F" + ) + + has_efg = not has_efg + + +@reactive.effect +@reactive.event(input.toggle_updates) +def _(): + global has_updates + if has_updates: + ui.update_accordion_panel( + "acc", + "updated_section_a", + "Some narrative for section A", + title="Section A", + value="Section A", + icon="", + ) + else: + with reactive.isolate(): + # print(acc()) + if "Section A" not in acc(): + ui.notification_show("Opening Section A", duration=2) + ui.update_accordion_panel("acc", "Section A", show=True) + ui.update_accordion_panel( + "acc", + "Section A", + "Updated body", + value="updated_section_a", + title=ui.tags.h3("Updated title"), + icon=ui.tags.div( + "Look! An icon! -->", + ui.HTML( + """\ + + + + + """ + ), + ), + ) + + has_updates = not has_updates diff --git a/tests/playwright/shiny/components/accordion/test_accordion.py b/tests/playwright/shiny/components/accordion/test_accordion.py index f2d366c4b..18c14febb 100644 --- a/tests/playwright/shiny/components/accordion/test_accordion.py +++ b/tests/playwright/shiny/components/accordion/test_accordion.py @@ -12,7 +12,7 @@ def test_accordion(page: Page, local_app: ShinyAppProc) -> None: acc = controller.Accordion(page, "acc") acc_panel_A = acc.accordion_panel("Section A") - output_txt_verbatim = controller.OutputTextVerbatim(page, "acc_txt") + output_txt_verbatim = controller.OutputText(page, "acc_txt") alternate_button = controller.InputActionButton(page, "alternate") open_all_button = controller.InputActionButton(page, "open_all") close_all_button = controller.InputActionButton(page, "close_all") From d3802c93c47dd74930d02e8fb92ae8a1c2bcf54b Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Mon, 28 Jul 2025 21:59:42 +0530 Subject: [PATCH 8/8] for python 3.9 compatibility --- shiny/express/ui/_insert.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shiny/express/ui/_insert.py b/shiny/express/ui/_insert.py index c3be98b03..9b8e4d732 100644 --- a/shiny/express/ui/_insert.py +++ b/shiny/express/ui/_insert.py @@ -8,7 +8,7 @@ @ui.hold() pass the UI as a value without displaying it. """ -from typing import Literal, Optional +from typing import Literal, Optional, Union from htmltools import TagAttrs, TagChild @@ -21,8 +21,8 @@ def insert_accordion_panel( id: str, panel_title: str, - *panel_contents: TagChild | TagAttrs, - panel_value: str | MISSING_TYPE | None = MISSING, + *panel_contents: Union[TagChild, TagAttrs], + panel_value: Union[str, MISSING_TYPE, None] = MISSING, panel_icon: TagChild = None, target: Optional[str] = None, position: Literal["after", "before"] = "after",