yonicd / slickR

slick carousel htmlwidget for R
https://yonicd.github.io/slickR/
Other
158 stars 14 forks source link

slickR shiny vignette broken #51

Closed ismirsehregal closed 3 years ago

ismirsehregal commented 3 years ago

I just tried to run the slickR shiny vignette but it seems to be broken.

Once the radioButtons are used to switch to stacked or synch mode the input$slick_output_current isn't providing any data. Also the lower carousel isn't rendered properly:

screen

> sessionInfo()
R version 4.0.3 (2020-10-10)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19042)

Matrix products: default

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] gdtools_0.2.3   svglite_1.2.3.2 slickR_0.5.0    xml2_1.3.2      shiny_1.6.0    

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.6        rstudioapi_0.13   magrittr_2.0.1    xtable_1.8-4      R6_2.5.0          rlang_0.4.10     
 [7] fastmap_1.1.0     tools_4.0.3       jquerylib_0.1.3   withr_2.4.1       systemfonts_1.0.1 htmltools_0.5.1.1
[13] ellipsis_0.3.1    yaml_2.2.1        digest_0.6.27     lifecycle_0.2.0   crayon_1.4.0      later_1.1.0.1    
[19] sass_0.3.1        base64enc_0.1-3   htmlwidgets_1.5.3 promises_1.1.1    cachem_1.0.1      mime_0.9         
[25] compiler_4.0.3    bslib_0.2.4       jsonlite_1.7.2    httpuv_1.5.5    

library(shiny)
library(svglite)
library(slickR)

ui <- fluidPage(
  sidebarLayout(
    sidebarPanel(

      sliderInput(inputId = "plot_num", 
                  label = "Number of Plots:", 
                  min = 1, max = 20, value = 5),

      sliderInput(inputId = "n_obs", 
                  label = "Number of observations:", 
                  min = 10, max = 500, value = 100),

      shiny::radioButtons('slick_type',
                          label = 'Carousel Type',
                          choices = c('single','stack','synch'),
                          selected = 'single',
                          inline = TRUE),

      shiny::verbatimTextOutput('current')
    ),
    mainPanel(

      slickROutput("slick_output",width='100%',height='200px')

    )
  )
)

server <- function(input, output) {

  # Create content for the carousel

  plots <- eventReactive(c(input$n_obs,input$plot_num),{

    replicate(input$plot_num,{

      xmlSVG({hist(rnorm(input$n_obs),
                   col = 'darkgray',
                   border = 'white')},
             standalone=TRUE)

    },simplify = FALSE)
  })

  # renderSlickR (We create the slickR objects here)

  output$slick_output <- renderSlickR({

    x <- slickR(plots(),
                slideId = 'myslick',
                height = 600,
                width = '50%') + 
      settings(slidesToShow=3,centerMode=TRUE)

    switch(input$slick_type,
           'single' = x,
           'stack'  = x %stack% x,
           'synch'  = x %synch% x
    )

  })

  # Observe the active slick

  # The htmlwidget is observed by shiny and information can be retrieved. 

  # Using the output name you set for the `renderSlick` object in this example
  # it is `output$slick_output`

  # Using this you can interact server-side "on click" of the active carousel
  # by accessing elements in `input$slick_output_current$`

  # `.clicked_slide`   : The index of the clicked element|
  # `.relative_clicked`: The relative position of the clicked element|
  # `.center_slide`    : The index of the center element|
  # `.total_slide`     : The total number of elements in the carousel|
  # `.active_slide`    : The ID of the active carousel|

  # We will store this information in a new reactive environment
  active_slick <- shiny::reactiveValues()

  shiny::observeEvent(input$slick_output_current,{
    clicked_slide    <- input$slick_output_current$.clicked
    relative_clicked <- input$slick_output_current$.relative_clicked
    center_slide     <- input$slick_output_current$.center
    total_slide      <- input$slick_output_current$.total
    active_slide     <- input$slick_output_current$.slide

    if(!is.null(clicked_slide)){
      active_slick$clicked_slide    <- clicked_slide
      active_slick$center_slide     <- center_slide
      active_slick$relative_clicked <- relative_clicked
      active_slick$total_slide      <- total_slide
      active_slick$active_slide     <- active_slide
    }
  })

  # Show in the UI the values in active_slick

  output$current <- renderText({
    l <- shiny::reactiveValuesToList(active_slick)
    paste(gsub('_',' ',names(l)), unlist(l),sep=' = ',collapse='\n')
  })

}

