From 7d257ff2f095480bb2a386ceba3371504d766324 Mon Sep 17 00:00:00 2001 From: "Maarten A. Breddels" Date: Wed, 27 Sep 2023 15:02:19 +0200 Subject: [PATCH] refactor --- solara/lab/components/__init__.py | 1 + solara/lab/components/confirmation_dialog.py | 80 ++++++++++++------- .../website/pages/api/confirmation_dialog.py | 3 +- tests/unit/confirmation_dialog_test.py | 50 +++++++++--- 4 files changed, 93 insertions(+), 41 deletions(-) diff --git a/solara/lab/components/__init__.py b/solara/lab/components/__init__.py index 5e27f5c48..426522d11 100644 --- a/solara/lab/components/__init__.py +++ b/solara/lab/components/__init__.py @@ -1 +1,2 @@ +from .confirmation_dialog import ConfirmationDialog # noqa: F401 from .tabs import Tab, Tabs # noqa: F401 diff --git a/solara/lab/components/confirmation_dialog.py b/solara/lab/components/confirmation_dialog.py index 21f2e7729..1a24a66fd 100644 --- a/solara/lab/components/confirmation_dialog.py +++ b/solara/lab/components/confirmation_dialog.py @@ -7,16 +7,18 @@ @solara.component def ConfirmationDialog( - is_open: Union[solara.Reactive[bool], bool], - on_ok: Callable[[], None], - on_close: Callable[[], None] = lambda: None, + open: Union[solara.Reactive[bool], bool], + *, + on_open: Callable[[bool], None] = lambda open: None, content: Union[str, solara.Element] = "", title: str = "Confirm action", ok: Union[str, solara.Element] = "OK", + on_ok: Callable[[], None] = lambda: None, cancel: Union[str, solara.Element] = "Cancel", + on_cancel: Callable[[], None] = lambda: None, children: List[solara.Element] = [], max_width: Union[int, str] = 500, - persistent: bool = True, + persistent: bool = False, ): """A dialog used to confirm a user action. @@ -29,52 +31,65 @@ def ConfirmationDialog( ```solara import solara - from solara.lab.components.confirmation_dialog import ConfirmationDialog - is_open = solara.reactive(False) + open_delete_confirmation = solara.reactive(False) def delete_user(): print("User being deleted...") @solara.component def Page(): - solara.Button(label="Delete user", on_click=lambda: is_open.set(True)) - ConfirmationDialog(is_open, delete_user, content="Are you sure you want to delete this user?") + solara.Button(label="Delete user", on_click=lambda: open_delete_confirmation.set(True)) + solara.lab.ConfirmationDialog(open_delete_confirmation, on_ok=delete_user, content="Are you sure you want to delete this user?") ``` ## Arguments - * `is_open`: Indicates whether the dialog is being shown or not. - * `on_ok`: Callback to be called when the OK button is clicked. - * `on_close`: Optional callback to be called when dialog is closed. + * `open`: Indicates whether the dialog is being shown or not. + * `on_open`: lalalal read about two-way binding vs ... * `content`: Message that is displayed. * `title`: Title of the dialog. * `ok`: If a string, this text will be displayed on the confirmation button (default is "OK"). If a Button, it will be used instead of the default button. + * `on_ok`: Callback to be called when the OK button is clicked. * `cancel`: If a string, this text will be displayed on the cancellation button (default is "Cancel"). If a Button, it will be used instead of the default button. + * `on_cancel`: Callback to be called when the Cancel button is clicked. When persistent is False, clicking outside of the element or pressing esc key will + also trigger cancel. * `children`: Additional components that will be shown under the dialog message, but before the buttons. * `max_width`: Maximum width of the dialog window. - * `persistent`: ... + * `persistent`: When False (the default), clicking outside of the element or pressing esc key will trigger cancel. """ + open_reactive = solara.use_reactive(open, on_open) + del open - is_open_reactive = solara.use_reactive(is_open) - del is_open + def close(): + open_reactive.set(False) - def perform_action(callback=None): + user_on_click_ok = None + user_on_click_cancel = None + + def perform_ok(): + if user_on_click_ok: + user_on_click_ok() on_ok() - if callback: - callback() close() - def close(): - on_close() # possible additional actions when closing - is_open_reactive.set(False) + def perform_cancel(): + if user_on_click_cancel: + user_on_click_cancel() + on_cancel() + close() + + def on_v_model(value): + if not value: + on_cancel() + open_reactive.set(value) with v.Dialog( - v_model=is_open_reactive.value, - on_v_model=is_open_reactive.set, - persistent=True, + v_model=open_reactive.value, + on_v_model=on_v_model, + persistent=persistent, max_width=max_width, ): with solara.Card(title=title): @@ -85,17 +100,20 @@ def close(): if children: solara.display(*children) with solara.CardActions(): + if isinstance(ok, str): - solara.Button(label=ok, on_click=perform_action) + solara.Button(label=ok, on_click=perform_ok) else: + # the user may have passed in on_click already + user_on_click_ok = ok.kwargs.get("on_click") + # override or add our own on_click handler + ok.kwargs = {**ok.kwargs, "on_click": perform_ok} solara.display(ok) - action = ok.kwargs.get("on_click") - if action: - # perform both on_ok and the on_click action of the custom Button - ok.kwargs = {**ok.kwargs, "on_click": lambda: perform_action(callback=action)} - else: - ok.kwargs = {**ok.kwargs, "on_click": perform_action} + + # similar as ok if isinstance(cancel, str): - solara.Button(label=cancel, on_click=close) + solara.Button(label=cancel, on_click=perform_cancel) else: + user_on_click_cancel = cancel.kwargs.get("on_click") + cancel.kwargs = {**cancel.kwargs, "on_click": perform_cancel} solara.display(cancel) diff --git a/solara/website/pages/api/confirmation_dialog.py b/solara/website/pages/api/confirmation_dialog.py index 2d271ed0a..20bd4b81f 100644 --- a/solara/website/pages/api/confirmation_dialog.py +++ b/solara/website/pages/api/confirmation_dialog.py @@ -9,6 +9,7 @@ from solara.lab.components.confirmation_dialog import ConfirmationDialog from solara.website.utils import apidoc +title = "ConfirmationDialog" users = solara.reactive("Alice Bob Cindy Dirk Eve Fred".split()) user_to_be_deleted: solara.Reactive[Union[str, None]] = solara.reactive(users.value[0]) is_open = solara.reactive(False) @@ -46,7 +47,7 @@ def Page(): style="max-width: 400px;", disabled=not users.value, ) - ConfirmationDialog(is_open, delete_user, title="Delete user", content=f"Are you sure you want to delete user **{user_to_be_deleted.value}**?") + ConfirmationDialog(is_open, on_ok=delete_user, title="Delete user", content=f"Are you sure you want to delete user **{user_to_be_deleted.value}**?") __doc__ += apidoc(solara.lab.components.confirmation_dialog.ConfirmationDialog.f) # type: ignore diff --git a/tests/unit/confirmation_dialog_test.py b/tests/unit/confirmation_dialog_test.py index f371f6397..708510b95 100644 --- a/tests/unit/confirmation_dialog_test.py +++ b/tests/unit/confirmation_dialog_test.py @@ -9,27 +9,48 @@ def test_confirmation_dialog_ok(): is_open = solara.reactive(True) on_ok = MagicMock() - el = ConfirmationDialog(is_open, on_ok=on_ok, content="Hello") + on_open = MagicMock() + el = ConfirmationDialog(is_open, on_ok=on_ok, on_open=on_open, content="Hello") _, rc = solara.render(el, handle_error=False) buttons = rc.find(vw.Btn) assert len(buttons) == 2 buttons[0].widget.click() assert on_ok.call_count == 1 # was OK button clicked? + assert on_open.call_count == 1 # always triggered assert not is_open.value # is dialog closed? def test_confirmation_dialog_cancel(): is_open = solara.reactive(True) on_ok = MagicMock() - el = ConfirmationDialog(is_open, on_ok=on_ok, content="Hello") + on_open = MagicMock() + el = ConfirmationDialog(is_open, on_ok=on_ok, on_open=on_open, content="Hello") _, rc = solara.render(el, handle_error=False) buttons = rc.find(vw.Btn) assert len(buttons) == 2 buttons[1].widget.click() assert on_ok.call_count == 0 # on_ok action should not have been executed + assert on_open.call_count == 1 # always triggered assert not is_open.value # is dialog closed? +def test_confirm_external_close(): + # e.g. when persistent=False, clicking away from the dialog closes it + is_open = solara.reactive(True) + on_ok = MagicMock() + on_cancel = MagicMock() + on_open = MagicMock() + el = ConfirmationDialog(is_open, on_ok=on_ok, on_cancel=on_cancel, on_open=on_open, content="Hello") + _, rc = solara.render(el, handle_error=False) + dialog = rc.find(vw.Dialog)[0].widget + assert dialog.v_model + dialog.v_model = False # trigger an external close, like escape or clicking away + assert not is_open.value # is dialog closed? + assert on_ok.call_count == 0 # on_ok action should not have been executed + assert on_cancel.call_count == 1 # on_cancel action should not have been executed + assert on_open.call_count == 1 # always triggered + + def test_confirmation_dialog_custom_button_no_onclick(): is_open = solara.reactive(True) on_ok = MagicMock() @@ -49,17 +70,28 @@ def test_confirmation_dialog_custom_button_with_onclick(): values = [] def on_ok(): - values.append(1) + values.append("on_ok") - def on_click(): - values.append(2) + def on_cancel(): + values.append("on_cancel") - my_button = solara.Button(label="Not OK", on_click=on_click) - el = ConfirmationDialog(is_open, on_ok=on_ok, ok=my_button, content="Are you sure?") + def on_click_ok(): + values.append("on_click_ok") + + def on_click_cancel(): + values.append("on_click_cancel") + + ok = solara.Button(label="Not OK", on_click=on_click_ok) + cancel = solara.Button(label="Not Cancel", on_click=on_click_cancel) + el = ConfirmationDialog(is_open, on_ok=on_ok, on_cancel=on_cancel, ok=ok, cancel=cancel, content="Are you sure?") _, rc = solara.render(el, handle_error=False) buttons = rc.find(vw.Btn) assert len(buttons) == 2 assert buttons[0].widget.children == ["Not OK"] - assert buttons[1].widget.children == ["Cancel"] + assert buttons[1].widget.children == ["Not Cancel"] buttons[0].widget.click() - assert values == [1, 2] # assert on_ok and on_click were both called, in that order + assert values == ["on_click_ok", "on_ok"] # assert on_ok and on_click were both called, in that order + values.clear() + # now the same for cancel + buttons[1].widget.click() + assert values == ["on_click_cancel", "on_cancel"]