From 591d54da10c82c348440c3bf0e8e3dc0f5f8eeb2 Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Mon, 18 Dec 2023 15:50:21 -0500 Subject: [PATCH] feat: `as_fill_*()` and `as_fillable_*()` now include the fill dependency (#946) --- NEWS.md | 11 +++++++- R/fill.R | 21 ++++++++++++-- tests/testthat/test-fill.R | 57 -------------------------------------- 3 files changed, 28 insertions(+), 61 deletions(-) diff --git a/NEWS.md b/NEWS.md index 907faad3c..28d5d1eed 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,10 @@ # 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) @@ -8,8 +12,13 @@ * 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 diff --git a/R/fill.R b/R/fill.R index a27a2735f..e48d04afc 100644 --- a/R/fill.R +++ b/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, diff --git a/tests/testthat/test-fill.R b/tests/testthat/test-fill.R index 306691f4e..ecc8d1809 100644 --- a/tests/testthat/test-fill.R +++ b/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 -----------------------------------------------------------