trestletech/shinyAce

fix for singe values + combining auto-complete lists

vnijs opened this issue · 13 comments

vnijs commented

@saurfang I just pushed several updates to master and have two questions for you

  1. Could you take a look at these lines that should address an issue when the number of values for a key on the comps list is of length 1. This line I added to the autocomplete example will throw an comps[key].map is not a function error without this change. If there is a better way to fix this issue please let me know
  2. I'm still having trouble determining how to best combine lists of auto completion hints. I added inst/examples/07-autocomplete-combine to illustrate something that I think would be useful
vnijs commented

@saurfang I found a way to combine static lists. There does seem to be a problem using autocomplete with shiny's renderUI however. I edited example 07 to illustrate. When the editor is specified in ui.R it works as intended. When you use renderUI, however, it does not. Could you take a look?

Glad to hear the progress! I'll take a look tomorrow.

vnijs commented

I wasn't able to the reproduce the issue you described. Or are you experiencing a different problem?

Initial state:

image

Select airquality:

image

Select mtcars again:

image

Session info ──────────────────────────────────────────────────────────────────────────────────────────────────
 setting  value                       
 version  R version 3.4.4 (2018-03-15)
 os       macOS High Sierra 10.13.4   
 system   x86_64, darwin15.6.0        
 ui       RStudio                     
 language (EN)                        
 collate  en_US.UTF-8                 
 tz       America/Los_Angeles         
 date     2018-04-22Packages ──────────────────────────────────────────────────────────────────────────────────────────────────────
 package     * version     date       source                             
 assertthat    0.2.0       2017-04-11 cran (@0.2.0)                      
 backports     1.1.2       2017-12-13 cran (@1.1.2)                      
 bindr         0.1.1       2018-03-13 cran (@0.1.1)                      
 bindrcpp      0.2.2       2018-03-29 CRAN (R 3.4.4)                     
 callr         2.0.3       2018-04-12 Github (r-lib/callr@2a73c8f)       
 cli           1.0.0       2017-11-05 cran (@1.0.0)                      
 clisymbols    1.2.0       2017-05-21 cran (@1.2.0)                      
 crayon        1.3.4       2018-04-12 Github (r-lib/crayon@95b3eae)      
 debugme       1.1.0       2017-10-22 cran (@1.1.0)                      
 desc          1.1.1.9002  2018-03-26 Github (r-lib/desc@2f8308e)        
 devtools    * 1.13.5.9000 2018-04-03 Github (r-lib/devtools@2f7bc84)    
 digest        0.6.15      2018-01-28 cran (@0.6.15)                     
 dplyr       * 0.7.4       2017-09-28 CRAN (R 3.4.2)                     
 glue          1.2.0       2017-10-29 cran (@1.2.0)                      
 htmltools     0.3.6       2017-04-28 cran (@0.3.6)                      
 httpuv        1.3.6.2     2018-03-02 cran (@1.3.6.2)                    
 jsonlite      1.5         2017-06-01 cran (@1.5)                        
 magrittr      1.5         2014-11-22 cran (@1.5)                        
 memoise       1.1.0       2017-04-21 CRAN (R 3.4.0)                     
 mime          0.5         2016-07-07 CRAN (R 3.4.0)                     
 pillar        1.2.1       2018-03-26 Github (hadley/pillar@2065df9)     
 pkgbuild      1.0.0       2018-04-03 Github (r-lib/pkgbuild@015f7f6)    
 pkgconfig     2.0.1       2017-03-21 cran (@2.0.1)                      
 pkgload       1.0.0       2018-04-03 Github (r-lib/pkgload@f827a27)     
 R6            2.2.2       2017-06-17 CRAN (R 3.4.0)                     
 Rcpp          0.12.16     2018-03-13 CRAN (R 3.4.4)                     
 rlang         0.2.0.9001  2018-04-17 Github (r-lib/rlang@82b2727)       
 rprojroot     1.3-2       2018-01-03 cran (@1.3-2)                      
 rsconnect     0.8.8       2018-03-09 CRAN (R 3.4.4)                     
 rstudioapi    0.7.0-9000  2018-03-26 Github (rstudio/rstudioapi@423d925)
 sessioninfo   1.0.0       2017-06-21 cran (@1.0.0)                      
 shiny       * 1.0.5       2017-08-23 cran (@1.0.5)                      
 shinyAce    * 0.3.0.1     2018-04-22 local                              
 testthat      2.0.0       2017-12-13 cran (@2.0.0)                      
 tibble        1.4.2       2018-01-22 cran (@1.4.2)                      
 usethis     * 1.3.0.9000  2018-03-26 Github (r-lib/usethis@59b0837)     
 withr         2.1.2       2018-04-12 Github (jimhester/withr@79d7b0d)   
 xtable        1.8-2       2016-02-05 cran (@1.8-2)                      
 yaml          2.1.18      2018-03-08 cran (@2.1.18) 
vnijs commented

When I first start the app (or after refreshing the browser) I see:

