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

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

Open
1 task done
Arlisaha opened this issue Dec 7, 2022 · 1 comment · Fixed by #437
Open
1 task done

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

Arlisaha opened this issue Dec 7, 2022 · 1 comment · Fixed by #437
Labels

Comments

@Arlisaha
Copy link

Arlisaha commented Dec 7, 2022

Guidelines

  • I agree to follow this project's Contributing 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 !")  
)
  1. 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 Arlisaha added the bug label Dec 7, 2022
@Arlisaha
Copy link
Author

Arlisaha commented Dec 7, 2022

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

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

Successfully merging a pull request may close this issue.

1 participant