Skip to content

fixed: issue #1365 - #1410

Open
poonam-16 wants to merge 1 commit into
pharmaverse:mainfrom
poonam-16:main
Open

fixed: issue #1365#1410
poonam-16 wants to merge 1 commit into
pharmaverse:mainfrom
poonam-16:main

Conversation

@poonam-16

@poonam-16 poonam-16 commented Jul 22, 2026

Copy link
Copy Markdown

Issue

Closes #1365

Description

Added a helper text that states the maximum upload limit

Additionally, sourced the limit from a single constant so the UI text and run_app.R stay in sync

Definition of Done

  • Max upload size is visible in the upload UI
  • Value matches the actual shiny.maxRequestSize setting

Contributor checklist

  • Code passes lintr checks
  • Code passes all unit tests
  • New logic covered by unit tests
  • New logic is documented
  • App or package changes are reflected in NEWS
  • Package version is incremented
  • R script works with the new implementation (if applicable)
  • Settings upload works with the new implementation (if applicable)
  • If any .scss change was done, run data-raw/compile_css.R
  • If a package dependency was added/changed, run data-raw/test_suggests_hidden.R

@Gero1999
Gero1999 self-requested a review July 23, 2026 12:43
@Gero1999

Copy link
Copy Markdown
Collaborator

Thanks for the contribution @poonam-16! I like the idea of a single source of truth, very witty!

Unfortunately, this does not seem t work on an installed package. I added some comments

Comment thread R/run_app.R
#' }
#' @export

max_upload_size_mb <- 30

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.

issue: as you see this is cutting the roxygen docx (all that is before @export) which may produce a fail in geenerating the documentation. Instead, move this line right before:

options(shiny.maxRequestSize = max_upload_size_mb * 1024^2)

p("Upload your PK dataset and Settings file (optional).",
tags$br(),
tags$small(style = "color: #6c757d; display: block; margin-top: 4px;",
sprintf("Maximum upload size: %s MB", max_upload_size_mb))),

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.

Nice idea! Having a single source of truth is always good. However, for an installed pacakge the max_upload_size_mb variable won't be available in this submodule.

I would recommend that we instead search for the value in the settings getOption("shiny.maxRequestSize", but in case for any reason run_app.R and the setting was not set (i.e, someone decided to run the App manually without using our function, shiny::runApp(...)) then we tell teh code to assume we are using the default that shiny has (5 MB). In code this will translate to this:

Suggested change
sprintf("Maximum upload size: %s MB", max_upload_size_mb))),
sprintf("Maximum upload size: %s MB",
getOption("shiny.maxRequestSize", 5 * 1024^2) / 1024^2))),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UX: show the max upload size in the file upload UI

2 participants