dreamRs / esquisse

RStudio add-in to make plots interactively with ggplot2
https://dreamrs.github.io/esquisse
Other
1.78k stars 229 forks source link

Possible bug - Target boxes not populated when 'selected' is used in latest version (1.0.1) #165

Closed JamesBrownSEAMS closed 3 years ago

JamesBrownSEAMS commented 3 years ago

I had an app deployed for a client using the selected functionality in the dragulaInput() function and have recieved a comment that items in the target boxes "disappeared" following the clients update to v1.0.1

I investigated and have reproduced the issue but have been unable to resolve it. Test code is:

# esquisse bug testing ---------------------------------------------------------
library(shiny)
library(esquisse)

labels <- LETTERS[1:4]
values <- 1:4
select <- list(B = 2, D = 4)

ui <- fluidPage(
  tags$h2("Demo dragulaInput"),
  tags$br(),
  fluidRow(
    column(
      width = 6,
      dragulaInput(
        inputId = "dad1",
        label = "Default:",
        sourceLabel = "Source",
        targetsLabels = labels,
        choices = values,
        selected = select,
        width = "100%",
        replace = FALSE
      ),
      verbatimTextOutput(outputId = "result1"),
    )
  )
)

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

  output$result1 <- renderPrint(str(input$dad1))
}

shinyApp(ui = ui, server = server)

and this produced the following output when viewed in the STudio built in browser: image

Previously, the same code produced the image seen in this post https://github.com/dreamRs/esquisse/issues/135#issuecomment-739815504

pvictor commented 3 years ago

Sorry for that, this should be fixed if you re-install from GitHub.

Victor

JamesBrownSEAMS commented 3 years ago

I have updated the package and the test app above is now behaving correctly however the full app is not; only 3 of the selected labels (out of about 10) appear in the UI. I am debugging to see if I can find out why only a subset of the selected items is displayed and will update again with findings.

pvictor commented 3 years ago

Arf.. Can you share your application ? I'll give a look

JamesBrownSEAMS commented 3 years ago

Thanks for the offer, I'm not sure if I can share the app but I'll talk to a few people and see what's said.

JamesBrownSEAMS commented 3 years ago

I wasn't able to get permission to send the app but I have been doing some additonal debugging. In the app, the 3 items that appeared via selected were all instances were the choice value and target label were the same. The ones that were missing were instances were this was not the case. I altered the app code so that the selected list was was named for the target labels rather than the target values (as it was previously) and this appears to have resolved the issue.

In the test case I recreated this using:

# esquisse bug testing ---------------------------------------------------------
library(shiny)
library(esquisse)

values <- LETTERS[1:4]
labels <- c("A and B", "C and D", "E and F", "G and H")
ids    <- c("AB", "CD", "E and F", "GH")

select        <- values[1:2]
names(select) <- ids[2:3]
select        <- as.list(select)

ui <- fluidPage(
  tags$h2("Demo dragulaInput"),
  tags$br(),
  fluidRow(
    column(
      width = 6,
      dragulaInput(
        inputId = "dad1",
        label = "Default:",
        sourceLabel = "Source",
        targetsLabels = labels,
        targetsIds = ids,
        choices = values,
        selected = select,
        width = "100%",
        replace = FALSE,
        ncolGrid = 4
      ),
      verbatimTextOutput(outputId = "result1"),
    )
  )
)

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

  output$result1 <- renderPrint(str(input$dad1))
}

shinyApp(ui = ui, server = server)

Which gave image

As you can see, the instance where the target label and target id were the same shows the selected item, the other does not. This suggests that recent changes to the package have changed the behaviour of the selected list so that the list names must match the targetLabels instead of the targetIds (which was working previously).

pvictor commented 3 years ago

Yes you're right, labels were used for selected values even if IDs were provided. This is fixed. Let me know how it works for you.

JamesBrownSEAMS commented 3 years ago

Retested and it looks like selected list names are back to responding to the targetIds. This means that if targetIds and targetLabels are supplied the selected list names must match the Ids, which is consistent with the previous versions of the package before I raised this case. You latest updates appear to have resolved the issue