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

reset button to restore filters to default state #298

Closed
Tracked by #165 ...
donyunardi opened this issue May 24, 2023 · 8 comments · Fixed by #354
Closed
Tracked by #165 ...

reset button to restore filters to default state #298

donyunardi opened this issue May 24, 2023 · 8 comments · Fixed by #354
Assignees

Comments

@donyunardi
Copy link
Contributor

donyunardi commented May 24, 2023

Summary

  • As a user, I would like to restore the filters to their original default settings, just like when the app was first run.
  • The reset could possibly apply to a single filter card or all filter cards.
  • Take into account that we have module_specific filters, so the restore on a FilteredData level might conflict existing states in other modules.
  • Consider scenario when we have multiple teal_slices saved and user might be able to switch between "bookmarked" states in the future.

Acceptance Criteria

  1. Conduct research to identify the necessary components and approaches for enabling the desired feature.
  2. Propose steps to complete the feature in small pieces. Breaking down the implementation into manageable steps can help ensure a systematic and organized approach.
@lcd2yyz
Copy link

lcd2yyz commented May 25, 2023

Propose to add a button with the turning black arrow icon at two levels

  • beside the on/off toggle at the global filter panel level
  • beside the collapse/expand accordion button at the dataset level

@donyunardi
Copy link
Contributor Author

donyunardi commented May 25, 2023

Just want to mention that the example app that @gogonzo mentioned in #165 (funny_module) already has reset button 😃

image

@chlebowa chlebowa self-assigned this Jun 19, 2023
@chlebowa
Copy link
Contributor

chlebowa commented Jun 20, 2023

Please clarify:

  • @lcd2yyz OP refers to all filters or a single card, whereas you refer to one whole dataset
  • Should we also consider an "undo" functionality (placeholder name) to remember the most recent state or store history of all changes? If so, should resetting clear history?

@chlebowa chlebowa changed the title [Research] A reset button to restore filters to default state reset button to restore filters to default state Jun 20, 2023
@lcd2yyz
Copy link

lcd2yyz commented Jun 21, 2023

Can we try to come up with a design for as much flexibility as possible - meaning having a "reset" for each potential level of interaction

  • For each filter card/variable
  • For each module or module set (now that we can do module specific)
  • At the global filter level (as if app have restarted)

On a second thought, resetting at each dataset-level probably does not make as much sense as resetting at module-level, so let's scratched that.

Once there is a rough design we can use to start discussion, we can go into the details of whether implementation is possible technically, and/or whether it make sense from UX perspective (too many buttons may not be a good thing either). Be as creative as you see fit.

  • Should we also consider an "undo" functionality (placeholder name) to remember the most recent state or store history of all changes? If so, should resetting clear history?

I wouldn't consider this to be a core requirement, but it will be very cool if possible! Do feel free to build that into the design as much as possible. My initial thought is "resetting" can either clear history, or be saved as another step in the history. Both can make sense depending on the actual usage, so I don't have a strong opinion either way at the moment.

@chlebowa
Copy link
Contributor

All right. No dataset level then.

I must warn that module-level may well be impossible because - as @gogonzo pointed out - some filters are shared across modules. But perhaps we will come up with some mechanism.

@chlebowa
Copy link
Contributor

chlebowa commented Jun 21, 2023

I prepared reset and undo/back mechanics on the filter card level. I expect it will not be hard to implement the same at the global level. However, I foresee a problem. Consider this:

  1. app runs 2 modules
  2. 5 filters are shared between modules
  3. module1 has 2 specific filters, module2 has 4
  4. the user sets the common filters in module1
  5. the user sets some specific filters in module1
  6. the user moves to module2 and sets specific filters there
  7. the user moves back to module1 and wants to go back a few steps
  8. the user clicks the global back button
    Nothing changes as the last recorded state of the app is identical to the current one as far as filters active in module1 are concerned. Meanwhile, changes do happen in module2, out of sight of the user.

The back action may not make sense in the global context.

Any thoughts?

@chlebowa chlebowa linked a pull request Jun 22, 2023 that will close this issue
@lcd2yyz
Copy link

