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

turn off on panel #91

Closed
wants to merge 19 commits into from
68 changes: 68 additions & 0 deletions R/FilteredData.R
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,7 @@ FilteredData <- R6::R6Class( # nolint
)
}
logger::log_trace("FilteredData$set_filter_state initialized, dataname: { paste(names(state), collapse = ' ') }")

invisible(NULL)
},

Expand All @@ -529,6 +530,8 @@ FilteredData <- R6::R6Class( # nolint
#'
#' @return `NULL`
remove_filter_state = function(state) {
checkmate::assert_subset(names(state), self$datanames())

logger::log_trace("FilteredData$remove_filter_state called, dataname: { paste(names(state), collapse = ' ') }")

for (dataname in names(state)) {
Expand All @@ -537,6 +540,7 @@ FilteredData <- R6::R6Class( # nolint
}

logger::log_trace("FilteredData$remove_filter_state done, dataname: { paste(names(state), collapse = ' ') }")

invisible(NULL)
},

Expand Down Expand Up @@ -586,6 +590,33 @@ FilteredData <- R6::R6Class( # nolint
restore_state_from_bookmark = function(state, check_data_hash = TRUE) {
stop("Pure virtual method")
},
#' @description disable the filter panel
filter_panel_disable = function() {
private$filter_turn <- FALSE
shinyjs::disable("filter_add_vars")
shinyjs::disable("filter_active_vars")
private$cached_states <- self$get_filter_state()
self$remove_all_filter_states()
invisible(NULL)
},
#' @description enable the filter panel
filter_panel_enable = function() {
private$filter_turn <- TRUE
shinyjs::enable("filter_add_vars")
shinyjs::enable("filter_active_vars")
if (length(private$cached_states) && (length(self$get_filter_state()) == 0)) {
self$set_filter_state(private$cached_states)
}
invisible(NULL)
},
#' @description get the state of filter panel, if it is activated
get_filter_turn = function() {
private$filter_turn
},
#' @description get the id of the filter panel ui
get_filter_panel_ui_id = function() {
private$filter_panel_ui_id
},

# shiny modules -----

Expand All @@ -602,6 +633,22 @@ FilteredData <- R6::R6Class( # nolint
div(
id = ns(NULL), # used for hiding / showing
include_css_files(pattern = "filter-panel"),
div(
id = ns("switch-button"),
class = "flex justify-content-right",
div(
title = "Active Filtering",
shinyWidgets::prettySwitch(
ns("filter_turn_onoff"),
label = "",
status = "success",
fill = TRUE,
value = TRUE,
inline = FALSE,
width = 30
)
)
),
div(
id = ns("filters_overview"), # not used, can be used to customize CSS behavior
class = "well",
Expand Down Expand Up @@ -772,6 +819,20 @@ FilteredData <- R6::R6Class( # nolint
}
)

private$filter_panel_ui_id <- session$ns(NULL)
observeEvent(
eventExpr = input[["filter_turn_onoff"]],
handlerExpr = {
pawelru marked this conversation as resolved.
Show resolved Hide resolved
if (isTRUE(input[["filter_turn_onoff"]])) {
self$filter_panel_enable()
logger::log_trace("Enable the Filtered Panel with the filter_panel_enable method")
} else {
self$filter_panel_disable()
logger::log_trace("Disable the Filtered Panel with the filter_panel_enable method")
}
}, ignoreNULL = TRUE
)

observeEvent(
eventExpr = active_datanames(),
handlerExpr = {
Expand Down Expand Up @@ -904,6 +965,12 @@ FilteredData <- R6::R6Class( # nolint
# private attributes ----
filtered_datasets = list(),

# turn on / off filter panel
filter_turn = TRUE,
Copy link
Contributor

@pawelru pawelru Sep 2, 2022

Choose a reason for hiding this comment

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

  1. I dislike the "turn" term here. You can turn left/right or on/off. It's a verb whereas you are looking for a noun. I propose state / status / active instead. Please change other places as well including method docs.

  2. What's the point of this field? I ask from the class simplicity point of view.
    I can see that it's used in the api class - there is a check whether it's active and if yes then you can do set/get/etc. Is that needed? The functionality would be anyway disabled from the GUI perspective (vide: disabling inputs). What I am suggesting here is that the big disable/enable button would just propagate this action to all relevant inputs + save/restore action and that's it. An object would not store its state as this opens a door for unnecessary complexity I believe. I am happy to discuss it further if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By default shinyWidgets::switchInput use the logical(1) for value so this should not be surprising I stay with this type.

filter_turn variable is needed only for filter_panel_api, so we could provide a proper warning if sb want to update the state when filter panel is turn off.

I like the Turn Off/On naming, although I see you agree with Nik in this area so please update to the one you are preferring.


# filter panel ui id
filter_panel_ui_id = character(0),

# whether the datasets had a reproducibility check
.check = FALSE,

Expand All @@ -915,6 +982,7 @@ FilteredData <- R6::R6Class( # nolint

# reactive i.e. filtered data
reactive_data = list(),
cached_states = NULL,

# we implement these functions as checks rather than returning logicals so they can
# give informative error messages immediately
Expand Down
34 changes: 30 additions & 4 deletions R/filter_panel_api.R
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ FilterPanelAPI <- R6::R6Class( # nolint
#'
#' @return `NULL`
set_filter_state = function(filter) {
private$filtered_data$set_filter_state(filter)
if (private$filtered_data$get_filter_turn()) {
private$filtered_data$set_filter_state(filter)
} else {
warning("Filter Panel is turn off so the action can not be applied with api.")
}
invisible(NULL)
},

Expand All @@ -73,7 +77,11 @@ FilterPanelAPI <- R6::R6Class( # nolint
#'
#' @return `NULL`
remove_filter_state = function(filter) {
private$filtered_data$remove_filter_state(filter)
if (private$filtered_data$get_filter_turn()) {
private$filtered_data$remove_filter_state(filter)
} else {
warning("Filter Panel is turn off so the action can not be applied with api.")
}
invisible(NULL)
},

Expand All @@ -83,8 +91,26 @@ FilterPanelAPI <- R6::R6Class( # nolint
#'
#' @return `NULL`
remove_all_filter_states = function(datanames) {
datanames_to_remove <- if (missing(datanames)) private$filtered_data$datanames() else datanames
private$filtered_data$remove_all_filter_states(datanames = datanames_to_remove)
if (private$filtered_data$get_filter_turn()) {
datanames_to_remove <- if (missing(datanames)) private$filtered_data$datanames() else datanames
private$filtered_data$remove_all_filter_states(datanames = datanames_to_remove)
} else {
warning("Filter Panel is turn off so the action can not be applied with api.")
}
invisible(NULL)
},
#' @description Toggle the state of the Filter Panel turn on/off button.
#' @param id (`character(1)`)\cr
#' `id` to the toggle button.
#'
#' @return `NULL`
filter_panel_toggle = function() {
shinyjs::runjs(
sprintf(
'$("#%s-filter_turn_onoff").click();',
private$filtered_data$get_filter_panel_ui_id()
)
)
invisible(NULL)
}
),
Expand Down
12 changes: 12 additions & 0 deletions inst/css/filter-panel.css
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
position: relative;
}

.inline-block {
position: inline-block;
}

.flex {
display: flex;
}
Expand Down Expand Up @@ -40,10 +44,18 @@
margin-bottom: 1rem;
}

.mr-4 {
margin-right: 1rem;
}

.justify-content-center {
justify-content: center;
}

.justify-content-right {
justify-content: right;
}

.date_reset_button {
padding: 0;
padding-top: 4px;
Expand Down
4 changes: 4 additions & 0 deletions man/CDISCFilteredData.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions man/FilterPanelAPI.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

52 changes: 48 additions & 4 deletions man/FilteredData.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

64 changes: 64 additions & 0 deletions tests/testthat/test-FilteredData.R
Original file line number Diff line number Diff line change
Expand Up @@ -576,3 +576,67 @@ testthat::test_that("get_data assert the `filtered` argument is logical(1)", {
regexp = "Assertion on 'filtered' failed: Must be of type 'logical flag', not 'character'"
)
})

testthat::test_that("filter_panel_disable", {
filtered_data <- FilteredData$new(data_objects = list("iris" = list(dataset = iris)), join_keys = NULL)
filtered_data$set_filter_state(list(iris = list(Sepal.Width = c(3, 4))))
shiny::testServer(
filtered_data$srv_filter_panel,
expr = {
filtered_data$filter_panel_disable()
testthat::expect_length(filtered_data$get_filter_state(), 0)
}
)
})

testthat::test_that("filter_panel_enable", {
filtered_data <- FilteredData$new(data_objects = list("iris" = list(dataset = iris)), join_keys = NULL)
filtered_data$set_filter_state(list(iris = list(Sepal.Width = c(3, 4))))
shiny::testServer(
filtered_data$srv_filter_panel,
expr = {
filtered_data$filter_panel_enable()
testthat::expect_length(filtered_data$get_filter_state(), 1)
testthat::expect_equal(filtered_data$get_filter_state()$iris$Sepal.Width$selected, c(3, 4))
}
)
})

testthat::test_that("filter_panel_disable and filter_panel_enable", {
filtered_data <- FilteredData$new(data_objects = list("iris" = list(dataset = iris)), join_keys = NULL)
filtered_data$set_filter_state(list(iris = list(Sepal.Width = c(3, 4))))
shiny::testServer(
filtered_data$srv_filter_panel,
expr = {
testthat::expect_length(filtered_data$get_filter_state(), 1)
testthat::expect_true(filtered_data$get_filter_turn())
filtered_data$filter_panel_disable()
testthat::expect_length(filtered_data$get_filter_state(), 0)
testthat::expect_false(filtered_data$get_filter_turn())
filtered_data$filter_panel_enable()
testthat::expect_length(filtered_data$get_filter_state(), 1)
testthat::expect_true(filtered_data$get_filter_turn())
}
)
})

testthat::test_that("turn filed by default equal to TRUE", {
filtered_data <- FilteredData$new(data_objects = list("iris" = list(dataset = iris)), join_keys = NULL)
testthat::expect_true(filtered_data$get_filter_turn())
})

testthat::test_that("get_filter_panel_ui_id - empty when no shiny session", {
filtered_data <- FilteredData$new(data_objects = list("iris" = list(dataset = iris)), join_keys = NULL)
testthat::expect_length(filtered_data$get_filter_panel_ui_id(), 0)
})

testthat::test_that("get_filter_panel_ui_id - non-empty when in shiny session", {
filtered_data <- FilteredData$new(data_objects = list("iris" = list(dataset = iris)), join_keys = NULL)
shiny::testServer(
filtered_data$srv_filter_panel,
expr = {
testthat::expect_length(filtered_data$get_filter_panel_ui_id(), 1)
}
)
})

Loading