feat: as_fill_*() and as_fillable_*() now include the fill depend…

…ency (#946)

NEWS.md

@@ -1,15 +1,24 @@
 # bslib (development version)
 
-* `layout_columns()` now always uses a 12-unit grid when the user provides an integer value to `col_widths` for any breakpoint. For example, `breakpoints(md = 3, lg = NA)` will pick a best-fitting layout for large screen sizes using the 12-column grid. Previously, the best fit algorithm might adjust the number of columns as a shortcut to an easy solution. That shortcut is only taken when an auto-fit layout is requested for every breakpoint, e.g. `col_widths = breakpoints(md = NA, lg = NA)` or `col_widths = NA`. (#928)
+## Breaking changes
+
+* `as_fillable_container()`, `as_fill_item()` and `as_fill_carrier()` now always include the htmltools fill CSS dependency. This means that they are no longer usable with the `$addAttr()` `htmltools::tagQuery` method; authors should instead pass elements to the `as_fillable_container()` and `as_fill_*()` functions and use the `css_selector` argument to apply fill options to specific elements.  (#946)
+
+## Improvements
 
 * `layout_columns()` was rewritten in Typescript as a custom element to improve the portability of the component. (#931)
 
 * When `layout_columns()` is given a `row_heights` value that is not a `breakpoints()` object, that value is used for the row heights at all breakpoints. Previously, it was used for the row heights from `"sm"` up. (#931)
 
 * When `layout_columns()` is given a `col_widths` value with `breakpoints()` at `lg` or wider, it now uses a better default column width for the smaller breakpoints not listed in the `col_widths` value. That said, you can always include `sm` or `md` in your `breakpoints()` definition to have complete control over column widths at those sizes. (#931)
 
+## Bug fixes
+
+* `layout_columns()` now always uses a 12-unit grid when the user provides an integer value to `col_widths` for any breakpoint. For example, `breakpoints(md = 3, lg = NA)` will pick a best-fitting layout for large screen sizes using the 12-column grid. Previously, the best fit algorithm might adjust the number of columns as a shortcut to an easy solution. That shortcut is only taken when an auto-fit layout is requested for every breakpoint, e.g. `col_widths = breakpoints(md = NA, lg = NA)` or `col_widths = NA`. (#928)
+
 * Fixed an issue where the page might be given a window title of `NA` if the primary `title` argument of a page function, such as `page_sidebar()`, is `NULL` or a suitable window title could not be inferred. (#933)
 
+
 # bslib 0.6.1
 
 ## Bug fixes

R/fill.R

@@ -73,7 +73,10 @@ as_fill_carrier <- function(x, ..., min_height = NULL, max_height = NULL, gap =
 
   if (rlang::is_missing(x)) {
     warn_css_selector_null(css_selector)
-    attrs <- c(attrs, list(class = "html-fill-item html-fill-container"))
+    attrs <- c(
+      attrs,
+      list(class = "html-fill-item html-fill-container", fill_dependency())
+    )
     return(rlang::splice(attrs))
   }
 
@@ -98,7 +101,7 @@ as_fillable_container <- function(x, ..., min_height = NULL, max_height = NULL,
 
   if (rlang::is_missing(x)) {
     warn_css_selector_null(css_selector)
-    attrs <- c(attrs, list(class = "html-fill-container"))
+    attrs <- c(attrs, list(class = "html-fill-container", fill_dependency()))
     return(rlang::splice(attrs))
   }
 
@@ -122,7 +125,7 @@ as_fill_item <- function(x, ..., min_height = NULL, max_height = NULL, class = N
 
   if (rlang::is_missing(x)) {
     warn_css_selector_null(css_selector)
-    attrs <- c(attrs, list(class = "html-fill-item"))
+    attrs <- c(attrs, list(class = "html-fill-item", fill_dependency()))
     return(rlang::splice(attrs))
   }
 
@@ -131,6 +134,18 @@ as_fill_item <- function(x, ..., min_height = NULL, max_height = NULL, class = N
   tagAppendAttributes(x, .cssSelector = css_selector, !!!attrs)
 }
 
+fill_dependency <- local({
+  fill_dep <- NULL
+
+  function() {
+    if (!is.null(fill_dep)) return(fill_dep)
+    fill_dep <<- htmltools::findDependencies(
+      htmltools::bindFillRole(htmltools::tags$div(), container = TRUE)
+    )[[1]]
+    fill_dep
+  }
+})
+
 fillable_attributes <- function(
   min_height = NULL,
   max_height = NULL,

tests/testthat/test-fill.R

@@ -82,22 +82,7 @@ test_that("as_fill_item() with a nested tag on inner tag", {
     rep(TRUE, 3)
   )
 
-  nested_fill_tq <-
-    tagQuery(tag_nested())$
-    find(".inner")$
-    addAttrs(as_fill_item())$
-    allTags()
-
-  expect_equal(
-    renders_to_tag_class(nested_fill_tq, "html-fill-item", selector = ".inner"),
-    rep(TRUE, 3)
-  )
-
   expect_snapshot(cat(format(nested_fill)))
-  expect_equal(
-    format(nested_fill),
-    format(nested_fill_tq)
-  )
 })
 
 
@@ -177,22 +162,7 @@ test_that("as_fillable_container() with a nested tag on inner tag", {
     rep(TRUE, 3)
   )
 
-  nested_fillable_tq <-
-    tagQuery(tag_nested())$
-    find(".inner")$
-    addAttrs(as_fillable_container())$
-    allTags()
-
-  expect_equal(
-    renders_to_tag_class(nested_fillable_tq, "html-fill-container", selector = ".inner"),
-    rep(TRUE, 3)
-  )
-
   expect_snapshot(cat(format(nested_fillable)))
-  expect_equal(
-    format(nested_fillable),
-    format(nested_fillable_tq)
-  )
 })
 
 
@@ -214,10 +184,6 @@ test_that("as_fill_carrier() with a simple tag", {
   )
 
   expect_snapshot(cat(format(as_fill_carrier(tag_simple()))))
-  expect_equal(
-    format(as_fill_carrier(tag_simple())),
-    format(tag_simple(as_fill_carrier()))
-  )
 })
 
 test_that("as_fill_carrier() with a simple tag with arguments", {
@@ -259,10 +225,6 @@ test_that("as_fill_carrier() with a nested tag on outer tag", {
   )
 
   expect_snapshot(cat(format(as_fill_carrier(tag_nested()))))
-  expect_equal(
-    format(as_fill_carrier(tag_nested())),
-    format(tag_nested(as_fill_carrier()))
-  )
 })
 
 test_that("as_fill_carrier() with a nested tag on inner tag", {
@@ -286,26 +248,7 @@ test_that("as_fill_carrier() with a nested tag on inner tag", {
     rep(TRUE, 3)
   )
 
-  nested_carrier_tq <-
-    tagQuery(tag_nested())$
-    find(".inner")$
-    addAttrs(as_fill_carrier())$
-    allTags()
-
-  expect_equal(
-    renders_to_tag_class(nested_carrier_tq, "html-fill-container", selector = ".inner"),
-    rep(TRUE, 3)
-  )
-  expect_equal(
-    renders_to_tag_class(nested_carrier_tq, "html-fill-item", selector = ".inner"),
-    rep(TRUE, 3)
-  )
-
   expect_snapshot(cat(format(nested_carrier)))
-  expect_equal(
-    format(nested_carrier),
-    format(nested_carrier_tq)
-  )
 })
 
 # Remove All Fill -----------------------------------------------------------