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

navbarMenu horizontal dividers #1147

Merged
merged 4 commits into from Mar 30, 2016
Merged

navbarMenu horizontal dividers #1147

merged 4 commits into from Mar 30, 2016

Conversation

wch
Copy link
Collaborator

@wch wch commented Mar 25, 2016

This closes #888.

Example app:

shinyApp(
  ui = navbarPage("App Title",
    tabPanel("Plot"),
    navbarMenu("More",
      tabPanel("Summary"),
      "----",
      "Header text",
      tabPanel("Table")
    )
  ),
  server = function(input, output) { }
)

image

@vnijs
Copy link
Contributor

vnijs commented Mar 27, 2016

Just tried out the PR. Very nice! I did notice that it will also turn an id into a dropdown-header.

shinyApp(
  ui = navbarPage("App Title",
                  tabPanel("Plot"),
                  navbarMenu("More",
                             id = "Not a header",
                             tabPanel("Summary"),
                             "----",
                             "Header text",
                             tabPanel("Table")
                  )
  ),
  server = function(input, output) { }
)

image

@wch
Copy link
Collaborator Author

wch commented Mar 28, 2016

Thanks for finding that case! I think that with the previous code, the id argument would basically get thrown away -- it wouldn't actually be used.

I think probably the right thing to do is to make sure that all the tab items are unnamed arguments, though it's possible I'm overlooking something. If we don't add that check, it's probably also makes sense to just use the behavior that you found in this pull request, and have the id argument (and any others) be treated like other unnamed arguments to navbarMenu. The first option is probably better for the sake of correctness; the second option is better for backward compatibility.

@wch wch force-pushed the navbar-horizontal-divider branch from f0082d1 to 77ac3a6 Compare March 30, 2016 03:13
@wch wch merged commit 67e2799 into master Mar 30, 2016
@wch wch deleted the navbar-horizontal-divider branch March 30, 2016 03:13
@vnijs
Copy link
Contributor

vnijs commented Mar 30, 2016

Does this mean that the id parameter can be removed from the function definition of navbarMenu?

@wch
Copy link
Collaborator Author

wch commented Mar 30, 2016

@vnijs
Copy link
Contributor

vnijs commented Mar 30, 2016

Ah. I misread the diff on github. Thanks again for adding this feature!

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.

Feature request: divider and dropdown-header in navbarMenu
2 participants