234 range selection with `plotly` #289

chlebowa · 2023-05-17T14:08:18Z

Closes #234
Closes #233

Replaces the range slider in RangeFilterState with an interactive plotly graph. Two shapes (lines) are drawn on the plot that can be dragged and their position is tracked. An observer listens to events emitted by the plot when shapes are altered (this event is called a "plotly_relayout") and updates selection. Another observer listens to the manual input and updates selection. Finally, a third observer listens to the selection and updates the manual input as well as the shapes on the plot.

Since the graph is slower to render, a spinner is added to it to alleviate the negative effect on UX.

Numeric (manual) input is now displayed simultaneously with the graphic input, not alternatively.

Numeric input receives a debounce.

Use this application to test:

options("teal.bs_theme" = bslib::bs_theme(version = 4))
devtools::load_all()
library(teal)
library(scda)

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")
        filter_panel_api |> get_filter_state() |> yaml::as.yaml()
      })
      output$filter_calls <- renderText({
        logger::log_trace("rendering text2")
        attr(data, "code")()
      })
    }
  )
}

### CDISC ###
options(teal.log_level = "TRACE", teal.show_js_log = FALSE)
devtools::load_all()
library(teal)
library(scda)

ADSL <- synthetic_cdisc_data("latest")$adsl
ADSL$numeric <- ADSL$numeric4 <- rnorm(nrow(ADSL))
ADSL$numeric2 <- sample(1:100000, size = nrow(ADSL))
ADSL$numeric3 <- rep(0, nrow(ADSL))

ADTTE <- synthetic_cdisc_data("latest")$adtte
ADRS <- synthetic_cdisc_data("latest")$adrs

ADTTE$numeric <- rnorm(nrow(ADTTE))
ADTTE$numeric[sample(1:nrow(ADTTE), size = 10,)] <- NA

default_filters <- filter_settings(
  filter_var("ADSL", "numeric", selected = c(0, 1.40), keep_na = TRUE, keep_inf = TRUE, fixed = FALSE),
  filter_var("ADSL", "numeric2", selected = c(10, 10000), keep_na = TRUE, keep_inf = TRUE, fixed = FALSE),
  filter_var("ADSL", "numeric3", selected = c(0, 0), keep_na = TRUE, keep_inf = TRUE, fixed = FALSE),
  filter_var("ADSL", "numeric4", selected = c(0, 1.40), keep_na = TRUE, keep_inf = TRUE, fixed = TRUE),
  filter_var("ADTTE", "AVAL", selected = c(10, 1000), keep_na = TRUE, keep_inf = TRUE, fixed = FALSE),
  exclude_varnames = list(ADTTE = setdiff(c("AVAL", "PARAMCD", "PARAM"), names(ADTTE)))
)

data <- cdisc_data(
  cdisc_dataset("ADSL", ADSL),
  cdisc_dataset("ADTTE", ADTTE),
  cdisc_dataset("ADRS", ADRS)
)

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

runApp(app, launch.browser = TRUE)

chlebowa · 2023-05-18T12:22:25Z

Update: made a breakthrough in hyperreactivity and disabling. It's not quite ready but getting somewhere.

asbates · 2023-05-18T19:14:07Z

This is so cool!!!

I will make my obligatory comment about not hard-coding colors ;) but that's out of scope for this.

The only real things I see are

It looks like some overlap of the code in server_inputs and server_inputs_fixed so that could be moved out into a method/function.
Selections are still limited to a specific set of values.

These are probably pretty obvious though and because this is a WIP. I can see this feature is going to be useful and cool.

Update: I think it would be a good idea to make this a stand-alone widget, maybe inteal.widgets. I think people would find this useful outside of teal.

…ter_panel_refactor@main

chlebowa · 2023-05-22T07:16:52Z

I will make my obligatory comment about not hard-coding colors ;) but that's out of scope for this.

Duly noted, the colors used here are placeholders.

It looks like some overlap of the code in server_inputs and server_inputs_fixed so that could be moved out into a method/function.

I will try to optimize this but it may be necessary, actually, because the plot in server_inputs emits events that must be namespaced. Perhaps the plot_* building blocks can be stored in the object and only be put together in the server.

Update: I think it would be a good idea to make this a stand-alone widget, maybe inteal.widgets. I think people would find this useful outside of teal.

Hmm, that's certainly something to think about.

Thanks, @asbates!

donyunardi · 2023-05-25T21:07:21Z

Truly great stuff!

Played around with this a bit more and found an interesting behavior.
If I use keyboard down arrow in the range input and I press up or down quickly several times, the reactive loops kicks in and the app is stuck in computing state while keep refreshing the range input box.

Here's a video where I press down in the max range input box several times rapidly.
During the video, I didn't press any key and you will see the filter panel keeps on refreshing/computing.

Screen.Recording.2023-05-25.at.2.03.29.PM.mov

@chlebowa

