r-lib / pkgdown

Generate static html documentation for an R package
https://pkgdown.r-lib.org/
Other
714 stars 334 forks source link

Rmd articles are not highlighted in navbar #2678

Closed olivroy closed 3 months ago

olivroy commented 3 months ago

Go to https://pkgdown.r-lib.org/dev/articles/quarto.html

image

Vs https://pkgdown.r-lib.org/dev/articles/translations.html

image

See that the Articles button remains selected in Quarto vignettes. The difference is not bad (and I wouldn't have noticed it, but as gt has different css, it is very apparent)

I did some tests in gt. Let me know if gt should fix its css or if pkgdown + quarto will?

Example of quarto vignette with gt ![image](https://github.com/r-lib/pkgdown/assets/52606734/d616803c-910e-45f3-b494-ea17f7612928)
hadley commented 3 months ago

I'm pretty sure that the intent is for articles to be highlighted for all articles; i.e. the fact that it's not for the non-quarto articles is the bug.

olivroy commented 3 months ago

Makes sense, I will wait for your resolution and then see what's the best way for gt to respond to this change

hadley commented 3 months ago

I'd encourage you to take a look at the code that makes this work (I think it's in javascript?). I'm a terrible at getting the logic right.

olivroy commented 3 months ago

I don't really know where to start. This can always be left untouched until someone else cares about this.

hadley commented 3 months ago

Neither do I so we're in the same boat 😆

hadley commented 3 months ago

Adding a few pointers as I look into this a bit more. First discovery is that the code in activate_navbar()

When I look at navbar_haystack() for the quarto article I see:

  nav_item   links                similar
  <list>     <chr>                  <dbl>
1 <xml_node> articles/quarto.html       2
2 <xml_node> articles                   1

When I look at it for customise I see:

# A tibble: 0 × 3
# ℹ 3 variables: nav_item <list>, links <chr>, similar <dbl>

So something is going wrong in navbar_links_haystack()

It also looks like maybe the intent is to actually highlight the specific quarto link?

hadley commented 3 months ago

Ah the reason that it's not highlighting the Rmd articles is that render_page() is called on the template, so the active path is something like "tmp/RtmpL8xSQj/pkgdown-rmd-template-146dd6b0d2544.html", which obviously doesn't match anything in the navbar.

...

No that's not quite right — we expect that to get called on the template, but we're also supposed to be calling render_page() again on the article, once it's in the right place.

....

Oh, the problem is that activate_navbar() is called only from render_page() not from tweak_page

...

Oooh and we do that because activate_navbar() is the only function that also requires the path to the page.