Make search a proper menu component (#2449)

Fixes #2320 I tested the visual appearance manually in a few places, and it looks good to me. @maelle it'd be useful if you could kick the tires on it a bit too. While working on this code, I realised that we'd been incorrectly labelling the search box as "toggle navigation" to screen readers 🤦 I've fixed that in this PR too.
r-lib · Apr 19, 2024 · e4a72c2 · e4a72c2
1 parent d76131d
commit e4a72c2
Show file tree

Hide file tree

Showing 9 changed files with 82 additions and 33 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,7 @@
 # pkgdown (development version)
 
+* New translation for "Search site", the label applied to the search box for screenreaders. This was previously incorrectly labelled as "Toggle navigation" (#2320).
+* You can now choose where the search box is placed with the "search" navbar component. This has been documented for a very long time, but as far as I can tell, never worked (#2320). If you have made your own template with a custom `navbar`, you will need to remove the `<form>` with `role="search"` to avoid getting two search boxes.
 * The mobile version of pkgdown sites no longer has a scrollburglar (a small amount of horizontal scroll) (#2179, @netique).
 
 # pkgdown 2.0.9

diff --git a/R/navbar.R b/R/navbar.R
@@ -31,7 +31,7 @@ navbar_style <- function(navbar = list(), theme = "_default", bs_version = 3) {
 navbar_structure <- function() {
   print_yaml(list(
     left = c("intro", "reference", "articles", "tutorials", "news"),
-    right = "github"
+    right = c("search", "github")
   ))
 }
 
@@ -104,6 +104,11 @@ navbar_components <- function(pkg = ".") {
   menu <- list()
   menu$reference <- menu_link(tr_("Reference"), "reference/index.html")
 
+  # in BS3, search is hardcoded in the template
+  if (pkg$bs_version == 5) {
+    menu$search <- list(search = NULL)
+  }
+
   if (!is.null(pkg$tutorials)) {
     menu$tutorials <- menu(tr_("Tutorials"),
       menu_links(pkg$tutorials$title, pkg$tutorials$file_out)
@@ -191,6 +196,23 @@ menu_spacer <- function() {
   menu_text("---------")
 }
 
+menu_search <- function(depth = 0) {
+  paste0(
+    '<li><form class="form-inline" role="search">\n',
+    '<input ',
+      'type="search" ',
+      'class="form-control" ',
+      'name="search-input" ', 
+      'id="search-input" ',
+      'autocomplete="off" ',
+      'aria-label="', tr_("Search site"), '" ',
+      'placeholder="', tr_("Search for"), '" ',
+      'data-search-index="', paste0(up_path(depth), "search.json"), '"',
+    '>\n',
+    '</form></li>'
+  )
+}
+
 bs4_navbar_links_html <- function(links) {
   as.character(bs4_navbar_links_tags(links), options = character())
 }
@@ -208,6 +230,10 @@ bs4_navbar_links_tags <- function(links, depth = 0L) {
   # function for links
   tackle_link <- function(x, index, is_submenu, depth) {
 
+    if (has_name(x, "search")) {
+      return(htmltools::HTML(menu_search(depth)))
+    }
+
     if (!is.null(x$menu)) {
 
       if (is_submenu) {
@@ -316,7 +342,8 @@ pkg_navbar <- function(meta = NULL, vignettes = pkg_navbar_vignettes(),
       src_path = file_temp(),
       meta = meta,
       vignettes = vignettes,
-      repo = list(url = list(home = github_url))
+      repo = list(url = list(home = github_url)),
+      bs_version = 5
     ),
     class = "pkgdown"
   )

diff --git a/R/render.R b/R/render.R
@@ -89,7 +89,6 @@ data_template <- function(pkg = ".", depth = 0L) {
   out$translate <- list(
     skip = tr_("Skip to contents"),
     toggle_nav = tr_("Toggle navigation"),
-    search_for = tr_("Search for"),
     on_this_page = tr_("On this page"),
     source = tr_("Source"),
     abstract = tr_("Abstract"),

diff --git a/R/tweak-navbar.R b/R/tweak-navbar.R
@@ -24,7 +24,7 @@ navbar_links_haystack <- function(html, pkg, path) {
   get_hrefs <- function(nav_item, pkg = pkg) {
     href <- xml2::xml_attr(xml2::xml_child(nav_item), "href")
 
-    if (href != "#") {
+    if (!is.na(href) && href != "#") {
       links <- href
     } else {
       # links in a drop-down

diff --git a/inst/BS5/templates/navbar.html b/inst/BS5/templates/navbar.html
@@ -18,10 +18,6 @@
       </ul>
       {{/left}}
 
-      <form class="form-inline my-2 my-lg-0" role="search">
-        <input type="search" class="form-control me-sm-2" aria-label="{{#translate}}{{toggle_nav}}{{/translate}}" name="search-input" data-search-index="{{#site}}{{root}}{{/site}}search.json" id="search-input" placeholder="{{#translate}}{{search_for}}{{/translate}}" autocomplete="off">
-      </form>
-
       {{#right}}
       <ul class="navbar-nav">
       {{{.}}}

diff --git a/po/R-pkgdown.pot b/po/R-pkgdown.pot
@@ -1,7 +1,7 @@
 msgid ""
 msgstr ""
 "Project-Id-Version: pkgdown 2.0.9.9000\n"
-"POT-Creation-Date: 2024-04-18 17:22-0500\n"
+"POT-Creation-Date: 2024-04-19 08:40-0500\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
@@ -18,7 +18,7 @@ msgstr ""
 msgid "Content not found. Please use links in the navbar."
 msgstr ""
 
-#: build-articles.R:395 navbar.R:142 navbar.R:149 navbar.R:164
+#: build-articles.R:395 navbar.R:147 navbar.R:154 navbar.R:169
 msgid "Articles"
 msgstr ""
 
@@ -42,7 +42,7 @@ msgstr ""
 msgid "Developers"
 msgstr ""
 
-#: build-home-authors.R:77 render.R:96
+#: build-home-authors.R:77 render.R:95
 msgid "Authors"
 msgstr ""
 
@@ -90,7 +90,7 @@ msgstr ""
 msgid "translator"
 msgstr ""
 
-#: build-home-citation.R:37 build-home-citation.R:94 render.R:99
+#: build-home-citation.R:37 build-home-citation.R:94 render.R:98
 msgid "Citation"
 msgstr ""
 
@@ -182,7 +182,7 @@ msgstr ""
 msgid "Usage"
 msgstr ""
 
-#: build-tutorials.R:46 navbar.R:108
+#: build-tutorials.R:46 navbar.R:113
 msgid "Tutorials"
 msgstr ""
 
@@ -202,14 +202,22 @@ msgstr ""
 msgid "Reference"
 msgstr ""
 
-#: navbar.R:136
+#: navbar.R:141
 msgid "Get started"
 msgstr ""
 
-#: navbar.R:162
+#: navbar.R:167
 msgid "More articles..."
 msgstr ""
 
+#: navbar.R:208
+msgid "Search site"
+msgstr ""
+
+#: navbar.R:209
+msgid "Search for"
+msgstr ""
+
 #: rd-data.R:33
 msgid "Details"
 msgstr ""
@@ -222,7 +230,7 @@ msgstr ""
 msgid "References"
 msgstr ""
 
-#: rd-data.R:45 render.R:94
+#: rd-data.R:45 render.R:93
 msgid "Source"
 msgstr ""
 
@@ -263,22 +271,18 @@ msgid "Toggle navigation"
 msgstr ""
 
 #: render.R:92
-msgid "Search for"
-msgstr ""
-
-#: render.R:93
 msgid "On this page"
 msgstr ""
 
-#: render.R:95
+#: render.R:94
 msgid "Abstract"
 msgstr ""
 
-#: render.R:97
+#: render.R:96
 msgid "Version"
 msgstr ""
 
-#: render.R:98
+#: render.R:97
 msgid "Examples"
 msgstr ""
 

diff --git a/tests/testthat/_snaps/navbar.md b/tests/testthat/_snaps/navbar.md
@@ -3,13 +3,17 @@
     reference:
       text: Reference
       href: reference/index.html
+    search:
+      search: ~
 
 
 ---
 
     reference:
       text: Reference
       href: reference/index.html
+    search:
+      search: ~
     github:
       icon: fab fa-github fa-lg
       href: https://github.com/r-lib/pkgdown
@@ -21,6 +25,8 @@
     reference:
       text: Reference
       href: reference/index.html
+    search:
+      search: ~
     github:
       icon: fab fa-gitlab fa-lg
       href: https://gitlab.com/r-lib/pkgdown
@@ -32,6 +38,8 @@
     reference:
       text: Reference
       href: reference/index.html
+    search:
+      search: ~
     intro:
       text: Get started
       href: test.html
@@ -127,16 +135,13 @@
 # data_navbar() can re-order default elements
 
     Code
-      data_navbar(pkg)
+      data_navbar(pkg)[c("left", "right")]
     Output
-      $type
-      [1] "default"
-      
       $left
-      [1] "<li>\n  <a href=\"https://github.com/r-lib/pkgdown/\">\n    <span class=\"fab fa-github fa-lg\"></span>\n     \n  </a>\n</li>\n<li>\n  <a href=\"reference/index.html\">Reference</a>\n</li>"
+      [1] "<li class=\"nav-item\">\n  <a class=\"nav-link\" href=\"https://github.com/r-lib/pkgdown/\" aria-label=\"github\">\n    <span class=\"fab fa fab fa-github fa-lg\"></span>\n     \n  </a>\n</li>\n<li><form class=\"form-inline\" role=\"search\">\n<input type=\"search\" class=\"form-control\" name=\"search-input\" id=\"search-input\" autocomplete=\"off\" aria-label=\"Search site\" placeholder=\"Search for\" data-search-index=\"search.json\">\n</form></li>"
       
       $right
-      [1] "<li>\n  <a href=\"news/index.html\">Changelog</a>\n</li>"
+      [1] "<li class=\"nav-item\">\n  <a class=\"nav-link\" href=\"news/index.html\">Changelog</a>\n</li>"
       
 
 # data_navbar() can remove elements
@@ -282,3 +287,12 @@
     Output
       <a class="dropdown-item" href="href" target="_blank">text</a>
 
+# can render search helper
+
+    Code
+      bs4_navbar_links_tags(list(menu = list(search = TRUE)))
+    Output
+      <li><form class="form-inline" role="search">
+      <input type="search" class="form-control" name="search-input" id="search-input" autocomplete="off" aria-label="Search site" placeholder="Search for" data-search-index="search.json">
+      </form></li>
+
diff --git a/tests/testthat/_snaps/render.md b/tests/testthat/_snaps/render.md
@@ -23,7 +23,6 @@
     translate:
       skip: Skip to contents
       toggle_nav: Toggle navigation
-      search_for: Search for
       on_this_page: On this page
       source: Source
       abstract: Abstract

diff --git a/tests/testthat/test-navbar.R b/tests/testthat/test-navbar.R
@@ -82,18 +82,20 @@ test_that("data_navbar() works by default", {
 
 test_that("data_navbar() can re-order default elements", {
   pkg <- local_pkgdown_site(meta = "
+    template: 
+      bootstrap: 5
     repo:
       url:
         home: https://github.com/r-lib/pkgdown/
 
     navbar:
       structure:
-        left: [github, reference]
-        right: news
+        left: [github, search]
+        right: [news]
   ")
   file.create(file.path(pkg$src_path, "NEWS.md"))
 
-  expect_snapshot(data_navbar(pkg))
+  expect_snapshot(data_navbar(pkg)[c("left", "right")])
 })
 
 test_that("data_navbar() can remove elements", {
@@ -208,3 +210,9 @@ test_that("can specific link target", {
     )
   })
 })
+
+test_that("can render search helper", {
+  expect_snapshot({
+    bs4_navbar_links_tags(list(menu = list(search = TRUE)))
+  })
+})