fix: fix and improve align()'s default align argument

- written by @keithnewman

See #623

DESCRIPTION

@@ -1,7 +1,7 @@
 Package: flextable
 Type: Package
 Title: Functions for Tabular Reporting
-Version: 0.9.6.009
+Version: 0.9.6.010
 Authors@R: c(
     person("David", "Gohel", role = c("aut", "cre"),
     email = "[email protected]"),
@@ -16,7 +16,8 @@ Authors@R: c(
     person("Paul", "Julian", role = "ctb", comment = "support for gam objects"),
     person("Sean", "Browning", role = "ctb", comment = "work on footnote positioning system"),
     person("Rémi", "Thériault", role = "ctb", comment = c(ORCID = "0000-0003-4315-6788", ctb = "theme_apa")),
-    person("Samuel", "Jobert", role = "ctb", comment = "work on pagination")
+    person("Samuel", "Jobert", role = "ctb", comment = "work on pagination"),
+    person("Keith", "Newman", role = "ctb")
     )
 Description: Use a grammar for creating and customizing pretty tables.
  The following formats are supported: 'HTML', 'PDF', 'RTF', 

NEWS.md

@@ -4,10 +4,28 @@
 
 - `headers_flextable_at_bkm()` and `footers_flextable_at_bkm()` are defunct.
 - `flextable_to_rmd()` is now using `knit_child()` for safer usage from `for`
-loops or `if` statements.
-- Add explanation about caption limitations in the manual of functions 
-`save_as_image()` and `ph_with.flextable()`.
-- Deprecate `as_raster()` since `gen_grob()` is easier to use and render nicer.
+  loops or `if` statements.
+- Add explanation about caption limitations in the manual of functions
+  `save_as_image()` and `ph_with.flextable()`.
+- Deprecate `as_raster()` since `gen_grob()` is easier to use and render
+  nicer.
+- BREAKING CHANGE: in `align()`, the default argument value for `align` is now
+  `"left"`, rather than `c("left", "center", "right", "justify")`. This
+  returns the default value to how it was in older versions of {flextable}.
+    - in `align()`, use of the old default `align` argument could cause an
+      error if the number of columns being adjusted was not a multiple of 4.
+    - The documentation specified that `align` had to be a single value, when
+      it could actually accept multiple values. This is why a default value of
+      `c("left", "center", "right", "justify")`, was problematic. This
+      documentation has now been updated and new examples included in the
+      documentation.
+    - The default `align` argument will now apply left alignment to all
+      columns in the body.
+    - If the user specifies an alignment that is invalid, a error will be
+      displayed.
+    - The `path` argument now has a signature of `part = c("body", "header",
+      "footer", "all")`, but because only a single value can be selected, it
+      will pick `"body"` by default, as before.
 
 ## Issues
 

R/as_flextable.tabular.R

@@ -171,9 +171,13 @@ as_flextable.tabular <- function(x,
 
   for (j in columns_keys) {
     ft <- align(ft, j = j, align = text_align$header[, j], part = "header")
+    aligns <- text_align$body[, j]
+    if (".type" %in% colnames(text_align$body)) {
+      aligns <- aligns[text_align$body$.type %in% c("one_row", "list_data")]
+    }
     ft <- align(ft,
       j = j, i = ~ .type %in% c("one_row", "list_data"),
-      align = text_align$body[, j], part = "body"
+      align = aligns, part = "body"
     )
   }
 

R/styles.R

@@ -600,24 +600,46 @@ padding <- function(x, i = NULL, j = NULL, padding = NULL,
 #' @param x a flextable object
 #' @param i rows selection
 #' @param j columns selection
-#' @param part partname of the table (one of 'all', 'body', 'header', 'footer')
-#' @param align text alignment - character values, expected value
-#' must be 'left', 'right', 'center', 'justify'. It can be a single value or
-#' multiple values, the argument is vectorized over columns.
+#' @param part partname of the table (one of 'body', 'header', 'footer', 'all')
+#' @param align text alignment - a single character value, or a vector of
+#' character values equal in length to the number of columns selected by `j`.
+#' Expected values must be from the set of ('left', 'right', 'center', or 'justify').
+#'
+#' If the number of columns is a multiple of the length of the `align` parameter,
+#' then the values in `align` will be recycled across the remaining columns.
 #' @family sugar functions for table style
 #' @examples
-#' ft <- flextable(head(mtcars)[, 3:6])
-#' ft <- align(ft, align = "right", part = "all")
-#' ft <- theme_tron_legacy(ft)
-#' ft
-align <- function(x, i = NULL, j = NULL, align = "left", part = "body") {
+#' # Table of 6 columns
+#' ft_car <- flextable(head(mtcars)[, 2:7])
+#'
+#' # All 6 columns right aligned
+#' align(ft_car, align = "right", part = "all")
+#'
+#' # Manually specify alignment of each column
+#' align(
+#'   ft_car,
+#'   align = c("left", "right", "left", "center", "center", "right"),
+#'   part = "all")
+#'
+#' # Center-align column 2 and left-align column 5
+#' align(ft_car, j = c(2, 5), align = c("center", "left"), part = "all")
+#'
+#' # Alternate left and center alignment across columns 1-4 for header only
+#' align(ft_car, j = 1:4, align = c("left", "center"), part = "header")
+align <- function(x, i = NULL, j = NULL, align = "left", part = c("body", "header", "footer", "all")) {
   if (!inherits(x, "flextable")) {
     stop(sprintf("Function `%s` supports only flextable objects.", "align()"))
   }
-  part <- match.arg(part, c("all", "body", "header", "footer"), several.ok = FALSE)
+  part <- match.arg(part, c("body", "header", "footer", "all"), several.ok = FALSE)
 
-  if (any(!align %in% c("left", "center", "right", "justify"))) {
-    stop("align values can only be 'left', 'center', 'right', 'justify'.")
+  allowed_alignments <- c("left", "center", "right", "justify")
+  if (!all(align %in% allowed_alignments)) {
+    stop(
+      "Invalid `align` argument(s) provided to `align()`: \"",
+      paste(setdiff(align, allowed_alignments), collapse = "\", \""),
+      "\".\n  `align` should be one of: \"", paste(allowed_alignments, collapse = "\", \""),
+      "\"."
+    )
   }
 
   if (part == "all") {
@@ -635,7 +657,7 @@ align <- function(x, i = NULL, j = NULL, align = "left", part = "body") {
   i <- get_rows_id(x[[part]], i)
   j <- get_columns_id(x[[part]], j)
 
-  if (length(align) == length(j)) {
+  if (length(j) %% length(align) == 0) {
     align <- rep(align, each = length(i))
   }
 

tests/testthat/test-styles.R

@@ -85,12 +85,82 @@ test_that("borders with office docs are sanitized", {
   expect_equal(xml_attr(bot_nodes, "w"), c("0", "12700", "0", "12700"))
 })
 
-test_that("align is vectorized over columns", {
-  ft <- flextable(head(mtcars[, 2:6]))
-  align_vals <- c("center", "right", "right", "right", "right")
-  ft <- align(ft, align = align_vals, part = "all")
+test_that("align() accepts default align argument when columns is not a multiple of 4", {
+  ft <- flextable(head(mtcars, n = 2)[, 1:6])
+  ft1 <- align(ft, part = "all")
   expect_equal(
-    rep(align_vals, 7),
-    information_data_paragraph(ft)$text.align
+    rep("left", 18),
+    information_data_paragraph(ft1)$text.align
   )
 })
+
+test_that("align() accepts combinations of align arguments.", {
+  ft <- flextable(head(mtcars, n = 2)[, 1:6])
+
+  # All columns right-aligned
+  ft2 <- align(ft, align = "right", part = "all")
+  expect_equal(
+    rep("right", 18),
+    information_data_paragraph(ft2)$text.align
+  )
+
+  # Custom alignment for each column
+  custom_alignment <- c("left", "right", "left", "center", "center", "right")
+  ft3 <- align(ft, align = custom_alignment, part = "all")
+  expect_equal(
+    rep(custom_alignment, 3),
+    information_data_paragraph(ft3)$text.align
+  )
+
+  # Custom alignment for only columns 3 and 5 in body only
+  custom_alignment <- c("center", "left")
+  ft4 <- align(ft, j = c("disp", "drat"), align = custom_alignment, part = "body")
+  subdat <- information_data_paragraph(ft4)
+  subdat <- subdat[subdat$.col_id %in% c("disp", "drat"),]
+  subdat <- subdat[subdat$.part %in% c("body"),]
+  expect_equal(
+    rep(custom_alignment, 2),
+    subdat$text.align
+  )
+
+  # Custom alignment for only columns 3 and 5 in body only (using default body arg)
+  ft4b <- align(ft, j = c("disp", "drat"), align = custom_alignment)
+  subdat <- information_data_paragraph(ft4b)
+  subdat <- subdat[subdat$.col_id %in% c("disp", "drat"),]
+  subdat <- subdat[subdat$.part %in% c("body"),]
+  expect_equal(
+    rep(custom_alignment, 2),
+    subdat$text.align
+  )
+
+  # Center alignment for only columns 3 and 5
+  ft5 <- align(ft, j = c("disp", "drat"), align = "center", part = "all")
+  subdat <- information_data_paragraph(ft5)
+  subdat <- subdat[subdat$.col_id %in% c("disp", "drat"),]
+  expect_equal(
+    rep("center", 6),
+    subdat$text.align
+  )
+
+  # Alternate left and center alignment across columns 1-4 for header only
+  ft6 <- align(ft, j = 1:4, align = c("left", "center"), part = "header")
+  subdat <- information_data_paragraph(ft6)
+  subdat <- subdat[subdat$.part %in% c("header"),]
+  expect_equal(
+    c(rep(c("left", "center"), 2), "right", "right"),
+    subdat$text.align
+  )
+})
+
+test_that("align() will error if invalid align and part arguments are supplied", {
+  ft <- flextable(head(mtcars, n = 2)[, 1:6])
+
+  # Invalid "part" argument
+  expect_error(align(ft, align = c("left", "center", "right"), part = "everything"))
+
+  # Invalid "align" argument
+  expect_error(align(ft, align = "top"))
+
+  # Invalid "align" argument mixed in with valid arguments throws warning
+  expect_error(align(ft, align = c("top", "left")), "Invalid `align` argument")
+})