-
Notifications
You must be signed in to change notification settings - Fork 336
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
support for tabs in BS4 #1694
support for tabs in BS4 #1694
Conversation
I wonder whether there'd be a way to make each tabset height fixed at the height of the highest tab. |
tablist_item <- function(tab, html, parent_id) { | ||
id <- xml2::xml_attr(tab, "id") | ||
text <- xml_text1(xml2::xml_child(tab)) | ||
ul_nav <- xml2::xml_find_first(html, sprintf("//ul[@id='%s']", parent_id)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't think this would be needed as HTML here is supposed to be the tabset, but if I didn't use the ID, all tabs ended up in the first tabset when there were two tabsets.
@@ -465,3 +465,8 @@ summary { | |||
details p { | |||
margin-top: -.5rem; | |||
} | |||
|
|||
/* tabsets */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another tweak that might make sense would be more padding/margin at the top of tab content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there normally some styling around the content of the tabs? For the pills example in particular, it's hard to tell how the tab names are related to the content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added left and bottom borders for the pills example. Its color won't be hard-coded forever, since the blslib variables PR will prevent this kind of hard-coding.
Co-authored-by: Hadley Wickham <[email protected]>
@@ -129,6 +129,113 @@ tweak_footnotes <- function(html) { | |||
xml2::xml_remove(container) | |||
} | |||
|
|||
# Tabsets tweaking: find Markdown recommended in https://bookdown.org/yihui/rmarkdown-cookbook/html-tabs.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the record, in rmarkdown the transformation happens via JS https://github.com/rstudio/rmarkdown/blob/145046175c721af185aa6ba3ecc9262f12dc7369/inst/rmd/h/navigation-1.1/tabsets.js#L50
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cderv is this transformation something that you might consider hosting in an Rmd-adjacent package at some point in the future? Or do you think the xml2 based transformation is too foreign for the Rmd ecosystem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are currently discussing a bit tabset feature because of rstudio/rmarkdown#2054. We got input from @cpsievert that bslib will offer functions for creating navs / tabs UI (https://rstudio.github.io/bslib/reference/index.html#section-create-navs-and-navbars) and that we may be able to leverage that in some way in the future for rmarkdown.
Until now our direction for reworking tabset feature would be to use Lua filters rather than R directly. Also because Quarto has already a Lua filter for this (but using a fenced div syntax). But nothing is planned and started on this.
I think Lua filter would not help at all pkgdown usage currently, am I right ?
If we don't go the Lua filter road, then yes regarding xml2, I think it could be included in the Rmd ecosystem. it has no dependency and I feel we should use it more for some html post processing. But it is not already in the package tree for rmarkdown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If users use bslib for creating navs, then the resulting divs should be protected i.e. not treated by pkgdown. The code in this PR recognizes future tabsets via the XPath query .//div[contains(@class, 'tabset')]
which if I follow correctly would not pick up navs created by bslib. 🤔
I think tweaking HTML with xml2 is easier than using https://rstudio.github.io/bslib/reference/index.html#section-create-navs-and-navbars, given what we start from. (but still wanted to make a note of the existence of those functions) |
@@ -129,6 +129,113 @@ tweak_footnotes <- function(html) { | |||
xml2::xml_remove(container) | |||
} | |||
|
|||
# Tabsets tweaking: find Markdown recommended in https://bookdown.org/yihui/rmarkdown-cookbook/html-tabs.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cderv is this transformation something that you might consider hosting in an Rmd-adjacent package at some point in the future? Or do you think the xml2 based transformation is too foreign for the Rmd ecosystem?
@@ -465,3 +465,8 @@ summary { | |||
details p { | |||
margin-top: -.5rem; | |||
} | |||
|
|||
/* tabsets */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there normally some styling around the content of the tabs? For the pills example in particular, it's hard to tell how the tab names are related to the content.
Co-authored-by: Hadley Wickham <[email protected]>
Note from the PR linked by @cderv: in R Markdown you can have nav dropdowns but it's not documented, therefore I won't support it in this PR. 😇 |
Correct. This is a special feature to turn tabs in a dropdown menu instead. And there is suggestion to make it more like Bootstrap dropdowns with tabs / navs: rstudio/rmarkdown#2054 |
Regarding the fading effect: if I remove the dependencies created by bslib and add Bootstrap via a CDN instead, I can see the fading effect. It's as if something were missing from the bslib JS? When I click on the tabs, the "fade" class disappears (and the "show" class isn't added). |
As a side-note, because the Netlify preview isn't a direct Netlify-GitHub integration, we don't get to use the fancy new Netlify preview tools. 😭 (How nice would screenshots be!) |
<h3 class="hasAnchor"> | ||
<a href="#tab-1" class="anchor" aria-hidden="true"></a>Tab 1</h3> | ||
<p>blablablabla</p> | ||
<div class="sourceCode" id="cb9"><pre class="downlit sourceCode r"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it important to have source code in here? Otherwise it would be better to keep the test as short and simple as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find of another solution right now #1725 (comment)
# no matter the heading level -- the headings one level down are the tabs | ||
# and transform to tabsets HTML a la Bootstrap | ||
|
||
tweak_tabsets <- function(html) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it's time to start breaking this file up into smaller pieces?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh no, it was my favorite R script in pkgdown 😢 More seriously: #1725
Fix #1667