Revert to visibility css property and allows fixed picks to show error

[averissimo]

Pull Request

Note for reviewer:

• It adds a manual generated text input with disabled property for it to trick validate_input to think it exists
• it should not affect any of the shiny reactivity, but please take this into account.

Changes description

• Using display: none for the popup hides the input from shiny which can have adverse effects
  • Some errors were delayed because of this
  • In theory we can now use the input to trigger change in shinytest2
  • Visual glitch no longer exists as width is set to 0 (zero)
• Fixed picks were not showing error as they didn't generate inputs for the CSS red border rule to activate

Example app

Note. please use picks branch 279-interactive_variables@main from teal.modules.clinical

# Patient Profile Vitals - original (choices_selected) implementation

pkgload::load_all("../teal.logger", export_all = FALSE)
pkgload::load_all("../teal.code", export_all = FALSE)
pkgload::load_all("../teal.data", export_all = FALSE)
pkgload::load_all("../teal.widgets", export_all = FALSE)
pkgload::load_all("../teal.transform", export_all = FALSE)
pkgload::load_all("../teal.reporter", export_all = FALSE)
pkgload::load_all("../teal.slice", export_all = FALSE)
pkgload::load_all("../teal.picks", export_all = FALSE)
pkgload::load_all("../teal", export_all = FALSE)
pkgload::load_all("../teal.modules.clinical", export_all = FALSE)

library(nestcolor)

data <- teal_data()
data <- within(data, {
  library(dplyr)
  ADSL <- teal.modules.clinical::tmc_ex_adsl
  ADVS <- teal.modules.clinical::tmc_ex_advs
})
join_keys(data) <- default_cdisc_join_keys[names(data)]

ADSL <- data[["ADSL"]]
ADVS <- data[["ADVS"]]

app <- init(
  data = data,
  modules = modules(
    tm_g_pp_vitals(
      label = "Vitals",
      dataname = "ADVS",
      parentname = "ADSL",
      patient_col = "USUBJID",
      plot_height = c(600L, 200L, 2000L),
      paramcd = teal.picks::variables("PARAMCD", multiple = FALSE, fixed = FALSE),
      xaxis = teal.picks::variables("ADY", multiple = FALSE, fixed = FALSE),
      aval_var = picks(
        teal.picks::variables(c("AAVAL"), selected = "AAVAL", multiple = TRUE, fixed = TRUE),
        check_dataset = FALSE
      )
    ),
    tm_g_pp_vitals(
      label = "Vitals (legacy)",
      dataname = "ADVS",
      parentname = "ADSL",
      patient_col = "USUBJID",
      plot_height = c(600L, 200L, 2000L),
      paramcd = choices_selected(
        choices = variable_choices(ADVS, "PARAMCD"),
        selected = "PARAMCD"
      ),
      xaxis = choices_selected(
        choices = variable_choices(ADVS, "ADY"),
        selected = "ADY"
      ),
      aval_var = choices_selected(
        choices = variable_choices(ADVS, "AVAL"),
        selected = "AVAL"
      )
    )
  )
) |> shiny::runApp()

[averissimo]

Tagging @osenan as you recently worked on fixed picks

[github-actions]

Unit Tests Summary

  1 files   10 suites   16s ⏱️
256 tests 242 ✅ 14 💤 0 ❌
384 runs  369 ✅ 15 💤 0 ❌

Results for commit 9f0dfbb.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Performance Difference

Test Suite	$Status$	Time on main	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
badge_dropdown	💀	$0.15$	$-0.15$	$-1$	$-1$	$0$	$0$

Additional test case details

Test Suite	$Status$	Time on main	$±Time$	Test Case
assertion	👶		$+0.02$	unnamed
badge_dropdown	💀	$0.15$	$-0.15$	shinytest2_badge_dropdown_is_visible_when_clicking_on_it_multiple_times

Results for commit 0f24ca2

♻️ This comment has been updated with latest results.

[github-actions]

Code Coverage Summary

Filename                  Stmts    Miss  Cover    Missing
----------------------  -------  ------  -------  -------------------------------------------------------------
R/as_picks.R                140      21  85.00%   63, 167-171, 182-186, 201-207, 229-231
R/assertion.R                 5       0  100.00%
R/call_utils.R              147      22  85.03%   23-28, 65, 132-138, 259, 279-280, 283-287, 292
R/helpers.R                   9       0  100.00%
R/interaction.R              42       1  97.62%   95
R/module_merge.R            257       2  99.22%   328, 607
R/module_picks.R            323      23  92.88%   47-53, 72, 109-110, 299-301, 303-307, 441, 490, 528, 540, 548
R/picks.R                   183       1  99.45%   335
R/print.R                    36       2  94.44%   50, 58
R/resolver.R                141      15  89.36%   110-118, 284-289
R/tidyselect-helpers.R       29       0  100.00%
R/tm_merge.R                 54      54  0.00%    42-102
R/ui_containers.R            55       0  100.00%
R/zzz.R                       5       5  0.00%    3-11
TOTAL                      1426     146  89.76%

Diff against main

Filename             Stmts    Miss  Cover
-----------------  -------  ------  --------
R/module_picks.R        +8       0  +0.18%
R/ui_containers.R       +8       0  +100.00%
TOTAL                  +16       0  +0.12%

Results for commit: 9f0dfbb

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

[osenan]

Hi, I've been playing with the change you created in the branch of tmc and with modifications of the example you provided. It does not fail. I also searched for errors using tm_merge module and it worked. Overall, it is a good catch.
The reactive change is less than it seems. It only affects for multiple = TRUE where deselection of all choices triggers reactivity, unlike before.