Merge pull request #125 from rstudio/parse-length-unit-fix

Allow percentage values to be used as `height` and `width` args
rstudio · Nov 15, 2024 · a5c698e · a5c698e
2 parents e0a38c2 + efe9bd3
commit a5c698e
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 24 deletions.
diff --git a/R/fa.R b/R/fa.R
@@ -108,27 +108,46 @@ fa <- function(
   height_num <- parse_length_unit(height)
   width_num <- parse_length_unit(width)
 
-  # Fill in height/width defaults
   if (is.null(height) && is.null(width)) {
 
+    # Fill in height/width defaults where `width` and `height` not provided
+
     height <- "1em"
     width <- paste0(round(icon_width / 512, 2), "em")
 
   } else if (!is.null(height) && is.null(width)) {
 
-    width <-
-      paste0(
-        round((icon_width / 512) * height_num, 2),
-        attr(height_num, "unit")
-      )
+    # Case where `height` is provided but not `width`
+
+    if (grepl("%$", height)) {
+
+      width <- height
+
+    } else {
+
+      width <-
+        paste0(
+          round((icon_width / 512) * height_num, 2),
+          attr(height_num, "unit")
+        )
+    }
 
   } else if (is.null(height) && !is.null(width)) {
 
-    height <-
-      paste0(
-        round(width_num / (icon_width / 512), 2),
-        attr(width_num, "unit")
-      )
+    # Case where `width` is provided but not `height`
+
+    if (grepl("%$", width)) {
+
+      height <- width
+
+    } else {
+
+      height <-
+        paste0(
+          round(width_num / (icon_width / 512), 2),
+          attr(width_num, "unit")
+        )
+    }
   }
 
   # Generate accessibility attributes if either of
@@ -192,7 +211,7 @@ parse_length_unit <- function(css_length) {
     return(NULL)
   }
 
-  if (!grepl("^^[0-9]*\\.?[0-9]+[a-z]+$", css_length)) {
+  if (!grepl("^^[0-9]*\\.?[0-9]+[a-z%]+$", css_length)) {
 
     stop(
       "Values provided to `height` and `width` must have a numerical value ",
@@ -210,7 +229,7 @@ parse_length_unit <- function(css_length) {
     )
   }
 
-  value <- as.numeric(gsub("[a-z]+$", "", css_length))
+  value <- as.numeric(gsub("[a-z%]+$", "", css_length))
   attr(value, "unit") <- unit
   value
 }

diff --git a/tests/testthat/test-fa_icon.R b/tests/testthat/test-fa_icon.R
@@ -91,40 +91,64 @@ test_that("Inserting attributes and styles works for FA icons", {
     "stroke:blue;stroke-width:2px;stroke-opacity:0.5;"
   )
 
+  # Expect that not providing width or height results in default values
+  expect_match(
+    as.character(fa(name = "file")),
+    "height:1em;width:0.75em;"
+  )
+
   # Expect that the `height = "30em"` CSS rule is rendered
   expect_match(
     as.character(fa(name = "file", height = "30em")),
     "height:30em;"
   )
 
-  # Expect a default height of 1em
+  # Expect that the `width = "1em"` CSS rule is rendered and the
+  # default height of 1em is adjusted
   expect_match(
-    as.character(fa(name = "file")),
-    "height:1em;"
+    as.character(fa(name = "file", width = "1em")),
+    "height:1.33em;width:1em;"
   )
 
-  # Expect that the `width = "1em"` CSS rule is rendered
+  # Expect that width and height values as percentages are rendered
   expect_match(
-    as.character(fa(name = "file", width = "1em")),
-    "width:1em;"
+    as.character(fa(name = "file", width = "53%")),
+    "height:53%;width:53%;"
+  )
+  expect_match(
+    as.character(fa(name = "file", width = "62%")),
+    "height:62%;width:62%;"
+  )
+  expect_match(
+    as.character(fa(name = "file", height = "53%", width = "62%")),
+    "height:53%;width:62%;"
+  )
+
+  # Expect that mixed CSS lengths and percentages will be passed through
+  expect_match(
+    as.character(fa(name = "file", height = "2em", width = "62%")),
+    "height:2em;width:62%;"
+  )
+  expect_match(
+    as.character(fa(name = "file", height = "53%", width = "14px")),
+    "height:53%;width:14px;"
   )
 
   # Expect that fractional width values are rendered properly
   expect_match(
     as.character(fa(name = "file", width = "0.75em")),
-    "width:0.75em;"
+    "height:1em;width:0.75em;"
   )
   expect_match(
-    as.character(fa(name = "file", width = ".75em")),
-    "width:.75em;"
+    as.character(fa(name = "file", height = ".75em")),
+    "height:.75em;width:0.56em;"
   )
-
   expect_match(
     as.character(fa(name = "file", width = ".756789em")),
     "width:.756789em;"
   )
 
-  # Expect that not supplying a width value will result in an error
+  # Expect that not supplying a value width value will result in an error
   expect_error(
     as.character(fa(name = "file", width = "em"))
   )