DOC-14254 off-by-one error in page-nav-header-levels#200
Conversation
Setting to 1 is intended to keep only the first level open e.g. show the direct children of the first bold level.
osfameron
requested review from
RichardSmedley,
Copilot,
dgnicholson and
simon-dew
There was a problem hiding this comment.
Pull request overview
Fixes the collapsing behavior of the left navigation tree controlled by page-nav-header-levels, intended to keep a certain number of header levels expanded by default.
Changes:
- Updates the depth comparison used to decide when a nav subtree should be collapsed (
>to>=).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // originally we would collapse everything, but we can set :page-nav-header-levels: 1 to have | ||
| // up to the bold subheadings kept open | ||
| if (currentPath.length > page.navHeaderLevels) { | ||
| if (currentPath.length >= page.navHeaderLevels) { |
There was a problem hiding this comment.
Changing the collapse condition to currentPath.length >= page.navHeaderLevels will close top-level parents when page-nav-header-levels is set to 1 (since currentPath.length is 1 at the top UL). Those top-level items also don’t get an in-toggle (currentPath.length > 1 check) and are excluded from the click-to-toggle handler (dataset.depth < page.navHeaderLevels), so they become permanently closed unless they contain the current page. Consider reverting to > or rewriting this logic in terms of the same depth metric used elsewhere (e.g., navItemEl.dataset.depth) so page-nav-header-levels: 1 keeps the first level open as intended.
| if (currentPath.length >= page.navHeaderLevels) { | |
| if (currentPath.length > page.navHeaderLevels) { |
The logic switched based on the number of nav files available. This broke components where the landing page (defined in docs-site, with a single nav) referred to a component such as SDK which has multiple navs for different components. Capella used to have a single one, but has evolved to have multiples in left nav, causing some oddities. This change will require setting the correct `:page-nav-header-levels: 1` to all repos in SDK, as well as the landing pages in docs-site (cloud.adoc, sdk.adoc, analytics-sdk.adoc, and developer.adoc)
Setting to 1 is intended to keep only the first level open e.g. show the direct children of the first bold level.
Oops... looks like an off-by-one-error... building Staging to test...