Skip to content

Greg filter select default #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

greg-minshall
Copy link

hi. responding to your plea for help in issue 70, i made these changes, which seem to work.

i am not a javascript/front-end expert. but, i did make one change to your crosstalk-input-select initializer, which is to wait for "window" to be "load"ed. i hope that makes both more robust. (but, if that's a mistake, i'd love to know.)

i hope this helps. and, i'd love to see this functionality pop up in a CRAN near me!

and, apologies, i'm also not sure i'm going to do this pull correctly. i tried to only commit the bits that i changed (rather than the knock-ons from roxygen2, etc.). i can re-do, resubmit.

thanks for crosstalk, and cheers.

(and, maybe???? make filter_select()'s 'selected' more robust, by
waiting until "window" is "loaded".)
@japhir
Copy link

japhir commented Apr 21, 2020

Hey! I've just tried to use this by installing your branch and just wanted to say that (barring some startup problems with making sure I'm loading your branch's code instead of the default), it works perfectly! Thanks for the effort!

  devtools::install_github("greg-minshall/crosstalk",
                           ref = "greg-filter-select-default",
                           lib = "~/R/x86_64-pc-linux-gnu-library/")
library(crosstalk, lib.loc = "~/R/x86_64-pc-linux-gnu-library/")
library(tibble)

dat <- tibble::tibble(a = 1:10,
                      b = 11:20,
                      c = stats::rnorm(10),
                      d = sample(letters[1:3], 10, TRUE),
                      log = sample(c(TRUE, FALSE), size = 10, TRUE))

dt <- SharedData$new(dat)

# works
fd <- filter_checkbox("col_d", "Column D", dt, ~d, selected = "c")

# expected syntax, works
fc <- filter_checkbox("col_log", "Column Logical", dt, ~log, selected = TRUE)

# based on docstring that says it should be a string, works!
fc2 <- filter_checkbox("col_log", "Column Logical", dt, ~log, selected = "TRUE")

Created on 2020-04-21 by the reprex package (v0.3.0)

sessioninfo
sessionInfo()
#> R version 3.6.3 (2020-02-29)
#> Platform: x86_64-pc-linux-gnu (64-bit)
#> Running under: Arch Linux
#> 
#> Matrix products: default
#> BLAS:   /usr/lib/libopenblasp-r0.3.9.so
#> LAPACK: /usr/lib/liblapack.so.3.9.0
#> 
#> locale:
#>  [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
#>  [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
#>  [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
#>  [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
#>  [9] LC_ADDRESS=C               LC_TELEPHONE=C            
#> [11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] tibble_3.0.0         crosstalk_1.0.1.9000
#> 
#> loaded via a namespace (and not attached):
#>  [1] Rcpp_1.0.4.6     knitr_1.28       magrittr_1.5     xtable_1.8-4    
#>  [5] R6_2.4.1         rlang_0.4.5      fastmap_1.0.1    fansi_0.4.1     
#>  [9] stringr_1.4.0    highr_0.8        tools_3.6.3      xfun_0.13       
#> [13] cli_2.0.2        htmltools_0.4.0  ellipsis_0.3.0   lazyeval_0.2.2  
#> [17] assertthat_0.2.1 yaml_2.2.1       digest_0.6.25    lifecycle_0.2.0 
#> [21] crayon_1.3.4     shiny_1.4.0      later_1.0.0      vctrs_0.2.4     
#> [25] promises_1.1.0   glue_1.4.0       evaluate_0.14    mime_0.9        
#> [29] rmarkdown_2.1    stringi_1.4.6    pillar_1.4.3     compiler_3.6.3  
#> [33] jsonlite_1.6.1   httpuv_1.5.2     pkgconfig_2.0.3

@apacheco61
Copy link

I tried @japhir code but I got the following error and can't install this version of crosstalk. Any idea why is this happening?
Error: Failed to install 'crosstalk' from GitHub: (converted from warning) 'lib = "~/R/x86_64-pc-linux-gnu-library/"' is not writable

@greg-minshall
Copy link
Author

@apacheco61 probably you should leave off the lib = "~/R/x86_64-pc-linux-gnu-library/" from the call to install_github() (and, libloc from the library() call), as your directory layout is probably different.

@japhir
Copy link

japhir commented Apr 21, 2020

Yeah that was just needed on my part, as on Linux I've installed some packages to my root directory and others (usually dev packages) to this local folder. But I'm glad you've tried out the code to install the branch. I think it's probably useful to include template code on how to install it in feature branch comments so that the adventurous can try it out!

@jcheng5
Copy link
Member

jcheng5 commented Apr 21, 2020

Thank you for this, @greg-minshall! Bit busy at the moment but I'll review this in a couple of days.

@japhir
Copy link

japhir commented Apr 21, 2020

Perhaps the docstring should be updated that it doesn't only work with strings but with any vector? Or doesn't it work with integers/doubles?

# based on docstring that says it should be a string, works!
fc2 <- filter_checkbox("col_log", "Column Logical", dt, ~log, selected = "TRUE")

@apacheco61
Copy link

Ok this seems to be kind of working for me but I found two things: 1) my code is also calling leaflet that looks like loads crosstalk automatically (?) so before I install crosstalk from here I need to remove leaflet first (otherwise this won't work). This is a problem because I use leaflet later in my code. 2) I have plotly bar graphs and when generating the output I receive the following message. It looks like I need to modify my code so I can successfully get the outputs that I need.
No trace type specified: Based on info supplied, a 'scatter' trace seems appropriate. Read more about this trace type -> https://plot.ly/r/reference/#scatter

@CLAassistant
Copy link

CLAassistant commented Apr 23, 2020

CLA assistant check
All committers have signed the CLA.

@jcheng5
Copy link
Member

jcheng5 commented Apr 23, 2020

@greg-minshall Code looks good! Don't worry about the conflicts, I'll resolve them.

If you wouldn't mind signing the CLA, I can take this forward. Thanks again!

@greg-minshall
Copy link
Author

thanks!

@cpsievert cpsievert changed the base branch from master to joe/feature/filter-select-default June 3, 2021 18:37
@@ -32,6 +32,11 @@ input.register({
lastKnownKeys = keyArray;
ctHandle.set(keyArray);
}
}
$el.on("change", "input[type='checkbox']", updateFilter);
// https://stackoverflow.com/a/2926235/1527747
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// https://stackoverflow.com/a/2926235/1527747

@@ -49,7 +49,10 @@ input.register({
}
}
selectize.on("change", updateFilter);
updateFilter();
// https://stackoverflow.com/a/2926235/1527747
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// https://stackoverflow.com/a/2926235/1527747

@cpsievert
Copy link
Contributor

Thanks for your contribution @greg-minshall, we'll be taking this on in #70

@cpsievert cpsievert closed this Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants