Skip to content

Shiny Express is missing panel_title() #1011

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

Closed
Tracked by #969
gadenbuie opened this issue Jan 16, 2024 · 8 comments · Fixed by #1039
Closed
Tracked by #969

Shiny Express is missing panel_title() #1011

gadenbuie opened this issue Jan 16, 2024 · 8 comments · Fixed by #1039
Labels

Comments

@gadenbuie
Copy link
Collaborator

gadenbuie commented Jan 16, 2024

panel_title() is missing from Shiny Express. Its usage is partially covered by shiny.ui.page_opts(), which exposes title (but not window_title) and passes title to page functions that take the argument.

I think that, for consistency, we should offer shiny.express.ui.panel_title(). Secondarily, we can augment shiny.express.ui.page_opts() to take window_title and to also, possibly, insert title into the older page functions like page_fluid().

panel_title() takes two arguments, title and window_title. When provided by the user, window_title is always static, but title could be a simple static value or it could involve dynamic UI:

from shiny import render
from shiny.express import input, ui

# Static title option ----
# ui.panel_title(title="Page title", window_title="Window title")

# Dynamic title option ----
with ui.panel_title(window_title="Static window title"):

    @render.ui
    def page_title():
        page = input.page() if "page" in input else "Home"
        return ui.h2(f"{page} title")


ui.input_select("page", "Page", ["Home", "About", "Contact"])
@gadenbuie gadenbuie mentioned this issue Jan 16, 2024
55 tasks
@wch
Copy link
Collaborator

wch commented Jan 19, 2024

We can add panel_title, but I don't see a straightforward way of supporting both the static and dynamic versions of the code above.

If we were to allow passing in the title from either the function call or in the with block, then for the function call form, it would have to be an unnamed argument, as in:

ui.panel_title("Page title", window_title="Window title")

@gadenbuie
Copy link
Collaborator Author

@wch would it be easy to support the unnamed argument version that allows either the direct or the context manager versions to be used? Personally, that'd be my preference and feels like a good fit for Express.

@wch
Copy link
Collaborator

wch commented Jan 23, 2024

Here are three different possible implementations. It looks like I was wrong about it being possible to have a version that could be used as a context manager and as a regular function.

  1. This version is what's in Add express.ui.panel_title #1039.
def panel_title1(
    *,
    title: str | Tag | TagList,
    window_title: str | MISSING_TYPE = MISSING,
) -> RecallContextManager[TagList]:
    return RecallContextManager(
        ui.panel_title,
        kwargs=dict(
            title=title,
            window_title=window_title,
        ),
    )

# Error
with panel_title1(window_title="window_title"):
    "title"

# Error
panel_title1("title", window_title="window_title")

# OK
panel_title1(title="title", window_title="window_title")
def panel_title2(
    title: str | Tag | TagList,
    *,
    window_title: str | MISSING_TYPE = MISSING,
) -> RecallContextManager[TagList]:
    return RecallContextManager(
        ui.panel_title,
        args=(title,),
        kwargs=dict(
            window_title=window_title,
        ),
    )

# Error
with panel_title2(window_title="window_title"):
    "title"

# OK
panel_title2("title", window_title="window_title")

# OK
panel_title2(title="title", window_title="window_title")
def panel_title3(
    *,
    window_title: str | MISSING_TYPE = MISSING,
) -> RecallContextManager[TagList]:
    return RecallContextManager(
        ui.panel_title,
        kwargs=dict(
            window_title=window_title,
        ),
    )

# OK
with panel_title3(window_title="window_title"):
    "title"

# Error
panel_title3("title", window_title="window_title")

# Error
panel_title3(title="title", window_title="window_title")

@gadenbuie
Copy link
Collaborator Author

@wch Do you still like the kwargs approach best? I don't have any strong feelings, but I lean toward option 2 (panel_title("Title") or panel_title(title="Title"))

@gadenbuie
Copy link
Collaborator Author

@wch Unless there's an advantage to requiring title as a keyword arg, it does seem advantageous to not have to include title=, since most people will use some form of

ui.panel_title("My page title")

# or...
ui.panel_title(ui.h1("My page title"))

@wch
Copy link
Collaborator

wch commented Jan 23, 2024

Wait, there is one more version that I overlooked;

def panel_title4(
    *args: str | Tag | TagList,
    window_title: str | MISSING_TYPE = MISSING,
) -> RecallContextManager[TagList]:
    return RecallContextManager(
        ui.panel_title,
        args=args,
        kwargs=dict(
            window_title=window_title,
        ),
    )

# OK
with panel_title4(window_title="window_title"):
    "title"

# OK
panel_title4("title", window_title="window_title")

# Error
panel_title4(title="title", window_title="window_title")

This supports both the context manager and regular function call, but the function signature is a bit off -- it takes *args, which might lead the user to expect that they can pass in multiple arguments, and it doesn't accept title.

One last possibility: if we were to simply pass through the original panel_title() function and not wrap it in a RecallContextManager, then it looks like this (which has the same user interface as option 2, but with less complexity under the hood):

# Error
with panel_title(window_title="window_title"):
    "title"

# OK
panel_title("title", window_title="window_title")

# OK
panel_title(title="title", window_title="window_title")

@gadenbuie
Copy link
Collaborator Author

Ooh, option 4 was kind of what I had in mind initially. But I can recognize it's not a common use case (and could be handled in other ways as well).

The simplicity of simply passing through to shiny.ui.panel_title() is appealing

@wch
Copy link
Collaborator

wch commented Jan 23, 2024

The weirdness in option 4 with *args is a drawback, and the simplicity of the passthrough appeals to me, so I'll update the PR to do that.

@wch wch closed this as completed in #1039 Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants