Appsilon / shiny.semantic

Shiny support for powerful Fomantic UI library.
http://appsilon.github.io/shiny.semantic
Other
497 stars 96 forks source link

[Bug]: modal function does not handle JS callbacks #426

Open Arlisaha opened 1 year ago

Arlisaha commented 1 year ago

Guidelines

Project Version

No response

Platform and OS Version

No response

Existing Issues

No response

What happened?

When attaching rules on settings, modal function does not check on html::htmlwidgets::JS class. Consequently, when changing callbacks behavior (like onApprove and onDeny settings), JS code is sent to UI as a string instead of a function.

A solution could be to change :

attach_rule <- function(id, behavior, target, value) {
        is_boolean <- (value == "false" || value == "true")
        paste0("$(\'#", id, "\').modal(\'", behavior, "\', \'", target, "\', ", if (!is_boolean) "\'", value, if (!is_boolean) "\'", ")")
    }

to :

attach_rule <- function(id, behavior, target, value) {
        do_not_wrap <- (value == "false" ||
            value == "true" ||
            "JS_EVAL" %in% class(value))
        paste0("$(\'#", id, "\').modal(\'", behavior, "\', \'", target, "\', ", if (!do_not_wrap) "\'", value, if (!do_not_wrap) "\'", ")")
    }

Steps to reproduce

  1. In UI, init a modal like
    modal(
    id       = "my-id",
    settings = list(
        c("onApprove", htmlwidgets::JS("() => false"))
    ),
    ... # modal content,
    footer   = button(input_id = "my-btn", label = "Do not close modal on click !")  
    )
  2. When running, click on my-btn button should not close modal because JS function return false. However, not only it closes modal, but JS console raises an error due to onApprove being a string instead of a function.

Expected behavior

Code wrapped in html::htmlwidgets::JS should be escaped.

Attachments

No response

Screenshots or Videos

No response

Additional Information

No response

Arlisaha commented 1 year ago

After further investigations, it seems that it demands to change the way settings are given to modal, instead of using ordered vector, one could use lists :

settings = list(
    c("blurring", "true"),
    c("closable", "false"),
    c("onApprove", htmlwidgets::JS("() => false"))
)

should become :

settings = list(
    blurring  = "true",
    closable  = "false",
    onApprove = htmlwidgets::JS("() => false")
)

and modal apply part would go from :

...
shiny::tagList(lapply(settings, function(x) tags$script(attach_rule(id, "setting", x[1], x[2]))))

to :

...
shiny::tagList(lapply(names(settings), function(x) tags$script(attach_rule(id, "setting", x, settings[[x]]))))