initial commit of bookmark script for bookmark include instead of exc… #69
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| print('bookmarked') | ||
| # Trigger bookmarking whenever relevant inputs change | ||
| session$doBookmark() |
Check warning
Code scanning / lintr
no visible binding for global variable 'session' Warning
There was a problem hiding this comment.
Had an initial scan through. A few initial things we could do with having on this:
- Run
styler::style_dir()to clean up the code layout a little - Run
lintr::lint_dir()and resolve the issues that it flags - Set up a handful of tests for the new function to get run by
devtools::test()alongside the rest of the testing suite
| # print(paste("ExcludedIDs:", paste(toExclude, collapse = ", "))) | ||
|
|
||
| # exclude all remaining inputs from bookmark | ||
| setBookmarkExclude(toExclude) |
Check warning
Code scanning / lintr
no visible global function definition for 'setBookmarkExclude' Warning
| #' }) | ||
| set_bookmark_include <- function(bookmarking_whitelist) { | ||
| # exclude the white list from complete list | ||
| to_exclude <- setdiff(names(input), bookmarking_whitelist) |
Check warning
Code scanning / lintr
local variable 'to_exclude' assigned but may not be used Warning
| #' }) | ||
| set_bookmark_include <- function(bookmarking_whitelist) { | ||
| # exclude the white list from complete list | ||
| to_exclude <- setdiff(names(input), bookmarking_whitelist) |
Check warning
Code scanning / lintr
no visible binding for global variable 'input' Warning
| to_exclude <- setdiff(names(input), bookmarking_whitelist) | ||
|
|
||
| # exclude all remaining inputs from bookmark | ||
| setBookmarkExclude(toExclude) |
Check warning
Code scanning / lintr
no visible binding for global variable 'toExclude' Warning
| to_exclude <- setdiff(names(input), bookmarking_whitelist) | ||
|
|
||
| # exclude all remaining inputs from bookmark | ||
| setBookmarkExclude(toExclude) |
There was a problem hiding this comment.
Looks like you've switched from to_exclude to toExclude here?
There was a problem hiding this comment.
Also, can you call functions explicitly, so in this case shiny::setBookmarkExclude(...)
| # exclude all remaining inputs from bookmark | ||
| setBookmarkExclude(toExclude) | ||
|
|
||
| print("bookmarked") |
There was a problem hiding this comment.
Should use message(), warning() etc rather than print() wherever possible
| #' onBookmarked(function(url) { | ||
| #' updateQueryString(url) | ||
| #' }) | ||
| set_bookmark_include <- function(bookmarking_whitelist) { |
There was a problem hiding this comment.
Do you need to pass session and input as variables here for them to pass through to the function content?
…lude
Brief overview of changes
Added a script to create bookmark include function
...
Why are these changes being made?
for explicit include instead of exclude
...
Detailed description of changes
script added to dfeshiny package
...
Additional information for reviewers
...
Issue ticket number/s and link
to close issue #96
...