Improve picks ui when single choice by osenan · Pull Request #49 · insightsengineering/teal.picks

osenan · 2026-04-15T06:07:17Z

It fixed #33

The solution has changed the badge_dropdown internal function that creates the badge that contains the picks dropdown. For cases where there is fixed dataset and variable, it has been changed to a different style and removed the icon. Thus, the user is not confused whether this variable can be changed. In addition, if the app developer wants I allowed for single choice with ui display.

This app shows both scenarios:

devtools::load_all()

# --- Custom dummy teal module using a single-choice picks object ---

ui_dummy <- function(id, var_picks, var_picks2, ...) {
  ns <- shiny::NS(id)
  shiny::tagList(
    teal.widgets::standard_layout(
      output = shiny::verbatimTextOutput(ns("out")),
      encoding = shiny::tags$div(
        shiny::tags$label("Encodings", class = "text-primary"),
        shiny::tags$div(
          shiny::tags$strong("Variable 1"),
          teal.picks::picks_ui(id = ns("var"), picks = var_picks)
        ),
        shiny::tags$div(
          shiny::tags$strong("Variable 2"),
          teal.picks::picks_ui(id = ns("var2"), picks = var_picks2)
        )
      )
    )
  )
}

srv_dummy <- function(id, data, dataname, var_picks, var_picks2) {
  checkmate::assert_class(data, "reactive")
  shiny::moduleServer(id, function(input, output, session) {
    selectors <- teal.picks::picks_srv(
      picks = list(var = var_picks, var2 = var_picks2),
      data = data
    )

    merged <- teal.picks::merge_srv(
      id = "merge",
      data = data,
      selectors = selectors,
      output_name = "ANL"
    )

    output$out <- shiny::renderPrint({
      selected_var <- merged$variables()$var
      selected_var2 <- merged$variables()$var2
      anl <- merged$data()[["ANL"]]
      cat("Selected variable 1:", selected_var, "\n")
      print(head(anl[[selected_var]]))
      cat("\nSelected variable 2:", selected_var2, "\n")
      print(head(anl[[selected_var2]]))
    })
  })
}

tm_dummy <- function(label, dataname,
                     var_picks = teal.picks::variables(
                       choices = "ADY",
                       selected = "ADY"
                     ),
                     var_picks2 = teal.picks::variables(
                       choices = "ADY",
                       selected = "ADY"
                     )) {
  var_picks <- teal.picks::picks(teal.picks::datasets(dataname), var_picks)
  var_picks2 <- teal.picks::picks(teal.picks::datasets(dataname), var_picks2)
  teal::module(
    label = label,
    datanames = dataname,
    ui = ui_dummy,
    ui_args = list(var_picks = var_picks, var_picks2 = var_picks2),
    server = srv_dummy,
    server_args = list(dataname = dataname, var_picks = var_picks, var_picks2 = var_picks2)
  )
}

# --- App ---

data <- teal_data() %>%
  within({
    ADTR <- teal.data::rADTR
  })

join_keys(data) <- default_cdisc_join_keys[names(data)]

app <- init(
  data = data,
  modules = modules(
    tm_dummy(
      label = "Dummy module (single choice picks)",
      dataname = "ADTR",
      var_picks = teal.picks::variables(
        choices = "ADY",
        selected = "ADY",
        fixed = FALSE
      ),
      var_picks2 = teal.picks::variables(
        choices = "ADY",
        selected = "ADY"
      )
    )
  )
)

shinyApp(app$ui, app$server)

To verify the behavior I have added more tests.

…cks arguments

github-actions · 2026-04-15T06:07:33Z

🎉 Thank you for your contribution! Before this PR can be accepted, we require that you read and agree to our Contributor License Agreement.
You can digitally sign the CLA by posting a comment on this Pull Request in the format shown below. This agreement will apply to this PR as well as all future contributions on this repository.

I have read the CLA Document and I hereby sign the CLA

osenan seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.}

osenan · 2026-04-15T06:09:18Z

I have read the CLA Document and I hereby sign the CLA

github-actions · 2026-04-15T06:17:23Z

Code Coverage Summary

