Skip to content

Commit 258284c

Browse files
committed
Apply maintainer feedback for PR#246
- Update NEWS.md to reference PR#246 instead of Issue #232 - Refactor checkSelectorNames() to use sprintf instead of paste0 - Remove unnecessary if(length==0) check and explicit return(NULL) - Remove all empty lines from test_that blocks per style guidelines
1 parent e6d8ae9 commit 258284c

File tree

3 files changed

+7
-26
lines changed

3 files changed

+7
-26
lines changed

NEWS.md

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
1-
<<<<<<< HEAD
21
# Changes in version 2025.10.9 (PR#242)
32

43
- Improve common chunk detection, output `na_group` and `row_in_group` when there are missing values.
5-
=======
6-
# Changes in version 2025.10.6 (Issue #232)
4+
5+
# Changes in version 2025.10.6 (PR#246)
76

87
- Added validation for selector names to prevent browser rendering failures. Selector names (from data values used in `clickSelects` and `showSelected`) cannot contain CSS special characters like `#`, `@`, `!`, `$`, etc., as these interfere with JavaScript DOM selectors and cause blank visualizations in the browser. The compiler now stops with a clear error message identifying problematic selector names, helping users fix data issues before attempting to render.
9-
>>>>>>> 32e15c5d (Add validation for selector names to prevent browser rendering failures(#232).)
108

119
# Changes in version 2025.10.3 (PR#240)
1210

R/z_animintHelpers.R

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -642,10 +642,6 @@ checkSingleShowSelectedValue <- function(selectors){
642642
#' @param selectors selectors to validate
643643
#' @return \code{NULL}. Throws error if invalid characters found.
644644
checkSelectorNames <- function(selectors){
645-
if(length(selectors) == 0){
646-
return(NULL)
647-
}
648-
649645
selector.names <- names(selectors)
650646

651647
## Characters that are invalid in CSS selectors and cause issues in browser
@@ -671,18 +667,17 @@ checkSelectorNames <- function(selectors){
671667
paste0("'", found.chars, "'", collapse = ", ")
672668
})
673669

674-
error.msg <- paste0(
670+
error.msg <- sprintf(
671+
"%s%s\n%s\n%s\n\n%s",
675672
"Invalid character(s) in selector name(s). ",
676-
"Selector names cannot contain special characters that interfere with CSS selectors.\n",
677-
"The following selector(s) contain invalid characters:\n",
673+
"Selector names cannot contain special characters that interfere with CSS selectors.",
674+
"The following selector(s) contain invalid characters:",
678675
paste0(" - '", invalid.names, "' contains ", problematic, collapse = "\n"),
679-
"\n\nPlease remove or replace these characters in your variable names."
676+
"Please remove or replace these characters in your variable names."
680677
)
681678

682679
stop(error.msg)
683680
}
684-
685-
return(NULL)
686681
}
687682

688683

tests/testthat/test-compiler-invalid-selector-characters.R

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,11 @@ test_that("selector name with # causes error in clickSelects", {
1616
clickSelects = c(regularization = "parameter")
1717
)
1818
)
19-
2019
expect_error(
2120
animint2dir(viz, open.browser = FALSE),
2221
"Invalid character\\(s\\) in selector name\\(s\\)",
2322
info = "Selector names with '#' should cause an error"
2423
)
25-
2624
expect_error(
2725
animint2dir(viz, open.browser = FALSE),
2826
"# nearest neighbors",
@@ -44,13 +42,11 @@ test_that("selector name with @ causes error in clickSelects", {
4442
clickSelects = c(regularization = "parameter")
4543
)
4644
)
47-
4845
expect_error(
4946
animint2dir(viz, open.browser = FALSE),
5047
"Invalid character\\(s\\) in selector name\\(s\\)",
5148
info = "Selector names with '@' should cause an error"
5249
)
53-
5450
expect_error(
5551
animint2dir(viz, open.browser = FALSE),
5652
"model@version",
@@ -70,7 +66,6 @@ test_that("selector name with ! causes error", {
7066
clickSelects = c(model = "parameter")
7167
)
7268
)
73-
7469
expect_error(
7570
animint2dir(viz, open.browser = FALSE),
7671
"Invalid character\\(s\\) in selector name\\(s\\)",
@@ -90,8 +85,6 @@ test_that("valid selector names work with clickSelects", {
9085
clickSelects = c(regularization = "parameter")
9186
)
9287
)
93-
94-
# Should NOT error
9588
info <- animint2dir(viz, open.browser = FALSE)
9689
expect_true(TRUE, info = "Valid selector names should not cause errors")
9790
})
@@ -108,8 +101,6 @@ test_that("selector names with spaces work", {
108101
clickSelects = c(regularization = "parameter")
109102
)
110103
)
111-
112-
# Should NOT error - spaces are fine
113104
info <- animint2dir(viz, open.browser = FALSE)
114105
expect_true(TRUE, info = "Selector names with spaces should work")
115106
})
@@ -126,13 +117,10 @@ test_that("multiple values with invalid characters all reported", {
126117
clickSelects = c(regularization = "parameter")
127118
)
128119
)
129-
130120
error_msg <- tryCatch(
131121
animint2dir(viz, open.browser = FALSE),
132122
error = function(e) as.character(e$message)
133123
)
134-
135-
# Should catch both bad selector names
136124
expect_match(error_msg, "Invalid character", info = "Should report invalid characters")
137125
expect_match(error_msg, "#bad|!worse", info = "Should mention at least one problematic selector")
138126
})

0 commit comments

Comments
 (0)