Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UAT All X-Axis & Y-Axis Range Zooms #133

Closed
npaszty opened this issue Mar 30, 2022 · 5 comments · Fixed by #319
Closed

UAT All X-Axis & Y-Axis Range Zooms #133

npaszty opened this issue Mar 30, 2022 · 5 comments · Fixed by #319
Assignees
Labels
bug Something isn't working core low-priority

Comments

@npaszty
Copy link
Contributor

npaszty commented Mar 30, 2022

using CRP as assay
range zoom defaults to slider
Boxplot Tick marks don't look right
image

changing range zoom by toggle to inputs and setting max to max+10 results in expected 0 to max+10 range axis.
but when toggle back to slider the slider value range returns to 0 to max but the plot still displays 0 to max+10. need to slide to max-1 to trigger reactivity and now the axis range is 0 to max-1.

@npaszty npaszty added bug Something isn't working low-priority labels Mar 30, 2022
@npaszty npaszty changed the title UAT BoxPlot Y-Axis Range Zoom UAT All X-Axis & Y-Axis Range Zooms Mar 30, 2022
@gogonzo
Copy link
Contributor

gogonzo commented Mar 31, 2022

Moving to backlog to refine item

@npaszty
Copy link
Contributor Author

npaszty commented Jul 25, 2024

@gogonzo

there is still something odd going on here. using the study app we have been testing new version with it looks like the plot is okay but the slider seems to get displayed with incorrect tick marks right out of the gate.

image

@donyunardi
Copy link
Contributor

donyunardi commented Aug 8, 2024

Might have something to do with how steps are generated/updated in teal.goshawk:
https://github.com/insightsengineering/teal.goshawk/blob/main/R/utils-keep_range_slider_updated.r

Acceptance Criteria:

  • Slider tick should show appropriate steps and not repetitive
  • Propose new solution if slider is not efficient

@m7pr
Copy link
Contributor

m7pr commented Oct 7, 2024

Confirming that I was able to reproduce with ? example

The code
# Example using ADaM structure analysis dataset.
data <- teal_data()
data <- within(data, {
  library(dplyr)
  library(nestcolor)
  library(stringr)
  
  # use non-exported function from goshawk
  h_identify_loq_values <- getFromNamespace("h_identify_loq_values", "goshawk")
  
  # original ARM value = dose value
  arm_mapping <- list(
    "A: Drug X" = "150mg QD",
    "B: Placebo" = "Placebo",
    "C: Combination" = "Combination"
  )
  set.seed(1)
  ADSL <- rADSL
  ADLB <- rADLB
  var_labels <- lapply(ADLB, function(x) attributes(x)$label)
  ADLB <- ADLB %>%
    mutate(
      AVISITCD = case_when(
        AVISIT == "SCREENING" ~ "SCR",
        AVISIT == "BASELINE" ~ "BL",
        grepl("WEEK", AVISIT) ~ paste("W", str_extract(AVISIT, "(?<=(WEEK ))[0-9]+")),
        TRUE ~ as.character(NA)
      ),
      AVISITCDN = case_when(
        AVISITCD == "SCR" ~ -2,
        AVISITCD == "BL" ~ 0,
        grepl("W", AVISITCD) ~ as.numeric(gsub("[^0-9]*", "", AVISITCD)),
        TRUE ~ as.numeric(NA)
      ),
      AVISITCD = factor(AVISITCD) %>% reorder(AVISITCDN),
      TRTORD = case_when(
        ARMCD == "ARM C" ~ 1,
        ARMCD == "ARM B" ~ 2,
        ARMCD == "ARM A" ~ 3
      ),
      ARM = as.character(arm_mapping[match(ARM, names(arm_mapping))]),
      ARM = factor(ARM) %>% reorder(TRTORD),
      ACTARM = as.character(arm_mapping[match(ACTARM, names(arm_mapping))]),
      ACTARM = factor(ACTARM) %>% reorder(TRTORD),
      ANRLO = 50,
      ANRHI = 75
    ) %>%
    rowwise() %>%
    group_by(PARAMCD) %>%
    mutate(LBSTRESC = ifelse(
      USUBJID %in% sample(USUBJID, 1, replace = TRUE),
      paste("<", round(runif(1, min = 25, max = 30))), LBSTRESC
    )) %>%
    mutate(LBSTRESC = ifelse(
      USUBJID %in% sample(USUBJID, 1, replace = TRUE),
      paste(">", round(runif(1, min = 70, max = 75))), LBSTRESC
    )) %>%
    ungroup()
  
  attr(ADLB[["ARM"]], "label") <- var_labels[["ARM"]]
  attr(ADLB[["ACTARM"]], "label") <- var_labels[["ACTARM"]]
  attr(ADLB[["ANRLO"]], "label") <- "Analysis Normal Range Lower Limit"
  attr(ADLB[["ANRHI"]], "label") <- "Analysis Normal Range Upper Limit"
  
  # add LLOQ and ULOQ variables
  ALB_LOQS <- h_identify_loq_values(ADLB, "LOQFL")
  ADLB <- left_join(ADLB, ALB_LOQS, by = "PARAM")
})

