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

ui.modal_remove fails unless fade is set to False #1318

Open
Pierre-Bartet opened this issue Apr 16, 2024 · 10 comments
Open

ui.modal_remove fails unless fade is set to False #1318

Pierre-Bartet opened this issue Apr 16, 2024 · 10 comments
Assignees

Comments

@Pierre-Bartet
Copy link
Contributor

In the documentation example of ui.modal_remove, actually trying to use it fails (the modal is never removed), unless fade is set to False.

image

@gadenbuie
Copy link
Collaborator

gadenbuie commented Apr 16, 2024

Hi @Pierre-Bartet! The real culprit is easy_close=False. That argument controls whether or not you can close the modal by clicking on the page outside the modal window. When easy_close=False, your app will need to either:

  1. include a ui.modal_button() to provide a button that closes the modal, or
  2. programmatically close the modal on the server using ui.modal_remove() on the server.

The second option is less common, typically with easy_close=False, you'll want to add the ui.modal_button(). Without the modal close button, easy_close=False is good for blocking error messages, which could be used for situations like when a user tries to login but fails.

Here's an example (run the app on shinylive) using easy_close=False and adding the modal close button.

from shiny import App, Inputs, Outputs, Session, reactive, ui

app_ui = ui.page_fluid(
    ui.input_action_button("show", "Show modal dialog"),
)


def server(input: Inputs, output: Outputs, session: Session):
    @reactive.effect
    @reactive.event(input.show)
    def _():
        m = ui.modal(
            "This is a somewhat important message.",
            title="Somewhat important message",
            fade=False,
            easy_close=False,
            footer=ui.modal_button("Close"),
        )
        ui.modal_show(m)


app = App(app_ui, server)

@Pierre-Bartet
Copy link
Contributor Author

Hi and thanks for the answer.
My point is that:

  1. programmatically close the modal on the server using ui.modal_remove() on the server.

doesn't work (except when deactivating fading).
So I don't see how the example you gave addresses the issue here (it doesn't even use ui.modal_remove()).

@gadenbuie
Copy link
Collaborator

My apologies, I missed that detail in your post -- because the code was in the screenshot, I went to the docs and tried to recreate your issue rather than being able to run your example directly.

I agree that we should have individual examples for ui.modal_remove() and ui.modal_button(). I'll take the examples from this thread and add them to our docs.

The problem with your original post is that you've included ui.modal_show() and ui.modal_remove() in the same @reactive.effect(). That leads to undefined behavior because you're trying to both show and remove the modal at the same time.

The idea of modal_remove() is that you would use a separate @reactive.effect() that keys off a @reactive.event() that's triggered within the modal UI. Here's an example (also on shinylive) where I've added an action link that, when clicked, removes the modal:

from shiny import App, Inputs, Outputs, Session, reactive, ui

app_ui = ui.page_fluid(
    ui.input_action_button("show", "Show modal dialog"),
)


def server(input: Inputs, output: Outputs, session: Session):
    @reactive.effect
    @reactive.event(input.show)
    def _():
        m = ui.modal(
            ui.p("This is a somewhat important message."),
            ui.p(
                "All done? ",
                ui.input_action_link("close", "Close the modal.")
            ),
            title="Somewhat important message",
            fade=False,
            easy_close=False,
            footer=None
        )
        ui.modal_show(m)

    @reactive.effect
    @reactive.event(input.close)
    def _():
        ui.modal_remove()


app = App(app_ui, server)

@gadenbuie gadenbuie self-assigned this Apr 16, 2024
@Pierre-Bartet
Copy link
Contributor Author

Thanks for the explanation!

That leads to undefined behavior because you're trying to both show and remove the modal at the same time.

I thought that first but was then confused by the weird interaction with fading.
Thanks.

@corey-dawson
Copy link

corey-dawson commented Apr 30, 2024

I have also seen issues where a modal fails to close based on ui.modal_remove(). In my case, the modal contains a loading icon while data is processed via a model. It is hard to provide an example because it seems intermittent where the modal doesnt close. However, I dont want the user be able to click off the dialog and close the loading bar when the processing is happening so easy_close should be false. My temporary fix has been to add a sleep after the function before the ui.modal_remove is called

@reactive.effect
@reactive.event(input.btn_run_model)
def runModel():
  data = v_data()
  m = get_loading_modal("Running Model")
  ui.modal_show(m, session=session)
  results = run_model(data) # can take 10-30 seconds to run
  v_results.set(results)
  ui.modal_remove(session=session)
  

@gadenbuie
Copy link
Collaborator

Thanks @corey-dawson. My guess is that as long as run_model() takes long enough, the modal is shown and then hidden correctly. But when run_model() is fast, both ui.modal_show() and ui.modal_remove() are called in the same reactive tick, i.e. both messages are sent to the server at the same time.

Here's a complete example (also on shinylive) that demonstrates the issue:

import time

from shiny import reactive
from shiny.express import input, render, ui, session


def run_model(delay=10.0):
    start_time = time.time()
    while time.time() - start_time < delay:
        pass
    return time.time()

def the_modal():
    return ui.modal(
            "The model is running, please wait.",
            title="Running model",
            easy_close=False,
            footer=None,
        )


ui.input_action_button("run", "Run Model")

v_results = reactive.value()


@reactive.effect
@reactive.event(input.run)
def runModel():
    ui.modal_show(the_modal())

    # adjust the delay in seconds down to zero
    results = run_model(delay=2)
    
    v_results.set(results)
    ui.modal_remove()

If you adjust the model running delay down to a small value close to or equal to zero, the modal is shown but doesn't go away. After working with this example a bit, I couldn't find a way to restructure the server logic to reliably avoid this problem.

I'm beginning to suspect that this is a subtle side-effect of the client-side showModal function being asynchronous while the removeModal() function is synchronous (shinyapp.ts). If that's the case, the upstream fix could be relatively straightforward.

@Pierre-Bartet
Copy link
Contributor Author

I tried closing the modal in a separate effect, but the problem is still there (probably because it is still inside the same reactive tick):

@reactive.effect
@reactive.event(input.run)
def runModel():
    ui.modal_show(the_modal())
    results = run_model(delay=0)
    v_results.set(results)

@reactive.effect
@reactive.event(v_results)
def remove_modal():
    ui.modal_remove()

@gadenbuie
Copy link
Collaborator

I can't reproduce this in Shiny for R using the same app; I now think this bug is specific to Shiny for Python.

@wch
Copy link
Collaborator

wch commented Apr 30, 2024

It's possible that it's happening in Python and not R because Python sends the back-to-back messages closer together.

I dug into it a little bit and I think the problem has to do with the Bootstrap Modal component.

I stepped through the code I noticed that when the .modal("hide") is called, the modal wasn't visible yet.

From https://getbootstrap.com/docs/4.0/components/modal/#methods:

All API methods are asynchronous and start a transition. They return to the caller as soon as the transition is started but before it ends. In addition, a method call on a transitioning component will be ignored.

So one possibility is in our .show() method, to wait for the shown.bs.modal event before returning from the function, which we should be able to do because .show() is async.

@wch
Copy link
Collaborator

wch commented Apr 30, 2024

I looked at it a bit more, and shown.bs.modal is triggered only after the entire thing has faded in, which would be an unacceptable delay for stopping processing of all the client side code.

I can think of some other solutions to this, but they are kind of complicated for this small issue -- maybe it would be best to just wait for a tick before calling modal("hide"). (I wish we could just set a state and have the UI reflect that state, instead of doing this all imperatively!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants