search

RMI-PACTA / pacta.pkgdown.rmitemplate

https://rmi-pacta.github.io/pacta.pkgdown.rmitemplate/
Other
2 stars 0 forks source link

multiple repos' pkgdown sites that use this template are suddenly not formatted correctly #32

Closed cjyetman closed 4 months ago

cjyetman commented 5 months ago

so I suspect something went wrong here, or a recent update to {pkgdown} or its dependencies broke things, e.g. https://rmi-pacta.github.io/pacta.data.preparation/ https://rmi-pacta.github.io/pacta.portfolio.utils/ https://rmi-pacta.github.io/pacta.portfolio.import/ https://rmi-pacta.github.io/pacta.scenario.data.preparation/

I ran the pkgdown on this repo (on main) and now this repo's pkgdown site has the same problem, so I suspect the problem is from a change in pkgdown and/or its upstream dependencies https://rmi-pacta.github.io/pacta.pkgdown.rmitemplate/

cjyetman commented 5 months ago

may already be fixed by this: https://github.com/r-lib/pkgdown/pull/2443

(my minimal testing hinted that something was wrong with Bootstrap and/or jQuery not loading for some reason)

cjyetman commented 5 months ago

FYI, a temporary fix is here: https://github.com/RMI-PACTA/pacta.portfolio.allocate/pull/33

As I understand it, our template sets and expects Bootstrap 5, but the current version of pkgdown lets the repo that is using a template override the Bootstrap setting and if no Bootstrap version is specified in the repo it uses Bootstrap 3 as the default and overrides the template's setting. So explicitly setting Bootstrap 5 in a repo that uses this template will force it to use Bootstrap 5, which is what this template requires.

jdhoffa commented 5 months ago

Thanks for investigating!

Explicitly pinning the bootstrap version is probably best practice in any case

cjyetman commented 5 months ago

Thanks for investigating!

Explicitly pinning the bootstrap version is probably best practice in any case

I think ideally one would not do that, so that the template can define what version of Bootstrap it needs. But I believe the next version of pkgdown will allow the template to set the version if it's not explicitly over-written.

jdhoffa commented 5 months ago

Ah yes I see the point, and agree. Then let's monitor for the next pkgdown release

cjyetman commented 4 months ago

appears to be fixed in pkgdwon 2.0.9 https://cloud.r-project.org/web/packages/pkgdown/news/news.html

jdhoffa commented 4 months ago

All of the pkgdown websites you initially pointed to seem to generate fine now: https://rmi-pacta.github.io/pacta.data.preparation/ https://rmi-pacta.github.io/pacta.portfolio.utils/ https://rmi-pacta.github.io/pacta.portfolio.import/ https://rmi-pacta.github.io/pacta.scenario.data.preparation/

Shall we close this issue as complete?

jdhoffa commented 4 months ago

We may also want to revert this PR: https://github.com/RMI-PACTA/pacta.portfolio.allocate/pull/33

cjyetman commented 4 months ago
cjyetman commented 4 months ago

closing this as it has resolved itself (and the 2 preemptive PRs in other repos have been reverted)