rstudio · cpsievert · May 31, 2023 · May 19, 2023 · May 19, 2023 · May 19, 2023
diff --git a/NEWS.md b/NEWS.md
@@ -8,11 +8,13 @@ This significant release focuses on making dashboards (e.g., `page_sidebar()`) a
 
 * `card_body()` now provides the same behavior as `card_body_fill()` (i.e., it is both a fillable container and fill item) by default. And, now, since `card_body()` can do everything `card_body_fill()` can do, `card_body_fill()` has been deprecated. The main benefit of this change is that `card(full_screen = TRUE, ...)` with output(s) passed to `...` "just works" in an intuitive way. To revert to the previous behavior, set `fillable = FALSE` and `fill = FALSE` in calls to `card_body()` and set `wrapper = function(x) card_body(x, fillable = FALSE, fill = FALSE)` in calls to `card()`. (#498)
 * Closed #375: `margin-top` is no longer included on header tags that aren't created via pandoc. If this negatively impacts spacing above headers, consider adding a suitable [utility class](https://rstudio.github.io/bslib/articles/utility-classes.html) (for example, change `shiny::titlePanel("My title")` to `tagAppendAttributes(titlePanel("My title"), class = "mt-3", .selector = "h2")`). (#396)
-* `page_fill()` (now called `page_fillable()`) had several breaking changes (listed below). If this breaks existing behavior, consider using `shiny::fillPage(theme = bslib::bs_theme(), ...)` instead of `page_fill()`.
+* `page_fill()` (now called `page_fillable()`) had several breaking changes (listed below) to better accommodate filling layouts. If this breaks existing behavior, consider using `shiny::fillPage(theme = bslib::bs_theme(), ...)` instead of `page_fill()`.
   * `page_fill()` now produces a `<body>` tag with `display:flex` (instead of `display:block`).
-  * `page_fill()` no longer fills the windows height on mobile (i.e., narrow screens) by default (set `fill_mobile = TRUE` to restore the old behavior).
+  * `page_fill()` no longer fills the windows height on mobile (i.e., narrow screens) by default (set `fillable_mobile = TRUE` to restore the old behavior).
   * `page_fill()` now adds `padding` and `gap` by default, set `padding = 0` and `gap = 0` to restore the old behavior.
-* `page_navbar()` (and consequently `shiny::navbarPage()`) no longer implicitly wrap `header` and `footer` in an additional `shiny::fluidRow()` container for Bootstrap 5+ (i.e., `theme = bs_theme()`) usage. Similarly, `navset_bar()` no longer does this (for any version of Bootstrap). If this breaks existing behavior, consider wrapping the `header` and `footer` value(s) with `shiny::fluidRow()`). (#479)
+* `page_navbar()` (and also `shiny::navbarPage()` with `theme = bs_theme()`) had a couple breaking changes:
+  * The container of each page is now `display:flex` (instead of `display:block`). If this breaks existing behavior, set `page_navbar()`'s `fillable` argument to `FALSE`.
+  * `header` and `footer` is no longer wrapped in an additional `shiny::fluidRow()` container. If this breaks existing behavior, consider wrapping the `header` and `footer` value(s) with `shiny::fluidRow()`). (#479)
 * `layout_column_wrap()`'s `fill` argument now controls whether or not the _layout container_ is allowed to grow/shrink to fit a fillable container (e.g., `page_fillable()`). It also gains a new `fillable` argument for controlling whether _UI elements_ are allowed to fill their row height. This is more consistent with the meaning of `fill` in other functions, like `card()`, `card_body()`, `layout_sidebar()`, etc. (#498)
 * Defaults for the following Bootstrap 5 Sass variables were changed to `null`: `$accordion-button-active-bg`, `$accordion-button-active-color`, and `$accordion-icon-active-color`. To restore the old behavior, do `bs_add_variables(theme, "accordion-button-active-bg" = "tint-color($component-active-bg, 90%)", "accordion-button-active-color" = "shade-color($primary, 10%)", "accordion-icon-active-color" = "$accordion-button-active-color", .where = "declarations")`. (#475)
 
@@ -25,7 +27,7 @@ This significant release focuses on making dashboards (e.g., `page_sidebar()`) a
 * `page_navbar()`, `navset_card_tab()`, and ` navset_card_pill()` gain a `sidebar` argument for putting a `sidebar()` on every page/tab/pill. (#479)
 * `page_navbar()` gains a `fillable` argument to make the content of particular page(s) fit the window/card. (#479)
 * `page_fillable()` (aka, `page_fill()`) is now considered a `fillable` container, meaning that `fill` items like `card()`, `layout_column_wrap()`, and `layout_sidebar()` now grow/shrink to fit the window's height when they appear as a direct child of `page_fillable()`. (#479)
-* `page_navbar()` and `page_fillable()` gain `fill_mobile` arguments to control whether the page should grow/shrink to fit the viewport on mobile. (#479)
+* `page_navbar()` and `page_fillable()` gain `fillable_mobile` arguments to control whether the page should grow/shrink to fit the viewport on mobile. (#479)
 * `card()`, `value_box()`, and `card_image()` gain `max_height` and `fill` arguments. (#498)
 * `card_body()` gains a `padding` argument. (#587)
 * Added new `as_fill()`, `as_fillable()`, `as_fill_carrier()`, `is_fill()`, and `is_fillable()` for testing and coercing potential to fill. (#498)

diff --git a/R/layout.R b/R/layout.R
@@ -167,7 +167,7 @@ layout_column_wrap <- function(
 #' )
 #'
 #' page_fillable(
-#'   fill_mobile = TRUE,
+#'   fillable_mobile = TRUE,
 #'   layout_columns(
 #'     col_widths = breakpoints(
 #'       sm = c(12, 12, 12),

diff --git a/R/navs-legacy.R b/R/navs-legacy.R
@@ -64,6 +64,7 @@ navset_hidden <- function(..., id = NULL, selected = NULL,
 }
 
 #' @inheritParams shiny::navbarPage
+#' @inheritParams page_fillable
 #' @param sidebar A [sidebar()] component to display on every [nav_panel()]
 #'   page.
 #' @param fillable Whether or not to allow `fill` items to grow/shrink to fit
@@ -77,7 +78,8 @@ navset_hidden <- function(..., id = NULL, selected = NULL,
 #' @export
 #' @rdname navset
 navset_bar <- function(..., title = NULL, id = NULL, selected = NULL,
-                     sidebar = NULL, fillable = FALSE,
+                     sidebar = NULL, fillable = TRUE,
+                     gap = NULL, padding = NULL,
                      # TODO: add sticky-top as well?
                      position = c("static-top", "fixed-top", "fixed-bottom"),
                      header = NULL, footer = NULL,
@@ -86,6 +88,7 @@ navset_bar <- function(..., title = NULL, id = NULL, selected = NULL,
   navs_bar_(
     ..., title = title, id = id, selected = selected,
     sidebar = sidebar, fillable = fillable,
+    gap = gap, padding = padding,
     position = position,
     header = header, footer = footer,
     bg = bg, inverse = inverse,
@@ -101,7 +104,8 @@ navset_bar <- function(..., title = NULL, id = NULL, selected = NULL,
 # we can use addition theme information as an indication of whether we need
 # to handle backwards compatibility
 navs_bar_ <- function(..., title = NULL, id = NULL, selected = NULL,
-                      sidebar = NULL, fillable = FALSE,
+                      sidebar = NULL, fillable = TRUE,
+                      gap = NULL, padding = NULL,
                       position = c("static-top", "fixed-top", "fixed-bottom"),
                       header = NULL, footer = NULL,
                       bg = NULL, inverse = "auto",
@@ -120,6 +124,7 @@ navs_bar_ <- function(..., title = NULL, id = NULL, selected = NULL,
   navbar <- navbarPage_(
     title = title, ..., id = id, selected = selected,
     sidebar = sidebar, fillable = fillable,
+    gap = gap, padding = padding,
     position = match.arg(position),
     header = header, footer = footer, collapsible = collapsible,
     inverse = inverse, fluid = fluid,
@@ -146,7 +151,9 @@ navbarPage_ <- function(title,
                        id = NULL,
                        selected = NULL,
                        sidebar = NULL,
-                       fillable = FALSE,
+                       fillable = TRUE,
+                       gap = NULL,
+                       padding = NULL,
                        position = c("static-top", "fixed-top", "fixed-bottom"),
                        header = NULL,
                        footer = NULL,
@@ -224,7 +231,7 @@ navbarPage_ <- function(title,
 
   # If fillable is truthy, give the relevant .tab-content > .tab-pane containers
   # the potential to fill
-  tabset$content <- makeTabsFillable(tabset$content, fillable, navbar = TRUE)
+  tabset$content <- makeTabsFillable(tabset$content, fillable, navbar = TRUE, gap = gap, padding = padding)
 
   # For backwards compatibility reasons, wrap header & footer in a .row
   # container if we're not using BS5+. I'm not entirely sure what the motivation

diff --git a/R/navs.R b/R/navs.R
@@ -108,7 +108,7 @@ navs_card_body <- function(content, sidebar) {
 
 # Given a .tab-content container, mark each relevant .tab-pane as a
 # fill container/item.
-makeTabsFillable <- function(content, fillable = FALSE, navbar = FALSE) {
+makeTabsFillable <- function(content, fillable = TRUE, navbar = FALSE, gap = NULL, padding = NULL) {
   if (!inherits(content, "shiny.tag") || !tagQuery(content)$hasClass("tab-content")) {
     abort("Expected `content` to be a tag with a tab-content class")
   }
@@ -127,8 +127,13 @@ makeTabsFillable <- function(content, fillable = FALSE, navbar = FALSE) {
 
       if (isTRUE(fillable) || isTRUE(tagGetAttribute(x, "data-value") %in% fillable)) {
         x <- tagAppendAttributes(
-          # Remove the margin between nav and content (for page_navbr())
-          style = css("--bslib-navbar-margin" = if (navbar) 0),
+          class = "bslib-gap-spacing",
+          style = css(
+            # Remove the margin between nav and content (for page_navbar())
+            "--bslib-navbar-margin" = if (navbar) 0,
+            gap = gap,
+            padding = padding
+          ),
           bindFillRole(x, container = TRUE, item = TRUE)
         )
       }

diff --git a/R/page.R b/R/page.R
@@ -37,13 +37,20 @@ page_fixed <- function(..., title = NULL, theme = bs_theme(), lang = NULL) {
 }
 
 #' @rdname page
-#' @inheritParams shiny::fillPage
-#' @param fill_mobile Whether or not the page should fill the viewport's
+#' @param padding Padding to use for the body. This can be a numeric vector
+#'   (which will be interpreted as pixels) or a character vector with valid CSS
+#'   lengths. The length can be between one and four. If one, then that value
+#'   will be used for all four sides. If two, then the first value will be used
+#'   for the top and bottom, while the second value will be used for left and
+#'   right. If three, then the first will be used for top, the second will be
+#'   left and right, and the third will be bottom. If four, then the values will
+#'   be interpreted as top, right, bottom, and left respectively.
+#' @param fillable_mobile Whether or not the page should fill the viewport's
 #'   height on mobile devices (i.e., narrow windows).
 #' @param gap A [CSS length unit][htmltools::validateCssUnit()] defining the
 #'   `gap` (i.e., spacing) between elements provided to `...`.
 #' @export
-page_fillable <- function(..., padding = NULL, gap = NULL, fill_mobile = FALSE, title = NULL, theme = bs_theme(), lang = NULL) {
+page_fillable <- function(..., padding = NULL, gap = NULL, fillable_mobile = FALSE, title = NULL, theme = bs_theme(), lang = NULL) {
   page(
     title = title,
     theme = theme,
@@ -55,7 +62,7 @@ page_fillable <- function(..., padding = NULL, gap = NULL, fill_mobile = FALSE,
         style = css(
           padding = validateCssPadding(padding),
           gap = validateCssUnit(gap),
-          "--bslib-page-fill-mobile-height" = if (fill_mobile) "100%" else "auto"
+          "--bslib-page-fill-mobile-height" = if (fillable_mobile) "100%" else "auto"
         ),
         ...
       ),
@@ -114,7 +121,7 @@ validateCssPadding <- function(padding = NULL) {
 #'
 #' shinyApp(ui, server)
 #'
-page_sidebar <- function(..., sidebar = NULL, title = NULL, fillable = TRUE, fill_mobile = FALSE, theme = bs_theme(), window_title = NA, lang = NULL) {
+page_sidebar <- function(..., sidebar = NULL, title = NULL, fillable = TRUE, fillable_mobile = FALSE, theme = bs_theme(), window_title = NA, lang = NULL) {
 
   if (rlang::is_bare_character(title) || rlang::is_bare_numeric(title)) {
     title <- h1(title, class = "bslib-page-title")
@@ -126,7 +133,7 @@ page_sidebar <- function(..., sidebar = NULL, title = NULL, fillable = TRUE, fil
     title = infer_window_title(title, window_title),
     theme = theme,
     lang = lang,
-    fill_mobile = fill_mobile,
+    fillable_mobile = fillable_mobile,
     title,
     layout_sidebar(
       sidebar = sidebar,
@@ -140,16 +147,16 @@ page_sidebar <- function(..., sidebar = NULL, title = NULL, fillable = TRUE, fil
 
 #' @rdname page
 #' @inheritParams navset_bar
-#' @inheritParams bs_page
 #' @seealso [shiny::navbarPage()]
-#' @param fill_mobile Whether or not `fillable` pages should fill the viewport's
+#' @param fillable_mobile Whether or not `fillable` pages should fill the viewport's
 #'   height on mobile devices (i.e., narrow windows).
 #' @param window_title the browser window title. The default value, `NA`, means
 #'   to use any character strings that appear in `title` (if none are found, the
 #'   host URL of the page is displayed by default).
 #' @export
 page_navbar <- function(..., title = NULL, id = NULL, selected = NULL,
-                        sidebar = NULL, fillable = FALSE, fill_mobile = FALSE,
+                        sidebar = NULL, fillable = TRUE, fillable_mobile = FALSE,
+                        gap = NULL, padding = NULL,
                         position = c("static-top", "fixed-top", "fixed-bottom"),
                         header = NULL, footer = NULL,
                         bg = NULL, inverse = "auto",
@@ -163,7 +170,7 @@ page_navbar <- function(..., title = NULL, id = NULL, selected = NULL,
   page_func <- if (isFALSE(fillable) && is.null(sidebar)) {
     page
   } else {
-    function(...) page_fillable(..., fill_mobile = fill_mobile, padding = 0, gap = 0)
+    function(...) page_fillable(..., fillable_mobile = fillable_mobile, padding = 0, gap = 0)
   }
 
   page_func(
@@ -173,6 +180,7 @@ page_navbar <- function(..., title = NULL, id = NULL, selected = NULL,
     navs_bar_(
       ..., title = title, id = id, selected = selected,
       sidebar = sidebar, fillable = fillable,
+      gap = gap, padding = padding,
       position = match.arg(position), header = header,
       footer = footer, bg = bg, inverse = inverse,
       collapsible = collapsible, fluid = fluid,
@@ -181,7 +189,6 @@ page_navbar <- function(..., title = NULL, id = NULL, selected = NULL,
   )
 }
 
-
 # https://github.com/rstudio/shiny/issues/2310
 infer_window_title <- function(title = NULL, window_title = NA) {
   if (!isTRUE(is.na(window_title))) {

diff --git a/R/sysdata.rda b/R/sysdata.rda
diff --git a/inst/components/sidebar.scss b/inst/components/sidebar.scss
@@ -316,21 +316,37 @@
 }
 
 
-// In the case of page_navbar(nav_panel("Foo", layout_sidebar())), we want the
-// sidebar to be flush with the viewport (just like it will be in the
-// case of page_navbar(sidebar = sidebar()))
 .navbar + .container-fluid {
-  &:has(>.tab-content>.tab-pane.active>.bslib-sidebar-layout) {
+  // When the tab/page is fillable, undo the padding on the container-fluid
+  // (in this case, the user has control over padding/gap)
+  &:has(> .tab-content > .tab-pane.active.html-fill-container) {
     padding-left: 0;
     padding-right: 0;
   }
-  > .tab-content > .tab-pane.active > .bslib-sidebar-layout {
-    &:not([data-bslib-sidebar-border="true"]) {
-      border: none;
+  // When the tab/page is fillable, add sensible default padding
+  > .tab-content > .tab-pane.active.html-fill-container {
+    padding: $spacer;
+    gap: $spacer;
+    // ...but if it holds a single sidebar layout, remove the padding
+    &:has(> .bslib-sidebar-layout:only-child) {
+      padding: 0;
     }
-    &:not([data-bslib-sidebar-border-radius="true"]) {
-      border-radius: 0;
+    // And smart border defaults for nav_panel("Foo", layout_sidebar())
+    > .bslib-sidebar-layout:only-child {
+      &:not([data-bslib-sidebar-border="true"]) {
+        border-left: none;
+        border-right: none;
+        border-bottom: none;
+      }
+      &:not([data-bslib-sidebar-border-radius="true"]) {
+        border-radius: 0;
+      }
     }
-  }
+  } 
 }
 
+// Make sure a border appears between the navbar and the sidebar layout
+// (especially important when page_navbar(inverse = FALSE, sidebar = sidebar())
+.navbar + div > .bslib-sidebar-layout {
+  border-top: var(--bslib-sidebar-border);
+}
diff --git a/inst/css-precompiled/5/bootstrap.min.css b/inst/css-precompiled/5/bootstrap.min.css
diff --git a/man/figures/navset_card_pill.png b/man/figures/navset_card_pill.png
diff --git a/man/figures/page_navbar.png b/man/figures/page_navbar.png
diff --git a/man/layout_columns.Rd b/man/layout_columns.Rd
diff --git a/man/navset.Rd b/man/navset.Rd
diff --git a/man/page.Rd b/man/page.Rd
diff --git a/man/page_sidebar.Rd b/man/page_sidebar.Rd
diff --git a/tests/testthat/_snaps/page.md b/tests/testthat/_snaps/page.md
@@ -4,13 +4,15 @@
       renderTags(page_navbar(title = div(h1("foo"), h2("bar"))))$head
     Output
         <title>foo bar</title>
+        <style>html { height: 100%; }</style>
 
 ---
 
     Code
       renderTags(page_navbar(title = "foo", window_title = "bar"))$head
     Output
         <title>bar</title>
+        <style>html { height: 100%; }</style>
 
 ---