datanames <- c("ADSL", "ADLB")
datanames(data) <- datanames


join_keys(data) <- default_cdisc_join_keys[datanames]

app <- init(
  data = data,
  modules = modules(
    tm_g_gh_boxplot(
      label = "Box Plot",
      dataname = "ADLB",
      param_var = "PARAMCD",
      param = choices_selected(c("ALT", "CRP", "IGA"), "ALT"),
      yaxis_var = choices_selected(c("AVAL", "BASE", "CHG"), "AVAL"),
      xaxis_var = choices_selected(c("ACTARM", "ARM", "AVISITCD", "STUDYID"), "ARM"),
      facet_var = choices_selected(c("ACTARM", "ARM", "AVISITCD", "SEX"), "AVISITCD"),
      trt_group = choices_selected(c("ARM", "ACTARM"), "ARM"),
      loq_legend = TRUE,
      rotate_xlab = FALSE,
      hline_arb = c(60, 55),
      hline_arb_color = c("grey", "red"),
      hline_arb_label = c("default_hori_A", "default_hori_B"),
      hline_vars = c("ANRHI", "ANRLO", "ULOQN", "LLOQN"),
      hline_vars_colors = c("pink", "brown", "purple", "black"),
    )
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

State 1 - app initialized

image

State 2 - slider toggled to numeric input

image

Stage 3 - slider toggled to numeric input and extended and plot changed

image

Stage 4 - back to slider, slider shows 0-55, plot uses 0-65 - no reactivity triggered

image

@m7pr
Copy link
Contributor

m7pr commented Oct 7, 2024

tm_g_gh_boxplot uses toggle_slider_ui

 toggle_slider_ui(
    ns("yrange_scale"),
    label = "Y-Axis Range Zoom",
    min = -1000000,
    max = 1000000,
    value = c(-1000000, 1000000)
  )

for Y-Axis Range Zoom so the work probably needs to be done around toggle_slider_ui and toggle_slider_server

m7pr added a commit that referenced this issue Oct 9, 2024
This is solution for #133 

`toggle_slider` is a component that is a sliderInput that can be toggled
to become a numericInput.

<img width="172" alt="image"
src="https://github.com/user-attachments/assets/df97ef6a-e7a5-4f96-bb43-8159f5f9dd83">

<img width="172" alt="image"
src="https://github.com/user-attachments/assets/64aa88ec-a041-4c3a-93ee-d5fd12a1def2">


The PR fixes the issue with ranges on sliderInput that should be
adjusted based on numericInput.

So if slider had ranges min_sliderInput and max_sliderInput and values
for numercInput are changed, so that min(numercInput) is lower than
min_sliderInput or max(numercInput) is greater than max_sliderInput,
then `min_sliderInput` or `max_sliderInput` should be extended for new
ranges.

# Example 1

min_sliderInput, max_sliderInput - on start 0, 55 

<img width="166" alt="image"
src="https://github.com/user-attachments/assets/e3bfcb1a-a2f4-435e-8569-54d45c549486">

hence numericInput on start 0, 55

<img width="166" alt="image"
src="https://github.com/user-attachments/assets/e4988200-5e89-452e-a287-12fbabc6f4bb">

numericInput changes to -5, 65

<img width="173" alt="image"
src="https://github.com/user-attachments/assets/1635521c-ace1-468a-8a8c-a3d758093fcc">

sliderInput changes to -5, 65

<img width="170" alt="image"
src="https://github.com/user-attachments/assets/62aee129-083c-4286-afef-86cb5ab0e0a8">

# Example 2

Continuation of the previous one, if you change numericInput from -5, 65

<img width="170" alt="image"
src="https://github.com/user-attachments/assets/416ff11a-b5ed-4e0c-9ff6-3695019b8c1f">

to 4, 45

<img width="172" alt="image"
src="https://github.com/user-attachments/assets/0d25721f-9da0-4d77-ae21-d7bc579d2963">

sliderInput is reset (the min and max) to original 0, 55 but values are
set to 4, 45

<img width="172" alt="image"
src="https://github.com/user-attachments/assets/e39ac970-5bb2-4297-89f1-062fcab74d5e">

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
@vedhav vedhav closed this as completed in 7907046 Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core low-priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants