Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update markup of BS5 tabs + conditional TOC JS inclusion #1890

Closed
wants to merge 5 commits into from
Closed

Conversation

maelle
Copy link
Collaborator

@maelle maelle commented Nov 9, 2021

@maelle maelle changed the title Update markup of BS5 tabs Update markup of BS5 tabs + conditional TOC JS inclusion Nov 9, 2021
@maelle maelle mentioned this pull request Nov 9, 2021
@@ -23,6 +23,10 @@
render_page <- function(pkg = ".", name, data, path = "", depth = NULL, quiet = FALSE) {
pkg <- as_pkgdown(pkg)

if (is.null(data$has_toc)) {
data$has_toc <- TRUE
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I only tested quickly is creating a vignette with tabsets and no h2: then the tabs work because there's a TOC even if it's empty.

@maelle maelle marked this pull request as ready for review November 9, 2021 08:30
@hadley
Copy link
Member

hadley commented Nov 11, 2021

Do you mind pulling the tabs markup change into a separate PR? I'd want to contemplate the tabs fix a bit more before we apply it — it introduces quite a lot of additional complexity in order to fix a pretty minor bug.

@maelle
Copy link
Collaborator Author

maelle commented Nov 12, 2021

👍 See #1903

@maelle
Copy link
Collaborator Author

maelle commented Nov 12, 2021

I was also thinking it's a good thing we now set the active class in the navbar via HTML tweaks. 😅 (or maybe there could still be a bug there one day) Things that can get an active class in a pkgdown page:

  • navbar item;
  • TOC item;
  • tabs in tabsets.

@maelle
Copy link
Collaborator Author

maelle commented Nov 12, 2021

For the record I re-read https://afeld.github.io/bootstrap-toc/ and didn't see any relevant configuration to change.

If I were to make this PR today, I wouldn't use logic inside templates but instead add a tweak that'd add the TOC JS when there's a TOC. 🤷‍♀️

@maelle
Copy link
Collaborator Author

maelle commented Dec 9, 2021

See #1959 (comment) by @gadenbuie

@maelle maelle closed this Dec 9, 2021
@hadley hadley deleted the tabs5 branch November 1, 2023 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A small tabset oddity
2 participants