Merged
Various improvements to tab panels#3315
Conversation
cpsievert
commented
Mar 4, 2021
Comment on lines
+991
to
+995
| # Replace <li class="nav-item"><a class="nav-link"></a></li> | ||
| # with <a class="dropdown-item"></a> | ||
| navItem <- x$children[[1]] | ||
| navItem$attribs$class <- "dropdown-item" | ||
| navItem |
Collaborator
Author
There was a problem hiding this comment.
The main unfortunate thing about all of this is that BS4 doesn't support <li><a></a></li> inside of dropdowns, which apparently they brought back in BS5
https://getbootstrap.com/docs/4.6/components/navs/#tabs-with-dropdowns
https://getbootstrap.com/docs/5.0/components/navs-tabs/#tabs-with-dropdowns
Might be worth asking for a backport into BS4, but I doubt we'll see it
cpsievert
commented
Mar 5, 2021
cpsievert
commented
Mar 5, 2021
…nto 'empty' nav/tab
…e is precedence in navbarPage() and mention them in the warning
cpsievert
commented
Mar 12, 2021
cpsievert
commented
Mar 12, 2021
cpsievert
commented
Mar 17, 2021
cpsievert
force-pushed
the
bs4-tab-panels
branch
from
March 22, 2021 14:43
496ac66 to
b5b0ba1
Compare
wch
reviewed
Mar 22, 2021
| } | ||
|
|
||
| # TODO: something like this should exist in htmltools | ||
| tagHasClass <- function(x, class) { |
Collaborator
There was a problem hiding this comment.
The complexity of this code inspired this issue:
rstudio/htmltools#205
wch
reviewed
Mar 22, 2021
Co-authored-by: Winston Chang <winston@stdout.org>
This was referenced Mar 22, 2021
shalutiwari
added a commit
to rstudio/shinycoreci-apps
that referenced
this pull request
Apr 13, 2021
cpsievert
added a commit
that referenced
this pull request
Apr 19, 2021
cpsievert
added a commit
that referenced
this pull request
May 5, 2021
* Follow up to #3315: reduce complexity and 'black-boxed' nature of tab panel logic * asTags(selected = FALSE) is now root() * tagAddRenderHook * Add bslib to remotes * Document (GitHub Actions) * root() was recently changed to allTags() * code review * tagQuery() doesn't necessarily preserve order of attributes * place href attribute before data attributes * add nav-item/nav-link to BS4+ dropdowns * Make sure .nav-item is removed in .dropdown-menu Co-authored-by: cpsievert <cpsievert@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR closes numerous issues with
tabsetPanel()/navlistPanel()/navbarPage(). See the respective issue links for examples:!!!may now be used to splicetabPanel()s into them (closes tabsetPanel() doesn't work with list of panels #2927)tabsetPanel()andnavlistPanel()functions gainheader/footerarguments, akin to the already existingheader/footerarguments innavbarPage(), making it easier to include content that should appear on every tab (closes Make it easier to include header/footer content in tabsetPanel()/navlistPanel() #3322).NULLvalues are now dropped instead of producing an empty nav item (closes tabsetPanel doesn't ignore NULLs #1928).tabPanel()/shiny.tagobjects are supplied to...(closes tabsetPanel() et al give confusing error messages when they receive incorrect input #3313, closes tabSetpanel first argument #1823).shiny.tagobject(s) are supplied to.... In this case we will continue to create an "empty" nav item and include the content on every tab, but the warning will mention the (new)header/footerargs, which is likely what the user wants (closes Tab panel behavior when shiny tags are supplied #3321).theme = bslib::bs_theme()(closes Tab panel functions don't produce Bootstrap 4 compatible markup #3320). At least for now, you'll still want{bslibs}'s BS3 compatibility layer to provide sensible styles innavlistPanel()andnavbarPage(), but perhaps in follow up PRs we can explore completely removing the compatibility layer dependency for those as well. The main downside to this change in behavior is thattabsetPanel()now returnstagFunction()s, meaning that user code which makes assumptions about the return value may break, but hopefully by providing more intelligent tag modification functions (see, for example, Add tagFunction() awareness to tag modifying functions htmltools#202), this won't be much of an issue.Testing notes
Install
remotes::install_github("rstudio/shiny")then verify there is no regression in the example app above as well as the one below (in the app below, when you click add 'dynamic' tab, a tab should be inserted between Static1 and Static2 in the dropdown)