-
Notifications
You must be signed in to change notification settings - Fork 2
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
Cookie module #19
Cookie module #19
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lintr found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
This reverts commit e2b5686.
…aining about object defs
…aining about object defs
methodological information, is available at the following link: ", | ||
tags$a( | ||
href = paste0( | ||
"https://explore-education-statistics.service.gov.uk/find-statistics/", |
Check notice
Code scanning / lintr
Lines should not be more than 80 characters. This line is 89 characters. Note
". The statistical release provides additional ", | ||
tags$a( | ||
href = paste0( | ||
"https://explore-education-statistics.service.gov.uk/find-statistics/", |
Check notice
Code scanning / lintr
Lines should not be more than 80 characters. This line is 89 characters. Note
" and ", | ||
tags$a( | ||
href = paste0( | ||
"https://explore-education-statistics.service.gov.uk/find-statistics/", |
Check notice
Code scanning / lintr
Lines should not be more than 80 characters. This line is 89 characters. Note
#' @param input_cookies The cookie input passed from cookies.js (should always | ||
#' be reactive(input$cookies)) | ||
#' @param input_clear The state of the cookie reset button provided by | ||
#' dfeshiny::support_panel(). Should always be set to reactive(input$cookie_consent_clear). |
Check notice
Code scanning / lintr
Lines should not be more than 80 characters. This line is 91 characters. Note
@@ -0,0 +1,9 @@ | |||
server <- function(input, output, session) { | |||
output$cookie_status <- cookie_banner_server( |
Check warning
Code scanning / lintr
no visible global function definition for 'cookie_banner_server' Warning test
@@ -0,0 +1,18 @@ | |||
ui <- function(input, output, session) { | |||
fluidPage( |
Check warning
Code scanning / lintr
no visible global function definition for 'fluidPage' Warning test
ui <- function(input, output, session) { | ||
fluidPage( | ||
shinyjs::useShinyjs(), | ||
dfe_cookie_script(), |
Check warning
Code scanning / lintr
no visible global function definition for 'dfe_cookie_script' Warning test
fluidPage( | ||
shinyjs::useShinyjs(), | ||
dfe_cookie_script(), | ||
cookie_banner_ui("cookies"), |
Check warning
Code scanning / lintr
no visible global function definition for 'cookie_banner_ui' Warning test
id = "navlistPanel", | ||
widths = c(2, 8), | ||
well = FALSE, | ||
support_panel( |
Check warning
Code scanning / lintr
no visible global function definition for 'support_panel' Warning test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there's a few lintr things to sort, and I suspect from pulling into my branch we may have to mop up / handle some things to stop failures in the CI checks. Nothing to stop this getting merged in though.
I think there's a wider thing we might need to be wary of around expecting certain object names from elsewhere in the dashboard between functions like this and the support_panel() tab, though we can look at that separately.
In general it seems to work pretty reliably (much more so than before in the template), and very excited to the modules working neatly too for chunking the code!
|
||
## Improvements | ||
|
||
* Implemented basic unit testing (currently just on e-mail validation) and UI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may need changing as I've added a couple of other bits of validation, but can do in a separate mop up before 0.2.0 is formally released.
Adding a module to make it easier to add cookie consent tracking to dashboards.
The module covers the following:
This can be added to a dashboard with just two lines of code in the ui.R and one line in server.R, plus copying the js file to the www/ folder.
As part of testing, I've set up a demo / test dashboard in the tests folder and added some basic shiny tests to check the cookie consent control is working as it should do. Does a basic cycle of clear consent -> accept cookies -> clear consent -> reject cookies.