-
Notifications
You must be signed in to change notification settings - Fork 738
add default values option for Dialog #1563
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. A few small comments. Thanks for the pull request!
@@ -175,6 +175,7 @@ def radiolist_dialog( | |||
ok_text: str = "Ok", | |||
cancel_text: str = "Cancel", | |||
values: Optional[List[Tuple[_T, AnyFormattedText]]] = None, | |||
default_values: List[_T] = [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mutable values should not be used as keyword arguments. State could leak from one call to the next.
Better is to do:
default_values: Optional[List[_T]] = None,
and in the body of the function:
if default_values is None:
default_values = []
@@ -213,6 +214,7 @@ def checkboxlist_dialog( | |||
ok_text: str = "Ok", | |||
cancel_text: str = "Cancel", | |||
values: Optional[List[Tuple[_T, AnyFormattedText]]] = None, | |||
default_values: List[_T] = [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment here about mutable defaults.
@@ -686,14 +686,25 @@ class _DialogList(Generic[_T]): | |||
multiple_selection: bool = False | |||
show_scrollbar: bool = True | |||
|
|||
def __init__(self, values: Sequence[Tuple[_T, AnyFormattedText]]) -> None: | |||
def __init__( | |||
self, values: Sequence[Tuple[_T, AnyFormattedText]], default_values: List[_T] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about mutable defaults here.
self.current_values: List[_T] = [ | ||
default_value | ||
for default_value in default_values | ||
if (default_value in candidate) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no parentheses are required around this expression.
@motty-mio2: I did some changes to this pull request, and merged it from here: #1569 |
Oh Thank you so much. |
Hello, I'm happy to post here.
Why
Default Value settings are important for some people (includes me).
What
I add an option to set default value for Dialogs(Checkbox list dialog and Radio list dialog).
Usage
Checkbox list dialog
Radio list dialog
First index (default_values[0], in this case "blue") item will be used for default value.
How It Works
checkbox dialogs
I set default_values as

["banana", "eggs", "croissants"]
radio dialogs
I set default_values as

["blue"]
"blue" is selected.
Related
default
parameter to RadioList and radiolist_dialog #651Thank you.