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

Creating basic pkgdown generated documentation #34

Merged
merged 11 commits into from
May 13, 2024
Merged

Creating basic pkgdown generated documentation #34

merged 11 commits into from
May 13, 2024

Conversation

rmbielby
Copy link
Contributor

@rmbielby rmbielby commented May 10, 2024

I've added the yml scripting to create a pkgdown generated site on github.io.

This needed some fiddling around with how the donttest / dontrun flags are used as the way it was set up led to errors in rendering the code examples. Seemed like there were a few intermingled issues along the following lines:

  • tidy_code() was scanning an intermediate example file (dfeshiny-Ex.R) when run by devools::checks(). That would fail depending on how \donttest and \dontrun were applied in the examples. I can't get to the intermediate file to see what tidy_code() was flagging, but there's a chance there's the output in there from other functions being run. Whatever it is, we don't have control over how dfeshiny-Ex.R is styled, so if it's created, it seems likely it will fail tidy_code() as an unintended consequence of running the checks (assuming I'm about understanding it right). I've added a \dontrun around tidy_code and that seems to be the key thing to prevent issues happening (note sure if I removed a \dontrun from it in the first place, will find out once I've seen the diff on this PR!).
  • styler throws a parsing error when \dontrun and \donttest are added on subsequent lines to each other
  • cookie_banner_server() needs \dontrun as it doesn't have access to the inputs it needs if run as an example
  • cookie_banner_ui() and dfe_cookie_script both run fine as examples as inputs are either not needed or dummy variables can be given (so I've made sure \dontrun and \donttest aren't used around those)

Other than that, I've structured the reference in what I think is a sensible way and I've used the same (dark) theming as we use in the Analysts Guide (i.e. cyborg).

This covers items #26 and #33 in the Issues log.

@rmbielby rmbielby requested a review from cjrace May 10, 2024 12:58
@cjrace cjrace self-assigned this May 10, 2024
DESCRIPTION Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem like the favicon is set up / working correctly?

Can be a separate issue / PR though as it can get fiddly so not worth investing much time in before we get the main documentation live.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, wasn't sure if that was just not working with launching in preview or if it was properly not working. Agreed it's one to fix down the line.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raised #36 to come back to later

README.md Show resolved Hide resolved
url: https://dfe-analytical-services.github.io/dfeshiny/
template:
bootstrap: 5
bootswatch: cyborg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Distinctly less jazzy than the rainbow you could have had...

In all seriousness it works well / looks good, aside from I think we need to add some custom CSS for the selected state in the navbar as it's pretty hard to read
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Gone for some nice blues!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you have - they work very nicely with the logo 👏

_pkgdown.yml Show resolved Hide resolved
R/cookies.R Show resolved Hide resolved
R/cookies.R Show resolved Hide resolved
@cjrace cjrace assigned rmbielby and unassigned cjrace May 10, 2024
DESCRIPTION Show resolved Hide resolved
R/cookies.R Show resolved Hide resolved
R/cookies.R Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raised #36 to come back to later

url: https://dfe-analytical-services.github.io/dfeshiny/
template:
bootstrap: 5
bootswatch: cyborg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you have - they work very nicely with the logo 👏

@rmbielby rmbielby merged commit fb0a97e into main May 13, 2024
9 checks passed
@rmbielby rmbielby deleted the pkgdown branch January 10, 2025 13:37
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.

2 participants