dreamRs / shinyWidgets

shinyWidgets : Extend widgets available in shiny
https://dreamrs.github.io/shinyWidgets/
GNU General Public License v3.0
826 stars 153 forks source link

Problem using Shinyvalidate's sv_between and numericInput range #483

Open ixodid198 opened 2 years ago

ixodid198 commented 2 years ago

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)
ismirsehregal commented 2 years ago

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)
samssann commented 1 year ago

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.

pvictor commented 1 year ago

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.

samssann commented 1 year ago

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