From bce3c32f462d6fd4236aa6b4d954cd6d4f3377d7 Mon Sep 17 00:00:00 2001 From: Peter Mosses <18308236+pdmosses@users.noreply.github.com> Date: Mon, 6 Jan 2025 08:41:16 +0100 Subject: [PATCH] Fix: auto-generated child navigation (TOC) (#1590) Fix #1587 As reported in issue #1587, the auto-generated child navigation (TOC) has a bug: it can be incorrectly omitted. This happens when the first page built is a parent page. The omission is caused by a side-effect of including the cached site-nav HTML: the code that generates the site-nav is executed the first time the cached HTML is included, and assignments in the executed code may overwrite the values of variables. @kevinlin1 suggested a simple and safe way to fix this bug: move the inclusion of the site-nav in `components/children_nav.html` so that it is executed before all local assignments. This PR implements that suggestion, and applies the same fix to two other files. ### Testing A test for this bug has been added to the [_Just the Docs Tests_ repo](https://github.com/just-the-docs/just-the-docs-tests). The first page rendered when building the website is `About this site` in the TESTS collection, and it is now the `parent` of the page [`Test TOC`](https://just-the-docs.github.io/just-the-docs-tests/tests/about/test-toc/), which should be listed in its auto-generated child navigation. (The test page uses `nav_exclude: true` to avoid the link to it appearing in the main navigation, but that doesn't affect the test of this PR.) The following steps check that the bug appears when building _Just the Docs Tests_ with v0.10.0 of the theme: 1. Clone the [Just the Docs Tests repo](https://github.com/just-the-docs/just-the-docs-tests). 2. Build and serve the website locally using: ```sh JTD_ORG=just-the-docs JTD_REF=v0.10.0 bundle install JTD_ORG=just-the-docs JTD_REF=v0.10.0 bundle exec jekyll serve ``` 3. Check that the `Test TOC` page at `.../just-the-docs-tests/tests/about/test-toc/` is a child of the `About this site` page at `.../just-the-docs-tests/tests/about/`. 4. Check that no auto-generated child navigation appears on the latter page. The following steps check that the bug does not appear when building _Just the Docs Tests_ with this PR branch: 5. Build and serve the website locally using: ```sh JTD_ORG=pdmosses JTD_REF=fix-toc bundle install JTD_ORG=pdmosses JTD_REF=fix-toc bundle exec jekyll serve ``` 6. Check that the `Test TOC` page at `.../just-the-docs-tests/tests/about/test-toc/` is a child of the `About this site` page at `.../just-the-docs-tests/tests/about/`. 7. Check that an auto-generated child navigation with a link to the `Test TOC` page appears on the latter page. (It seems unnecessary to check that the reported bug does not appear on other pages, since subsequent includes of the cached site-nav cannot assign to any variables.) --- CHANGELOG.md | 2 ++ _includes/components/breadcrumbs.html | 8 ++++---- _includes/components/children_nav.html | 11 ++++++----- _includes/css/activation.scss.liquid | 8 ++++---- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c1408f..263c355 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,12 +18,14 @@ This website is built from the `HEAD` of the `main` branch of the theme reposito Code changes to `main` that are *not* in the latest release: - Fixed: `back_to_top` not displaying when no other footer variables are set by [@mattxwang] in [#1461] +- Fixed: auto-generated child navigation (TOC) by [@pdmosses] in [#1590] Docs changes made since the latest release: - N/A [#1461]: https://github.com/just-the-docs/just-the-docs/pull/1461 +[#1590]: https://github.com/just-the-docs/just-the-docs/pull/1590 ## Release v0.10.0 diff --git a/_includes/components/breadcrumbs.html b/_includes/components/breadcrumbs.html index c945277..6bdcdad 100644 --- a/_includes/components/breadcrumbs.html +++ b/_includes/components/breadcrumbs.html @@ -11,14 +11,14 @@ {%- if page.url != "/" and page.parent and page.title -%} -{%- capture nav_list_link -%} - -{%- endcapture -%} - {%- capture site_nav -%} {%- include_cached components/site_nav.html all=true -%} {%- endcapture -%} +{%- capture nav_list_link -%} + +{%- endcapture -%} + {%- capture nav_list_simple -%}