lcd2yyz commented Jun 27, 2023

Thanks for starting the design and thinking through the detail of the behaviors.

With the example you outlined, I agree "undo/back" action may not make sense at global level. We should definitely avoid changing filter settings unknowingly to the users. At global level, it should require more conscious interaction from the user to save some "states" perhaps (as oppose to save all states with undo).
It would still be useful at individual filter card level, as users may be exploring different settings/thresholds. Will be cool if we can still retain this at filter-card level.

@chlebowa
Copy link
Contributor

Yup, after some talks we agreed that it makes more sense to only be able to save snapshots globally, but perhaps track some history of filter cards in between global snapshots. That is what I am trying to figure out at the moment.

chlebowa added a commit that referenced this issue Jul 21, 2023
Closes #298 

#### In this PR

##### State history
+ `FilterState` receives a new private field, `private$state_history`,
that holds a `reactiveVal` that contains a `list`. This list stores all
states that this `FilterState` has had since istantiation. States are
stored not as `teal_slice` but as lists because `teal_slice` are now
`reactiveValues` that can be shared between `FilteredData` objects.
+ Updates to `private$get_selected` , `private$get_keep_na`, and
`private$get_keep_inf` cause the current state to be appended to state
history.
+ `FilterState` ui module receives two buttons, `back` and `reset`,
placed next to the `remove` button.
+ The `reset` button sets the state to the one with in which
`FilterState$server` started.
+ The `back` button sets the state to the penultimate one stored in
`private$state_history`.
+ Since we do not envision a `forward` button, state history is only
stored backwards, i.e. clicking the `back` button erases the last stored
state.
+ The `setdiff_teal_slices` function is added to assist in removing
subsets of filters.
+ Some behavior in getting and setting filter states is modified to be
more amenable to module-specific situations.
+ Changes in `FilterStates` cause server updates when new states are
added, not just when states with new names are added. The previous state
caused pointers to removed filter states to linger and new states could
access private fields of the removed ones. Solved by @gogonzo

Note that buttons become active only when applicable (i.e. state history
is of length > 1).


#### TESTING
```
# rm(list = ls()) # uncomment to clear environment before each app start

options(teal.log_level = "WARN", teal.show_js_log = FALSE)
options("teal.bs_theme" = bslib::bs_theme(version = 3))
# options(shiny.trace = TRUE)
devtools::load_all("../teal.slice")
devtools::load_all("../teal")


funny_module <- function (label = "Filter states", datanames = "all") {
  checkmate::assert_string(label)
  module(
    label = label,
    filters = datanames,
    ui = function(id, ...) {
      ns <- NS(id)
      div(
        h2("The following filter calls are generated:"),
        verbatimTextOutput(ns("filter_states")),
        verbatimTextOutput(ns("filter_calls")),
        actionButton(ns("reset"), "reset_to_default")
      )
    },
    server = function(input, output, session, data, filter_panel_api) {
      checkmate::assert_class(data, "tdata")
      observeEvent(input$reset, set_filter_state(filter_panel_api, default_filters))
      output$filter_states <- renderText({
        logger::log_trace("rendering text1")
        stage1 <- filter_panel_api |> get_filter_state()
        stage2 <- lapply(stage1, as.list)
        stage3 <- yaml::as.yaml(stage2)
        stage3
      })
      output$filter_calls <- renderText({
        logger::log_trace("rendering text2")
        attr(data, "code")()
      })
    }
  )
}


Iris <- iris
levels(Iris$Species) #%<>% toupper()
Iris$Species2 <- Iris$Species
Iris$Species3 <- Iris$Species
Iris$Sepal.Length[1] <- NA
Iris$Sepal.Length[2] <- Inf
Iris$Species3[2] <- NA
attr(Iris[[1]], "label") <- "variable label"
attr(Iris[[2]], "label") <- "variable label"
attr(Iris[[3]], "label") <- "variable label"
attr(Iris[[4]], "label") <- "variable label"
data <- teal_data(
  dataset("iris", Iris),
  dataset("mtcars", mtcars)
)

default_filters <- teal_slices(
  teal_slice("iris", "Sepal.Length", sel = c(5.4, 6.1), fixed = FALSE),
  teal_slice("iris", "Sepal.Width", sel = c(2.5, 4.0), fixed = FALSE, anchored = TRUE),
  teal_slice("iris", "Petal.Length", sel = c(4.5, 5.1), fixed = TRUE),
  teal_slice("iris", "Petal.Width", sel = c(0.3, 1.8), fixed = TRUE, anchored = TRUE),
  teal_slice("iris", "Species", sel = c("versicolor", "virginica")),
  # teal_slice("iris", "Species2", sel = c("versicolor", "virginica"), fixed = TRUE),
  # teal_slice("iris", "Species3", sel = c("versicolor", "virginica"), anchored = TRUE),
  count_type = NULL
)

app <- init(
  data = data,
  modules = list(funny_module(), funny_module("module2")), filter = default_filters
)

runApp(app, launch.browser = TRUE)
```