Filename                    Stmts    Miss  Cover    Missing
------------------------  -------  ------  -------  -----------------------------------------------------------------------------
R/0-as_picks.R                139      79  43.17%   61, 91-92, 126-209, 227-229
R/0-badge_dropdown.R           41       0  100.00%
R/0-call_utils.R              127      15  88.19%   23-28, 65, 132-138, 259, 276
R/0-module_merge.R            244       2  99.18%   316, 595
R/0-module_picks.R            329      34  89.67%   47-53, 76-77, 79, 115-116, 259-267, 308-310, 312-316, 437, 486, 524, 536, 544
R/0-picks.R                   168       1  99.40%   362
R/0-print.R                    38       2  94.74%   50, 58
R/0-resolver.R                131      15  88.55%   95-103, 269-274
R/0-tidyselect-helpers.R       29      17  41.38%   24-41
R/0-tm_merge.R                 54      54  0.00%    42-102
R/assertion.R                   5       0  100.00%
R/zzz.R                         3       3  0.00%    3-5
TOTAL                        1308     222  83.03%

Diff against main

Filename                Stmts    Miss  Cover
--------------------  -------  ------  --------
R/0-as_picks.R             -1       0  -0.41%
R/0-badge_dropdown.R       +6     -35  +100.00%
R/0-call_utils.R          -20     -18  +10.64%
R/0-module_merge.R        -22     -12  +4.44%
R/0-module_picks.R        +14      -5  +2.05%
R/0-resolver.R            -10      -4  +2.02%
R/zzz.R                    -2      -2  +100.00%
TOTAL                     -35     -76  +6.42%

Results for commit: c541422

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

github-actions · 2026-04-15T06:17:28Z

Unit Tests Summary

1 files 8 suites 13s ⏱️
200 tests 187 ✅ 13 💤 0 ❌
302 runs 288 ✅ 14 💤 0 ❌

Results for commit c541422.

♻️ This comment has been updated with latest results.

github-actions · 2026-04-15T06:17:31Z

Unit Test Performance Difference

Test Suite	$Status$	Time on `main`	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
0-interaction	💀	$0.06$	$-0.06$	$-3$	$0$	$0$	$0$

Additional test case details

Test Suite	$Status$	Time on `main`	$±Time$	Test Case
0-as_picks	💀	$0.01$	$-0.01$	as.picks_does_not_throw_warning_with_quiet_TRUE_list_of_filter_specs
0-as_picks	💀	$0.01$	$-0.01$	as.picks_does_not_throw_warning_with_quiet_TRUE_with_data_extract_spec
0-as_picks	💀	$0.01$	$-0.01$	as.picks_does_not_throw_warning_with_quiet_TRUE_with_filterspect
0-as_picks	💀	$0.01$	$-0.01$	as.picks_does_not_throw_warning_with_quiet_TRUE_with_non_supporter_base_types
0-interaction	💀	$0.05$	$-0.05$	interaction_vars_is_compatible_with_eval_select
0-interaction	💀	$0.01$	$-0.01$	interaction_vars_stores_interactions_in_environment
0-module_picks	👶		$+0.02$	picks_ui_creates_correct_badge_dropdown_with_fixed_FALSE_when_picks_has_one_dataset_and_single_choice_var_with_explicit_fixed_FALSE
0-module_picks	👶		$+0.03$	picks_ui_creates_correct_badge_dropdown_with_fixed_FALSE_when_picks_has_two_datasets_and_one_single_choice_var
0-module_picks	👶		$+0.02$	picks_ui_creates_correct_badge_dropdown_with_fixed_TRUE_when_picks_has_one_dataset_and_single_choice_var

Results for commit 3c6d5b1

♻️ This comment has been updated with latest results.

llrs-roche

The app currently uses the secondary color to generate the badge:

I am not sure we need to reuse badge_dropdown() given that it doesn't generate a dropdown. Instead we might want a new function to generate the UI. There was a comment to show a lock on the UI to convey the user they cannot change that. I think it would be helpful as secondary colors might be used for less important buttons but still functional ones.

I haven't run the tests (yet)

llrs-roche · 2026-04-15T08:26:58Z

+#' @param fixed (`logical(1)`) Whether to return a badge with dropdown (default) or simple fixed badge if set to TRUE
 #' @keywords internal