image

After switching datasets hints for dplyr do indeed show up but not at first.

ah. got it. I am able to reproduce that. Let me take a look

The issue is that even though on the server side, observe ran after renderUI, on the frontend side, completion update message is received before AceEditor is initialized.

The least pervasive solution I came up with is to check whether the editor has been initialized before updating the autocompletion list. I simply used input[[editor_id]] as a proxy but the downside is that if you re-initialize ace editor in renderUI with the same id, the update would still run before the editor (re)initialization. Therefore you should use a random editor id in renderUI.

The other option is that if you refactor you completion list into a function and specify them both in renderUI as argument to aceEditor, and in observe as with updateAceEditor. This is probably simpler then my reactiveVal gymnastic.


The first option:

ui.R

library(shiny)
# library(shinyAce)
library(dplyr)

shinyUI(fluidPage(
  aceEditorDependencies(),
  titlePanel("shinyAce auto completion - combine completion lists"),
  radioButtons("dataset", "Dataset: ", c("mtcars", "airquality"), inline = TRUE),
  actionButton("rerender", "Re-initialize Editor"),
  # works as expected
  # shinyAce::aceEditor(
  #   "editor",
  #   mode = "r",
  #   value = "select(wt, mpg)\n",
  #   height = "500px",
  #   autoComplete = "live",
  #   autoCompleteList = list(dplyr = getNamespaceExports("dplyr"))
  # )
  # doesn't work as expected
  uiOutput("ace_editor")
))

server.R

library(shiny)
# library(shinyAce)
library(dplyr)

shinyServer(function(input, output, session) {
  
  ## Dataset Selection
  dataset <- reactive({
    get(input$dataset)
  })
  
  editor_id <- reactiveVal()
  initialization_required <- reactiveVal(FALSE)
  
  ## doesn't work as expected
  output$ace_editor <- renderUI({
    input$rerender
    
    message("rendering...")
    # generate random editor id
    id <- basename(tempfile("editor"))
    editor_id(id)
    initialization_required(TRUE)
    
    shinyAce::aceEditor(
      id,
      mode = "r",
      value = "select(wt, mpg)\n",
      height = "500px",
      autoComplete = "live"
    )
  })
 
  ## Update static auto complete list according to dataset
  update_comps <- function() {
    message("updating...")
    comps <- list()
    comps[[input$dataset]] <- colnames(dataset())
    comps <- c(comps, list(dplyr = getNamespaceExports("dplyr")))
    shinyAce::updateAceEditor(session,
      editor_id(),
      autoCompleters = "static",
      autoCompleteList = comps
    )
  }
  observe({
    id <- editor_id()
    if (!initialization_required() || is.null(id) || is.null(input[[id]])) {
      return()
    }
    initialization_required(FALSE)
    
    isolate(update_comps())
  })
  observe({
    update_comps()
  })
})
vnijs commented

I like option 2 :) See the updated code for example 07. Works great. Thanks @saurfang!

The one thing that is missing here, I guess, is to allow the user to set/initialize the autoCompleters to use in the call to aceEditor as well. Now when you add the autocomplete list in aceEditor you cannot restrict the completers to just "static", for example, when you call updateAceEditor

Nice! Agreed. It's just a lot of duplicate code that makes me unhappy :) I'll see if I can find some time next weekend to poke around it.

vnijs commented

I added the autoCompleter argument to the shinyAce function and code to activate the specified auto-complete type. I expected the following lines to do-the-trick but it is not working as intended.

https://github.com/trestletech/shinyAce/blob/master/R/ace-editor.R#L268-L284

In example 07 local recommendations should now not be shown when the editor is initialized but they are (see screenshot). The observe fixes this after the data set has been changed but was not the intention.

  output$ace_editor <- renderUI({
    shinyAce::aceEditor(
      "editor",
      mode = "r",
      value = "select(wt, mpg)\n",
      height = "500px",
      autoComplete = "live",
      autoCompleters = "static",
      autoCompleteList = isolate(comps()),
    )
  })

image

I don't quite have the time to look into this until maybe week after next. For the time being, maybe you can check

  1. Verify if . completers is empty before you push since editor comes with default completers
  2. Verify all completers you referred in the constructor exists at the time of construction by checking JS console for any errors
vnijs commented

Well spotted @saurfang. I was able to get "static" working but not "rlang". The default option when calling aceEditor is not to restrict the set of possible completers. If you want, however, you can restrict to specific completers (or disable all) and update using observe and aceAutocomplete as needed.

I'd like to be able to restrict to "rlang" as well but was not able to get that working. Also, removing some of the code duplication between ace-editor.R and shinAce.js would be very nice.

That said, most of the functionality we want is there and I'd like to submit to CRAN mid-late next week. If you are able to review/refine before then, great. Else we can do that in the next release if that is ok with you.

Thanks again for your contributions @saurfang!

vnijs commented

Should be resolved (sufficiently) in 0.4.0 on its way to CRAN now