@chlebowa I have implemented a debounce of 500ms for the input$selection_manual. Additionally, I have replaced the modal with a popover (which can alternatively be replaced with a tooltip). Initially, I attempted to address the issue by creating a separate JavaScript (JS) file named "popover.js" and calling it within the "filterState" function using the "include_js_files" method. However, I encountered a problem where the popover functionality only applied to the first defined help link, while the others did not display the popover. To overcome this issue, I decided to keep the script inline within the HTML code. LEt me know you thoughts and suggestions --------- Signed-off-by: kartikeya kirar <[email protected]> Signed-off-by: Aleksander Chlebowski <[email protected]> Co-authored-by: kartikeya <[email protected]> Co-authored-by: Aleksander Chlebowski <[email protected]> Co-authored-by: Aleksander Chlebowski <[email protected]>

kartikeyakirar · 2023-06-13T11:23:29Z

Debouce functionality for range numeric input and popover is added in place of modal pop-up.
#334 (PR is merged )

…b.com:insightsengineering/teal.slice into 234_range_selector@filter_panel_refactor@main

kartikeyakirar · 2023-06-13T12:44:21Z

Test is failing. with error Objects equal but not identical for one of the test in filterstateRange

chlebowa · 2023-06-13T12:45:31Z

Many thanks to @m7pr for helping me figure out the source of persistent check notes. It appears that prefixed object names (package::function) are ignored when checking for dependencies in code when they are within R6 classes, which results in

  Namespaces in Imports field not imported from:
    <package name>
    All declared Imports should be used.

A workaround was used (with no explanation) whereby single functions from some packages were imported, e.g. ggplot from ggplot2. This caused clutter in NAMESPACE. I used a solution found by @m7pr, which is to define a function that doesis never called but its body contains calls to (prefixed) functions from all problematic packages. I put it in zzz.R.

chlebowa · 2023-06-13T12:47:56Z

Test is failing. with error Objects equal but not identical for one of the test in filterstateRange

Are you sure you are up to date? I get a clean pass.

gogonzo · 2023-06-13T13:05:10Z

If the issue below will be solved in this PR please close as well

#114

chlebowa · 2023-06-13T14:50:34Z

The PR is now ready to merge so let's have a final round of review @lcd2yyz @m7pr @gogonzo @Melkiades (contributors have been omitted).

gogonzo · 2023-06-13T16:13:24Z

R/zzz.R

+.rectify_dependencies_check <- function() {
+  dplyr::filter
+  grDevices::rgb
+  lifecycle::badge
+  logger::log_trace
+  plotly::plot_ly
+  shinycssloaders::withSpinner
+  shinyWidgets::pickerOptions
+  teal.widgets::optionalSelectInput
+}


I don't understand this. Can you explain?
Do I understand properly that checks doesn't recognise usage of the functions in methods of R6 classes?

That is exactly what happens.

If package is listed in Imports, (1) check looks through NAMESPACE to see if there are any importFrom(package,<foo>) statements or an import(package) statement. If none are found, (2) check looks for package::* calls in the code. If none are found again, (3) check throws the NOTE, which is banned by our CI.
Now, when package::* statements are made within an R6 class, they are not registered. Since plotly and shinycssloaders were only used within RangeFilterState$server_inputs, I kept getting

Namespaces in Imports field not imported from: 'plotly' 'shinycssloaders' All declared Imports should be used.

gogonzo · 2023-06-13T16:26:34Z

R/FilterStateRange.R

+      if (values[1L] < private$choices[1L]) values[1L] <- private$choices[1L]
+      if (values[2L] > private$choices[2L]) values[2L] <- private$choices[2L]


way better to remove values in remove_out_of_bound_values instead of cast_and_validate!
Thanks

The values_adjusted part removed from cast_and_validate did have the effect of removing out of bound values, but it was mostly to match arbitrary numbers to slider ticks. Before contain_interval was introduced a few months ago, remove_out_of_bound_values was actually doing its job.

gogonzo

It's amazing!

chlebowa · 2023-06-13T16:40:19Z

Thanks @gogonzo. I will add the detailed explanation of the dependency check issue to comments in zzz.R.

m7pr · 2023-06-13T17:09:37Z

R/teal.slice.R

-#' @importFrom dplyr filter
-#' @importFrom ggplot2 ggplot
-#' @importFrom grDevices rgb
-#' @importFrom lifecycle badge


respect for deleting all those @impoftFrom functions lol

R/FilterStateRange.R

kartikeyakirar

Although my contribution was minimal, I must acknowledge that the overall work done by Aleks is truly amazing.

Aleksander Chlebowski added 3 commits May 17, 2023 15:31

add plotly graph

7dfaa98

add plotly dependency

b307feb

move histogram creation to server

5798d54

chlebowa added the core label May 17, 2023

Aleksander Chlebowski added 2 commits May 18, 2023 14:21

modify plot creation

10f6cc2

make plot static on disable

b50eb3a

Aleksander Chlebowski added 7 commits May 18, 2023 14:50

add mask do disabled plot

83f2208

adjust colors

898bad3

prevent plot resizing

b55aeec

update fixed server

68662d7

clean up

6e29ba5

simplify

cac8483

