fix for singe values + combining auto-complete lists
vnijs opened this issue · 13 comments
@saurfang I just pushed several updates to master and have two questions for you
- 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 - 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
@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.
I wasn't able to the reproduce the issue you described. Or are you experiencing a different problem?
Initial state:
Select airquality
:
Select mtcars
again:
─ 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-22
─ Packages ──────────────────────────────────────────────────────────────────────────────────────────────────────
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)
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()
})
})
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.
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()),
)
})
I don't quite have the time to look into this until maybe week after next. For the time being, maybe you can check
- Verify if
. completers
is empty before you push since editor comes with default completers - Verify all completers you referred in the constructor exists at the time of construction by checking JS console for any errors
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!
Should be resolved (sufficiently) in 0.4.0 on its way to CRAN now