feat(bookmarking): Add session.bookmark.show_bookmark_url_modal()

[schloerke]

Fixes #1909

If no session.bookmark.on_bookmarked callbacks have been registered, the bookmark modal will be shown when bookmarking: https://github.com/posit-dev/py-shiny/pull/1922/files#diff-c91259520702d312a5af26c95bfd1e86ef9c7ba0e859ae44cef271a01a53a0f0R555

[Copilot]

Pull Request Overview

This pull request introduces changes to bookmarking functionality by adding a new on_bookmarked callback that updates the query string and by adjusting module naming and recursion for improved bookmark state management. Key changes include:

• Adding an async on_bookmarked callback to call session.bookmark.update_query_string() in both express and recursive modes.
• Renaming and updating functions and module calls (e.g. recursive_mod to ex_mod) alongside increasing recursion depth (k increased from 2 to 4).
• Removing unused/commented code and deprecated state exclusions (e.g. _always_exclude).

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tests/playwright/shiny/bookmark/modules/app-express.py	Adds a new on_bookmarked callback and renames the module function to ex_mod.
tests/playwright/shiny/bookmark/modules/app-core-recursive.py	Adjusts submodule naming and recursion logic; adds conditional on_bookmarked callback registration.
tests/playwright/shiny/bookmark/modules/test_bookmark_modules.py	Updates expected bookmark key structure to reflect new module naming and recursion depth.
tests/playwright/shiny/bookmark/modules/app-core.py	Increases recursion depth, removes commented out blocks, and registers a new on_bookmarked callback.
shiny/bookmark/_save_state.py	Removes unused _always_exclude attribute from the state saving logic.

Comments suppressed due to low confidence (7)

tests/playwright/shiny/bookmark/modules/app-express.py:29

• [nitpick] The renaming from 'recursive_mod' to 'ex_mod' may reduce clarity regarding this function's role in the express module. Consider renaming it to something more descriptive, such as 'express_module', to better reflect its purpose.

def ex_mod(input: Inputs, output: Outputs, session: Session, recurse: int = 3):

tests/playwright/shiny/bookmark/modules/app-core-recursive.py:32

• Verify that the modified submodule naming using 'sub{idx - 1}' correctly maps to the intended bookmark key structure, in line with updated test expectations.

mod_btn(f"sub{idx - 1}", idx - 1) if idx > 0 else None,

tests/playwright/shiny/bookmark/modules/app-core-recursive.py:83

• Ensure that calling on_bookmarked in the else branch to update the query string meets the overall design intent, without conflicting with other bookmark callbacks.

session.bookmark.on_bookmarked(session.bookmark.update_query_string)

tests/playwright/shiny/bookmark/modules/app-core.py:82

• [nitpick] The increased recursion depth (k = 4) will generate a deeper UI hierarchy. Confirm that all related UI components and bookmark state handling are correctly updated to manage the additional nesting.

k = 4

shiny/bookmark/_save_state.py:43

• The removal of the '_always_exclude' attribute may impact code that depends on always excluding certain keys from saved state; verify that no unintended state persistence issues arise from this change.

self._always_exclude: list[str] = []

tests/playwright/shiny/bookmark/modules/test_bookmark_modules.py:15

• Ensure that the updated expected bookmark keys in tests align with the changes in module naming and the new recursion depth, reflecting the intended behavior.

('app-express.py', 'mod0', 'mod3'),

tests/playwright/shiny/bookmark/modules/app-core.py:96

• Double-check that using only the update_query_string callback for on_bookmarked satisfies the new modal display behavior without interfering with other bookmark restoration functions.

session.bookmark.on_bookmarked(session.bookmark.update_query_string)