clean up

73fc589

Merge branch 'filter_panel_refactor@main' into 234_range_selector@fil…

9f78919

…ter_panel_refactor@main

Aleksander Chlebowski added 11 commits May 22, 2023 11:00

remove excess

4fe4f7a

reduce code duplication for histogram creation

35067c4

minor edits

f08864f

remove adjusting values to former slider ticks

d208903

round selection to 4 significant digits

db4317b

add function to determine bootstrap colors

7f193b7

specify plot colors better(?)

1b3ba5d

amend documentation

ae70252

update dependencies

3d486df

temporarily suspend unit tests

1f62853

modify step for numeric input

bfde124

chlebowa marked this pull request as ready for review May 24, 2023 08:41

Aleksander Chlebowski and others added 4 commits June 13, 2023 13:05

fix dependencies check

06fa62c

Merge 77b43bb into 64a1ea0

8cf081d

[skip actions] Restyle files

9832277

Aleksander Chlebowski and others added 6 commits June 13, 2023 13:48

Merge branch '234_range_selector@filter_panel_refactor@main' of githu…

b3b0214

…b.com:insightsengineering/teal.slice into 234_range_selector@filter_panel_refactor@main

linter

f0dbd3a

add logger to observer

3e7bdf2

clean up

c3e64e3

Merge c3e64e3 into 64a1ea0

2d3e99d

[skip actions] Restyle files

c176ea0

fix NAMESPACE

885e377

gogonzo reviewed Jun 13, 2023

View reviewed changes

gogonzo approved these changes Jun 13, 2023

View reviewed changes

add comment explaining .rectify_dependencies_check function

b2f327f

m7pr reviewed Jun 13, 2023

View reviewed changes

lcd2yyz reviewed Jun 14, 2023

View reviewed changes

R/FilterStateRange.R Outdated Show resolved Hide resolved

kartikeyakirar approved these changes Jun 14, 2023

View reviewed changes

modify step for manual selection

fedd0bf

chlebowa merged commit 4ec0499 into filter_panel_refactor@main Jun 14, 2023

chlebowa deleted the 234_range_selector@filter_panel_refactor@main branch June 14, 2023 15:59

This was referenced Jun 14, 2023

Research on better solution to manually enter numeric ranges #234

Closed

Research on improving numeric range input using plotly #233

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

234 range selection with `plotly` #289

234 range selection with `plotly` #289

chlebowa commented May 17, 2023 •

edited

Loading

chlebowa commented May 18, 2023 •

edited

Loading

asbates commented May 18, 2023 •

edited

Loading

chlebowa commented May 22, 2023

donyunardi commented May 25, 2023 •

edited

Loading

kartikeyakirar commented Jun 13, 2023

kartikeyakirar commented Jun 13, 2023 •

edited

Loading

chlebowa commented Jun 13, 2023

chlebowa commented Jun 13, 2023 •

edited

Loading

gogonzo commented Jun 13, 2023

chlebowa commented Jun 13, 2023

gogonzo Jun 13, 2023

chlebowa Jun 13, 2023

gogonzo Jun 13, 2023

chlebowa Jun 13, 2023

gogonzo left a comment

chlebowa commented Jun 13, 2023

m7pr Jun 13, 2023

kartikeyakirar left a comment

		if (values[1L] < private$choices[1L]) values[1L] <- private$choices[1L]
		if (values[2L] > private$choices[2L]) values[2L] <- private$choices[2L]

234 range selection with plotly #289

234 range selection with plotly #289

Conversation

chlebowa commented May 17, 2023 • edited Loading

chlebowa commented May 18, 2023 • edited Loading

asbates commented May 18, 2023 • edited Loading

chlebowa commented May 22, 2023

donyunardi commented May 25, 2023 • edited Loading

kartikeyakirar commented Jun 13, 2023

kartikeyakirar commented Jun 13, 2023 • edited Loading

chlebowa commented Jun 13, 2023

chlebowa commented Jun 13, 2023 • edited Loading

gogonzo commented Jun 13, 2023

chlebowa commented Jun 13, 2023

gogonzo Jun 13, 2023

Choose a reason for hiding this comment

chlebowa Jun 13, 2023

Choose a reason for hiding this comment

gogonzo Jun 13, 2023

Choose a reason for hiding this comment

chlebowa Jun 13, 2023

Choose a reason for hiding this comment

gogonzo left a comment

Choose a reason for hiding this comment

chlebowa commented Jun 13, 2023

m7pr Jun 13, 2023

Choose a reason for hiding this comment

kartikeyakirar left a comment

Choose a reason for hiding this comment

234 range selection with `plotly` #289

234 range selection with `plotly` #289

chlebowa commented May 17, 2023 •

edited

Loading

chlebowa commented May 18, 2023 •

edited

Loading

asbates commented May 18, 2023 •

edited

Loading

donyunardi commented May 25, 2023 •

edited

Loading

kartikeyakirar commented Jun 13, 2023 •

edited

Loading

chlebowa commented Jun 13, 2023 •

edited

Loading