shinyApp(ui = ui, server = server)
yonicd commented 3 years ago

thanks for opening the issue, will investigate

yonicd commented 3 years ago

This should fix the problem which was having the same ID for two elements in the app.

you can install this branch and it should run fine

The reactive text will observing the carousels will be attached to the last carousel built ie if only 1 carousel the slick1 if two carousels it will track slick2

here is the specific fix:

  output$slick_output <- renderSlickR({

    x <- slickR(plots(),
                slideId = 'slick1',
                height = 600,
                width = '50%') + 
      settings(slidesToShow=3,centerMode=TRUE)

    y <- slickR(plots(),
                slideId = 'slick2',
                height = 600,
                width = '50%') + 
      settings(slidesToShow=3,centerMode=TRUE)    

    switch(input$slick_type,
           'single' = x,
           'stack'  = x %stack% y,
           'synch'  = x %synch% y
           )

  })
ismirsehregal commented 3 years ago

Thanks for the very fast update!

With the new version when i switch from single to stack all is fine. When I then switch to synch I have 3 carousels, back to stack and there are 4.

It's piling up new carousels:

screen

yonicd commented 3 years ago

ha nice. i'll take another look

yonicd commented 3 years ago

try this commit https://github.com/yonicd/slickR/commit/417fd60e013b70540970c1b798897050c3580d2c it should accommodate multiple sliders in the same app including the observers to know which is active

ismirsehregal commented 3 years ago

Using your latest commit the vignette runs fine! Thanks for your time and effort!

ismirsehregal commented 3 years ago

Just saw that the description given in the vignette needs to be aligned with the actual outputs:

.clicked_slide != .clicked etc. (_slide suffix needs to be removed.. or added)

Using this you can interact server-side "on click" of the active carousel by accessing elements in input$slick_output_current$

.clicked _slide : The index of the clicked element| .relative_clicked: The relative position of the clicked element| .center _slide : The index of the center element| .total _slide : The total number of elements in the carousel| .active _slide : The ID of the active carousel|

We will store this information in a new reactive environment active_slick <- shiny::reactiveValues()

shiny::observeEvent(input$slick_output_current,{

clicked_slide <- input$slick_output_current$.clicked relative_clicked <- input$slick_output_current$.relative_clicked center_slide <- input$slick_output_current$.center total_slide <- input$slick_output_current$.total active_slide <- input$slick_output_current$.slide

yonicd commented 3 years ago

Thanks for the comment 👍

ismirsehregal commented 3 years ago

I just realized, that when using https://github.com/yonicd/slickR/commit/417fd60e013b70540970c1b798897050c3580d2c there still is an issue: when stack is selected and we are going back to single both carousels remain.

screen

Furthermore library(gdtools) should be added to the vignette. After a fresh R install:

package or namespace load failed for ‘svglite’ in loadNamespace(j <- i[[1L]], c(lib.loc, .libPaths()), versionCheck = vI[[j]]): there is no package called ‘gdtools’

(svglite was installed via: install.packages("svglite", dependencies = TRUE))

yonicd commented 3 years ago

you are right.

the way the js is written only 1 slider is being removed because it is removing based on the number of slicks that are being created and not the existing ones.

so when moving from 2 to 1 it is looking for only 1. the original code was written assuming you are updating the same number of slicks every time.

yonicd commented 3 years ago

this should fix the issue https://github.com/yonicd/slickR/commit/3505e63400bce35a4b321a03bc4c43e3556a42f8

yonicd commented 3 years ago

closed with pr https://github.com/yonicd/slickR/pull/56