DOC-14254 expandHeaderLevels in playbook site.keys.navGroups#201
DOC-14254 expandHeaderLevels in playbook site.keys.navGroups#201
Conversation
Instead of having to configure `:page-nav-header-levels:` in every antora.yml, we can just update in the playbook
```
site:
keys:
navGroups:
[
{ "title": "Capella", "startPage": "home::cloud.adoc", "components": ["cloud", "app-services", "ai", "analytics"], "expandHeaderLevels": 1 },
...
{ "title": "Develop", "startPage": "home::developer.adoc",
"expandHeaderLevels": 1,
"subGroups": [
{
"title": "Operational SDKs",
...
```
osfameron
requested review from
RichardSmedley,
Copilot,
dgnicholson and
simon-dew
There was a problem hiding this comment.
Pull request overview
This PR shifts navigation header expansion configuration from per-page AsciiDoc attributes (:page-nav-header-levels:) to a per-nav-group playbook setting (expandHeaderLevels), reducing repeated config across component antora.yml files.
Changes:
- Removed the
page-nav-header-levelsmeta tag from the page head. - Updated client-side nav tree building to use
group.expandHeaderLevelsto determine which levels remain expanded by default.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/partials/head-meta.hbs | Removes the meta tag previously used to pass page-nav-header-levels into client JS. |
| src/js/01-nav.js | Switches nav collapsing logic to use group.expandHeaderLevels and updates buildNavTree signature accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Returns an object with attributes extracted from Meta | ||
| function getPage () { | ||
| var head = document.head | ||
| return { | ||
| component: head.querySelector('meta[name="dcterms.subject"]').getAttribute('content'), | ||
| version: head.querySelector('meta[name="dcterms.identifier"]').getAttribute('content'), | ||
| url: head.querySelector('meta[name=page-url]').getAttribute('content'), | ||
| navHeaderLevels: parseInt(head.querySelector('meta[name="page-nav-header-levels"]')?.content) || 0, | ||
| } | ||
| } |
There was a problem hiding this comment.
getPage() no longer includes navHeaderLevels, but activateNav() still reads page.navHeaderLevels to decide which menu levels should be non-toggleable. With the meta tag removed, this becomes undefined and changes the click/toggle behavior (and makes the new expandHeaderLevels setting ineffective there). Consider threading group.expandHeaderLevels (or a derived numeric value) into activateNav() and removing the last page.navHeaderLevels usage.
| const expandHeaderLevels = group.expandHeaderLevels || 0 | ||
| // build the navTree. | ||
| // At least one of these componentVersions must return a navTree in order for us to | ||
| // use this componentVersionNavEl | ||
| if (buildNavTree(items, componentVersionNavEl, page, [])) { | ||
| if (buildNavTree(items, componentVersionNavEl, page, [], expandHeaderLevels)) { |
There was a problem hiding this comment.
expandHeaderLevels is constant for the whole nav group, but it's currently recomputed inside the componentNavData.versions.forEach loop. Moving it once (e.g., near the top of buildNav) would reduce repetition and keep the config source clearer.
simon-dew
left a comment
simon-dew
left a comment
There was a problem hiding this comment.
LGTM as a useful interim step 👍
(Ideally I think we should have consistent nav behavior across the entire docs set, i.e. I don't think we should have different levels of header expansion for different nav groups at all)
|
thanks @simon-dew! A key thing here is that we can have different levels, as long as they are consistent for each nav group (e.g. what we're doing in this PR) Otherwise, you get weird inconsistencies as you move between components. On which note, @RichardSmedley you mentioned you might want something like that for SDKs? Are you happy to accept this per-nav behaviour for now? |
dgnicholson
left a comment
dgnicholson
left a comment
There was a problem hiding this comment.
👍 Makes sense to me. No objection from the Capella docs point of view.
| const group = JSON.parse(pageNavigationGroup.innerText) | ||
| group.expandHeaderLevels = group.expandHeaderLevels || 0 | ||
| expandHeaderLevels = group.expandHeaderLevels | ||
|
|
||
| buildNav( | ||
| navContainer, // container | ||
| getPage(), // page | ||
| pageVersions, // pageVersions | ||
| JSON.parse(pageNavigationGroup.innerText), //group | ||
| siteNavigationData // navData | ||
| navContainer, | ||
| getPage(), | ||
| pageVersions, | ||
| group, | ||
| siteNavigationData | ||
| ) | ||
| } // else Presumably Components already/now exist | ||
|
|
||
| activateNav(navContainer, getPage()) | ||
| activateNav(navContainer, getPage(), expandHeaderLevels) |
| var pageVersions = document.getElementById('page-versions') | ||
|
|
||
| const group = JSON.parse(pageNavigationGroup.innerText) | ||
| group.expandHeaderLevels = group.expandHeaderLevels || 0 |
Instead of having to configure
:page-nav-header-levels:in every antora.yml, we can just update in the playbook