Fixes #1427: add event delegation so that modals do not close by mistake

[bborgesr]

See #1427: when using shinyjs, modals were closing incorrectly when an element inside it them was triggered as hidden.

Example app:

library(shiny)
library(shinyjs)

ui <- basicPage(
  useShinyjs(),
  actionButton("show", "Show modal dialog")
)

server <- function(input, output) {
  observeEvent(input$show, {
    showModal(modalDialog(
      title = "Important message",
      div(id = "foo", "bar"),
      actionButton("toggle", "Show/Hide")
    ))
  })

  observeEvent(input$toggle, {
    toggle("foo")
  })
}

shinyApp(ui = ui, server = server)

Before this fix, the modal in the app above would be removed entirely when the toggle button was pressed. I think this is because the modal's "hidden.bs.modal" event was bubbling down to the elements inside of it (i.e. if any element inside it was triggered as hidden, it would register that as a hidden event for the whole modal and remove it). With this fix, the "hidden.bs.modal" event only applies to elements with class "shiny-modal-wrapper", so the app above works as expected.

[wch]

Looks good. Can you add a NEWS item?

[jcheng5]

I think the traditional way to do this is not to use a selector, but in the handler to check whether this === event.target or event.currentTarget === event.target.

[wch]

Oh, also, shouldn't the selector be #shiny-modal-wrapper instead of .shiny-modal-wrapper?

[bborgesr]

Oh wow, great catch @wch. But this was working -- does this mean this code is just never called? (the modal closes just fine when either the Dismiss button is clicked or we click outside it and easyClose = TRUE...)

Anyway, I changed it to Joe's suggestion (although I wasn't sure if I should use e.target or e.currentTarget). It works once again. But now I'm a little afraid I'm not testing it right...

[wch]

Yes, I think that the code was never getting called, and contents were only getting hidden, but not unbound and removed from the DOM.

I think e.target is correct., but can you test to make sure that it actually enters the if statement in the right cases? That is, when the modal is hidden, but not when other hide events occur.

[bborgesr]

Alright, this really does work now. It turns out that e.target must be equal to $("#shiny-modal"), not this. So, it is not the most elegant piece of code ever, but it makes sense (this refers to the wrapper, which has 0 width, so it will never be the e.target).