`page_sidebar` and `page_navbar` should drop `sidebar` parameter and accept `Sidebar` objects in `*args`

[wch]

Currently, page_sidebar() and page_navbar() take an explicit sidebar argument.

However, for page_auto(), which is used in Express mode, there's no sidebar argument (because children can't be passed as named arguments in Express). Instead, we inspect the *args, pull out any Sidebar objects, and then explicitly pass them to the sidebar argument.

py-shiny/shiny/ui/_page.py

Lines 529 to 530 in a020af6

	page_fn = page_sidebar # pyright: ignore[reportGeneralTypeIssues]
	args = tuple(sidebars + [x for x in args if x not in sidebars])

Also, because of how Python handles named and unnamed ages:

• For page_sidebar(), the signature is page_sidebar(sidebar, *args). This means that the sidebar must be the first argument and must not be named.
• For page_navbar(), the signature is page_navbar(*args, sidebar). This means that the sidebar must not be the first argument, and must be named.

I think we should remove the sidebar parameter from these functions and simply inspect the *args for Sidebar objects -- essentially, take the logic from page_auto() and move it into page_sidebar() and page_navbar().

[schloerke]

take the logic from page_auto() and move it into page_sidebar() and page_navbar().

This motivation feels off to me as the API for Core is being driven by Express. (However, I am happy to have differences between Express and Core for usability!)

I'd like to keep as much static type checking as possible within Core. Because the sidebar parameters are special and not interleaved within *args children, I believe sidebar argument should be exposed and typed.

I am ok with continuing to have page_auto handle the sidebar differently than the two other page methods as its job is to auto detect the *args objects. (If we don't like the API for page_sidebar() or page_navbar() and these proposed changes help improve the API, let me know.)

---

If we do keep this proposal, should we also apply it to layout_sidebar(sidebar, *args) for consistency? (Or any other method that takes a sidebar and set of tag children (*args)?

---

@wch What about having shiny.express.ui.page_{sidebar, navbar}() accept *args and have them separate the args into Sidebar and not Sidebar args? This would fit the model of Express is different than Core where needed. (But not having the API be driven by Express at the cost of losing typing within Core.)

[wch]

The motivation is in part to improve the Express API, but also to improve the Core API. That said, I think it can make sense to make changes to the Core API to reflect the Express API, if it provides a more consistent experience and learning ramp. If we were to keep the status quo, then there would be one way of structuring the UI code for Express, and a different way of structuring the code for Core:

• For page_sidebar(): In Express, a Sidebar can go anywhere, but in Core, it must go first.
• For page_navbar(): In Express, a Sidebar can go anywhere (and is unnamed), but in Core, it must go last, and be a named argument.

This means that the transition from Express to Core would require learning a new way of structuring the layout code.

There is also the inconsistency in the Core between thepage_sidebar() and page_navbar() API, which I mentioned earlier:

• For page_sidebar(), the signature is page_sidebar(sidebar, *args). This means that the sidebar must be the first argument and must not be named.
• For page_navbar(), the signature is page_navbar(*args, sidebar). This means that the sidebar must not be the first argument, and must be named.

The drawback to the proposed change is that in page_sidebar(), there is no enforcement of requiring a Sidebar component by the static type checker. But there still is a helpful runtime error.

[jcheng5]

Couple of thoughts:

1. Why would an Express user expect Core's page_sidebar and page_navbar to work the same way as Express's default layout? Isn't that where page_auto would come in?
2. What if we keep the page_sidebar and page_navbar signatures the way they are, but also sniff out the *args? I don't even care if we document it. This way, the signatures for Core look sensible, while a naive port of the Express code to Core also does what that user would expect--except that page_sidebar would still require the sidebar argument to be first. (Which I really don't see as a problem in Core; I think it's weird to have page_sidebar(output1, output2, sidebar) and just ignore the sidebar's position in the list of arguments. I mean, fine if we happen to have that behavior, but I'd rather have a sensible function signature personally.)

Longer-term, maybe it doesn't make sense to have page_sidebar, page_navbar, page_fluid, page_fixed, and page_fillable in core--there's so much overlap between their capabilities.

[wch]

There are a number of open questions, so we're going to defer this for after the 0.7.0 release.

[gadenbuie]

Even just within Core, I think there's a strong argument to be made to align the function signatures of page_sidebar() and page_navbar(). It's relatively common to start with a page_sidebar() and realize you need a navbar page, in which case you have to significantly rearrange your code. Allowing page_navbar() to find a sidebar child in *args would be smoother transition

# from
ui.page_sidebar(
	ui.sidebar(),
	*children
)

# to
ui.page_navbar(
	ui.sidebar(),
	ui.nav_panel(
		*children
	)
)

# instead of
ui.page_navbar( 
	ui.nav_panel(
		*children
	),
	sidebar = ui.sidebar()
)