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

Regression bug: dragcreate event callback is not called when draggable is created dynamically #58

Closed
daattali opened this issue Mar 10, 2020 · 12 comments

Comments

@daattali
Copy link

This broke in commit 9f1e613fef43809d2e85253771c11747ccee6345 (it worked up until the commit before that one)

Example: here is a simple app that start off with a draggable element and every time you click the "Add" button it adds another draggable element. Every time a draggable is created, you should see an alert message. This works for draggable elements that are created on page load, but it does not work for draggables added with insertUI. It did work up until the commit mentioned above.

library(shiny)

ui <- fluidPage(
  tags$script("$(document).on('dragcreate', '.ui-draggable', function(e, ui) { alert('test'); });"),
  shinyjqui::jqui_draggable(div("test")),
  actionButton("add", "add")
)

server <- function(input, output, session) {
  observeEvent(input$add, {
    insertUI(
      "#add", "afterEnd", 
      shinyjqui::jqui_draggable(div(paste("test", input$add)))
    )
  })
}

shinyApp(ui, server)
@daattali
Copy link
Author

daattali commented May 5, 2020

This problem is happening because you changed from inserting the javascript to the head, to just inserting javascript code at a random location. This works for Rmd, but breaks in shiny apps. Here's a bug report about this rstudio/shiny#1545

I would wager that shiny is a more common usecase than Rmd so it would be worthwhile to revert this. You could also have some logic that conditionally adds to the head tag or not depending on whether it's a shiny app or not.

@trafficonese
Copy link

Could #18 be related aswell? It's a closed issue, that should be reopened I guess. At least both given examples dont work for me.

@daattali
Copy link
Author

daattali commented May 5, 2020

The issue I'm describing is happening only since February 2020 because of the issue I linked to above. Your issue is from 2 years ago, before this change happened, so I don't think it's related. But I haven't actually looked into your issue so I can't say for sure.

@trafficonese
Copy link

The original issue is from 2 years ago, I encountered a similar problem yesterday and added my example there.

@daattali
Copy link
Author

daattali commented May 5, 2020

You'll have to debug it to see if it's the same code that breaks it. The way I debugged this issue is that I kept going backwards in git commits until I found a commit where the code worked, and then I looked at exactly what changed in the commit and tried slowly adding those changes to the code until it broke. That's how I pinpointed the problem. I'm sorry but I dont have the bandwidth to troubleshoot your issue, from first glance it doesnt seem to be the same, but maybe it is if you look deeper.

@daattali
Copy link
Author

@Yang-Tang Would you accept a PR that fixes this issue, if it means Rmd won't work? This is a tradeoff between rmd and shiny.

@smouksassi
Copy link

hi, @Yang-Tang is there hope to fix this regression? best regards

@Yang-Tang
Copy link
Owner

Hi @daattali @trafficonese @smouksassi , I finally got time to look into this. Apologize for such a long time delay in response.
As Dean mentioned, this is related to rstudio/shiny#1545 (comment), so I used the onload trick mentioned by bborgesr to get around. Now, with this commit af36f95, both the current issue and #18 should have been resolved.

@daattali
Copy link
Author

@smouksassi you might be interested in this :)

@smouksassi
Copy link

absolutely looking forward to make gradientinput work again in ggquikeda CRAN ! awaiting merge to main thanks @Yang-Tang

@Yang-Tang
Copy link
Owner

Hi @smouksassi I have submitted the new shinyjqui to CRAN. Hope it works in you package. I'll close this issue now.

@smouksassi
Copy link

thanks a million ! have a great day !

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

No branches or pull requests

4 participants