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

898 save app state version 3 #1011

Merged
merged 134 commits into from
Mar 28, 2024
Merged

898 save app state version 3 #1011

merged 134 commits into from
Mar 28, 2024

Conversation

chlebowa
Copy link
Contributor

@chlebowa chlebowa commented Dec 15, 2023

Closes #898
Closes #941

Incorporating bookmarking to teal applications. The functionality is handled by the shiny bookmarking mechanism, using the "server" mode. The new bookmark_manager module tracks bookmark history, should the user choose to create more than one in their session.

Overview

Inputs are stored with shiny's built-in bookmarking mechanism using the "server" mode.
Additional values may also be stored, which we use to "remember" filter state, filter snapshot history, and bookmark history.

literature

Using bookmarks requires that the shiny option "bookmarkStore" be set to "server". Normally this is achieved by calling enableBookmarking("server") before running the app or by setting enableBookmarking = "server" in a shinyApp call.
Here the option is set with a loading hook in teal. Furthermore, the bookmark manager will enforce this option.

Bookmarks are added to a list much like in the snapshot manager module but elements of the list are URLs pointing to bookmarked applications that open in new windows.

Current filter state, snapshot history, and bookmark history are included in the bookmark. The former two are stored by the snapshot manager, the latter - by the bookmark manager.

Note on bookmark history:
Bookmark URL is is obtained using the session$onBookmarked callback. As a result the URL is added to the bookmark list in the state manager module after the bookmark is created and so when one goes to the bookmarked application, the most recent bookmark is not on the list. This is a small price to pay for not relying on potentially unstable internal functions but it deserves a mention.

Manager modules

Prior to this PR, the filter_manager module was placed in the filter_manager_modal module. The former consisted on a single button that opened a modal dialog where the filter_manager was housed.
The snapshot_manager module was called by the filter_manager and its UI resided within the modal dialog opened by the filter_manager_modal button.

Here, the module that opened the modal is renamed to wunder_bar module. wunder_bar holds several buttons, each opening a different module. The wunder_bar server function passes values between its subordinate modules.

filter_manager and snapshot_manager are decoupled. The values that exist in the filter_manager_srv scope that the snapshot_manager needs are added to filter_manager_srv's return value and are passed to the snapshot_manager_srv function call in the wunder_bar server.
Likewise, snapshot_manager_srv returns an object needed by bookmark_manager_srv, which is passed accordingly.

Module compatibility

Some modules contain logic that precludes bookmarking. Therefore, each individual module will have to be proofed by its author and given the attribute: teal_bookmarkable = TRUE if bookmarking is possible. (The bookmarks_identical function will be helpful in checking.) teal will determine if the modules in the app support bookmarking and communicate it to the app user in the bookmark manager.

Miscellaneous

Adjusted some CSS class names and styles.

Additional changes

We know of three issues that preclude the bookmarking mechanism from working:

  1. creating inputs empty (e.g. choices = NULL) and modifying them server-side using update*Input
  2. hierarchical inputs where the value of the junior depends on the value of the senior
  3. delayed module initiation (specific to teal)

Issue (3) is remedied by a change in the module_nested_tabs: unlike starting a naive app, when modules are initiated upon their first viewing, when restoring a bookmark, all modules are initiated on app start. This was done by @gogonzo.

Issues (1) and (2) require certain preventative measures, which is explained in detail here.
Appropriate changes have been made:

Update: in the context of teal update*Input will never be compatible with bookmarking due to teal.transform.
All dynamic inputs must be rewritten with rendering. This will be done during bookmarkability verification (issue below). In some cases the logic still makes it impossible to fully bookmark a module (e.g. teal.modules.general::tm_g_distribution).

TO DO

@Polkas
Copy link
Contributor

Polkas commented Mar 26, 2024

It's an interesting solution, but it seems demanding to be well-validated, as it requires decent Shiny Server testing. I want to continue the discussion started by @pawelru. One of the questions is how to protect the server so as not to be overwhelmed with the number of saves. Possibly control it by expiration date for saves. It is interesting if other users can access another one's saves and if you want to control it. The same app state produces different state id for following saves, which seems inefficient (on a shiny level) but perhaps more secure. Why not additionally support the limited scope URL bookmarking? The URL bookmarking is much more light and transparent, but I understand it can not cover the whole app here. It is too easy to lose everything now by clicking a specific bookmark. There should be a more careful process to apply a bookmark from the bookmark list. There should be an easy way to save all bookmarks to a yaml/txt file.
This item seems to be a nice candidate for a colabo with Automation team for me, to chase proper testing.

Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wunderbar 👍

R/module_bookmark_manager.R Outdated Show resolved Hide resolved
@chlebowa chlebowa merged commit f331ffe into main Mar 28, 2024
24 checks passed
@chlebowa chlebowa deleted the 898_save_app_state3@main branch March 28, 2024 16:15
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants