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

express.ui.page_opts(title = ...) now always generates a header #1016

Merged
merged 3 commits into from
Jan 19, 2024

Conversation

cpsievert
Copy link
Collaborator

Closes #1015

@cpsievert cpsievert requested a review from wch January 16, 2024 20:40
@wch
Copy link
Collaborator

wch commented Jan 18, 2024

It would be useful to be able to set the window title separately from the on-page title. How about having two separate args, window_title and title, and if window_title isn't provided, it defaults to the value of title?

@cpsievert
Copy link
Collaborator Author

cpsievert commented Jan 19, 2024

Yea, we can definitely add that, but I think it'll time a bit of time to do properly -- let's maybe add that as an item for the next release and take this now?

@wch wch merged commit 16797a7 into main Jan 19, 2024
23 of 26 checks passed
@wch wch deleted the express-page-title-consistency branch January 19, 2024 05:44
@wch
Copy link
Collaborator

wch commented Jan 19, 2024

I just went ahead and implemented the window_title and title behavior.

@cpsievert
Copy link
Collaborator Author

cpsievert commented Jan 19, 2024

@wch FWIW 73463e2 won't handle Tag/TagList values for title as well as we do in other places. For example, this will put <h6>Title</h6> on the browser tab (i.e., <title>):

from shiny.express import ui

ui.page_opts(title=ui.h6("Title"))

Whereas, this will actually extract the string, so that the tab says Title, not <h6>Title</h6>

import shiny.express
from shiny.ui import panel_title

panel_title(ui.h6("Title"))

@wch
Copy link
Collaborator

wch commented Jan 19, 2024

Interesting... title is expected to be a string based on the type hints, but I suppose it is possible that someone would pass in a Tag/Taglist object. I'll file an issue about it.

schloerke added a commit that referenced this pull request Jan 19, 2024
* main: (26 commits)
  api!: Merge RendererBase class into Renderer (#1032)
  chore(render.display): Improve error message (#1020)
  `express.ui.page_opts(title = ...)` now always generates a header (#1016)
  api!: `Renderer.auto_output_ui()` drops `id` arg. Make `RendererBase.output_id` a non-namespaced value. (#1030)
  fix(page_sidebar): Add semicolon to end style declaration (#1027)
  chore: Remove experimental from app (#1028)
  chore: Expose `render.renderer.RendererBaseT` and do not require `| None` when defining Renderer types (#1026)
  bug: Restore legacy renderers while packages transition (#1023)
  Update deploy test apps to use render.code
  Update changelog
  Provide useful message in Express when `input` was not imported (#994)
  Calculate coordinate mapping after drawing figure (#999)
  Remove `express.ui.output_*` functions, add `shiny.express.render` (#1018)
  fix: Do not allow for renderer's to be given to reactive effects (#1017)
  Truncate the requirements.txt file before deploys (#998)
  Update changelog
  Fixes for flake8 (#1012)
  Pin starlette version below 0.35 (#1009)
  Remove shiny express warning
  Cause RecallContextManagers to run when used without `with` (#992)
  ...
cpsievert added a commit that referenced this pull request Jan 19, 2024
)

Co-authored-by: Winston Chang <winston@posit.co>
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.

express.ui.page_opts(title = ...) is sometimes not visible on the page
2 participants