Skip to content

Commit

Permalink
fix(navbar_options): Require all named arguments in ... and use `at…
Browse files Browse the repository at this point in the history
…tribs` for consistency
  • Loading branch information
gadenbuie committed Dec 3, 2024
1 parent 68f2408 commit ed14acf
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 17 deletions.
30 changes: 17 additions & 13 deletions R/navbar_options.R
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,12 @@ navbar_options <- function(
underline = underline
)

attrs <- rlang::dots_list(...)
if ("inverse" %in% names(attrs)) {
dots <- separate_arguments(...)
if (length(dots$children) > 0) {
abort("All arguments in `...` must be named attributes to be applied to the navbar container.")
}

if ("inverse" %in% names(dots$attribs)) {
# Catch muscle-memory for using `inverse`. We didn't release
# `navbar_options()` with an `inverse`, but it's reasonable people might try
# to use it and it did exist briefly in dev versions.
Expand All @@ -64,8 +68,8 @@ navbar_options <- function(
with = "navbar_options(type=)"
)
}
if (length(attrs)) {
opts[["attrs"]] <- attrs
if (length(dots$attribs)) {
opts[["attribs"]] <- dots$attribs
}

structure(
Expand All @@ -76,20 +80,20 @@ navbar_options <- function(
)
}

navbar_options_apply_attrs <- function(navbar, navbar_options = NULL) {
if (is.null(navbar_options[["attrs"]])) {
navbar_options_apply_attribs <- function(navbar, navbar_options = NULL) {
if (is.null(navbar_options[["attribs"]])) {
return(navbar)
}

attrs <- navbar_options[["attrs"]]
navbar[[1]] <- rlang::exec(tagAppendAttributes, navbar[[1]], !!!attrs)
attribs <- navbar_options[["attribs"]]
navbar[[1]] <- rlang::exec(tagAppendAttributes, navbar[[1]], !!!attribs)

if ("data-bs-theme" %in% names(attrs)) {
if ("data-bs-theme" %in% names(attribs)) {
# If you're setting this attribute directly, you know more about what you're
# doing than we do (we handle it for users via `type`). Also: the call to
# tagAppendAttributes ensures that `navbar[[1]]` is a tag object and has the
# attribs field.
navbar[[1]][["attribs"]][["data-bs-theme"]] <- attrs[["data-bs-theme"]]
navbar[[1]][["attribs"]][["data-bs-theme"]] <- attribs[["data-bs-theme"]]
}

navbar
Expand Down Expand Up @@ -184,10 +188,10 @@ navbar_options_resolve_deprecated <- function(
)
}

attrs <- options_user$attrs %||% list()
options_user$attrs <- NULL
attribs <- options_user$attrs %||% list()
options_user$attribs <- NULL

rlang::exec(navbar_options, !!!options_user, !!!attrs)
rlang::exec(navbar_options, !!!options_user, !!!attribs)
}

#' @export
Expand Down
2 changes: 1 addition & 1 deletion R/navs-legacy.R
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ navset_bar <- function(
theme = bs_theme()
)

navbar_options_apply_attrs(navbar, .navbar_options)
navbar_options_apply_attribs(navbar, .navbar_options)
}


Expand Down
2 changes: 1 addition & 1 deletion R/page.R
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ page_navbar <- function(
fluid = fluid,
theme = theme
)
navbar <- navbar_options_apply_attrs(navbar, .navbar_options)
navbar <- navbar_options_apply_attribs(navbar, .navbar_options)

page_func(
title = infer_window_title(title, window_title),
Expand Down
8 changes: 6 additions & 2 deletions tests/testthat/test-navbar_options.R
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,12 @@ test_that("navbar_options() print method", {
)
})

test_that("navbar_options() adds ... to `attrs`", {
expect_equal(navbar_options(foo = "bar")$attrs, list(foo = "bar"))
test_that("navbar_options() adds named args from ... to `attribs`", {
expect_equal(navbar_options(foo = "bar")$attribs, list(foo = "bar"))
})

test_that("navbar_options() throws for unnamed args in ...", {
expect_error(navbar_options("foo", "bar"))
})

test_that("navbar_options() warns `inverse` is used instead of `type`", {
Expand Down

0 comments on commit ed14acf

Please sign in to comment.