insightsengineering / teal

Exploratory Web Apps for Analyzing Clinical Trial Data
https://insightsengineering.github.io/teal/
Other
179 stars 37 forks source link

[Bug]: `TealAppDriver` setting numeric filter state #1151

Closed averissimo closed 6 months ago

averissimo commented 7 months ago

TealAppDriver$set_active_filter_selection only works for categorical variables. It should also work for numeric ranges.

Example below of test that should work

testthat::test_that("e2e: Set numeric filter state", {
  app <- TealAppDriver$new(
    data = simple_teal_data(),
    modules = example_module(label = "Example Module"),
    timeout = default_idle_timeout
  )
  withr::defer(app$stop())

  app$wait_for_idle()

  app$add_filter_var("iris", "Sepal.Length")
  app$view()
  app$set_active_filter_selection("iris", "Sepal.Length", c(4, 5), is_numeric = TRUE)
  app$wait_for_idle()
  testthat::expect_equal(
    app$get_active_filter_selection("iris", "Sepal.Length", is_numeric = TRUE), 
    c(4, 5)
  )
})
chlebowa commented 7 months ago
 app$set_active_filter_selection("iris", "Sepal.Length", c(4, 5), is_numeric = TRUE)

Are you sure you don't want to pass a teal_slice?

averissimo commented 7 months ago

That's a nice idea passing a teal_slice. It's familiar and avoids that is_numeric weirdness.

app$set_active_filter_selection(
  teal_slice("iris", "Sepal.Length", selected = c(4, 5))
)

The operations that this call triggers would still be UI/input-based, but this would allow us to keep consistency.

Care to give insights on reasons to keep current formals @vedhav ?

ps. this would be a separate issue though as potential follow-up to this one.

averissimo commented 7 months ago

The shinyWidgets::numericRangeInput widget requires a custom handler to change the values.

I was able to do it via javascript (#1152), but if someone knows how to hint the handler via Shiny API, please chip in.

The caveat of doing it this way is that the input boxes still hold the previous values.

image

image

Side quest: The range input does not work out of the box with shinytest2, but it has no issues with dynamic rendering as we do it :grin: https://github.com/rstudio/shinytest2/issues/376

chlebowa commented 7 months ago

Yeah, $setInputValue and $setInput are not entirely reliable. But doesn't shinyWidgets::updateNumericRangeInput, which uses $sendInputMessage, work?

averissimo commented 7 months ago

For that we need access to the session object, which I don't think we can easily access :-\

chlebowa commented 7 months ago

Oh, I see. I misunderstood the problem. Sorry.

averissimo commented 7 months ago

I wasn't totally explicit in the description.

The issue here is that AppDriver API does nothing when setting for the input_id.

So the downstream javascript call needs to be Shiny.setInput("an_input:sw.numericRange", [min, max])

Which is what the PR does skipping all middle layers of AppDriver.