insightsengineering / teal.modules.general

General Purpose Teal Modules
https://insightsengineering.github.io/teal.modules.general/
Other
9 stars 13 forks source link

[Bug]: Checkbox with label "Treat Variable as Factor" doesn't update #682

Closed kartikeyakirar closed 8 months ago

kartikeyakirar commented 8 months ago

What happened?

The dynamic behavior of the checkbox is not working as intended. The checkbox should be checked or unchecked based on the condition unique_entries < 6. However, it seems that the issue is caused by the condition is.null(isolate(input$numeric_as_factor)) always being evaluated as FALSE after the initial assignment.

Check mtcars$cyl (<6)andmtcars$hp (<30)

sample code

library(teal.widgets)

# Module specification used in apps below
tm_variable_browser_module <- tm_variable_browser(
  label = "Variable browser",
  ggplot2_args = ggplot2_args(
    labs = list(subtitle = "Plot generated by Variable Browser Module")
  )
)

# general data example
data <- teal_data()
data <- within(data, {
  iris <- iris
  mtcars <- mtcars
  women <- women
  faithful <- faithful
  CO2 <- CO2
})
datanames(data) <- c("iris", "mtcars", "women", "faithful", "CO2")

app <- init(
  data = data,
  modules = modules(tm_variable_browser_module)
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

sessionInfo()

No response

Relevant log output

No response

Code of Conduct

Contribution Guidelines

Security Policy

vedhav commented 8 months ago

Changing the default value over here to this should fix this:

value = unique_entries < .unique_records_default_as_factor
vedhav commented 8 months ago

However, based on the previous if statement. I believe that the actual feature was something different:

Feature: By default if there are < 6 distinct values it should be converted into a factor. However, if the user decides to keep the checkbox on/off for a given column, it should remain the same and not change.

☝🏽 That behaviour is broken if we just make the default selection as per our logic.

vedhav commented 8 months ago

The crux of the problem is that we use the same checkbox input for every column selection.

kartikeyakirar commented 8 months ago

Feature: By default if there are < 6 distinct values it should be converted into a factor. However, if the user decides to keep the checkbox on/off for a given column, it should remain the same and not change.

So here are two options

Simple Fix: Update the checkbox every time a user selects a row from the DT table. This will check the condition unique_entries < .unique_records_default_as_factor and update the checkbox accordingly.

Complex Approach: Implement a mechanism to store information about user selections on the checkbox for each column. If a user makes any changes after the default selection, the mechanism should retain that selection.