---------

Signed-off-by: Marek Blazewicz <110387997+BLAZEWIM@users.noreply.github.com>
Signed-off-by: Aleksander Chlebowski <114988527+chlebowa@users.noreply.github.com>
Signed-off-by: kartikeya kirar <kirar.kartikeya1@gmail.com>
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
Co-authored-by: Dawid Kałędkowski <dawid.kaledkowski@gmail.com>
Co-authored-by: Dawid Kałędkowski <6959016+gogonzo@users.noreply.github.com>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: chlebowa <chlebowa@users.noreply.github.com>
Co-authored-by: Andrew Bates <andrew.bates@atorusresearch.com>
Co-authored-by: Marek Blazewicz <110387997+BLAZEWIM@users.noreply.github.com>
Co-authored-by: Mahmoud Hallal <86970066+mhallal1@users.noreply.github.com>
Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Blazewicz <blazewim@emea.roche.com>
Co-authored-by: Nikolas Burkoff <nikolas.burkoff@roche.com>
Co-authored-by: Dony Unardi <donyunardi@gmail.com>
Co-authored-by: Marcin <133694481+m7pr@users.noreply.github.com>
Co-authored-by: kartikeya kirar <kirar.kartikeya1@gmail.com>
Co-authored-by: kartikeya <kartikeya.kirar@unicle.life>
Co-authored-by: kartikeyakirar <kartikeyakirar@users.noreply.github.com>
Co-authored-by: m7pr <m7pr@users.noreply.github.com>
Co-authored-by: gogonzo <gogonzo@users.noreply.github.com>
Co-authored-by: pawelru <pawelru@users.noreply.github.com>
chlebowa added a commit to insightsengineering/teal that referenced this issue Jul 21, 2023
Relates to [this
issue](insightsengineering/teal.slice#298).

Adds module for management of snapshots of application state.
A snapshot contains in formation on all filter states currently existing
within the app, as well as their application to particular modules.
A snapshot can be added at any time by the user.
A snapshot can be restored at any time by the user.
A snapshot can be saved to file.
The initial state of the application is the first snapshot (not
displayed in the list).

In addition, `teal_slices` handles global-module-specific specification
differently. The deciding argument is `module_specific`.
`mapping$global_filters` will decide which - if any - filters are active
on start-up in global mode.

---------

Signed-off-by: Aleksander Chlebowski <114988527+chlebowa@users.noreply.github.com>
Co-authored-by: go_gonzo <dawid.kaledkowski@gmail.com>
Co-authored-by: Andrew Bates <andrew.bates@atorusresearch.com>
Co-authored-by: asbates <asbates@users.noreply.github.com>
Co-authored-by: chlebowa <chlebowa@users.noreply.github.com>
Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Dawid Kałędkowski <6959016+gogonzo@users.noreply.github.com>
Co-authored-by: Marcin <133694481+m7pr@users.noreply.github.com>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: kartikeya kirar <kirar.kartikeya1@gmail.com>
Co-authored-by: kartikeya <kartikeya.kirar@unicle.life>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants