Merge pull request #1627 from obsaditelnost/master

fix for fmt-functions in combination with md/html-stubs
rstudio · Apr 20, 2024 · 69f431c · 69f431c · github-actions · Apr 20, 2024
2 parents 4da6952 + 27f190e
commit 69f431c
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 9 deletions.
diff --git a/R/resolver.R b/R/resolver.R
@@ -381,7 +381,9 @@ resolve_rows_l <- function(
 ) {
 
   if (is_gt_tbl(data = data)) {
-    row_names <- dt_stub_df_get(data = data)$row_id
+    # unlist because dt_stub_df_get might return a list instead of a vector
+    # (when helper functions such as md/html were used)
+    row_names <- unlist(dt_stub_df_get(data = data)$row_id)
     data <- dt_data_get(data = data)
   } else {
     row_names <- row.names(data)

diff --git a/tests/testthat/test-fmt_percent.R b/tests/testthat/test-fmt_percent.R
@@ -412,3 +412,34 @@ test_that("The `fmt_percent()` fn can render in the Indian numbering system", {
     )
   )
 })
+
+test_that("The `fmt_percent()` function works correctly with stubs", {
+  tbl <- tibble::tibble(
+    raw = c("[shiny](https://shiny.posit.co/)", "<a href='https://gt.rstudio.com/'>gt</a>"),
+    markdown = purrr::map(c("[shiny](https://shiny.posit.co/)", "[gt](https://gt.rstudio.com/)"), gt::md),
+    html = purrr::map(c("<a href='https://shiny.posit.co/'>shiny</a>", "<a href='https://gt.rstudio.com/'>gt</a>"), gt::html),
+    str_col = c("shiny", "gt"),
+    pct_col = c(0.75, 0.25)
+  )
+
+  # nothing special (raw)
+  # stub as html
+  # stub as markdown
+
+  for (test_case in c("raw", "markdown", "html")) {
+    tab <- tbl |>
+      gt(rowname_col = test_case) |>
+      fmt_percent(columns = pct_col, decimals = 2, dec_mark = ".") |>
+      render_formats_test(context = "html")
+
+    expect_equal(tab[["pct_col"]], c("75.00%", "25.00%"))
+
+    # with row filter
+    tab <- tbl |>
+      gt(rowname_col = test_case) |>
+      fmt_percent(columns = pct_col, decimals = 2, dec_mark = ".", rows = contains("gt")) |>
+      render_formats_test(context = "html")
+
+    expect_equal(tab[["pct_col"]], c("0.75", "25.00%"))
+  }
+})
diff --git a/tests/testthat/test-gtsave.R b/tests/testthat/test-gtsave.R
@@ -133,7 +133,7 @@ test_that("HTML saving with `gtsave()` is successful with different path defs",
   #
 
   # Form final path, check for non-existence
-  path_1 <- tempfile(fileext = ".html")
+  path_1 <- normalizePath(tempfile(fileext = ".html"), winslash = "/", mustWork = F)
   on.exit(unlink(path_1))
   expect_false(file.exists(path_1))
 
@@ -145,7 +145,7 @@ test_that("HTML saving with `gtsave()` is successful with different path defs",
   path_n <- length(split_path)
 
   # Set working directory
-  setwd(file.path(paste0("/", paste(split_path[2:3], collapse = "/"))))
+  setwd(file.path(paste0(ifelse(.Platform$OS.type == "windows", split_path[1], ""), "/", paste(split_path[2:3], collapse = "/"))))
 
   # Write the file
   exibble %>%
@@ -167,7 +167,7 @@ test_that("HTML saving with `gtsave()` is successful with different path defs",
   #
 
   # Form final path, check for non-existence
-  path_2 <- tempfile(fileext = ".html")
+  path_2 <- normalizePath(tempfile(fileext = ".html"), winslash = "/", mustWork = F)
   on.exit(unlink(path_2))
   expect_false(file.exists(path_2))
 
@@ -179,7 +179,7 @@ test_that("HTML saving with `gtsave()` is successful with different path defs",
   path_n <- length(split_path)
 
   # Set working directory
-  setwd(file.path(paste0("/", paste(split_path[2:3], collapse = "/"))))
+  setwd(file.path(paste0(ifelse(.Platform$OS.type == "windows", split_path[1], ""), "/", paste(split_path[2:3], collapse = "/"))))
 
   # Write the file
   exibble %>%
@@ -221,6 +221,8 @@ test_that("HTML saving with `gtsave()` is successful with different path defs",
       tidy_grepl("<!DOCTYPE html>")
   )
 
+  skip_on_os("windows")
+
   # [#4] Filename starting with ~/, absolute path (expect that path is ignored)
 
   # Form final path, check for non-existence
@@ -276,7 +278,7 @@ test_that("HTML saving with `gt_save_html()` with different path defs works", {
   #
 
   # Form final path, check for non-existence
-  path_1 <- tempfile(fileext = ".html")
+  path_1 <- normalizePath(tempfile(fileext = ".html"), winslash = "/", mustWork = F)
   on.exit(unlink(path_1))
   expect_false(file.exists(path_1))
 
@@ -288,7 +290,7 @@ test_that("HTML saving with `gt_save_html()` with different path defs works", {
   path_n <- length(split_path)
 
   # Set working directory
-  setwd(file.path(paste0("/", paste(split_path[2:3], collapse = "/"))))
+  setwd(file.path(paste0(ifelse(.Platform$OS.type == "windows", split_path[1], ""), "/", paste(split_path[2:3], collapse = "/"))))
 
   # Write the file
   exibble %>%
@@ -310,7 +312,7 @@ test_that("HTML saving with `gt_save_html()` with different path defs works", {
   #
 
   # Form final path, check for non-existence
-  path_2 <- tempfile(fileext = ".html")
+  path_2 <- normalizePath(tempfile(fileext = ".html"), winslash = "/", mustWork = F)
   on.exit(unlink(path_2))
   expect_false(file.exists(path_2))
 
@@ -322,7 +324,7 @@ test_that("HTML saving with `gt_save_html()` with different path defs works", {
   path_n <- length(split_path)
 
   # Set working directory
-  setwd(file.path(paste0("/", paste(split_path[2:3], collapse = "/"))))
+  setwd(file.path(paste0(ifelse(.Platform$OS.type == "windows", split_path[1], ""), "/", paste(split_path[2:3], collapse = "/"))))
 
   # Write the file
   exibble %>%
@@ -364,6 +366,8 @@ test_that("HTML saving with `gt_save_html()` with different path defs works", {
       tidy_grepl("<!DOCTYPE html>")
   )
 
+  skip_on_os("windows")
+
   # [#4] Filename starting with ~/, absolute path (expect that path is ignored)
 
   # Form final path, check for non-existence