shinyWidgets icon indicating copy to clipboard operation
shinyWidgets copied to clipboard

Problem using Shinyvalidate's sv_between and numericInput range

Open ixodid198 opened this issue 2 years ago • 4 comments

Both fields require numbers >=0 and < 300 Empty fields, spaces, ".", "e" are not allowed. When a field is invalid a red "!" should appear AND the button should be disabled.

The code below works BUT I had to make the minimum of the field = -1 to make the disable button work. This is a poor workaround because -1 is an invalid number and should show the red "!" .

How is it possible to get both the red "!" AND the disabled button to work if the valid numbers are: 0 <= x < 300

library("shiny")
library("bslib")
library("shinyjs")
library("shinyWidgets")
library("shinyvalidate")

per_month <- "/mth"
num_min <- -1
num_max <- 300

ui <- bootstrapPage(
  useShinyjs(),
  

  theme = bs_theme(version = 5, "font_scale" = 1.0), 
  div(class = "container-fluid",
      
      div(class = "row",
          div(class="col-4", 
              numericInputIcon(
                inputId = "a",
                label = "A",
                value = 100,
                min = num_min,
                max = num_max,
                width = "200px",
                icon = list(NULL, per_month)
              ),
          ),
          div(class="col-4", 
              numericInputIcon(
                inputId = "b",
                label = "B",
                value = 200,
                min = num_min,
                max = num_max,
                width = "200px",
                icon = list(NULL, per_month)
              ),
          ),
          htmlOutput("a")
      ), 
      div(class = "row",
          div(class = "col-12",
              actionButton("cc_calculate", "—— Submit ——")
          ), align = "center"
      )
  )
)

server <- function(input, output, session) {

  output$a <- renderText({
    paste("a:", input$a)
  })
  
       
  iv <- InputValidator$new()

  iv$add_rule("a", sv_required(message = ""))
  iv$add_rule("a", sv_between(num_min, num_max, message_fmt = "", inclusive = c(FALSE, FALSE)))

  iv$add_rule("b", sv_required(message = ""))
  iv$add_rule("b", sv_between(num_min, num_max, message_fmt = "", inclusive = c(FALSE, FALSE)))

  iv$enable()

  observe(
    if (iv$is_valid()) {
      enable("cc_calculate")
    } else {
      disable("cc_calculate")
    }
  )

}

shinyApp(ui = ui, server = server)

ixodid198 avatar Mar 18 '22 02:03 ixodid198

Here you can find the related SO post.

In my eyes here are just two different (incompatible) validation approaches in conflict.

The "issue" is, that the built in validation of numericInputIcon contradicts the use of shinyvalidate by forcing it's internal value back into the range of allowed values (even though the invalid value still is displayed) which shinyvalidate correctly detects as valid.

A example comparing the numericInputIcon's behaviour with numericInput:

Animation

library("shiny")
library("shinyjs")
library("shinyWidgets")
library("shinyvalidate")

per_month <- "/mth"
num_min <- 0
num_max <- 300

library(shiny)

ui <- fluidPage(
  useShinyjs(),
  fluidRow(column(2, numericInputIcon(
    inputId = "numericInputIconA",
    label = "numericInputIconA",
    value = 100,
    min = num_min,
    max = num_max,
    width = "200px",
    icon = list(NULL, per_month)
  )),
  column(2, numericInput(
    inputId = "numericInputB",
    label = "numericInputB",
    value = 100,
    min = num_min,
    max = num_max,
    width = "200px"
  )),
  actionButton("cc_calculate", "—— Submit ——", style = "margin-top:25px;")),
  fluidRow(column(2, uiOutput("servervaluesA")),
           column(2, uiOutput("servervaluesB")))
)

server <- function(input, output, session) {
  output$a <- renderText({
    paste("a:", input$a)
  })
  
  iv <- InputValidator$new()
  iv$add_rule("numericInputIconA", sv_required(message = "invalid"))
  iv$add_rule("numericInputIconA", sv_between(num_min, num_max, message_fmt = "invalid", inclusive = c(TRUE, TRUE)))
  iv$add_rule("numericInputB", sv_required(message = "invalid"))
  iv$add_rule("numericInputB", sv_between(num_min, num_max, message_fmt = "invalid", inclusive = c(TRUE, TRUE)))
  iv$enable()
  observe({
    if (iv$is_valid()) {
      enable("cc_calculate")
    } else {
      disable("cc_calculate")
    }
  })
  
  output$servervaluesA <- renderUI({paste("Server value numericInputIconA:", input$numericInputIconA)})
  output$servervaluesB <- renderUI({paste("Server value numericInputB:", input$numericInputB)})
  
}

shinyApp(ui = ui, server = server)

ismirsehregal avatar Mar 18 '22 07:03 ismirsehregal

Is this behaviour (only returning a valid value inside the min-max range rather than the set value itself) a design choice for the package? If not, the fix is rather simple: remove lines 79 and 83?

https://github.com/dreamRs/shinyWidgets/blob/bf0a001623d86f259c884adbbd6ba9f83f0c8579/inst/assets/input-icon/numeric-icon-bindings.js#L76-L88

Most users probably use the shinyWidgets::numericInputIcon() as a direct replacement for shiny::numericInput() which does return an invalid value outside the min-max range, and thus having a different logic here could be ill-advised. For backwards compability there could also be an extra argument allow_invalid_value that changes the behaviour to use that of shiny::numericInput(). At the very least, since this behaviour contradicts that of shiny::numericInput() this should be documented.

samssann avatar Feb 28 '23 07:02 samssann

Yes the behavior of this function is the expected one, when I developed this function it was to answer a specific problem and that's what it does here. Nevertheless I understand the problem, and if I am against changing the default behavior, I am not against making this function evolve by adding for example a parameter force_valid_value = TRUE or similar. PR are welcomed either for new functionnalities or documentation improvement.

pvictor avatar Feb 28 '23 14:02 pvictor

I couldn't find any tests that test javascript (i.e. shinytest/shinytest2), so a PR would not require any test tweaks?

samssann avatar Mar 02 '23 08:03 samssann