-
Notifications
You must be signed in to change notification settings - Fork 69
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
Include tooltip()
and popover()
in express
#949
Conversation
4ed6df3
to
1c954f8
Compare
1c954f8
to
c5aaf6d
Compare
I wasn't sure what API we want for these functions. With this PR, these functions need to be used in the non-context-manager way: from shiny.express import ui
ui.tooltip(
ui.input_action_button("btn", "A button"),
"A message",
id="btn_tooltip",
) This is quite different from the other context manager stuff in Express. Also, if the user wants to have nested content for either the trigger or tooltip/popover, they will need to have a separate import of Here are a few possible ways of doing this with context managers. Note that in all cases, the kwargs (like # (1) Passing values straight through
with ui.tooltip(id="btn_tooltip"):
ui.input_action_button("btn", "A button")
"A message"
# (2) Taking the `trigger` argument:
with ui.tooltip(ui.input_action_button("btn", "A button"), id="btn_tooltip"):
"A message"
# (3) Taking the *args:
with ui.tooltip("A message", id="btn_tooltip"):
ui.input_action_button("btn", "A button") I have to say that I don't really love any of these, but I think (1) is the least weird. With (1), it's weird that the kwargs have to go to the into the function call immediately, but then the unnamed args come later, after the With (2), it feels strange to me that the (3) feels better to me than (2), but is limiting in a similar way, if there's a lot of content in the tooltip. This is even more true for popovers. For both (2) and (3), it is not possible to have nested content in the args passed to the function, unless a separate import is made for |
Easily option 1 for me, personally. |
Agreed (option 1), I'll update the PR accordingly |
* main: (24 commits) Use dynamic version of py-shiny for deploy tests (#970) Add underscores to hide some imports (#978) Add rsconnect json files(shinyapps.io tests) and folium tests (#928) Express' `value_box()` no longer includes named positional args (#966) Include `tooltip()` and `popover()` in express (#949) Remove extra call to run_express() Call `tagify()` early to intercept `AttributeErrors` (#941) Don't pass sidebar twice to navbar_page Update changelog Update changelog Switch from `requests` to `urllib` (#940) Bump version to 0.6.1.1 Fix docstring for page_opts Fix API doc sections for Express Smarter, lazier, and more complete page default/api for express (#893) Change `express.layout` to `express.ui` (#904) Remove `@output` from examples (#790) feat: Allow for `App` `server=` to take `input` only (#920) Add fixes for type stub generation (#828) Move quarto express docs to bottom ...
* main: Use dynamic version of py-shiny for deploy tests (#970) Add underscores to hide some imports (#978) Add rsconnect json files(shinyapps.io tests) and folium tests (#928) Express' `value_box()` no longer includes named positional args (#966) Include `tooltip()` and `popover()` in express (#949)
Closes #946