From 9d0ce1c22a5c43961e6e2de385e1b6532fb455b1 Mon Sep 17 00:00:00 2001 From: Peter Mosses <18308236+pdmosses@users.noreply.github.com> Date: Fri, 18 Aug 2023 20:35:14 +0200 Subject: [PATCH] Fix the navigation panel (#1244) * Fix the nav html and cache it - Remove `active` class from nav html - Add js to insert `active` class on link to selected page - Include attempt to generate page-specific css for same styling when js is off * Refactor nav, breadcrumbs, children_nav Fix #1118 Improve the modularity of building the nav-panel, breadcrumbs, and children-nav by making them independent. This also significantly simplifies the Liquid code. * Manual merge of fix-leakage Also fix order of breadcrumbs * Fix order of breadcrumbs * Revert layout in HTML Revert to the previous layout in the HTML, to allow the use of `diff` to check the built site. * Update breadcrumbs.html Revert inclusion of single breadcrumb for top-level pages. * Update breadcrumbs.html Revert to the previous layout in the HTML, to allow the use of `diff` to check the built site. * Update children_nav.html Revert to the previous layout in the HTML, to allow the use of `diff` to check the built site. * Delete nav_init.html * Update sidebar.html Caches. * Add a comment * Update nav.html - Comment on independence from page. - Remove redundant comment. - Remove superfluous conditionals. * Update just-the-docs.gemspec Revert jekyll version spec change. * Update just-the-docs.gemspec Revert runtime dependency on kramdown-parser-gfm. * Revert inclusion of activation.scss.liquid Inclusion makes HTML of all pages differ from 0.5.1 * Update default.html Restore deleted "" * Update children_nav.html Restore line break. * Delete activation.scss.liquid This was merely an example of page-specific CSS for use when JS off. * Remove an unused include parameter `nav.html` does not depend on `include.key`. * Generate page-specific styling for nav links and lists in the side-nav In this PR, the code in `includes/nav.html` is fixed, and none of its elements have class `active`. When JS is enabled, `activateNav()` adds the class `active` to all nav-list-items that enclose the nav-list-link to the current page, so the navigation works as usual. Unobtrusive JS requires the same behaviour when JS is disabled. - Add `_includes/css/activation.scss.liquid` to compute the indices in the enclosing nav-lists of the nav-list-link to the current page, and generate page-specific styling. - Insert Liquid code in `_includes/head.html` to include the CSS generated by `_includes/css/activation.scss.liquid` (which depends on `site.color_scheme`). - Update the toggling in `initNav()` to allow also contraction of enclosing levels when JS is enabled. Caveat: When JS is enabled, buttons can be used to switch the colour scheme dynamically. The page-specific styling of the site-nav is generated statically, and doesn't change, so the background-image of the nav-list-link to the current page is incorrect. (I guess that could be fixed by generating a style element for each available colour scheme, and using JS to reorder the stylesheets in the DOM.) A further issue is that the `@import` rules used in `_includes/head.html` cause duplication. Replacing them by `@use` rules would avoid duplication, but that is out of scope for this PR. * Fix activation for collections - Adjust generated selectors to pages in collections. - Expand all folded collections when JS is disabled. This PR should now make unobtrusive use of JS: - When JS is disabled, the navigation panel shows links to the top pages in all collections (in contrast to the current version of the theme). - When JS is enabled, folded collections remain folded until their pages are selected. * Respect `child_nav_order` - Assumes reverse order when set to any truthy value. * Suppress liquid line breaks * Cache the search for favicon.ico - Move the code for finding the favicon.ico file to `_includes/favicon.html`. - Replace that code in `_includes.html` by a cached include of `favicon.html`. * Add "jekyll-include-cache" in fixtures Needed when CI ignores the gemspec. * Add gem "jekyll-include-cache" in fixtures Needed when CI ignores the gemspec. * Update head.html - Avoid duplication of color_scheme CSS in `style` element. - Avoid generation of whitespace by Liquid code. * Update sorted_pages.html - Minor optimisation. - Minor improvements to layout of Liquid code. * Ensure split is not at start of rules generated by css/activation.scss.liquid A custom color scheme might not import any highlighting style rules, so we should not assume that there is anything before the first occurrence of `.site-nav`. * Update head.html - Add implicit import of light color scheme. - Revert to previous Liquid code for removing color scheme rules. * Manual resolution of merge conflicts with v0.5.2 - Copied replacement of links on nav expanders by buttons. - Removed (page-dependent) conditions associated with `active`. * Manual resolution of merge conflict with v0.5.2 If previously "" (neither active nor passive), then `active && passive` is true, and the target is now "active". If previously only "active", then the target is now just "passive". If previously only "passive", then the target is now just "active". The state "active passive" is never used. The value of `active` is true just when the target is left "active". * Update fixtures/Gemfile-github-pages Co-authored-by: Matt Wang * Update head.html The result of `activation.scss.liquid` is "" for pages with no `title` or with `nav_exclude`. This update stops `head.html` including a `style` element with an invalid body on such pages. Note that when the result of `scssify` doesn't contain `.site-nav`, `split` produces a one-element array, so `shift` produces an empty array, and `join` then produces an empty string. * Fix omitted `.site-nav` Restore the previous prepending of `.site-nav`, which was dropped when suppressing the generation of an incorrect ` +{%- endunless %} diff --git a/_includes/nav.html b/_includes/nav.html index b631397..0b20b36 100644 --- a/_includes/nav.html +++ b/_includes/nav.html @@ -1,6 +1,6 @@ {%- comment -%} - Include as: {%- include nav.html pages = pages key = key -%} - Depends on: include.pages, include.key, page, site. + Include as: {%- include_cached nav.html pages=pages -%} + Depends on: include.pages. Results in: HTML for the navigation panel. Includes: sorted_pages.html @@ -15,6 +15,10 @@ {%- include sorted_pages.html pages = nav_pages -%} +{%- comment -%} + It might be more efficient to sort the pages at each level separately. +{%- endcomment -%} + {%- assign first_level_pages = sorted_pages | where_exp: "item", "item.parent == nil" -%} {%- assign second_level_pages = sorted_pages @@ -23,84 +27,49 @@ {%- assign third_level_pages = sorted_pages | where_exp: "item", "item.grand_parent != nil" -%} -{%- comment -%} - The order of sibling pages in `sorted_pages` determines the order of display of - links to them in lists of navigation links. - - Note that Liquid evaluates conditions from right to left (and it does not allow - the use of parentheses). Some conditions are not so easy to express clearly... - - For example, consider the following condition: - - C: page.collection = = include.key and - page.url = = node.url or - page.grand_parent = = node.title or - page.parent = = node.title and - page.grand_parent = = nil - - Here, `node` is a first-level page. The last part of the condition - -- namely: `page.parent = = node.title and page.grand_parent = = nil` -- - is evaluated first; it holds if and only if `page` is a child of `node`. - - The condition `page.grand_parent = = node.title or ...` holds when - `page` is a grandchild of node, OR `...` holds. - - The condition `page.url = = node.url or ...` holds when - `page` is `node`, OR `...` holds. - - The condition C: `page.collection = = include.key and ...` holds when we are - generating the nav links for a collection that includes `page`, AND `...` holds. -{%- endcomment -%} - diff --git a/_includes/sorted_pages.html b/_includes/sorted_pages.html index 0870aab..659be10 100644 --- a/_includes/sorted_pages.html +++ b/_includes/sorted_pages.html @@ -1,9 +1,9 @@ {%- comment -%} - Include as: {%- include sorted_pages.html pages = pages -%} + Include as: {%- include sorted_pages.html pages=array_of_pages -%} Depends on: include.pages. Assigns to: sorted_pages. Overwrites: - nav_order_pages, title_order_pages, + nav_order_pages, title_order_pages, double_quote, empty_array, nav_number_pages, nav_string_pages, nav_order_groups, group, title_number_pages, title_string_pages, title_order_groups. {%- endcomment -%} @@ -29,29 +29,55 @@ {%- endcomment -%} {%- assign nav_order_pages = include.pages - | where_exp: "item", "item.nav_order != nil" -%} + | where_exp: "item", "item.nav_order != nil" -%} {%- assign title_order_pages = include.pages - | where_exp: "item", "item.nav_order == nil" -%} + | where_exp: "item", "item.nav_order == nil" -%} {%- comment -%} - Divide the arrays of `nav_order_pages` and `title_order_pages` according to - the type of value. + First, filter `nav_order_pages` and `title_order_pages` according to the type + of value to be used for sorting. - The first character of the result of `jsonify` is `"` only for strings. - Grouping by a single character also ensures the number of groups is small. + The first character of the result of filtering with `jsonify` is `"` only for + strings. Removing `"` from its `slice : 0` has size 0 for strings and 1 for + numbers, so grouping the pages gives at most two groups. {%- endcomment -%} -{%- assign nav_number_pages = "" | split: "" -%} -{%- assign nav_string_pages = "" | split: "" -%} -{%- assign nav_order_groups = nav_order_pages - | group_by_exp: "item", "item.nav_order | jsonify | slice: 0" -%} -{%- for group in nav_order_groups -%} - {%- if group.name == '"' -%} - {%- assign nav_string_pages = group.items -%} - {%- else -%} - {%- assign nav_number_pages = nav_number_pages | concat: group.items -%} - {%- endif -%} -{%- endfor -%} +{%- assign double_quote = '"' -%} +{%- assign empty_array = "" | split: "" -%} + +{%- assign nav_string_pages = empty_array -%} +{%- assign nav_number_pages = empty_array -%} +{%- unless nav_order_pages == empty -%} + {%- assign nav_order_groups = nav_order_pages + | group_by_exp: "item", + "item.nav_order | jsonify | slice: 0 | remove: double_quote | size" -%} + {%- for group in nav_order_groups -%} + {%- if group.name == 0 -%} + {%- assign nav_string_pages = group.items -%} + {%- elsif group.name == 1 -%} + {%- assign nav_number_pages = group.items -%} + {%- endif -%} + {%- endfor -%} +{%- endunless -%} + +{%- assign title_string_pages = empty_array -%} +{%- assign title_number_pages = empty_array -%} +{%- unless title_order_pages == empty -%} + {%- assign title_order_groups = title_order_pages + | group_by_exp: "item", + "item.title | jsonify | slice: 0 | remove: double_quote | size" -%} + {%- for group in title_order_groups -%} + {%- if group.name == 0 -%} + {%- assign title_string_pages = group.items -%} + {%- elsif group.name == 1 -%} + {%- assign title_number_pages = group.items -%} + {%- endif -%} + {%- endfor -%} +{%- endunless -%} + +{%- comment -%} + Now sort each array of pages separately, then concatenate the sorted arrays. +{%- endcomment -%} {%- unless nav_number_pages == empty -%} {%- assign nav_number_pages = nav_number_pages | sort: "nav_order" -%} @@ -65,18 +91,6 @@ {%- endif -%} {%- endunless -%} -{%- assign title_number_pages = "" | split: "" -%} -{%- assign title_string_pages = "" | split: "" -%} -{%- assign title_order_groups = title_order_pages - | group_by_exp: "item", "item.title | jsonify | slice: 0" -%} -{%- for group in title_order_groups -%} - {%- if group.name == '"' -%} - {%- assign title_string_pages = group.items -%} - {%- else -%} - {%- assign title_number_pages = title_number_pages | concat: group.items -%} - {%- endif -%} -{%- endfor -%} - {%- unless title_number_pages == empty -%} {%- assign title_number_pages = title_number_pages | sort: "title" -%} {%- endunless -%} @@ -90,6 +104,6 @@ {%- endunless -%} {%- assign sorted_pages = nav_number_pages - | concat: nav_string_pages - | concat: title_number_pages - | concat: title_string_pages -%} + | concat: nav_string_pages + | concat: title_number_pages + | concat: title_string_pages -%} diff --git a/assets/js/just-the-docs.js b/assets/js/just-the-docs.js index 6367a3b..87fa9b5 100644 --- a/assets/js/just-the-docs.js +++ b/assets/js/just-the-docs.js @@ -31,13 +31,18 @@ function initNav() { } if (target) { e.preventDefault(); - target.ariaPressed = target.parentNode.classList.toggle('active'); + const active = target.parentNode.classList.toggle('active'); + const passive = target.parentNode.classList.toggle('passive'); + if (active && passive) target.parentNode.classList.toggle('passive'); + target.ariaPressed = active; } }); const siteNav = document.getElementById('site-nav'); const mainHeader = document.getElementById('main-header'); const menuButton = document.getElementById('menu-button'); + + disableHeadStyleSheet(); jtd.addEvent(menuButton, 'click', function(e){ e.preventDefault(); @@ -66,6 +71,14 @@ function initNav() { {%- endif %} } +// The page-specific