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

Widgets' inputIds behave incorrectly with spaces #2799

Open
jntrcs opened this issue Mar 19, 2020 · 9 comments · May be fixed by #3852
Open

Widgets' inputIds behave incorrectly with spaces #2799

jntrcs opened this issue Mar 19, 2020 · 9 comments · May be fixed by #3852

Comments

@jntrcs
Copy link

jntrcs commented Mar 19, 2020

With my non-comprehensive look through Shiny documentation, it appears that the only requirement for a Shiny widget inputId is that it is a character string. However, I have discovered that if you have spaces in that character string, the updateWidget function doesn't behave properly (it basically is ignored, with no error message). My simple repro, where I change the id from "bin" to "Number of bins" documents the problem:

library(shiny)
ui <- fluidPage(
    sidebarLayout(
        sidebarPanel(
            sliderInput("Number of bins",
                        "Number of bins:",
                        min = 1,
                        max = 50,
                        value = 30),
            actionButton("IncreaseBins", "Increase max number of bins")
        ),
        mainPanel(
           plotOutput("distPlot")
        )
    )
)

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

    output$distPlot <- renderPlot({
        x    <- faithful[, 2]
        bins <- seq(min(x), max(x), length.out = input[["Number of bins"]] + 1)
        hist(x, breaks = bins, col = 'darkgray', border = 'white')
    })
    observeEvent(input$IncreaseBins, {
        updateSliderInput(session, "Number of bins", max = 100)
    })
}

shinyApp(ui = ui, server = server)
@jntrcs
Copy link
Author

jntrcs commented Mar 19, 2020

Screen Shot 2020-03-19 at 12 43 24 PM
In case this is helpful

@wch
Copy link
Collaborator

wch commented Mar 19, 2020

Spaces in IDs aren't supported. It could help if we added a check for this and throw an error if someone tries to use it.

@jntrcs
Copy link
Author

jntrcs commented Mar 19, 2020

Yeah, it's definitely funny that including spaces works for setting ids, retrieving values, but not updating widgets, all without an error. It should definitely throw an error and be mentioned in the help pages for selectInput, numericInput, etc

@ismirsehregal
Copy link
Contributor

ismirsehregal commented Apr 18, 2020

This article provides a short note regarding special JavaScript characters in input id's:

You are not recommended to use special JavaScript characters such as a period . in the input id's, but if you do use them anyway, for example, inputId = "foo.bar", you will have to use input["foo.bar"] instead of input.foo.bar to read the input value.

An error message would be quite helpful 👍

@jntrcs
Copy link
Author

jntrcs commented May 12, 2020

Would a pull request that threw an error on bad inputIDs be welcome? I saw a note in the docs that said anything that creates backward compatibility issues would not be welcome. Since spaces in IDs work for many scenarios, there is probably a lot of functional code out there that would no longer work if this started throwing errors.

If it would be welcome, I would be open to trying if someone can point me in the direction of "these are the files that would need to change."

@wch
Copy link
Collaborator

wch commented May 12, 2020

We'd definitely take a look at the PR, but note that there may be some tricky stuff involved to get all the details right. We definitely don't want to break existing apps, so it would be better to throw a warning than an error.

@tyner
Copy link

tyner commented Jul 6, 2023

Spaces in IDs aren't supported. It could help if we added a check for this and throw an error if someone tries to use it.

Is there any mention of this being unsupported in the formal documentation? I just spent the better part of a week debugging a glitchy app and finally isolated the root cause to be the use of an inputId containing spaces (it was being dynamically generated from another variable whose value contained spaces).

We'd definitely take a look at the PR, but note that there may be some tricky stuff involved to get all the details right. We definitely don't want to break existing apps, so it would be better to throw a warning than an error.

How about something like options(shiny.inputid.strict = TRUE) to allow users to "opt-in" to strict validation of their inputIds?

@gadenbuie gadenbuie linked a pull request Jul 6, 2023 that will close this issue
@ismirsehregal
Copy link
Contributor

ismirsehregal commented Jul 7, 2023

Well hidden, but see the note here:

You are not recommended to use special JavaScript characters such as a period . in the input id's, but if you do use them anyway, for example, inputId = "foo.bar", you will have to use input["foo.bar"] instead of input.foo.bar to read the input value.

However, the space restriction seems to be inherited from HTML:

An id's value must not contain whitespace (spaces, tabs, etc.). Browsers treat non-conforming IDs that contain whitespace as if the whitespace is part of the ID. In contrast to the class attribute, which allows space-separated values, elements can only have one single ID value.

@gadenbuie
Copy link
Member

Browsers are funny like that: an ID must not contain whitespace, but if it does we'll consider the whitespace part of the ID.

I strongly recommend avoiding whitespace in input IDs, but given that almost anything will work in the browser – and most if not all of our inputs will work fine with spaces in them – I don't think we should be more aggressive unless we have a strong reason to do so. (In other words, if spaces in an ID break a specific input, we should proactively avoid those issues early.)

In this case, we can support input updates when spaces are involved by changing our JavaScript internals to use a CSS selector that queries by attribute value. E.g. the #spaced id selector will target an <id> element that's a child of an element with id="spaced", whereas [id="spaced id"] will find an element with id="spaced id". I've proposed an update in #3852 that fixes our input update methods, but also throws a warning message in the browser's developer console when spaced IDs are encountered.

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 a pull request may close this issue.

5 participants