Skip to content

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

Closed
wants to merge 4 commits into from
Closed

add default values option for Dialog #1563

wants to merge 4 commits into from

Conversation

motty-mio2
Copy link
Contributor

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).

  • It can check whether the value is valid or invalid.
    • later checkbox example, "banana" is not a valid option, so it is removed. "eggs" and "croissants" are selected.

Usage

Checkbox list dialog

results_array = checkboxlist_dialog(
    title="CheckboxList dialog",
    text="What would you like in your breakfast ?",
    values=[
        ("eggs", "Eggs"),
        ("bacon", "Bacon"),
        ("croissants", "20 Croissants"),
        ("daily", "The breakfast of the day")
    ],
    default_values=["banana", "eggs", "croissants"]
).run()
All values in default_values will be used for default value.

Radio list dialog

result = radiolist_dialog(
    values=[
        ("red", "Red"),
        ("green", "Green"),
        ("blue", "Blue"),
        ("orange", "Orange"),
    ],
    default_values=["blue"],
    title="Radiolist dialog example",
    text="Please select a color:",
).run()

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"]
checkbox

radio dialogs

I set default_values as ["blue"]
"blue" is selected.
radio

Related

Thank you.

Copy link
Member

@jonathanslenders jonathanslenders left a 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] = [],
Copy link
Member

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] = [],
Copy link
Member

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]
Copy link
Member

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)
Copy link
Member

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.

@jonathanslenders
Copy link
Member

@motty-mio2: I did some changes to this pull request, and merged it from here: #1569
Thanks again for the contribution!

@motty-mio2
Copy link
Contributor Author

Oh Thank you so much.

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.

2 participants