dreamRs / esquisse

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

Use of 'selected' to prepopulate targets and expected impact on source #135

Closed JamesBrownSEAMS closed 2 years ago

JamesBrownSEAMS commented 3 years ago

I have been using the esquisse package for a couple of internal projects at work and found it to be very useful. I am currently experimenting with the selected parameter to pre-populate some of the targets where the correct values can be identifed without user input. The output of this is not what I was expecting and I was curious if this was by design or a bug of some sort.

Example code:

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%"
      ),
      verbatimTextOutput(outputId = "result1"),
    )
  )
)

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

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

if (interactive())
  shinyApp(ui = ui, server = server)

Output given: image

Output expected: image

pvictor commented 3 years ago

Thanks, seems like a bug, I'll look into it.

Victor

trf66 commented 3 years ago

Maybe you can modify the statment in the R/esquisserUI.R replace TRUE by FALSE . I tred it and it's ok for me

   top_right = dragulaInput(
    inputId = ns("dragvars"), 
    sourceLabel = "Variables", 
    targetsLabels = c("X", "Y", "Fill", "Color", "Size", "Group", "Facet"), 
    targetsIds = c("xvar", "yvar", "fill", "color", "size", "group", "facet"),
    choices = "",
    badge = FALSE, 
    width = "100%", 
    height = "100%",
    replace = FALSE               <--------      
  ),
JamesBrownSEAMS commented 3 years ago

replace = FALSE is the default state, which is why I ommitted it in my original comment, but I tried replace = FALSE and replace = TRUE before raising the query and got the results I posted in the opening comment both times. I am not clear from the example code you supplied how this relates to the selected parameter given the suplied code does not use it.

JamesBrownSEAMS commented 3 years ago

I took another look at the behaviour and it appears that trf66's comment may indicate a second issue at play.

With selected defined, as in my opening example, there is duplication of the input values in source and target boxes. This happens regardless of the state of the replace parameter. I checked what happened if I set selected = NULL but kept everything else the same as the opening example. When replace = FALSE the dragable obejcts behave as expected, they move from source to target boxes, but when replace = TRUE they are copied from source to target creating a duplicate.

pvictor commented 3 years ago

Hello,

With replace=TRUE, labels can be dragged in several target boxes and there can be only one label in a target boxes at the time.

With replace=FALSE (default), labels can be dragged only once and there can be several labels in the same target boxes.

I pushed a fix for your example, if you want to try

Victor

JamesBrownSEAMS commented 3 years ago

I installed the development release and with replace = FALSE the updated behaviour with a selected parameter is now as I had expected it to be: image

Thanks for looking into this.

JamesBrownSEAMS commented 3 years ago

I am a little confused as to why replace = TRUE would create duplicate labels.

I had expected that for replace = TRUE the only difference would be that if a label already exists in a target box it is moved to where ever the replacement label came from, be it another target box or the source, and only source could hold multiple labels. I had not expected replace = TRUE to copy the labels rather than move them. I am not sure I understand why this would be a desired effect for the replace parameter.

It feels like there should be a separate copy = TRUE parameter that controls whether items are duplicated or not...

pvictor commented 3 years ago

Yes the name replace is badly chosen, originally the point was to use a same variable for different aesthetic in the main addin. An additional argument copy would be great, I've to look how I can implement this.