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

feat: Move navbar_options into single argument with helper navbar_options() #1822

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

gadenbuie
Copy link
Collaborator

@gadenbuie gadenbuie commented Jan 16, 2025

Moves navbar options position, bg, inverse, underline and collapsibe into a navbar_options argument in ui.page_navbar() and ui.navset_bar(). These options can be prepared with a new helper ui.navbar_options(). Further, inverse is replaced with theme in ui.navber_options().

Reflects changes in

The top-level direct arguments are now deprecated, but users can continue to use them (with a deprecation warning).

@gadenbuie gadenbuie changed the title feat: Move navbar_options() into single argument with helper feat: Move navbar_options into single argument with helper navbar_options() Jan 16, 2025
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schloerke Can you review the changes in this file for me? I wanted an object that operates like MISSING but gives a better hint to users in the function signature, so I created a new sentinel DEPRECATED object. Look at page_navbar() or navset_bar() (in _navs.py, which might be hidden for being a large diff) to see how this is used.


# Sentinel value - indicates a missing value in a function call.
class MISSING_TYPE:
pass


MISSING: MISSING_TYPE = MISSING_TYPE()
DEPRECATED: MISSING_TYPE = MISSING_TYPE() # A MISSING that communicates deprecation
MaybeMissing = Union[T, MISSING_TYPE]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find bg: MaybeMissing[str | None] = MISSING a bit more readable than bg: str | None | MISSING_TYPE = MISSING, so this just provides a bit of sugar around the common pattern.

Comment on lines +54 to +55
def is_missing(x: Any) -> TypeIs[MISSING_TYPE]:
return x is MISSING or isinstance(x, MISSING_TYPE)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL about the difference between TypeGuard and TypeIs. (TypeGuard only narrows the type scope in the positive case [if it is, it is, otherwise it might still be], while TypeIs narrows the type on both sides [it either is or it isn't].)

Anyway, this could also just be isinstance(x, MISSING_TYPE) but it's a little more succinct when used.

@gadenbuie gadenbuie marked this pull request as ready for review January 16, 2025 22:17
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.

1 participant