pkgdown layout is broken

[pawelru]

Visible only for main: https://insightsengineering.github.io/teal/main/

might be related: https://github.com/insightsengineering/r-pkgdown-multiversion/issues/22

[pawelru]

Hey @cicdguy is this something close to what you have just fixed?

[cicdguy]

I think it might be related. I'll rerun the workflow to see what it is.

[cicdguy]

This one is a bit different

Before (aka pre pkgdown v2.1.0):

After (pkgdown v2.1.0+):

The navbar items is being rendered as a button instead of a list item.

I think one or more settings in the _pkgdown settings need to be tweaked. Our actions do not modify the element types.

[pawelru]

I think it is related. I run the pkgdown::build_site() locally and it render nicely. That means that our injection is not working correctly.

Have a look at this https://github.com/insightsengineering/teal/commit/9745ffa9fb5badc3e2d780924c0212970e4c3bd9#diff-539e48ef777e8ae44686d203f7a8d44099150953e61b552694032fe7a3a428ba. This is the last commit on main from 5h ago so it includes yesterday fix. We did the modifications in line 79 whereas it should have been 78

<ul class="navbar-nav me-auto">

  (...)

  <!-- changelog -->
  <li class="nav-item">
    <a class="nav-link" ...> Changelog</a>
  </li>

                                                        # <- this is where it's should be injected

  <!-- Reports -->
  <li class="nav-item dropdown">                        # line 78 in 9745ffa9fb5badc3e2d780924c0212970e4c3bd9 that I refer to
                                                        # <- this is where it's being injected right now
    <button ...>Reports</button>
    <ul ...>
      <li>...</li>
      <li>...</li>
      ...
    </ul>
  </li>

</ul>

In other words: line 78 belongs to Reports and we should be right before it.

On main we have:

That comment should be inside <ul> (next to all the <li> tags) and not inside <li>.

WDYT? @cicdguy

[cicdguy]

Ah yes, it is indeed that. The bug still exists. Working on a more robust fix.

[cicdguy]

I have 2 proposals:

1. Rewrite the r-pkgdown-multiversion action with a more sophisticated approach (XML parsing). This will take longer but will guarantee stability with formatting changes that may arise with different versions of pkgdown.

2. Update the nesttemplate footer with the pkgdown version that the site was built with: https://github.com/insightsengineering/nesttemplate/blob/main/inst/pkgdown/templates/footer.html. This one is shorter and easier IMHO.

   As in the case of pkgdown when used without a custom template, the pkgdown version is added to the website footer: https://github.com/r-lib/pkgdown/blob/v2.1.1/R/build-footer.R#L38-L43

   This is the version that the r-pkgdown-multiversion action keys off in order to determine the structure of the HTML.

Thoughts?

[pawelru]

1. If I were to rewrite this functionality I would do this as a pre-processing before the main pkgdown job of creating the docs. That step would modify the _pkgdown.yaml and add hrefs there. Then we can run the pkgdown as usual. This is in the opposite of post-processing after the pkgdown run. What we are doing is to modify what pkgdown produces. This way we will be always affected by possible breaking changes in pkgdown. Either way - it's a considerable amount of work so let's try to find a quick win here.
2. I think we can add the pkgdown version in the footer but I don't really follow how this could help. We are injecting in the navbar element. How edits in footer could help? Apologies for my weak understanding of the logic.

[cicdguy]

I'll schedule a call for early next week and we can go over both options. I can explain the situation better that way.