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

Improve nav_*() and navset_*() function names #546

Merged
merged 34 commits into from
May 16, 2023
Merged

Conversation

gadenbuie
Copy link
Member

@gadenbuie gadenbuie commented Apr 12, 2023

For #476

Overview

This is a big, wide-ranging PR with simple naming changes. There are two over-arching goals:

  1. Improve the names of nav-item functions, in particular to make nav() more descriptive.
  2. Create a better separation between navs_ and nav_ prefixes by replacing the first with navset_.

API Changes

Here's a short summary of the changes. In all cases, deprecation warnings are emitted unless the functions are called from Shiny (to give us time to migrate the changes where bslib::nav() and others are called directly).

New Old Shiny equiv Notes
nav_panel() nav() tabPanel()
nav_panel_hidden() nav_content() tabPanelBody() 1
navset_tab() navs_tab() tabsetPanel()
navset_pill() navs_pill() tabsetPanel(type="pill")
navset_pill_list() navs_pill_list() navlistPanel()
navset_hidden() navs_hidden() tabSetPanel(type="hidden") 1
navset_bar() navs_bar() pageNavbar() adjacent?
navset_card_tab() navs_tab_card() (none) 2
navset_card_pill() navs_pill_card() (none) 2

In the revised API, all functions starting with navset_*() take items prefixed with nav_().

Notes:

  1. nav_content() is now nav_panel_hidden() (which pairs with navset_hidden()). The panel itself isn't hidden necessarily; the hidden refers to the fact that there isn't a <nav> container with controls (e.g. a wizard interface).
  2. The _card part of the function name was moved in front of tab|pill in navset_card_tab() and navset_card_pill() to reflect that card is higher priority in usage.

PR Notes

There's a lot of work in flight in the bslib articles, so I only made changes in the R functions, documentation, and examples. We can wait for those PRs to land or we can tackle those updates in a follow-up PR. Update: this is done now.

I also took care to ensure that the aliased functions appear at the end of any function lists in the documentation. All aliases are now collected in R/navs-aliases.R with TODO items so we can deprecate them in future version of bslib. Update: these functions will be formally deprecated.

I marked the deprecated functions as "renamed in v0.5.0". We're currently on v0.4.2.9000 and I'm assuming, from looking at our current NEWS situation that we'd be doing a minor version bump with the next release.

Remaining work

Need to:

  • Clean up navset example images and ensure correct naming.
  • Update usage in bslib's articles.

For discussion or possibly a future PR:

  • Also rename nav_item()? In Shiny for Python this became nav_control().
  • Rename fillable in page_navbar() to fillable_panel?

@gadenbuie gadenbuie marked this pull request as ready for review April 13, 2023 15:03
R/navs-aliases.R Outdated Show resolved Hide resolved
@gadenbuie
Copy link
Member Author

Oops, I thought I merged main into this branch before updating the articles. I'll finish this up early next week.

@nstrayer
Copy link
Contributor

I think this is much easier to understand!

@jcheng5
Copy link
Member

jcheng5 commented Apr 20, 2023

Looks a lot better to me too

@gadenbuie
Copy link
Member Author

gadenbuie commented May 15, 2023

Python API

Comparing the new names with the current Shiny for Python API:

bslib (new) Python (current) Changes?
nav_panel() ui.nav 👀
nav_panel_hidden() ui.nav_content 👀 ❗
navset_tab() ui.navset_tab
navset_pill() ui.navset_pill
navset_pill_list() ui.navset_pill_list
navset_hidden() ui.navset_hidden
navset_bar() ui.navset_bar
navset_card_tab() ui.navset_tab_card 👀
navset_card_pill() ui.navset_pill_card 👀

👀 new R name is different from current Python
❗ not in API refs

@gadenbuie gadenbuie requested a review from cpsievert May 15, 2023 18:51
R/deprecated.R Outdated Show resolved Hide resolved
gadenbuie and others added 2 commits May 16, 2023 11:50
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
@gadenbuie gadenbuie merged commit 145fe8d into main May 16, 2023
12 checks passed
@gadenbuie gadenbuie deleted the rename-nav-panel branch May 16, 2023 17:35
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.

None yet

5 participants