From fddbe22a379647ccb686e671fce04f8e52afa92b Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Mon, 9 Nov 2026 13:20:49 -0500 Subject: [PATCH] fix: write the bookmark URL to the address bar instead of a Bootstrap modal Shiny's default onBookmarked action shows the URL in a Bootstrap modal, and page_react() loads no Bootstrap, so session$doBookmark() / session.bookmark() threw "$(...).modal is not a function". The shinyreact.init handler now registers a default that calls updateQueryString() / update_query_string(), and steps aside once the app registers its own callback. Fixes posit-dev/shinyreact#326 --- FEATURES.md | 13 ++++ pkg-py/CHANGELOG.md | 6 ++ pkg-py/src/shinyreact/_bookmark.py | 35 +++++++++ pkg-py/src/shinyreact/_input_handler.py | 5 +- pkg-py/tests/test_bookmark_default.py | 81 ++++++++++++++++++++ pkg-r/NEWS.md | 6 ++ pkg-r/R/bookmark.R | 37 +++++++++ pkg-r/R/input-handler.R | 3 +- pkg-r/man/init_input_handler.Rd | 2 +- pkg-r/tests/testthat/test-bookmark-default.R | 76 ++++++++++++++++++ 10 files changed, 261 insertions(+), 3 deletions(-) create mode 100644 pkg-py/tests/test_bookmark_default.py create mode 100644 pkg-r/tests/testthat/test-bookmark-default.R diff --git a/FEATURES.md b/FEATURES.md index 6a8a4c15..36ab8d39 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -140,6 +140,19 @@ R has no e2e suite, so no `(e2e)` leaf covers R (issue #194). session, after Shiny initializes, so apps with no `useShinyInput` are covered too - it installs renderer dependency discovery for the session + - it replaces Shiny's bookmark default (#326) + - Shiny's default `onBookmarked` / `on_bookmarked` shows the URL in a + Bootstrap modal, which threw `$(...).modal is not a function` on a + Bootstrap-free `page_react()` page + - instead, bookmarking writes the URL to the address bar + (`updateQueryString()` / `update_query_string()`, `"replace"` mode) + - it acts only while it is the session's only bookmarked callback: an + app's own `onBookmarked()` / `on_bookmarked()` replaces it, just as it + would replace Shiny's default + - it installs once per session, and no-ops on sessions with no bookmark + callbacks (mock / stub sessions) + - it applies on every page the bundle runs on, including a + `page_react_html()` document that loads Bootstrap itself - it returns the value unchanged - the contract is defined in terms of the JSON the React hook sent - array of objects — `[{name, size}, ...]` diff --git a/pkg-py/CHANGELOG.md b/pkg-py/CHANGELOG.md index e2e85633..912b569c 100644 --- a/pkg-py/CHANGELOG.md +++ b/pkg-py/CHANGELOG.md @@ -8,6 +8,12 @@ IDs were found" warning grew and the console logged `Output not found` (#330). +* Bookmarking (`session.bookmark()`) on a `page_react()` page no longer fails + with `$(...).modal is not a function`. Shiny's default shows the URL in a + Bootstrap modal, and these pages load no Bootstrap; shinyreact now writes the + URL to the address bar instead. An app's own `on_bookmarked()` still replaces + the default (#326). + ## 0.1.0 * Initial PyPI release. diff --git a/pkg-py/src/shinyreact/_bookmark.py b/pkg-py/src/shinyreact/_bookmark.py index 5490b1b0..d7c99649 100644 --- a/pkg-py/src/shinyreact/_bookmark.py +++ b/pkg-py/src/shinyreact/_bookmark.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +from typing import Any from htmltools import HTML, HTMLDependency, head_content, tags from shiny.bookmark._restore_state import ( @@ -84,3 +85,37 @@ def _config_script_tag() -> HTMLDependency: return head_content( tags.script(HTML(payload), type="application/json", id="shinyreact-config") ) + + +_BOOKMARK_DEFAULT_FLAG = "_shinyreact_bookmark_default" + + +def install_bookmark_default(session: Any) -> bool: + """Replace Shiny's modal bookmark default with updating the address bar. + + Shiny's default ``on_bookmarked`` action shows the URL in a Bootstrap + modal, and ``page_react()`` loads no Bootstrap (#285), so + ``session.bookmark()`` failed in the browser (#326). + + Shiny runs its default only when no ``on_bookmarked`` callback exists, and + runs every registered callback otherwise. So ours acts only while it is the + only one: once the app registers its own, the app's callback alone decides + what happens, exactly as it would replace Shiny's default. + ``_on_bookmarked_callbacks`` is private; py-shiny has no public way to + count them (R reads ``private$bookmarkedCallbacks``, same deal). + + Installed by the ``shinyreact.init`` input handler, once per session. + Returns whether it was installed. + """ + bookmark: Any = getattr(session, "bookmark", None) + callbacks = getattr(bookmark, "_on_bookmarked_callbacks", None) + if callbacks is None or getattr(session, _BOOKMARK_DEFAULT_FLAG, False): + return False + setattr(session, _BOOKMARK_DEFAULT_FLAG, True) + + async def update_url(url: str) -> None: + if callbacks.count() == 1: + await bookmark.update_query_string(url) + + bookmark.on_bookmarked(update_url) + return True diff --git a/pkg-py/src/shinyreact/_input_handler.py b/pkg-py/src/shinyreact/_input_handler.py index 09828c48..b5e76415 100644 --- a/pkg-py/src/shinyreact/_input_handler.py +++ b/pkg-py/src/shinyreact/_input_handler.py @@ -14,6 +14,7 @@ from shiny.input_handler import input_handlers +from ._bookmark import install_bookmark_default from ._dep_discovery import install_dep_discovery if TYPE_CHECKING: @@ -35,8 +36,10 @@ def _shinyreact_asis(value: Any, name: ResolvedId, session: Session) -> Any: # The JS bundle sends one `.shinyreact_init:shinyreact.init` ping per session # after Shiny initializes (pkg-js/src/dep-discovery.ts); the handler bootstraps -# automatic output dependency discovery, matching R (issue #220). +# automatic output dependency discovery (issue #220) and the Bootstrap-free +# bookmark default (issue #326), matching R. @input_handlers.add("shinyreact.init", force=True) def _shinyreact_init(value: Any, name: ResolvedId, session: Session) -> Any: install_dep_discovery(session) + install_bookmark_default(session) return value diff --git a/pkg-py/tests/test_bookmark_default.py b/pkg-py/tests/test_bookmark_default.py new file mode 100644 index 00000000..82a9acb0 --- /dev/null +++ b/pkg-py/tests/test_bookmark_default.py @@ -0,0 +1,81 @@ +"""Bootstrap-free bookmark default (#326). + +Mirrors R's pkg-r/tests/testthat/test-bookmark-default.R. +""" + +from __future__ import annotations + +import asyncio +from types import SimpleNamespace +from typing import Any + +from shiny._utils import AsyncCallbacks, wrap_async +from shiny.input_handler import input_handlers +from shiny.module import ResolvedId +from shinyreact._bookmark import install_bookmark_default + + +class _FakeBookmark: + """The slice of shiny's BookmarkApp the default touches, with shiny's own + AsyncCallbacks held where BookmarkApp keeps it.""" + + def __init__(self) -> None: + self._on_bookmarked_callbacks = AsyncCallbacks() + self.queries: list[str | None] = [] + + def on_bookmarked(self, callback: Any) -> Any: + return self._on_bookmarked_callbacks.register(wrap_async(callback)) + + async def update_query_string(self, query_string: str | None = None) -> None: + self.queries.append(query_string) + + def bookmarked(self, url: str) -> None: + # What BookmarkApp.do_bookmark() does once it has the URL: run the + # callbacks, or Shiny's modal default when there are none. + if self._on_bookmarked_callbacks.count() == 0: + raise RuntimeError("Shiny's default modal: $(...).modal is not a function") + asyncio.run(self._on_bookmarked_callbacks.invoke(url)) + + +def _session() -> Any: + return SimpleNamespace(bookmark=_FakeBookmark()) + + +def test_bookmarking_updates_the_url_instead_of_a_modal() -> None: + # #326: page_react() has no Bootstrap, so Shiny's default modal failed. + session = _session() + assert install_bookmark_default(session) + + session.bookmark.bookmarked("http://app/?_inputs_&n=1") + + assert session.bookmark.queries == ["http://app/?_inputs_&n=1"] + + +def test_an_apps_own_on_bookmarked_replaces_the_default() -> None: + session = _session() + install_bookmark_default(session) + seen: list[str] = [] + session.bookmark.on_bookmarked(seen.append) + + session.bookmark.bookmarked("http://app/?_inputs_&n=1") + + assert seen == ["http://app/?_inputs_&n=1"] + assert session.bookmark.queries == [] + + +def test_installs_once_per_session() -> None: + session = _session() + assert install_bookmark_default(session) + assert not install_bookmark_default(session) + assert session.bookmark._on_bookmarked_callbacks.count() == 1 + + +def test_no_ops_without_bookmark_callbacks() -> None: + assert not install_bookmark_default(None) + assert not install_bookmark_default(SimpleNamespace(bookmark=None)) + + +def test_the_init_handler_installs_the_default() -> None: + session = _session() + input_handlers["shinyreact.init"](1, ResolvedId("x"), session) + assert session.bookmark._on_bookmarked_callbacks.count() == 1 diff --git a/pkg-r/NEWS.md b/pkg-r/NEWS.md index 4631eb33..a0f28318 100644 --- a/pkg-r/NEWS.md +++ b/pkg-r/NEWS.md @@ -6,6 +6,12 @@ IDs were found" warning grew and the console logged `Output not found` (#330). +* Bookmarking (`session$doBookmark()`) on a `page_react()` page no longer + fails with `$(...).modal is not a function`. Shiny's default shows the URL in + a Bootstrap modal, and these pages load no Bootstrap; shinyreact now writes + the URL to the address bar instead. An app's own `onBookmarked()` still + replaces the default (#326). + # shinyreact 0.1.0 * Initial CRAN release. shinyreact is the server-side plumbing for the diff --git a/pkg-r/R/bookmark.R b/pkg-r/R/bookmark.R index 95efce07..a56075de 100644 --- a/pkg-r/R/bookmark.R +++ b/pkg-r/R/bookmark.R @@ -115,3 +115,40 @@ config_script_tag <- function() { id = "shinyreact-config" ) } + +# Shiny's default `onBookmarked()` action shows the URL in a Bootstrap modal, +# and `page_react()` loads no Bootstrap (#285), so `session$doBookmark()` threw +# `$(...).modal is not a function` in the browser (#326). Replace that default +# with putting the URL in the address bar. +# +# Shiny runs its default only when no `onBookmarked()` callback exists, and +# runs every registered callback otherwise. So ours acts only while it is the +# only one: once the app registers its own, the app's callback alone decides +# what happens, exactly as it would replace Shiny's default. `bookmarkedCallbacks` is private; +# shiny has no public way to count them (Python reads +# `session.bookmark._on_bookmarked_callbacks`, same deal). +# +# Installed by the `shinyreact.init` input handler (input-handler.R), once per +# session. +install_bookmark_default <- function(session) { + user_data <- tryCatch(session$userData, error = function(e) NULL) + callbacks <- tryCatch( + session$.__enclos_env__$private$bookmarkedCallbacks, + error = function(e) NULL + ) + if ( + !is.environment(user_data) || + is.null(callbacks) || + isTRUE(user_data[[".shinyreact_bookmark_default"]]) + ) { + return(invisible(FALSE)) + } + user_data[[".shinyreact_bookmark_default"]] <- TRUE + + session$onBookmarked(function(url) { + if (callbacks$count() == 1L) { + shiny::updateQueryString(url, session = session) + } + }) + invisible(TRUE) +} diff --git a/pkg-r/R/input-handler.R b/pkg-r/R/input-handler.R index 4d77f465..f057527d 100644 --- a/pkg-r/R/input-handler.R +++ b/pkg-r/R/input-handler.R @@ -59,7 +59,7 @@ asis_input_handler <- function(value, session = NULL, name = NULL) { #' `.shinyreact_init:shinyreact.init` ping arrives (sent after Shiny #' initializes -- `pkg-js/src/dep-discovery.ts`). Its job is per-session #' bootstrap: installing automatic output dependency discovery -#' (dep-discovery.R). The hook is installed before the ping's own reactive +#' (dep-discovery.R) and the Bootstrap-free bookmark default (bookmark.R). The hook is installed before the ping's own reactive #' flush runs, so that same flush performs the first dependency harvest -- #' `server()` has already registered its outputs by then. A dedicated handler #' keeps the value-transforming handlers above pure, and the guaranteed ping @@ -67,5 +67,6 @@ asis_input_handler <- function(value, session = NULL, name = NULL) { #' @keywords internal init_input_handler <- function(value, session = NULL, name = NULL) { install_dep_discovery(session) + install_bookmark_default(session) value } diff --git a/pkg-r/man/init_input_handler.Rd b/pkg-r/man/init_input_handler.Rd index 2286b6fb..612f279e 100644 --- a/pkg-r/man/init_input_handler.Rd +++ b/pkg-r/man/init_input_handler.Rd @@ -12,7 +12,7 @@ shiny invokes it when the JS bundle's single per-session \code{.shinyreact_init:shinyreact.init} ping arrives (sent after Shiny initializes -- \code{pkg-js/src/dep-discovery.ts}). Its job is per-session bootstrap: installing automatic output dependency discovery -(dep-discovery.R). The hook is installed before the ping's own reactive +(dep-discovery.R) and the Bootstrap-free bookmark default (bookmark.R). The hook is installed before the ping's own reactive flush runs, so that same flush performs the first dependency harvest -- \code{server()} has already registered its outputs by then. A dedicated handler keeps the value-transforming handlers above pure, and the guaranteed ping diff --git a/pkg-r/tests/testthat/test-bookmark-default.R b/pkg-r/tests/testthat/test-bookmark-default.R new file mode 100644 index 00000000..a32c7d2f --- /dev/null +++ b/pkg-r/tests/testthat/test-bookmark-default.R @@ -0,0 +1,76 @@ +# Mirrors Python's test_bookmark_default.py. +# +# A stand-in for a ShinySession with the pieces install_bookmark_default() +# touches: userData, onBookmarked() backed by shiny's own Callbacks class (held +# where ShinySession keeps it, in private$bookmarkedCallbacks), and +# updateQueryString(), which records instead of messaging a client. +bookmark_session <- function() { + private <- new.env() + private$bookmarkedCallbacks <- shiny:::Callbacks$new() + enclos <- new.env() + enclos$private <- private + session <- new.env() + session$.__enclos_env__ <- enclos + session$userData <- new.env() + session$queries <- character() + session$onBookmarked <- function(fun) { + private$bookmarkedCallbacks$register(fun) + } + session$updateQueryString <- function(queryString, mode) { + session$queries <- c(session$queries, queryString) + } + # What ShinySession$doBookmark() does once it has the URL: run the + # callbacks, or Shiny's modal default when there are none. + session$bookmarked <- function(url) { + if (private$bookmarkedCallbacks$count() > 0) { + private$bookmarkedCallbacks$invoke(url) + } else { + stop("Shiny's default modal: $(...).modal is not a function") + } + } + session +} + +test_that("bookmarking puts the URL in the address bar instead of a modal", { + # #326: page_react() has no Bootstrap, so Shiny's default modal threw. + session <- bookmark_session() + expect_true(install_bookmark_default(session)) + + session$bookmarked("http://app/?_inputs_&n=1") + + expect_identical(session$queries, "http://app/?_inputs_&n=1") +}) + +test_that("an app's own onBookmarked() replaces the default", { + # Same as replacing Shiny's own default: only the app's callback acts. + session <- bookmark_session() + install_bookmark_default(session) + seen <- NULL + session$onBookmarked(function(url) seen <<- url) + + session$bookmarked("http://app/?_inputs_&n=1") + + expect_identical(seen, "http://app/?_inputs_&n=1") + expect_identical(session$queries, character()) +}) + +test_that("it installs once per session", { + session <- bookmark_session() + expect_true(install_bookmark_default(session)) + expect_false(install_bookmark_default(session)) + expect_identical( + session$.__enclos_env__$private$bookmarkedCallbacks$count(), + 1L + ) +}) + +test_that("it no-ops on sessions without bookmark callbacks", { + expect_false(install_bookmark_default(NULL)) + expect_false(install_bookmark_default(shiny::MockShinySession$new())) +}) + +test_that("the shinyreact.init handler installs the default", { + session <- bookmark_session() + init_input_handler(1, session = session) + expect_true(session$userData[[".shinyreact_bookmark_default"]]) +})