Skip to content
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

Shiny template module #5

Merged
merged 6 commits into from
Aug 22, 2024
Merged

Shiny template module #5

merged 6 commits into from
Aug 22, 2024

Conversation

JT-39
Copy link
Collaborator

@JT-39 JT-39 commented Aug 22, 2024

Pull request overview

Started out with aim to modularise the cookie logic in shiny-template. Turns out this has already been done and is available in the {dfeshiny} package. I have now included this into the ui.R and server.R scripts and cleared out the old code.

I did make some minor improvements - namely creating a helper function to simply add the .css file to the ui.R script.

Pull request checklist

Please check if your PR fulfills the following:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Tests have been run locally and are passing (shinytest2::test_app())
  • Code is styled according to tidyverse styling (checked locally with styler::style_dir() and lintr::lint_dir())

What is the current behaviour?

Cookie logic is verbose in the server.R and ui.R.

What is the new behaviour?

This has been replaced with the dfeshiny::cookies_ functions in the server.R and ui.R. I also added a function to simply add the .css file into the ui.R, this is called set_css_style_sheet().

Anything else

NOTE: New updates are coming to {dfeshiny} which may break code (due to function name changes). Also a test should be written for the new set_css_style_sheet() function. (Plus check if this is okay practice.)

@JT-39 JT-39 merged commit 2269131 into main Aug 22, 2024
3 checks passed
@JT-39 JT-39 deleted the shiny-template-module branch August 22, 2024 15:53
@JT-39 JT-39 linked an issue Aug 22, 2024 that may be closed by this pull request
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.

Improvement: Modularise cookies and bookmark logic
1 participant