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

Fix updateSelectizeInput(), fixes #2624 #2625

Merged
merged 1 commit into from Sep 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion NEWS.md
Expand Up @@ -26,7 +26,7 @@ shiny 1.4.0

* Added `resourcePaths()` and `removeResourcePaths()` functions. ([#2459](https://github.com/rstudio/shiny/pull/2459))

* Resolved [#2515](https://github.com/rstudio/shiny/issues/2515): `selectInput()` now deals appropriately with named factors. ([#2524](https://github.com/rstudio/shiny/pull/2524))
* Resolved [#2515](https://github.com/rstudio/shiny/issues/2515): `selectInput()` and `selectizeInput()` now deal appropriately with named factors. Note that `updateSelectInput()` and `updateSelectizeInput()` **do not** yet handle factors; their behavior is unchanged. ([#2524](https://github.com/rstudio/shiny/pull/2524), [#2540](https://github.com/rstudio/shiny/pull/2540), [#2625](https://github.com/rstudio/shiny/pull/2625))

* Resolved [#2433](https://github.com/rstudio/shiny/issues/2433): An informative warning is now thrown if subdirectories of the app's `www/` directory are masked by other resource prefixes and/or the same resource prefix is mapped to different local file paths. ([#2434](https://github.com/rstudio/shiny/pull/2434))

Expand Down
9 changes: 8 additions & 1 deletion R/input-utils.R
Expand Up @@ -92,7 +92,10 @@ generateOptions <- function(inputId, selected, inline, type = 'checkbox',

# True when a choice list item represents a group of related inputs.
isGroup <- function(choice) {
length(choice) > 1 || !is.null(names(choice))
is.list(choice) ||
!is.null(names(choice)) ||
length(choice) > 1 ||
length(choice) == 0
}

# True when choices is a list and contains at least one group of related inputs.
Expand Down Expand Up @@ -131,6 +134,10 @@ processFlatChoices <- function(choices) {
processGroupedChoices <- function(choices) {
# We assert choices is a list, since only a list may contain a group.
stopifnot(is.list(choices))
# The list might be unnamed by this point. We add default names of "" so that
# names(choices) is not zero-length and mapply can work. Within mapply, we
# error if any group's name is ""
choices <- asNamed(choices)
choices <- mapply(function(name, choice) {
choiceIsGroup <- isGroup(choice)
if (choiceIsGroup && name == "") {
Expand Down
33 changes: 32 additions & 1 deletion tests/testthat/test-bootstrap.r
Expand Up @@ -69,7 +69,7 @@ test_that("Repeated names for selectInput and radioButtons choices", {


test_that("Choices are correctly assigned names", {
# Empty non-list comes back with names
# Empty non-list comes back as a list with names
expect_identical(
choicesWithNames(numeric(0)),
stats::setNames(list(), character(0))
Expand All @@ -79,6 +79,16 @@ test_that("Choices are correctly assigned names", {
choicesWithNames(list()),
stats::setNames(list(), character(0))
)
# NULL comes back as an empty list with names
expect_identical(
choicesWithNames(NULL),
stats::setNames(list(), character(0))
)
# NA is processed as a leaf, not a group
expect_identical(
choicesWithNames(NA),
as.list(stats::setNames(as.character(NA), NA))
)
# Empty character vector
# An empty character vector isn't a sensical input, but we preserved this test
# in the off chance that somebody relies on the existing behavior.
Expand Down Expand Up @@ -151,8 +161,19 @@ test_that("Choices are correctly assigned names", {
choicesWithNames(list(A="a", "b", C=list("d", E="e"))),
list(A="a", b="b", C=list(d="d", E="e"))
)
# List, with a single-item unnamed group list
expect_identical(
choicesWithNames(list(C=list(123))),
list(C=list("123"="123"))
)
# Error when sublist is unnamed
expect_error(choicesWithNames(list(A="a", "b", list(1,2))))
# Error when list is unnamed and contains a group
# NULL, list(1,2), and anything of length() == 0 is considered a group.
# NA is NOT a group.
expect_error(choicesWithNames(list(NULL)), regexp = "must be named")
expect_error(choicesWithNames(list(list(1,2))), regexp = "must be named")
expect_error(choicesWithNames(list(character(0))), regexp = "must be named")
# Unnamed factor
expect_identical(
choicesWithNames(factor(c("a","b","3"))),
Expand All @@ -173,6 +194,16 @@ test_that("Choices are correctly assigned names", {
choicesWithNames(list(A="a", B="b", C=structure(factor(c("d", "e")), names = c("d", "e")))),
list(A="a", B="b", C=list(d="d", e="e"))
)
# List, named, with an empty group as an unnamed empty list
expect_identical(
choicesWithNames(list(C=list())),
list(C=stats::setNames(list(), character()))
)
# List, named, with an empty group as an unnamed empty vector
expect_identical(
choicesWithNames(list(C=c())),
list(C=stats::setNames(list(), character()))
)
})


Expand Down