Make sure NA attributes are rendered as blank. Fixes #99

DESCRIPTION

@@ -1,7 +1,7 @@
 Package: htmltools
 Type: Package
 Title: Tools for HTML
-Version: 0.3.6.9000
+Version: 0.3.6.9001
 Date: 2017-04-26
 Author: RStudio, Inc.
 Maintainer: Joe Cheng <joe@rstudio.com>

NEWS

@@ -1,11 +1,15 @@
-htmltools 0.3.6.9000
+htmltools 0.3.6.9001
 --------------------------------------------------------------------------------
 
 * Updated RcppExports for new version of Rcpp. (#93)
 
 * `as.character.shiny.tags()` will handle non-ASCII attributes correctly if they
   are not encoded in native encoding.
 
+* Fixed #99: `NA` attributes were sometimes rendered as `"NA"` in the HTML,
+  instead of being blank. (#100)
+
+
 htmltools 0.3.6
 --------------------------------------------------------------------------------
 

R/tags.R

@@ -398,8 +398,18 @@ tagWrite <- function(tag, textWriter, indent=0, eol = "\n") {
   attribs <- lapply(tag$attribs, as.character)
   # concatenate attributes
   # split() is very slow, so avoid it if possible
-  if (anyDuplicated(names(attribs)))
-    attribs <- lapply(split(attribs, names(attribs)), paste, collapse = " ")
+  if (anyDuplicated(names(attribs))) {
+    attribs <- lapply(split(attribs, names(attribs)), function(x) {
+      na_idx <- is.na(x)
+      if (any(na_idx)) {
+        if (all(na_idx)) {
+          return(NA)
+        }
+        x <- x[!na_idx]
+      }
+      paste(x, collapse = " ")
+    })
+  }
 
   # write attributes
   for (attrib in names(attribs)) {

tests/testthat/test-tags.r

@@ -418,6 +418,42 @@ test_that("Getting attributes from tags", {
   )
 })
 
+test_that("NA attributes are rendered correctly", {
+  expect_identical(
+    as.character(tags$div("text", foo = NA)),
+    '<div foo>text</div>'
+  )
+  expect_identical(
+    as.character(tags$div("text", class = "a", foo = NA)),
+    '<div class="a" foo>text</div>'
+  )
+  expect_identical(
+    as.character(tags$div("text", class = "a", foo = NA, class = "b")),
+    '<div class="a b" foo>text</div>'
+  )
+
+  # Multiple NA's are coalesced
+  expect_identical(
+    as.character(tags$div("text", class = "a", foo = NA, class = "b", foo = NA)),
+    '<div class="a b" foo>text</div>'
+  )
+
+  # A non-NA value supersedes NA
+  expect_identical(
+    as.character(tags$div("text", class = "a", foo = NA, foo = "b")),
+    '<div class="a" foo="b">text</div>'
+  )
+  expect_identical(
+    as.character(tags$div("text", class = "a", foo = "b", foo = NA, foo = "c")),
+    '<div class="a" foo="b c">text</div>'
+  )
+  expect_identical(
+    as.character(tags$div("text", class = "a", foo = "b", foo = NA, foo = NA, foo = "c")),
+    '<div class="a" foo="b c">text</div>'
+  )
+})
+
+
 test_that("Flattening a list of tags", {
   # Flatten a nested list
   nested <- list(