Don't update distribution params labels when variable is deselected in tm_g_distribution

[m7pr]

Fixes #510

[Copilot]

Pull Request Overview

Fixes an issue where distribution parameters were being reset unintentionally when switching variables in the tm_g_distribution module.

• Tracks manual parameter edits and previous selections with reactive values
• Updates the main observer to only recalculate parameters when truly needed
• Adds separate observers for manual edits and reset logic to manage the manual‐set flag

Comments suppressed due to low confidence (3)

R/tm_g_distribution.R:572

• [nitpick] The variable name prev_t_dist is somewhat ambiguous. Consider renaming it to prev_dist_type (and similarly prev_variable to prev_selected_variable) to make their purposes more explicit.

    params_manually_set <- reactiveVal(FALSE)

R/tm_g_distribution.R:572

• Adding a brief comment above the params_manually_set, prev_t_dist, and prev_variable declarations explaining their roles would improve readability and maintainability.

    params_manually_set <- reactiveVal(FALSE)

R/tm_g_distribution.R:586

• The new conditional logic for recalculating distribution parameters based on params_manually_set and previous values should be covered by unit or integration tests to verify that manual edits are preserved and reset behavior works correctly.

    observeEvent(

[github-actions]

Unit Tests Summary

  1 files  22 suites   1s ⏱️
144 tests 29 ✅ 115 💤 0 ❌
182 runs  67 ✅ 115 💤 0 ❌

Results for commit 49eda1a.

♻️ This comment has been updated with latest results.

[github-actions]

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  ---------------------------------------
R/tm_a_pca.R                    889     889  0.00%    139-1161
R/tm_a_regression.R             773     773  0.00%    178-1056
R/tm_data_table.R               201     201  0.00%    100-349
R/tm_file_viewer.R              172     172  0.00%    47-254
R/tm_front_page.R               144     133  7.64%    77-247
R/tm_g_association.R            344     344  0.00%    159-578
R/tm_g_bivariate.R              698     434  37.82%   331-826, 867, 978, 995, 1013, 1024-1046
R/tm_g_distribution.R          1123    1123  0.00%    156-1426
R/tm_g_response.R               369     369  0.00%    177-625
R/tm_g_scatterplot.R            734     734  0.00%    260-1098
R/tm_g_scatterplotmatrix.R      297     278  6.40%    198-532, 593, 607
R/tm_missing_data.R            1119    1119  0.00%    124-1420
R/tm_outliers.R                1045    1045  0.00%    163-1359
R/tm_t_crosstable.R             285     285  0.00%    175-509
R/tm_variable_browser.R         803     798  0.62%    89-1044, 1082-1265
R/utils.R                       151     135  10.60%   87-272, 302-338, 350-359, 364, 378-397
R/zzz.R                           2       2  0.00%    2-3
TOTAL                          9149    8834  3.44%

Diff against main

Filename                 Stmts    Miss  Cover
---------------------  -------  ------  --------
R/tm_g_distribution.R       +6      +6  +100.00%
TOTAL                       +6      +6  -0.00%

Results for commit: 49eda1a

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

[donyunardi]

Originally, issue arises when user deselect the "Variable" selection. This triggers the label being resetted to param1 and param2, even though distribution is selected, in this case "normal".

When I use main version, I can't deselect the "Variable" selection as it always select a variable.

Then only way to test this PR by setting the select_spect(selected = NULL) for the dist_var argument. However, I still see the same behavior where the labels are still showing 'param1' and 'param2', even though the distribution was selected.

Example code with `selected = NULL`

data <- teal_data()
data <- within(data, {
  ADSL <- teal.data::rADSL
})
join_keys(data) <- default_cdisc_join_keys[names(data)]
#'
vars1 <- choices_selected(
  variable_choices(data[["ADSL"]], c("ARM", "COUNTRY", "SEX")),
  selected = NULL
)
#'
app <- init(
  data = data,
  modules = modules(
    tm_g_distribution(
      dist_var = data_extract_spec(
        dataname = "ADSL",
        select = select_spec(
          choices = variable_choices(data[["ADSL"]], c("AGE", "BMRKR1")),
          selected = NULL,
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      strata_var = data_extract_spec(
        dataname = "ADSL",
        filter = filter_spec(
          vars = vars1,
          multiple = TRUE
        )
      ),
      group_var = data_extract_spec(
        dataname = "ADSL",
        filter = filter_spec(
          vars = vars1,
          multiple = TRUE
        )
      )
    )
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

[m7pr]

Ah sorry @donyunardi I though it's params values, but it actually params labels : )
Updated the PR.
Now, when there is no variable selected, and you select a distribution, the param names are set to their respective names