Skip to content
Open
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ inst/shiny/log/*
.DS_Store
# {shinytest2}: Ignore new debug snapshots for `$expect_values()`
*_.new.png
desktop.ini
desktop.initests/testthat/Rplots.pdf
10 changes: 10 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# aNCA (development version)

## Code Quality

* Removed debug `print()` statements from TLG module and replaced with proper logging (#1298)
* Removed unused `require(magrittr)` and `require(stats)` calls from `app.R` (#1298)
* Replaced deprecated HTML `align` attributes with inline CSS in TLG module (#1298)
* Standardized button CSS classes: removed redundant `btn` prefix where duplicate with `btn-*` (#1298)
* Added `aria-label` attributes to 15 icon-only buttons for accessibility (#1298)
* Removed redundant `disabled = FALSE` default from units table actionButton (#1298)
* Extracted inline hex colors to shared `colors.R` constants and SCSS variables (#1298)

## Features

### Settings & Configuration
Expand Down
2 changes: 2 additions & 0 deletions inst/WORDLIST
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ RefParameter
Req
Roadmap
SBS
SCSS
SDTM
SDTMIG
SMS
Expand Down Expand Up @@ -144,6 +145,7 @@ Walkthrough
XPT
YAML
aNCA's
actionButton
adnca
analysing
analyte
Expand Down
2 changes: 0 additions & 2 deletions inst/shiny/app.R
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ require(dplyr)
require(htmlwidgets)
require(logger)
require(formatters)
require(magrittr)
require(plotly)
require(purrr)
require(reactable)
Expand All @@ -15,7 +14,6 @@ require(shinycssloaders)
require(shinyjs)
require(shinyjqui)
require(shinyWidgets)
require(stats)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

require(stats) removal — likely safe but worth verifying

Removing require(magrittr) is clearly correct (%>% is re-exported by dplyr). Removing require(stats) is almost certainly fine since stats is a base package that's always loaded in R sessions, but worth a quick grep -r "stats::" inst/shiny/ to confirm no module code relies on the namespace being explicitly attached.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion, grep -r "stats::" inst/shiny/ returns zero results.


require(tidyr)
require(tools)
Expand Down
26 changes: 26 additions & 0 deletions inst/shiny/functions/colors.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Color constants — keep in sync with inst/shiny/www/styles/modules/_colors.scss

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Color constants duplicate and diverge from existing SCSS variables

The existing _colors.scss already defines a palette ($anca-blue: #007bc2, $grey-1 through $grey-5). The new constants here introduce a parallel set that doesn't reference or align with those:

  • ACCENT_BLUE is #0d6efd (Bootstrap blue) vs $anca-blue: #007bc2 — different blues, but naming doesn't distinguish intent.
  • BG_GREY_LIGHT is #eeeeee vs $grey-5: #dadada — similar purpose, different values.

The sync comment on line 1 is good, but there's no mechanism to enforce it. A future developer changing the SCSS variable won't know to update the R constant (and vice versa).

Suggestion: Add a matching comment in _colors.scss pointing back to this file (bidirectional sync note). Also consider whether some of these should reuse existing SCSS variable values rather than introducing new ones.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review. Now added bidirectional sync comment in _colors.scss pointing back to colors.R (commit a5b78fe). The new variables expand the shared palette — they mirror the R constants and will be consumed by future CSS rules. For now both files serve as the single source of truth for their respective domains (SCSS for CSS rules, R for inline styles).

# See https://github.com/pharmaverse/aNCA/issues/1298

# Backgrounds
BG_GREY_LIGHT <- "#eeeeee"
BG_GREY_HEADER <- "#e9e9e9"
BG_HIGHLIGHT_BLUE <- "#CCE5FF"
BG_BLUE_LIGHT <- "#e6f2ff"
ANCA_WHITE <- "#ffffff"

# Text
TEXT_MUTED <- "#999999"
TEXT_DIM <- "#888888"
TEXT_SUBTLE <- "#777777"
TEXT_SECONDARY <- "#666666"
TEXT_HEADING <- "#555555"

# Borders
BORDER_LIGHT <- "#dddddd"
BORDER_MUTED <- "#cccccc"
BORDER_PANEL <- "#dee2e6"

# Accent
ACCENT_BLUE <- "#0d6efd"
ACCENT_ORANGE <- "#ffa62d"
ACCENT_DANGER <- "#dc3545"
11 changes: 6 additions & 5 deletions inst/shiny/functions/utils-exclusions.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ EXCL_COLOR_PARAM <- "#FFF3CD" # yellow — parameter exclusion
.legend_swatch <- function(color, label) {
div(
style = "display:flex; align-items:center; gap:6px;",
div(style = paste0(
"width:14px; height:14px; background:", color,
"; border:1px solid #ddd;"
div(style = glue::glue(
"width:14px; height:14px; background:{color}",
"; border:1px solid {BORDER_LIGHT};"
)),
span(label, style = "font-size:0.9em;")
)
Expand Down Expand Up @@ -167,7 +167,8 @@ EXCL_COLOR_PARAM <- "#FFF3CD" # yellow — parameter exclusion
ns(value),
label = NULL,
icon = shiny::icon("times"),
class = "btn btn-link btn-sm",
`aria-label` = "Remove exclusion",
class = "btn-link btn-sm",
style = "padding:2px 6px;"
)
)
Expand Down Expand Up @@ -196,7 +197,7 @@ EXCL_COLOR_PARAM <- "#FFF3CD" # yellow — parameter exclusion
pagination = FALSE,
defaultPageSize = nrow(df),
theme = reactable::reactableTheme(
headerStyle = list(background = "#e9e9e9")
headerStyle = list(background = BG_GREY_HEADER)
),
columns = cols
)
Expand Down
10 changes: 7 additions & 3 deletions inst/shiny/modules/tab_about.R
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ tab_about_server <- function(id) {
}
)
tags$blockquote(
style = "border-left: 3px solid #ccc; padding-left: 1em; color: #555;",
style = glue::glue(
"border-left: 3px solid {BORDER_MUTED}; padding-left: 1em; color: {TEXT_HEADING};"
),
tags$p(cit_text)
)
})
Expand All @@ -163,7 +165,9 @@ tab_about_server <- function(id) {
cit <- utils::citation("PKNCA")
cit_text <- paste(format(cit, style = "text"), collapse = "\n\n")
tags$blockquote(
style = "border-left: 3px solid #ccc; padding-left: 1em; color: #555;",
style = glue::glue(
"border-left: 3px solid {BORDER_MUTED}; padding-left: 1em; color: {TEXT_HEADING};"
),
tags$p(cit_text)
)
})
Expand Down Expand Up @@ -255,7 +259,7 @@ tab_about_server <- function(id) {
style = "margin-bottom: 0.3em;",
tags$a(href = url, target = "_blank", label),
tags$span(
style = "color: #999; font-size: 0.85em; margin-left: 0.5em;",
style = glue::glue("color: {TEXT_MUTED}; font-size: 0.85em; margin-left: 0.5em;"),
url
)
)
Expand Down
2 changes: 1 addition & 1 deletion inst/shiny/modules/tab_data/data_mapping.R
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ data_mapping_server <- function(id, adnca_data, imported_mapping, trigger) {
if (dup_data[index, ".dup_group"] %% 2 == 0) {
list(background = "white")
} else {
list(background = "#e6f2ff")
list(background = BG_BLUE_LIGHT)
}
},
selection = "multiple",
Expand Down
4 changes: 2 additions & 2 deletions inst/shiny/modules/tab_data/data_upload.R
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,8 @@ data_upload_server <- function(id) {
defaultPageSize = 10,
theme = reactableTheme(
rowSelectedStyle = list(
backgroundColor = "#CCE5FF",
boxShadow = "inset 2px 0 0 0 #0d6efd"
backgroundColor = BG_HIGHLIGHT_BLUE,
boxShadow = paste0("inset 2px 0 0 0 ", ACCENT_BLUE)
)
),
columns = list(
Expand Down
2 changes: 1 addition & 1 deletion inst/shiny/modules/tab_explore.R
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ tab_explore_server <- function(id, pknca_data, extra_group_vars) {
footer = tagList(
modalButton("Cancel"),
actionButton(ns("confirm_add_to_exports"), "Save",
class = "btn btn-primary")
class = "btn-primary")
),
size = "s",
easyClose = TRUE
Expand Down
2 changes: 1 addition & 1 deletion inst/shiny/modules/tab_explore/plot_sidebar.R
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ plot_sidebar_ui <- function(id, is_mean_plot = FALSE, extra_ui = NULL) {
ns("add_to_exports"),
label = "Add to Exports",
icon = icon("plus"),
class = "btn btn-primary btn-sm",
class = "btn-primary btn-sm",
width = "100%"
),
extra_ui,
Expand Down
2 changes: 1 addition & 1 deletion inst/shiny/modules/tab_explore/qc_plot.R
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pk_dose_qc_plot_ui <- function(id, extra_ui = NULL) {
ns("add_to_exports"),
label = "Add to Exports",
icon = icon("plus"),
class = "btn btn-primary btn-sm",
class = "btn-primary btn-sm",
width = "100%"
),
extra_ui,
Expand Down
17 changes: 9 additions & 8 deletions inst/shiny/modules/tab_explore/saved_outputs.R
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ saved_outputs_ui <- function(id) {
ns("view_exports"),
label = "View Exports",
icon = icon("eye"),
class = "btn btn-primary btn-sm",
class = "btn-primary btn-sm",
width = "100%"
)
}
Expand Down Expand Up @@ -168,7 +168,7 @@ saved_outputs_server <- function(id, saved_plots_metadata, get_plot_obj,
"No plots saved yet. Use ",
tags$strong("Add to Exports"),
" to save plots.",
style = "color: #888; font-style: italic; margin: 16px 0;"
style = glue::glue("color: {TEXT_DIM}; font-style: italic; margin: 16px 0;")
))
}

Expand All @@ -186,9 +186,9 @@ saved_outputs_server <- function(id, saved_plots_metadata, get_plot_obj,
)

tags$div(
style = paste0(
"border: 1px solid #dee2e6; border-radius: 6px; ",
"padding: 12px; margin-bottom: 16px; background: #fff;"
style = glue::glue(
"border: 1px solid {BORDER_PANEL}; border-radius: 6px; ",
"padding: 12px; margin-bottom: 16px; background: {ANCA_WHITE};"
),
tags$div(
style = paste0(
Expand All @@ -199,16 +199,17 @@ saved_outputs_server <- function(id, saved_plots_metadata, get_plot_obj,
tags$strong(plot_name, style = "font-size: 1.1em;"),
tags$span(
paste0(" \u2014 ", plot_type, " \u00b7 ", plot_ts),
style = "color: #666; font-size: 0.9em;"
style = glue::glue("color: {TEXT_SECONDARY}; font-size: 0.9em;")
)
),
tags$a(
href = "#",
onclick = paste0(remove_js, "return false;"),
style = paste0(
"color: #dc3545; font-size: 1.2em; ",
style = glue::glue(
"color: {ACCENT_DANGER}; font-size: 1.2em; ",
"text-decoration: none;"
),
`aria-label` = "Remove from exports",
title = "Remove from exports",
icon("times")
)
Expand Down
2 changes: 1 addition & 1 deletion inst/shiny/modules/tab_nca.R
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ tab_nca_ui <- function(id) {
inputId = ns("run_nca"),
label = "Run NCA",
icon = icon("play"),
class = "btn btn-primary",
class = "btn-primary",
width = "100%"
),
nca_setup_ui(ns("nca_setup")),
Expand Down
2 changes: 1 addition & 1 deletion inst/shiny/modules/tab_nca/excretion_analysis.R
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ excretion_help_ui <- function() {
)
),
style = "unite",
icon = icon("question"),
icon = icon("question"), label = "Help",
status = "primary"
)
}
4 changes: 3 additions & 1 deletion inst/shiny/modules/tab_nca/nca_results.R
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ nca_results_ui <- function(id) {
span("MISSING", style = "font-size:0.9em;")
),
div(style = "display:flex; align-items:center; gap:6px;",
div(style = "width:14px; height:14px; background:#ffffff; border:1px solid #ddd;"),
div(style = glue::glue(
"width:14px; height:14px; background:{ANCA_WHITE}; border:1px solid {BORDER_LIGHT};"
)),
span("ACCEPTED", style = "font-size:0.9em;")
)
),
Expand Down
7 changes: 5 additions & 2 deletions inst/shiny/modules/tab_nca/non_nca_ratios.R
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,10 @@ non_nca_ratio_server <- function(id, data, grouping_vars) {
),
div(
class = "ratio-remove-btn-wrapper",
actionButton(ns(paste0("remove_", count)), "", icon = icon("trash-alt"))
actionButton(
ns(paste0("remove_", count)), "",
`aria-label` = "Remove ratio pair", icon = icon("trash-alt")
)
)
)

Expand Down Expand Up @@ -243,7 +246,7 @@ non_nca_ratio_help_ui <- function(title) {
),
style = "unite",
placement = "left",
icon = icon("question"),
icon = icon("question"), label = "Help",
status = "primary"
)
}
4 changes: 2 additions & 2 deletions inst/shiny/modules/tab_nca/setup/data_imputation.R
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ data_imputation_ui <- function(id) {
),
style = "unite",
right = TRUE,
icon = icon("question"),
icon = icon("question"), label = "Help",
status = "primary",
width = "500px"
)
Expand Down Expand Up @@ -123,7 +123,7 @@ data_imputation_ui <- function(id) {
),
style = "unite",
right = TRUE,
icon = icon("question"),
icon = icon("question"), label = "Help",
status = "primary",
width = "500px"
)
Expand Down
4 changes: 2 additions & 2 deletions inst/shiny/modules/tab_nca/setup/general_exclusions.R
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ general_exclusions_ui <- function(id) {
actionButton(
ns("add_exclusion_reason"),
label = "Add",
class = "btn btn-primary btn-sm"
class = "btn-primary btn-sm"
),
# Help button (dropdown)
dropdown(
Expand Down Expand Up @@ -78,7 +78,7 @@ general_exclusions_ui <- function(id) {
),
style = "unite",
right = TRUE,
icon = icon("question"),
icon = icon("question"), label = "Help",
status = "primary"
)
),
Expand Down
4 changes: 2 additions & 2 deletions inst/shiny/modules/tab_nca/setup/manual_slopes_table.R
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ manual_slopes_table_server <- function(
borderless = TRUE,
theme = reactableTheme(
rowSelectedStyle = list(
backgroundColor = "#eee",
boxShadow = "inset 2px 0 0 0 #ffa62d"
backgroundColor = BG_GREY_LIGHT,
boxShadow = glue::glue("inset 2px 0 0 0 {ACCENT_ORANGE}")
)
)
)
Expand Down
4 changes: 2 additions & 2 deletions inst/shiny/modules/tab_nca/setup/parameter_exclusions.R
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ parameter_exclusions_ui <- function(id) {
actionButton(
ns("add_exclusion"),
label = "Add",
class = "btn btn-primary btn-sm"
class = "btn-primary btn-sm"
),
dropdown(
div(
Expand All @@ -45,7 +45,7 @@ parameter_exclusions_ui <- function(id) {
),
style = "unite",
right = TRUE,
icon = icon("question"),
icon = icon("question"), label = "Help",
status = "primary"
)
),
Expand Down
4 changes: 2 additions & 2 deletions inst/shiny/modules/tab_nca/setup/parameter_selection.R
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ parameter_selection_ui <- function(id,
),
style = "unite",
right = TRUE,
icon = icon("question"),
icon = icon("question"), label = "Help",
status = "primary",
width = "500px"
),
Expand Down Expand Up @@ -428,7 +428,7 @@ parameter_selection_server <- function(id, processed_pknca_data, parameter_overr
href = func_url,
target = "_blank",
style = paste0(
"color: #0d6efd;",
"color: ", ACCENT_BLUE, ";",
"text-decoration: underline;"
),
value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ ratios_table_ui <- function(id) {
),
style = "unite",
right = TRUE,
icon = icon("question"),
icon = icon("question"), label = "Help",
status = "primary",
width = "500px"
)
Expand Down
Loading
Loading