refactor

solara/lab/components/__init__.py

@@ -1 +1,2 @@
+from .confirmation_dialog import ConfirmationDialog  # noqa: F401
 from .tabs import Tab, Tabs  # noqa: F401

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)

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

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