From df876005eb7da50db1fd71129d5b0bb7065da073 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20Ka=C5=82=C4=99dkowski?= <6959016+gogonzo@users.noreply.github.com> Date: Thu, 8 Aug 2024 10:36:05 +0200 Subject: [PATCH] fix duplicated FilterState when reloading (#609) Fix Bug found by @vedhav when reloading data. --- R/FilterState.R | 46 ++++++++++++---------------- R/FilterStateChoices.R | 6 ++-- R/FilterStateDate.R | 8 ++--- R/FilterStateDatettime.R | 10 +++--- R/FilterStateExpr.R | 30 ++++++++---------- R/FilterStateLogical.R | 4 +-- R/FilterStateRange.R | 10 +++--- R/FilterStates.R | 31 ++++++++++--------- R/FilterStatesSE.R | 17 +++++++--- R/FilteredData.R | 28 +++++++++++------ R/FilteredDataset.R | 19 +++++++++--- R/utils.R | 14 +++++++-- man/FilterState.Rd | 4 +-- man/FilterStateExpr.Rd | 2 +- man/FilterStates.Rd | 2 +- man/FilteredData.Rd | 2 +- man/FilteredDataset.Rd | 2 +- man/dot-finalize_session_bindings.Rd | 21 +++++++++++++ 18 files changed, 148 insertions(+), 108 deletions(-) create mode 100644 man/dot-finalize_session_bindings.Rd diff --git a/R/FilterState.R b/R/FilterState.R index 306711e1f..c73147e3d 100644 --- a/R/FilterState.R +++ b/R/FilterState.R @@ -215,7 +215,7 @@ FilterState <- R6::R6Class( # nolint private$server_inputs("inputs") } - private$observers[[session$ns("state")]] <- observeEvent( + private$session_bindings[[session$ns("state")]] <- observeEvent( eventExpr = list(private$get_selected(), private$get_keep_na(), private$get_keep_inf()), handlerExpr = { current_state <- as.list(self$get_state()) @@ -225,7 +225,7 @@ FilterState <- R6::R6Class( # nolint } ) - private$observers[[session$ns("back")]] <- observeEvent( + private$session_bindings[[session$ns("back")]] <- observeEvent( eventExpr = input$back, handlerExpr = { history <- rev(private$state_history()) @@ -236,7 +236,7 @@ FilterState <- R6::R6Class( # nolint } ) - private$observers[[session$ns("reset")]] <- observeEvent( + private$session_bindings[[session$ns("reset")]] <- observeEvent( eventExpr = input$reset, handlerExpr = { slice <- private$state_history()[[1L]] @@ -246,7 +246,7 @@ FilterState <- R6::R6Class( # nolint # Buttons for rewind/reset are disabled upon change in history to prevent double-clicking. # Re-enabling occurs after 100 ms, after they are potentially hidden when no history is present. - private$observers[[session$ns("state_history")]] <- observeEvent( + private$session_bindings[[session$ns("state_history")]] <- observeEvent( eventExpr = private$state_history(), handlerExpr = { shinyjs::disable(id = "back") @@ -268,25 +268,21 @@ FilterState <- R6::R6Class( # nolint } ) - private$observers[[session$ns("remove")]] <- observeEvent( + private$session_bindings[[session$ns("remove")]] <- observeEvent( once = TRUE, # remove button can be called once, should be destroyed afterwards ignoreInit = TRUE, # ignoreInit: should not matter because we destroy the previous input set of the UI eventExpr = input$remove, # when remove button is clicked in the FilterState ui handlerExpr = remove_callback() ) - private$destroy_shiny <- function() { - logger::log_debug("Destroying FilterState inputs and observers; id: { private$get_id() }") - - if (session$isEnded()) { - return(NULL) - } # skip input removal if session has ended - # remove values from the input list - lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) - - # remove observers - lapply(private$observers, function(x) x$destroy()) - } + private$session_bindings[[session$ns("inputs")]] <- list( + destroy = function() { + logger::log_debug("Destroying FilterState inputs and observers; id: { private$get_id() }") + if (!session$isEnded()) { + lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) + } + } + ) private$state_history <- reactiveVal(list()) @@ -388,17 +384,14 @@ FilterState <- R6::R6Class( # nolint }, #' @description - #' Destroy observers stored in `private$observers`. + #' Destroy inputs and observers stored in `private$session_bindings`. #' - #' The `destroy_shiny` definition is set in the server method. #' #' @return `NULL`, invisibly. #' finalize = function() { - if (!is.null(private$destroy_shiny)) { - private$destroy_shiny() - private$destroy_shiny <- NULL - } + .finalize_session_bindings(self, private) + invisible(NULL) } ), @@ -412,10 +405,9 @@ FilterState <- R6::R6Class( # nolint na_count = integer(0), filtered_na_count = NULL, # reactive containing the count of NA in the filtered dataset varlabel = character(0), # taken from variable labels in data; displayed in filter cards - destroy_shiny = NULL, # function is set in server # other is_choice_limited = FALSE, # flag whether number of possible choices was limited when specifying filter - observers = list(), # stores observers + session_bindings = list(), # stores observers and inputs to destroy afterwards state_history = NULL, # reactiveVal holding a list storing states this FilterState has had since instantiation # private methods ---- @@ -771,7 +763,7 @@ FilterState <- R6::R6Class( # nolint # this observer is needed in the situation when private$keep_inf has been # changed directly by the api - then it's needed to rerender UI element # to show relevant values - private$observers[[session$ns("keep_na_api")]] <- observeEvent( + private$session_bindings[[session$ns("keep_na_api")]] <- observeEvent( ignoreNULL = FALSE, # nothing selected is possible for NA ignoreInit = TRUE, # ignoreInit: should not matter because we set the UI with the desired initial state eventExpr = private$get_keep_na(), @@ -786,7 +778,7 @@ FilterState <- R6::R6Class( # nolint } } ) - private$observers[[session$ns("keep_na")]] <- observeEvent( + private$session_bindings[[session$ns("keep_na")]] <- observeEvent( ignoreNULL = FALSE, # ignoreNULL: we don't want to ignore NULL when nothing is selected in the `selectInput` ignoreInit = TRUE, # ignoreInit: should not matter because we set the UI with the desired initial state eventExpr = input$value, diff --git a/R/FilterStateChoices.R b/R/FilterStateChoices.R index 401f1f6a1..02402f37f 100644 --- a/R/FilterStateChoices.R +++ b/R/FilterStateChoices.R @@ -412,7 +412,6 @@ ChoicesFilterState <- R6::R6Class( # nolint id = id, function(input, output, session) { logger::log_debug("ChoicesFilterState$server_inputs initializing, id: { private$get_id() }") - # 1. renderUI is used here as an observer which triggers only if output is visible # and if the reactive changes - reactive triggers only if the output is visible. # 2. We want to trigger change of the labels only if reactive count changes (not underlying data) @@ -454,7 +453,7 @@ ChoicesFilterState <- R6::R6Class( # nolint }) }) - private$observers[[session$ns("selection")]] <- if (private$is_checkboxgroup()) { + private$session_bindings[[session$ns("selection")]] <- if (private$is_checkboxgroup()) { observeEvent( ignoreNULL = FALSE, ignoreInit = TRUE, # ignoreInit: should not matter because we set the UI with the desired initial state @@ -504,13 +503,12 @@ ChoicesFilterState <- R6::R6Class( # nolint ) } - private$keep_na_srv("keep_na") # this observer is needed in the situation when teal_slice$selected has been # changed directly by the api - then it's needed to rerender UI element # to show relevant values - private$observers[[session$ns("selection_api")]] <- observeEvent(private$get_selected(), { + private$session_bindings[[session$ns("selection_api")]] <- observeEvent(private$get_selected(), { # it's important to not retrigger when the input$selection is the same as reactive values # kept in the teal_slice$selected if (!setequal(input$selection, private$get_selected())) { diff --git a/R/FilterStateDate.R b/R/FilterStateDate.R index 97bc3069a..f8d053a4e 100644 --- a/R/FilterStateDate.R +++ b/R/FilterStateDate.R @@ -327,7 +327,7 @@ DateFilterState <- R6::R6Class( # nolint # this observer is needed in the situation when teal_slice$selected has been # changed directly by the api - then it's needed to rerender UI element # to show relevant values - private$observers[[session$ns("selection_api")]] <- observeEvent( + private$session_bindings[[session$ns("selection_api")]] <- observeEvent( ignoreNULL = TRUE, # dates needs to be selected ignoreInit = TRUE, eventExpr = private$get_selected(), @@ -344,7 +344,7 @@ DateFilterState <- R6::R6Class( # nolint } ) - private$observers[[session$ns("selection")]] <- observeEvent( + private$session_bindings[[session$ns("selection")]] <- observeEvent( ignoreNULL = TRUE, # dates needs to be selected ignoreInit = TRUE, # ignoreInit: should not matter because we set the UI with the desired initial state eventExpr = input$selection, @@ -374,7 +374,7 @@ DateFilterState <- R6::R6Class( # nolint private$keep_na_srv("keep_na") - private$observers[[session$ns("reset1")]] <- observeEvent(input$start_date_reset, { + private$session_bindings[[session$ns("reset1")]] <- observeEvent(input$start_date_reset, { logger::log_debug("DateFilterState$server@3 reset start date, id: { private$get_id() }") updateDateRangeInput( session = session, @@ -383,7 +383,7 @@ DateFilterState <- R6::R6Class( # nolint ) }) - private$observers[[session$ns("reset2")]] <- observeEvent(input$end_date_reset, { + private$session_bindings[[session$ns("reset2")]] <- observeEvent(input$end_date_reset, { logger::log_debug("DateFilterState$server@4 reset end date, id: { private$get_id() }") updateDateRangeInput( session = session, diff --git a/R/FilterStateDatettime.R b/R/FilterStateDatettime.R index 52a3f16be..c7347a0df 100644 --- a/R/FilterStateDatettime.R +++ b/R/FilterStateDatettime.R @@ -388,7 +388,7 @@ DatetimeFilterState <- R6::R6Class( # nolint # this observer is needed in the situation when teal_slice$selected has been # changed directly by the api - then it's needed to rerender UI element # to show relevant values - private$observers[[session$ns("selection_api")]] <- observeEvent( + private$session_bindings[[session$ns("selection_api")]] <- observeEvent( ignoreNULL = TRUE, # dates needs to be selected ignoreInit = TRUE, # on init selected == default, so no need to trigger eventExpr = private$get_selected(), @@ -417,7 +417,7 @@ DatetimeFilterState <- R6::R6Class( # nolint ) - private$observers[[session$ns("selection_start")]] <- observeEvent( + private$session_bindings[[session$ns("selection_start")]] <- observeEvent( ignoreNULL = TRUE, # dates needs to be selected ignoreInit = TRUE, # ignoreInit: should not matter because we set the UI with the desired initial state eventExpr = input$selection_start, @@ -445,7 +445,7 @@ DatetimeFilterState <- R6::R6Class( # nolint } ) - private$observers[[session$ns("selection_end")]] <- observeEvent( + private$session_bindings[[session$ns("selection_end")]] <- observeEvent( ignoreNULL = TRUE, # dates needs to be selected ignoreInit = TRUE, # ignoreInit: should not matter because we set the UI with the desired initial state eventExpr = input$selection_end, @@ -475,7 +475,7 @@ DatetimeFilterState <- R6::R6Class( # nolint private$keep_na_srv("keep_na") - private$observers[[session$ns("reset1")]] <- observeEvent( + private$session_bindings[[session$ns("reset1")]] <- observeEvent( ignoreInit = TRUE, # reset button shouldn't be trigger on init ignoreNULL = TRUE, # it's impossible and wrong to set default to NULL input$start_date_reset, @@ -488,7 +488,7 @@ DatetimeFilterState <- R6::R6Class( # nolint logger::log_debug("DatetimeFilterState$server@2 reset start date, id: { private$get_id() }") } ) - private$observers[[session$ns("reset2")]] <- observeEvent( + private$session_bindings[[session$ns("reset2")]] <- observeEvent( ignoreInit = TRUE, # reset button shouldn't be trigger on init ignoreNULL = TRUE, # it's impossible and wrong to set default to NULL input$end_date_reset, diff --git a/R/FilterStateExpr.R b/R/FilterStateExpr.R index bac01a8d3..aec04968b 100644 --- a/R/FilterStateExpr.R +++ b/R/FilterStateExpr.R @@ -141,16 +141,12 @@ FilterStateExpr <- R6::R6Class( # nolint }, #' @description - #' Destroy observers stored in `private$observers`. + #' Destroy inputs and observers stored in `private$session_bindings`. #' #' @return `NULL`, invisibly. #' finalize = function() { - lapply(private$observers, function(x) x$destroy()) - - if (!is.null(private$destroy_shiny)) { - private$destroy_shiny() - } + .finalize_session_bindings(self, private) invisible(NULL) }, @@ -173,22 +169,21 @@ FilterStateExpr <- R6::R6Class( # nolint function(input, output, session) { private$server_summary("summary") - private$observers[[session$ns("remove")]] <- observeEvent( + private$session_bindings[[session$ns("remove")]] <- observeEvent( once = TRUE, # remove button can be called once, should be destroyed afterwards ignoreInit = TRUE, # ignoreInit: should not matter because we destroy the previous input set of the UI eventExpr = input$remove, # when remove button is clicked in the FilterState ui handlerExpr = remove_callback() ) - private$destroy_shiny <- function() { - logger::log_debug("Destroying FilterStateExpr inputs; id: { private$get_id() }") - - if (session$isEnded()) { - return(NULL) - } # skip input removal if session has ended - # remove values from the input list - lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) - } + private$session_bindings[[session$ns("inputs")]] <- list( + destroy = function() { + logger::log_debug("Destroying FilterState inputs and observers; id: { private$get_id() }") + if (!session$isEnded()) { + lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) + } + } + ) NULL } @@ -245,9 +240,8 @@ FilterStateExpr <- R6::R6Class( # nolint # private members ---- private = list( - observers = NULL, # stores observers + session_bindings = list(), # stores observers and inputs to destroy afterwards teal_slice = NULL, # stores reactiveValues - destroy_shiny = NULL, # function is set in server # @description # Get id of the teal_slice. diff --git a/R/FilterStateLogical.R b/R/FilterStateLogical.R index 54f47c783..c421c9953 100644 --- a/R/FilterStateLogical.R +++ b/R/FilterStateLogical.R @@ -313,7 +313,7 @@ LogicalFilterState <- R6::R6Class( # nolint NULL }) - private$observers[[session$ns("selected_api")]] <- observeEvent( + private$session_bindings[[session$ns("selected_api")]] <- observeEvent( ignoreNULL = !private$is_multiple(), ignoreInit = TRUE, eventExpr = private$get_selected(), @@ -335,7 +335,7 @@ LogicalFilterState <- R6::R6Class( # nolint } ) - private$observers[[session$ns("selection")]] <- observeEvent( + private$session_bindings[[session$ns("selection")]] <- observeEvent( ignoreNULL = FALSE, ignoreInit = TRUE, eventExpr = input$selection, diff --git a/R/FilterStateRange.R b/R/FilterStateRange.R index 240a801eb..3fba266a7 100644 --- a/R/FilterStateRange.R +++ b/R/FilterStateRange.R @@ -506,7 +506,7 @@ RangeFilterState <- R6::R6Class( # nolint }) # Dragging shapes (lines) on plot updates selection. - private$observers[[session$ns("relayout")]] <- observeEvent( + private$session_bindings[[session$ns("relayout")]] <- observeEvent( ignoreNULL = FALSE, ignoreInit = TRUE, eventExpr = relayout_data(), @@ -540,7 +540,7 @@ RangeFilterState <- R6::R6Class( # nolint ) # Change in selection updates shapes (lines) on plot and numeric input. - private$observers[[session$ns("selection_api")]] <- observeEvent( + private$session_bindings[[session$ns("selection_api")]] <- observeEvent( ignoreNULL = FALSE, ignoreInit = TRUE, eventExpr = private$get_selected(), @@ -557,7 +557,7 @@ RangeFilterState <- R6::R6Class( # nolint ) # Manual input updates selection. - private$observers[[session$ns("selection_manual")]] <- observeEvent( + private$session_bindings[[session$ns("selection_manual")]] <- observeEvent( ignoreNULL = FALSE, ignoreInit = TRUE, eventExpr = selection_manual(), @@ -712,7 +712,7 @@ RangeFilterState <- R6::R6Class( # nolint # this observer is needed in the situation when private$teal_slice$keep_inf has been # changed directly by the api - then it's needed to rerender UI element # to show relevant values - private$observers[[session$ns("keep_inf_api")]] <- observeEvent( + private$session_bindings[[session$ns("keep_inf_api")]] <- observeEvent( ignoreNULL = TRUE, # its not possible for range that NULL is selected ignoreInit = TRUE, # ignoreInit: should not matter because we set the UI with the desired initial state eventExpr = private$get_keep_inf(), @@ -727,7 +727,7 @@ RangeFilterState <- R6::R6Class( # nolint } ) - private$observers[[session$ns("keep_inf")]] <- observeEvent( + private$session_bindings[[session$ns("keep_inf")]] <- observeEvent( ignoreNULL = TRUE, # it's not possible for range that NULL is selected ignoreInit = TRUE, # ignoreInit: should not matter because we set the UI with the desired initial state eventExpr = input$value, diff --git a/R/FilterStates.R b/R/FilterStates.R index 387f1534a..4fceca00d 100644 --- a/R/FilterStates.R +++ b/R/FilterStates.R @@ -363,7 +363,7 @@ FilterStates <- R6::R6Class( # nolint ) }) - private$observers[[session$ns("added_states")]] <- observeEvent( + private$session_bindings[[session$ns("added_states")]] <- observeEvent( added_states(), # we want to call FilterState module only once when it's added ignoreNULL = TRUE, { @@ -372,7 +372,7 @@ FilterStates <- R6::R6Class( # nolint lapply(added_states(), function(state) { state$server( id = fs_to_shiny_ns(state), - function() private$state_list_remove(state$get_state()$id) + remove_callback = function() private$state_list_remove(state$get_state()$id) ) }) added_states(NULL) @@ -440,12 +440,12 @@ FilterStates <- R6::R6Class( # nolint ) }) - output$add_filter <- renderUI({ logger::log_debug( "FilterStates$srv_add@1 updating available column choices, dataname: { private$dataname }" ) if (length(avail_column_choices()) == 0) { + # because input UI is not rendered on this condition but shiny still holds latest selected value tags$span("No available columns to add.") } else { tags$div( @@ -462,7 +462,7 @@ FilterStates <- R6::R6Class( # nolint } }) - private$observers[[session$ns("var_to_add")]] <- observeEvent( + private$session_bindings[[session$ns("var_to_add")]] <- observeEvent( eventExpr = input$var_to_add, handlerExpr = { logger::log_debug( @@ -477,13 +477,15 @@ FilterStates <- R6::R6Class( # nolint teal_slice(dataname = private$dataname, varname = input$var_to_add) ) ) - logger::log_debug( - sprintf( - "FilterStates$srv_add@2 added FilterState of variable %s, dataname: %s", - input$var_to_add, - private$dataname - ) - ) + } + ) + + # Extra observer that clears all input values in session + private$session_bindings[[session$ns("inputs")]] <- list( + destroy = function() { + if (!session$isEnded()) { + lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) + } } ) @@ -495,13 +497,13 @@ FilterStates <- R6::R6Class( # nolint #' @description #' Object cleanup. #' - #' - Destroy observers stored in `private$observers` + #' - Destroy inputs and observers stored in `private$session_bindings` #' - Clean `state_list` #' #' @return `NULL`, invisibly. #' finalize = function() { - .finalize_observers(self, private) # Remove all observers + .finalize_session_bindings(self, private) # Remove all inputs and observers private$state_list_empty(force = TRUE) isolate(private$state_list(NULL)) invisible(NULL) @@ -521,7 +523,7 @@ FilterStates <- R6::R6Class( # nolint fun = quote(subset), # function used to generate subset call keys = character(0), ns = NULL, # shiny ns() - observers = list(), # observers + session_bindings = list(), # inputs and observers state_list = NULL, # list of `reactiveVal`s initialized by init methods of child classes, # private methods ---- @@ -640,7 +642,6 @@ FilterStates <- R6::R6Class( # nolint state_list_remove = function(state_id, force = FALSE) { checkmate::assert_character(state_id) logger::log_debug("{ class(self)[1] } removing a filter, state_id: { toString(state_id) }") - isolate({ current_state_ids <- vapply(private$state_list(), function(x) x$get_state()$id, character(1)) to_remove <- state_id %in% current_state_ids diff --git a/R/FilterStatesSE.R b/R/FilterStatesSE.R index 4a5f94bb2..686bb44c4 100644 --- a/R/FilterStatesSE.R +++ b/R/FilterStatesSE.R @@ -194,7 +194,7 @@ SEFilterStates <- R6::R6Class( # nolint ) }) - private$observers[[session$ns("avail_row_data_choices")]] <- observeEvent( + private$session_bindings[[session$ns("avail_row_data_choices")]] <- observeEvent( avail_row_data_choices(), ignoreNULL = TRUE, handlerExpr = { @@ -219,7 +219,7 @@ SEFilterStates <- R6::R6Class( # nolint } ) - private$observers[[session$ns("avail_col_data_choices")]] <- observeEvent( + private$session_bindings[[session$ns("avail_col_data_choices")]] <- observeEvent( avail_col_data_choices(), ignoreNULL = TRUE, handlerExpr = { @@ -244,7 +244,7 @@ SEFilterStates <- R6::R6Class( # nolint } ) - private$observers[[session$ns("col_to_add")]] <- observeEvent( + private$session_bindings[[session$ns("col_to_add")]] <- observeEvent( eventExpr = input$col_to_add, handlerExpr = { logger::log_debug( @@ -270,7 +270,7 @@ SEFilterStates <- R6::R6Class( # nolint ) - private$observers[[session$ns("row_to_add")]] <- observeEvent( + private$session_bindings[[session$ns("row_to_add")]] <- observeEvent( eventExpr = input$row_to_add, handlerExpr = { logger::log_debug( @@ -295,6 +295,15 @@ SEFilterStates <- R6::R6Class( # nolint } ) + # Extra observer that clears all input values in session + private$session_bindings[[session$ns("inputs")]] <- list( + destroy = function() { + if (!session$isEnded()) { + lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) + } + } + ) + NULL } ) diff --git a/R/FilteredData.R b/R/FilteredData.R index 83527f871..9b56ee937 100644 --- a/R/FilteredData.R +++ b/R/FilteredData.R @@ -610,7 +610,7 @@ FilteredData <- R6::R6Class( # nolint private$srv_available_filters("available_filters") - private$observers[[session$ns("minimise_filter_active")]] <- observeEvent( + private$session_bindings[[session$ns("minimise_filter_active")]] <- observeEvent( eventExpr = input$minimise_filter_active, handlerExpr = { shinyjs::toggle("filter_active_vars_contents") @@ -640,7 +640,7 @@ FilteredData <- R6::R6Class( # nolint ) }) - private$observers[[session$ns("is_filter_removable")]] <- observeEvent( + private$session_bindings[[session$ns("is_filter_removable")]] <- observeEvent( eventExpr = is_filter_removable(), handlerExpr = { shinyjs::toggle("remove_all_filters", condition = is_filter_removable()) @@ -651,7 +651,7 @@ FilteredData <- R6::R6Class( # nolint } ) - private$observers[[session$ns("active_datanames")]] <- observeEvent( + private$session_bindings[[session$ns("active_datanames")]] <- observeEvent( eventExpr = active_datanames(), handlerExpr = lapply(self$datanames(), function(dataname) { if (dataname %in% active_datanames()) { @@ -682,7 +682,7 @@ FilteredData <- R6::R6Class( # nolint ) }) - private$observers[[session$ns("remove_all_filters")]] <- observeEvent( + private$session_bindings[[session$ns("remove_all_filters")]] <- observeEvent( eventExpr = input$remove_all_filters, handlerExpr = { logger::log_debug("FilteredData$srv_filter_panel@1 removing all non-anchored filters") @@ -691,6 +691,14 @@ FilteredData <- R6::R6Class( # nolint } ) + private$session_bindings[[session$ns("inputs")]] <- list( + destroy = function() { + if (!session$isEnded()) { + lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) + } + } + ) + NULL }) }, @@ -852,12 +860,12 @@ FilteredData <- R6::R6Class( # nolint #' @description #' Object and dependencies cleanup. #' - #' - Destroy observers stored in `private$observers` + #' - Destroy inputs and observers stored in `private$session_bindings` #' - Finalize `FilteredData` stored in `private$filtered_datasets` #' #' @return `NULL`, invisibly. finalize = function() { - .finalize_observers(self, private) + .finalize_session_bindings(self, private) lapply(private$filtered_datasets, function(x) x$finalize()) invisible(NULL) } @@ -882,8 +890,8 @@ FilteredData <- R6::R6Class( # nolint # flag specifying whether the user may add filters allow_add = TRUE, - # observers list - observers = list(), + # observers and inputs list + session_bindings = list(), # private methods ---- @@ -1014,7 +1022,7 @@ FilteredData <- R6::R6Class( # nolint ) }) - private$observers[[session$ns("available_slices_id")]] <- observeEvent( + private$session_bindings[[session$ns("available_slices_id")]] <- observeEvent( eventExpr = input$available_slices_id, ignoreNULL = FALSE, ignoreInit = TRUE, @@ -1039,7 +1047,7 @@ FilteredData <- R6::R6Class( # nolint } ) - private$observers[[session$ns("available_teal_slices")]] <- observeEvent( + private$session_bindings[[session$ns("available_teal_slices")]] <- observeEvent( eventExpr = private$available_teal_slices(), ignoreNULL = FALSE, handlerExpr = { diff --git a/R/FilteredDataset.R b/R/FilteredDataset.R index 9e3d568f5..998f5f4d8 100644 --- a/R/FilteredDataset.R +++ b/R/FilteredDataset.R @@ -293,6 +293,7 @@ FilteredDataset <- R6::R6Class( # nolint filter_count <- reactive({ length(self$get_filter_state()) }) + output$filter_count <- renderText( sprintf( "%d filter%s applied", @@ -339,7 +340,7 @@ FilteredDataset <- R6::R6Class( # nolint isTRUE(length(non_anchored) > 0) }) - private$observers[[session$ns("get_filter_state")]] <- observeEvent( + private$session_bindings[[session$ns("get_filter_state")]] <- observeEvent( self$get_filter_state(), ignoreInit = TRUE, { @@ -366,12 +367,20 @@ FilteredDataset <- R6::R6Class( # nolint ) }) - private$observers[[session$ns("remove_filters")]] <- observeEvent(input$remove_filters, { + private$session_bindings[[session$ns("remove_filters")]] <- observeEvent(input$remove_filters, { logger::log_debug("FilteredDataset$srv_active@1 removing all non-anchored filters, dataname: { dataname }") self$clear_filter_states() logger::log_debug("FilteredDataset$srv_active@1 removed all non-anchored filters, dataname: { dataname }") }) + private$session_bindings[[session$ns("inputs")]] <- list( + destroy = function() { + if (!session$isEnded()) { + lapply(session$ns(names(input)), .subset2(input, "impl")$.values$remove) + } + } + ) + self$srv_add(private$dataname) NULL @@ -420,12 +429,12 @@ FilteredDataset <- R6::R6Class( # nolint #' @description #' Object and dependencies cleanup. #' - #' - Destroy observers stored in `private$observers` + #' - Destroy inputs and observers stored in `private$session_bindings` #' - Finalize `FilterStates` stored in `private$filter_states` #' #' @return `NULL`, invisibly. finalize = function() { - .finalize_observers(self, private) + .finalize_session_bindings(self, private) lapply(private$filter_states, function(x) x$finalize()) invisible(NULL) } @@ -439,7 +448,7 @@ FilteredDataset <- R6::R6Class( # nolint dataname = character(0), keys = character(0), label = character(0), - observers = list(), + session_bindings = list(), # Adds `FilterStates` to the `private$filter_states`. # `FilterStates` is added once for each element of the dataset. diff --git a/R/utils.R b/R/utils.R index a7d595f63..725f5a8c9 100644 --- a/R/utils.R +++ b/R/utils.R @@ -67,7 +67,17 @@ make_c_call <- function(choices) { } } -.finalize_observers <- function(self, private) { - if (length(private$observers) > 0) lapply(private$observers, function(x) x$destroy()) +#' Destroys inputs and observers stored in `private$session_bindings` +#' +#' @description +#' Call a `destroy` method to remove `observer` and `input` from obsolete session which happens +#' when `filter_panel_srv` is called again in new `FilteredData` object. +#' Inputs are not stored directly in a field as they don't have `destroy` method. Instead, we +#' store callback `destroy` function for inputs which removes bindings from a `session`. +#' @param self,private slots of a `R6` class +#' @return `NULL` invisibly +#' @keywords internal +.finalize_session_bindings <- function(self, private) { + if (length(private$session_bindings) > 0) lapply(private$session_bindings, function(x) x$destroy()) invisible(NULL) } diff --git a/man/FilterState.Rd b/man/FilterState.Rd index 0b3feabb5..9ed204942 100644 --- a/man/FilterState.Rd +++ b/man/FilterState.Rd @@ -243,9 +243,7 @@ The UI for this class contains simple message stating that it is not supported. \if{html}{\out{}} \if{latex}{\out{\hypertarget{method-FilterState-finalize}{}}} \subsection{Method \code{finalize()}}{ -Destroy observers stored in \code{private$observers}. - -The \code{destroy_shiny} definition is set in the server method. +Destroy inputs and observers stored in \code{private$session_bindings}. \subsection{Usage}{ \if{html}{\out{
}}\preformatted{FilterState$finalize()}\if{html}{\out{
}} } diff --git a/man/FilterStateExpr.Rd b/man/FilterStateExpr.Rd index 128909b20..8ab314690 100644 --- a/man/FilterStateExpr.Rd +++ b/man/FilterStateExpr.Rd @@ -202,7 +202,7 @@ and must be executed in reactive or isolated context.} \if{html}{\out{}} \if{latex}{\out{\hypertarget{method-FilterStateExpr-finalize}{}}} \subsection{Method \code{finalize()}}{ -Destroy observers stored in \code{private$observers}. +Destroy inputs and observers stored in \code{private$session_bindings}. \subsection{Usage}{ \if{html}{\out{
}}\preformatted{FilterStateExpr$finalize()}\if{html}{\out{
}} } diff --git a/man/FilterStates.Rd b/man/FilterStates.Rd index 61bef72d6..b6898ef25 100644 --- a/man/FilterStates.Rd +++ b/man/FilterStates.Rd @@ -348,7 +348,7 @@ Removing a filter variable adds it back to available choices. \subsection{Method \code{finalize()}}{ Object cleanup. \itemize{ -\item Destroy observers stored in \code{private$observers} +\item Destroy inputs and observers stored in \code{private$session_bindings} \item Clean \code{state_list} } \subsection{Usage}{ diff --git a/man/FilteredData.Rd b/man/FilteredData.Rd index 9a4fbf411..fd641cf81 100644 --- a/man/FilteredData.Rd +++ b/man/FilteredData.Rd @@ -676,7 +676,7 @@ panel will be hidden.} \subsection{Method \code{finalize()}}{ Object and dependencies cleanup. \itemize{ -\item Destroy observers stored in \code{private$observers} +\item Destroy inputs and observers stored in \code{private$session_bindings} \item Finalize \code{FilteredData} stored in \code{private$filtered_datasets} } \subsection{Usage}{ diff --git a/man/FilteredDataset.Rd b/man/FilteredDataset.Rd index 302724784..ac51c9d76 100644 --- a/man/FilteredDataset.Rd +++ b/man/FilteredDataset.Rd @@ -357,7 +357,7 @@ contains one \code{FilterStates} object for \code{colData} and one for each expe \subsection{Method \code{finalize()}}{ Object and dependencies cleanup. \itemize{ -\item Destroy observers stored in \code{private$observers} +\item Destroy inputs and observers stored in \code{private$session_bindings} \item Finalize \code{FilterStates} stored in \code{private$filter_states} } \subsection{Usage}{ diff --git a/man/dot-finalize_session_bindings.Rd b/man/dot-finalize_session_bindings.Rd new file mode 100644 index 000000000..be4944d1e --- /dev/null +++ b/man/dot-finalize_session_bindings.Rd @@ -0,0 +1,21 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{.finalize_session_bindings} +\alias{.finalize_session_bindings} +\title{Destroys inputs and observers stored in \code{private$session_bindings}} +\usage{ +.finalize_session_bindings(self, private) +} +\arguments{ +\item{self, private}{slots of a \code{R6} class} +} +\value{ +\code{NULL} invisibly +} +\description{ +Call a \code{destroy} method to remove \code{observer} and \code{input} from obsolete session which happens +when \code{filter_panel_srv} is called again in new \code{FilteredData} object. +Inputs are not stored directly in a field as they don't have \code{destroy} method. Instead, we +store callback \code{destroy} function for inputs which removes bindings from a \code{session}. +} +\keyword{internal}