Favicon fix

CHANGELOG.md

@@ -2,8 +2,11 @@
 All notable changes to `dash` will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
-## [0.9.0] - 2020-10-31
+## [0.9.1] - 2020-11-16
+### Fixed
+- A regression which prevented favicons from displaying properly has been resolved, and a default Dash favicon is now supplied when none is provided in the `assets` directory. [#240](https://github.com/plotly/dashr/pull/240)
 
+## [0.9.0] - 2020-10-31
 ### Fixed
 - Fixes a minor bug in `setCallbackContext` (described in [#236](https://github.com/plotly/dashR/issues/236)) which prevented pattern-matching callbacks from working properly if one or more `input` statements did not include a selector. [#237](https://github.com/plotly/dashR/pull/237)
 

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: dash
 Title: An Interface to the Dash Ecosystem for Authoring Reactive Web Applications
-Version: 0.9.0
+Version: 0.9.1
 Authors@R: c(person("Chris", "Parmer", role = c("aut"), email = "chris@plotly.com"), person("Ryan Patrick", "Kyle", role = c("aut", "cre"), comment = c(ORCID = "0000-0001-5829-9867"), email = "ryan@plotly.com"), person("Carson", "Sievert", role = c("aut"), comment = c(ORCID = "0000-0002-4958-2844")), person("Hammad", "Khan", role = c("aut"), comment = c(ORCID = "0000-0003-2479-9841"), email = "hammadkhan@plotly.com"), person(family = "Plotly Technologies", role = "cph")) 
 Description: A framework for building analytical web applications, Dash offers a pleasant and productive development experience. No JavaScript required.
 Depends:

R/dash.R

@@ -116,7 +116,7 @@ Dash <- R6::R6Class(
       # ------------------------------------------------------------
       router <- routr::RouteStack$new()
       server$set_data("user-routes", list()) # placeholder for custom routes
- 
+
       # ensure that assets_folder is neither NULL nor character(0)
       if (!(is.null(private$assets_folder)) & length(private$assets_folder) != 0) {
         if (!(dir.exists(private$assets_folder)) && gsub("/+", "", assets_folder) != "assets") {
@@ -494,13 +494,17 @@ Dash <- R6::R6Class(
         }
         TRUE
       })
-
       dash_favicon <- paste0(self$config$routes_pathname_prefix, "_favicon.ico")
 
       route$add_handler("get", dash_favicon, function(request, response, keys, ...) {
         asset_path <- get_asset_path(private$asset_map,
                                      "/favicon.ico")
 
+        # If custom favicon is not present, get the path for the default Dash favicon
+        if (is.na(names(asset_path))) {
+          asset_path <- system.file("extdata", "favicon.ico", package = "dash")
+        }
+
         file_handle <- file(asset_path, "rb")
         response$body <- readBin(file_handle,
                                  raw(),
@@ -1891,17 +1895,18 @@ Dash <- R6::R6Class(
       # create tag for favicon, if present
       # other_files_map[names(other_files_map) %in% "/favicon.ico"]
       if ("/favicon.ico" %in% names(private$asset_map$other)) {
-        favicon <- sprintf("<link href=\"/_favicon.ico\" rel=\"icon\" type=\"image/x-icon\">")
+        favicon_url <- sprintf('\"%s_favicon.ico\"', self$config$requests_pathname_prefix)
+        favicon <- sprintf("<link href=%s rel=\"icon\" type=\"image/x-icon\">", favicon_url)
       } else {
-        favicon <- ""
+        favicon_url <- sprintf('\"%s_favicon.ico\"', self$config$requests_pathname_prefix)
+        favicon <- sprintf("<link href=%s rel=\"icon\" type=\"image/x-icon\">", favicon_url)
       }
 
       # set script tag to invoke a new dash_renderer
       scripts_invoke_renderer <- sprintf("<script id=\"%s\" type=\"%s\">%s</script>",
                                          "_dash-renderer",
                                          "application/javascript",
                                          "var renderer = new DashRenderer();")
-
       # add inline tags
       scripts_inline <- private$inline_scripts
 
@@ -1961,24 +1966,6 @@ Dash <- R6::R6Class(
         private$.index <- private$template_index
       }
 
-      # define the react-entry-point
-      app_entry <- "<div id='react-entry-point'><div class='_dash-loading'>Loading...</div></div>"
-      # define the dash default config key
-      config <- sprintf("<script id='_dash-config' type='application/json'> %s </script>", to_JSON(self$config))
-
-      if (is.null(private$name))
-        private$name <- 'Dash'
-
-      if (!is.null(private$custom_index)) {
-        string_index <- glue::glue(private$custom_index, .open = "{%", .close = "%}")
-
-        private$.index <- string_index
-      }
-
-      else if (length(private$template_index) == 1) {
-        private$.index <- private$template_index
-      }
-
       else {
         private$.index <- sprintf(
        '<!DOCTYPE html>

tests/testthat/test-attributes.R

@@ -4,7 +4,7 @@ test_that("stylesheets can be added with or without attributes", {
   library(dashHtmlComponents)
   stylesheet_pattern <- '^.*<link href="(https.*)">.*$'
   script_pattern <- '^.*<script src="(https.*)">.*$'
-      
+
   app <- Dash$new(external_stylesheets = list(
                                            list(
                                              href="https://codepen.io/chriddyp/pen/bWLwgP.css",
@@ -22,35 +22,35 @@ test_that("stylesheets can be added with or without attributes", {
                     )
                   )
           )
-  
+
   app$layout(htmlDiv(
     "Hello world!"
   )
   )
-  
+
   request_with_attributes <- fiery::fake_request(
     "http://127.0.0.1:8050"
   )
-  
+
   # start up Dash briefly to generate the index
   app$run_server(block=FALSE)
   app$server$stop()
-  
+
   response_with_attributes <- app$server$test_request(request_with_attributes)
 
   tags_by_line <- lapply(strsplit(response_with_attributes$body, "\n "), function(x) trimws(x))[[1]]
   stylesheet_hrefs <- grep(stylesheet_pattern, tags_by_line, value = TRUE)
   script_hrefs <- grep(script_pattern, tags_by_line, value = TRUE)
-    
+
   expect_equal(
     stylesheet_hrefs,
     "<link href=\"https://codepen.io/chriddyp/pen/bWLwgP.css\" hreflang=\"en-us\" rel=\"stylesheet\">"
   )
-  
+
   expect_equal(
     script_hrefs,
-    c("<script src=\"https://www.google-analytics.com/analytics.js\"></script>", 
-      "<script src=\"https://cdn.polyfill.io/v2/polyfill.min.js\"></script>", 
+    c("<script src=\"https://www.google-analytics.com/analytics.js\"></script>",
+      "<script src=\"https://cdn.polyfill.io/v2/polyfill.min.js\"></script>",
       "<script src=\"https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.10/lodash.core.js\" integrity=\"sha256-Qqd/EfdABZUcAxjOkMi8eGEivtdTkh3b65xCZL4qAQA=\" crossorigin=\"anonymous\"></script>"
     )
   )
@@ -72,20 +72,20 @@ test_that("stylesheets can be added with or without attributes", {
                     )
                   )
           )
-  
+
   app$layout(htmlDiv(
     "Hello world!"
   )
   )
-  
+
   request_with_attributes <- fiery::fake_request(
     "http://127.0.0.1:8050"
   )
-  
+
   # start up Dash briefly to generate the index
   app$run_server(block=FALSE)
   app$server$stop()
-  
+
   response_with_attributes <- app$server$test_request(request_with_attributes)
 
   tags_by_line <- lapply(strsplit(response_with_attributes$body, "\n "), function(x) trimws(x))[[1]]
@@ -110,7 +110,7 @@ test_that("stylesheets can be added with or without attributes", {
                     "&m=",
                     modtime)
 
-  all_tags <- glue::glue("<script src=\"{c(internal_hrefs[c(\"react-prod\", 
+  all_tags <- glue::glue("<script src=\"{c(internal_hrefs[c(\"react-prod\",
                                                             \"react-dom-prod\",
                                                             \"prop-types-prod\",
                                                             \"polyfill-prod\")],
@@ -125,8 +125,8 @@ test_that("stylesheets can be added with or without attributes", {
   expect_equal(
     script_hrefs,
       c(glue::glue_collapse(all_tags, sep="\n"),
-      "<script src=\"https://www.google-analytics.com/analytics.js\"></script>", 
-      "<script src=\"https://cdn.polyfill.io/v2/polyfill.min.js\"></script>", 
+      "<script src=\"https://www.google-analytics.com/analytics.js\"></script>",
+      "<script src=\"https://cdn.polyfill.io/v2/polyfill.min.js\"></script>",
       "<script src=\"https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.10/lodash.core.js\" integrity=\"sha256-Qqd/EfdABZUcAxjOkMi8eGEivtdTkh3b65xCZL4qAQA=\" crossorigin=\"anonymous\"></script>"
     )
   )
@@ -156,7 +156,7 @@ test_that("invalid attributes trigger an error", {
                         )
                       )
 
-  expect_error(dash:::assertValidExternals(external_scripts, external_stylesheets), 
+  expect_error(dash:::assertValidExternals(external_scripts, external_stylesheets),
     "The following script or stylesheet attributes are invalid: baz, foo, bar.")
 })
 
@@ -170,10 +170,10 @@ test_that("not passing named attributes triggers an error", {
                               "moredata"
                               )
                             )
-                                            
+
   external_scripts <- list()
 
-  expect_error(dash:::assertValidExternals(external_scripts, external_stylesheets), 
+  expect_error(dash:::assertValidExternals(external_scripts, external_stylesheets),
     "Please verify that all attributes are named elements when specifying URLs for scripts and stylesheets.")
 })
 
@@ -228,7 +228,7 @@ test_that("passing a list with no href/src fails", {
   library(dashHtmlComponents)
   stylesheet_pattern <- '^.*<link href="(https.*)">.*$'
   script_pattern <- '^.*<script src="(https.*)">.*$'
-      
+
   expect_error(app <- Dash$new(external_stylesheets = list(
                       href="https://codepen.io/chriddyp/pen/bWLwgP.css",
                             list(
@@ -269,3 +269,33 @@ test_that("passing a list with no href/src fails", {
                   ),
                   "A valid URL must be included with every entry in external_scripts. Please sure no 'src' entries are missing or malformed.")
 })
+
+test_that("default favicon resource is supplied when none is present in assets", {
+  library(dashHtmlComponents)
+  favicon_pattern <- '^.*<link href=".*/_favicon.*">.*$'
+
+  app <- Dash$new()
+
+  app$layout(htmlDiv(
+    "Hello world!"
+  )
+  )
+
+  request_with_attributes <- fiery::fake_request(
+    "http://127.0.0.1:8050"
+  )
+
+  # start up Dash briefly to generate the index
+  app$run_server(block=FALSE)
+  app$server$stop()
+
+  response_with_attributes <- app$server$test_request(request_with_attributes)
+
+  tags_by_line <- lapply(strsplit(response_with_attributes$body, "\n "), function(x) trimws(x))[[1]]
+  favicon_hrefs <- grep(favicon_pattern, tags_by_line, value = TRUE)
+
+  expect_equal(
+    favicon_hrefs,
+    "<link href=\"/_favicon.ico\" rel=\"icon\" type=\"image/x-icon\">"
+  )
+})