-
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 3 - remove tab dependencies #43
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
cjrace
changed the base branch from
add-branch-install-readme
to
update-cookie-to-cookies
July 31, 2024 14:32
… anything beyond if the tab was selected)
cjrace
changed the title
Part 2 - remove tab dependencies
Part 3 - remove tab dependencies
Aug 2, 2024
Base automatically changed from
update-cookie-to-cookies
to
add-branch-install-readme
August 7, 2024 14:21
Merge branch 'add-branch-install-readme' into remove-tab-dependencies # Conflicts: # R/cookies.R # man/cookies.Rd # man/cookies_banner_server.Rd # man/cookies_banner_ui.Rd # man/cookies_panel_server.Rd # man/cookies_panel_ui.Rd # tests/test_dashboard/tests/testthat/_snaps/UI-01-basic_load/basic_load-001_.new.png # tests/test_dashboard/tests/testthat/_snaps/UI-02-cookies/cookies_consent-001_.png # tests/test_dashboard/tests/testthat/_snaps/UI-02-cookies/cookies_consent-002_.png # tests/test_dashboard/tests/testthat/_snaps/UI-02-cookies/cookies_consent-003_.png # tests/test_dashboard/tests/testthat/_snaps/UI-02-cookies/cookies_consent-004_.png # tests/test_dashboard/tests/testthat/_snaps/UI-03-support_panel/support_panel-001.json
rmbielby
reviewed
Aug 19, 2024
rmbielby
approved these changes
Aug 28, 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.
Worth reviewing #42 first, before then re-pointing this at the branch for #41 which we're using as a big branch of changes for v0.3.0
This changes the support_panel() and cookies_panel_ui() functions to only return the content and not specify the type of tab that it belongs too as mentioned in #41. Aim here is to give flexibility in how the content is used and not tie ourselves down too much. For ourselves we'll want to be switching to bslib tabs soon anyway, and also this allows users to add more content or customise their dashboards a little more, while preserving the core of what we want.
It will be a breaking change but better to do that while adoption is low now!
This also updates some of the testing around these functions to make it a bit more thorough / robust. The original shinytest2 snapshots for the support panel were only actually tracking the navlist input had changed rather than anything about the panel itself. As a result, I have:
support_panel()
andcookies_panel_ui()
functionsEDIT:
The test_dashboard tests were failing on the GHA servers but passing locally, brute forced some extra waits in and it seemed to do the trick.