-badge_dropdown <- function(id, label, content) {
+badge_dropdown <- function(id, label, content, badge_context = "primary", fixed = FALSE) {


I like the description of the new badge_context argument and I see it inherits the color from teal.
But do we need the fixed = FALSE argument? If it is fixed there is no dropdown so we might want to use a different function that generates the UI.

llrs-roche · 2026-04-15T08:28:44Z

-badge_dropdown <- function(id, label, content) {
+badge_dropdown <- function(id, label, content, badge_context = "primary", fixed = FALSE) {
+  checkmate::assert_character(badge_context)
+  checkmate::assert_logical(fixed)


Not only logical but also to be of length one and not accept NA values

Suggested change

checkmate::assert_logical(fixed)

checkmate::assert_flag(fixed)

llrs-roche · 2026-04-15T08:29:40Z

-        class = "badge bg-primary rounded-pill badge-dropdown",
-        style = "cursor: pointer;",
+        class = sprintf("badge bg-%s rounded-pill badge-dropdown", badge_context),
+        style = ifelse(fixed, "", "cursor: pointer"),


Not important but I think there was some recommendations to use if instead of ifelse for these cases:

Suggested change

style = ifelse(fixed, "", "cursor: pointer"),

style = if (fixed) "" else "cursor: pointer",

llrs-roche · 2026-04-15T08:33:28Z

-      if (isTRUE(args$fixed) || length(choices()) <= 1) {
+      if (!length(choices()) || isTRUE(args$fixed)) {
        NULL
+      } else if (length(choices()) == 1 && isFALSE(args$fixed)) {


Do we need this new conditional? Wouldn't it be catch up by the latest else clause?

llrs-roche · 2026-04-15T08:41:25Z

  htmltools::tags$div(
    if (missing(container)) {
-      badge_dropdown(id = ns("inputs"), label = badge_label, htmltools::tagList(content))
+      if (isTRUE(attr(picks$variables, "fixed")) && isTRUE(attr(picks$datasets, "fixed"))) {


I would iterate through the whole elements of picks and check with is_pick_fixed() from #48
Maybe datasets and variables are fixed but values are not.

Se also my comment about fixed = TRUE and that we might want to have a different function to generate the UI below this: badge_locked() ? . If we don't I would only have one call and use badge_dropdown(..., fixed = all(locked)) to avoid an extra if else.

osenan · 2026-04-15T09:06:39Z

Thank you for the review @llrs-roche!
I think that including an icon with a lock will inform better the user. Moreover, I am open to change it to a ui element that is not a badge. It is worth giving a try as it will be more informative for the user.
I will wait for this PR to be merged in order to reuse the helper functions in here.
I also realized that this PR is not taking into consideration value picks, which should also look different if they are fixed.
After those refactors, I will address your specific comments if they remain relevant to the new implementation.

osenan and others added 4 commits April 15, 2026 07:50

feat: allow for different badge depending on number of choices and pi…

762d76a

…cks arguments

chore: fix style

dd473fb

docs: update documentation

55c8481

update verification

5f5743f

osenan added the core label Apr 15, 2026

update verification

8e466db

llrs-roche self-assigned this Apr 15, 2026

chore: update WORDLIST

c541422

llrs-roche requested changes Apr 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve picks ui when single choice#49

Improve picks ui when single choice#49
osenan wants to merge 6 commits intomainfrom
fix-single-choice-ui@main

osenan commented Apr 15, 2026

Uh oh!

github-actions bot commented Apr 15, 2026 •

edited

Loading

Uh oh!

osenan commented Apr 15, 2026

Uh oh!

github-actions bot commented Apr 15, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Apr 15, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Apr 15, 2026 •

edited

Loading

Uh oh!

llrs-roche left a comment

Uh oh!

llrs-roche Apr 15, 2026

Uh oh!

llrs-roche Apr 15, 2026

Uh oh!

llrs-roche Apr 15, 2026

Uh oh!

llrs-roche Apr 15, 2026

Uh oh!

llrs-roche Apr 15, 2026 •

edited

Loading

Uh oh!

osenan commented Apr 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	checkmate::assert_logical(fixed)
	checkmate::assert_flag(fixed)

	style = ifelse(fixed, "", "cursor: pointer"),
	style = if (fixed) "" else "cursor: pointer",

Conversation

osenan commented Apr 15, 2026

Uh oh!

github-actions bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

osenan commented Apr 15, 2026

Uh oh!

github-actions bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Summary

Diff against main

Uh oh!

github-actions bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Tests Summary

Uh oh!

github-actions bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Performance Difference

Uh oh!

llrs-roche left a comment

Choose a reason for hiding this comment

Uh oh!

llrs-roche Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

llrs-roche Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

llrs-roche Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

llrs-roche Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

llrs-roche Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

osenan commented Apr 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Apr 15, 2026 •

edited

Loading

github-actions bot commented Apr 15, 2026 •

edited

Loading

github-actions bot commented Apr 15, 2026 •

edited

Loading

github-actions bot commented Apr 15, 2026 •

edited

Loading

llrs-roche Apr 15, 2026 •

edited

Loading