JohnCoene / tippy

💬 Tippy.js for Shiny
http://tippy.john-coene.com/
Other
76 stars 2 forks source link

`tippy_show` doesn't trigger the appropriate tip, it triggers all visible. #33

Closed yogat3ch closed 1 year ago

yogat3ch commented 1 year ago

Hi @JohnCoene, Hope you're doing well! I'm still enthused about learning how Javascript and R function together and have been diving in and trying to understand how more of these R packages that port JS libraries are working. Hopefully this issue and associated PR are helpful.

I'm trying to use the new tippy_show method . This issue requires merging #32 to get the R side of the tippy_* methods functioning properly first.

The tippy_show method doesn't work to show the actual selectors passed into the function, instead it's defaulting to show all visible.

I think it has something to do with the Map get method.

This JS get method for the Map class is being passed an array of selectors via the tippy-method routine, however the get method doesn't seem to work properly to find DOM selectors. When I look at window.tooltips in the console, I don't see a get method, I only see those you've declared in the file linked above.

I found a workaround that works using shinyjs which is demoed below, which may provide a clue as to what needs to change to get the show method working properly.

Here's a reprex to demonstrate:

library(shiny)
# devtools::load_all("~/Documents/R/Contributor_Repos/tippy")
# Either merge the branch, or pull it and load your local tippy repo here
require(shinyjs)
ui <- fluidPage(
  shinyjs::useShinyjs(),
  div(
    tippy::tippy(
      actionButton("test", "Test"),
      content = "Im the test button"
    ),
    actionButton("show",
                 "I should show the tooltip, but I show all tooltips"),
    tippy::tippy(
      actionButton("test2", "Test2"),
      content = "But I show the actual tooltip"
    )

  )
)

server <- function(input, output, session) {

  observeEvent(input$show, {
    tippy::tippy_show("test")
  })
  observeEvent(input$test2, {
    shinyjs::runjs("window.tooltips.show(['#test'])")
  })

}

shinyApp(ui, server)

If you have some good guidance on how to fix this I might be able to fix it, if that's easier.

JohnCoene commented 1 year ago

Thank you, as far as I can tell your PR fixes it (merged), you found the issue :)

Or am I missing something?

Thanks for the fix!

yogat3ch commented 1 year ago

Hi @JohnCoene, Thanks for merging #32, though it does not fix this issue - it fixes an issue on the R side where the .is_tag argument wasn't passed down so the functions failed outright. The reprex demonstrates how the tippy_show method doesn't show the specified tip, it shows all tips on the page - I'm not sure of the reason because my JS isn't there yet, but I've highlighted some potential places to start in the description above. If you have some ideas and can point me in the right direction I might be able to implement a fix, or you can do it yourself, whichever is easiest. Does that make sense?

JohnCoene commented 1 year ago

I think I found the issue sapply returns a named list which serialises to a JSON object where we want an array.

Looks good now. Hope I got it right this time!

yogat3ch commented 1 year ago

@JohnCoene Ahh, Good catch. Thanks for fixing that!

JohnCoene commented 1 year ago

Great.

Correction:

I think I found the issue sapply returns a named list which serialises to a JSON object where we want an array.