-
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
Part 2 - Make init code inline #44
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rmbielby
approved these changes
Aug 19, 2024
cjrace
added a commit
that referenced
this pull request
Sep 4, 2024
* Bump roxygen2 version to latest * Add branch note to readme and copy text from dfeR, add contributing file * add lintr config with limit matching CRAN and style_dir() to tidy code * add PR template * fix contributing headers * tidy up after conflict resolution * attempt a github action for the test dashboard * try to stop false positives coming from global variable binding by pre-loading the packages * remove renv from test dashboard yaml * fix typo in test_dashboard gha * try adding devtools to description (test_dashboard gha) * make sure that GHA will fail if the test dashboard tests fail * Revert "make sure that GHA will fail if the test dashboard tests fail" This reverts commit 1503c49. * put the local package into the setup r dependencies step, update CodeQL version * add a code of conduct (same as dfeR) * copy dfeR issue templates * update test_dashboard tests (did some tidy up while looking at adding something for the support panel) * rebuild favicons to make them appear on pkgdown site * linting and responding to test notes * fix brackets in code of conduct * add missing families and update ids in test dashboard * Increment version number to 0.3.0 * PR response and standardise examples / vignette for cookies * Add init_analytics() into news! * remove duplicate branch install instructions (turns out we had added it on another branch!) * run r cmd check on all pull requests * Part 1 - update cookie to cookies (#42) * update cookie to cookies * run r cmd check on all pull requests * Part 2 - Make init code inline (#44) * Update init_analytics to have a create_file option and add more unit tests around the content * Move the init_analytics HTML / JS to be written inline * Prevent init_analytics from running in examples and creating scripts in the check directory * update init_cookies to have the JS code inline and add some tests * Part 3 - remove tab dependencies (#43) * update cookie to cookies * remove tabPanel from within our functions * remove images from snapshots as not needed * Switch support panel tests to unit tests (as the UI ones didn't track anything beyond if the tab was selected) * Add tests for cookies_panel_ui * Add note to contributing guidelines about lintr and loading package first * run r cmd check on all PRs * force some extra waits * Clarified usethis makes blank scripts and have updated readme so users will install the correct package * update news.md with changes on this branch * update news.md
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As mentioned in #41, this moves the HTML and JS code used in the init_cookies and init_analytics functions to be written inline in the R code, so that they are easier to maintain / test moving forwards and we aren't relying on a copy of a file from a branch on GitHub itself
As a part of this I've also added in some extra testing around the functions and a way of printing the output to the console instead (I don't expect users to use it much if ever, but I needed it for the testing and it might be generally handy for us moving forwards)
init_cookies() didn't seem to do what the description suggested so I've updated a little on both sides to cover what I think was originally intended. Here's the new messages for the console: