Add banner and main landmarks to page_navbar()

[jcheng5]

with @shelbylevel, and Posit Asisstant/Kimi K3

Fixes #1356

Wrap the top navigation bar in a <header> element so the page has a banner landmark, and render the content area as <main> when no sidebar is present (sidebar pages already get one via page_main_container()).

Both changes are made in page_navbar() rather than the shared navs_bar_() so that embedded navset_bar() instances don't produce inappropriate page-level landmarks. The <main> change is done in-place (the content div's tag is renamed) to preserve fill layout behavior.

Pull Request

Before you submit a pull request, please ensure you've completed the following checklist

• Ensure there is an already open and relevant GitHub issue describing the problem in detail and you've already received some indication from the maintainers that they are welcome to a contribution to fix the problem. This helps us to prevent wasting anyone's time.

• Add unit tests in the tests/testthat directory.

• This project uses roxygen2 for documentation. If you've made changes to documentation, run devtools::document().

• Run devtools::check() (or, equivalently, click on Build->Check Package in the RStudio IDE) to make sure your change did not add any messages, warnings, or errors.

  • Note there is a decent chance that some tests were already failing before your changes. Just make sure you haven't introduced any new ones.

• Ensure your code changes follow the style outlined in http://r-pkgs.had.co.nz/style.html

• Add an entry to NEWS.md concisely describing what you changed.

[gadenbuie]

Thanks for picking this up! Adding banner and main landmarks to page_navbar() is a real accessibility improvement, and keeping the change out of navset_bar() is the right call.

The blocking issue is that bslib's CSS for navbars assumes the .navbar and its content container are adjacent siblings, which is the shape navbarPage_() produces for page_navbar(), shiny::navbarPage(), and navset_bar() alike. Wrapping the nav in <header> and renaming the content div to <main> changes that shape for page_navbar() only, which causes visible layout regressions in existing apps (details inline). I tested an alternative that adds the same landmarks by renaming the existing elements instead, which keeps the DOM shape and needs no CSS changes.

[gadenbuie]

Once the approach is settled, could these two bullets be combined into one that describes the user-facing benefit without the element-level details, with attribution? Something like:

• page_navbar() now includes banner and main landmarks, making it easier for screen reader users to navigate the page. (@jcheng5, @shelbylevel, Add banner and main landmarks to page_navbar() #1357)

[gadenbuie]

Minor/non-blocking: this reaches into navbarPage_()'s return value by position ([[1]], [[2]], and with the approach above, [[1]]$children[[1]]). navs_bar_() already does navbar[[1]], but this adds more coupling to the internal structure. Consider having navbarPage_()/navs_bar_() accept an option that controls whether the landmark elements are produced, so the structure is defined in one place.

[gadenbuie]

Wrapping the <nav> in a <header> breaks several CSS rules that rely on .navbar being immediately followed by the content container:

• inst/components/scss/page_navbar.scss:1: .navbar+.container-fluid (fillable padding/gap, nav_panel(layout_sidebar()) borders)
• inst/components/scss/page_navbar.scss:37: .navbar+div>.bslib-sidebar-layout
• inst/bs3compat/_navbar_compat.scss:18-24: .navbar + div > .tab-content > .tab-pane margin
• inst/builtin/bs5/shiny/_rules.scss:187: .bslib-page-navbar > .navbar + div (the divider under the navbar in the default preset)

The same rules also style navset_bar() wherever users place it (e.g. page_fillable(navset_bar(...))), so they're written against the shared navbarPage_() output rather than a specific page function.

Computed styles in Chrome, main vs. this PR:

Case	main	PR
Default page_navbar(): content padding / pane padding	0 / 24px	12px / 0
fillable = FALSE (the navbarPage() default): pane margin-top	20px	0
Divider under the navbar (default preset), all layouts	1px	0
nav_panel(layout_sidebar()): sidebar border / radius	none	1px / 3px
navbar_options(class = "sticky-top"): navbar top after scrolling 600px	0	−582

The sticky case breaks because a sticky element can't move outside its parent, and the new <header> is exactly the navbar's height.

An approach that keeps the existing DOM shape is to rename the existing elements instead of adding a wrapper: the outer .navbar becomes the <header>, and its inner container becomes the <nav>:

navbar[[1]]$name <- "header"
navbar[[1]]$attribs <- navbar[[1]]$attribs[names(navbar[[1]]$attribs) != "role"]
navbar[[1]]$children[[1]]$name <- "nav"

if (is.null(sidebar)) {
  navbar[[2]] <- tagAppendAttributes(navbar[[2]], role = "main")
}

I tested this: it matches main on every measured style above, sticky positioning and the collapse toggle still work, Chrome's accessibility tree shows banner, navigation, and main landmarks, and nothing in bslib's SCSS/TS selects on the nav tag name.

[gadenbuie]

Renaming the content div to <main> breaks the + div selectors in the bs3compat and default-preset rules on its own, even without the <header> wrapper. Testing the rename together with the approach above still loses the pane margin, the navbar divider, and the layout_sidebar() border handling. role = "main" on the existing div produces the same landmark while keeping the selectors working.

[gadenbuie]

These tests match on HTML strings and only cover the default arguments (e.g. "<main class=\"container-fluid" assumes fluid = TRUE). Could you add cases for fluid = FALSE, fillable = FALSE, and shiny::navbarPage() (which goes through page_navbar() with different defaults)? They wouldn't have caught the CSS regressions, but they'd pin down the landmark structure across the main argument combinations.