Fix#1020
- Move the display of nav external links from `_includes/nav.html` to `_layouts/default.html`.
- Replace ` unless include.key` by `if site.nav_external_links`.
- Wrap the body of `if site.nav_external_links` in `<ul class="nav-list">…</ul>`.
To test this PR:
1. Add to `_config.yml`:
```yaml
defaults:
-
scope: {path: ""}
values: {nav_exclude: true}
```
2. Check that the only link to appear in the nav panel is external.
Co-authored-by: Matt Wang <matt@matthewwang.me>
* Fix top-level active link
Fixes#1014
1. Reverse the order of `page.parent == node.title or page.grand_parent == node.title`. This makes no difference.
2. Replace `page.parent == node.title` by `page.parent == node.title and page.grand_parent == nil`. The condition is evaluated first because it is rightmost.
We have `node in first_level_pages`.
The old condition holds not only when `page` is a child of `node`,
but also when `page` is a grandchild of a *different* top-level page and its parent happens to have the same title as `node`.
The new condition never holds for a grandchild.
This change has been tested locally: in v0.4.0.rc3, when the 3rd-level page `G` was selected, the link to the top-level page `F` was active, and the link to its child `G` was shown; after making the change, it is no longer active, so the link to its child `G` is not shown.
* Update nav.html
Add a comment to clarify just when top-level nodes are active.
The regression tests in https://just-the-docs.github.io/just-the-docs-tests/navigation/grandparent/index/ include a section "A grandchild with the same parent title as a child or top-level page". Its last item fails, as can currently be seen at https://just-the-docs.github.io/just-the-docs-tests/navigation/grandparent/f/ : the first occurrence of `G` links to the grandchild of `E`., and shouldn't be included.
The regression can be fixed by changing `where: "grand_parent", page.parent` to `where_exp: "item", "item.grand_parent == page.parent"`. This should be apparent when running the regressions tests with this PR branch as remote theme.
The two filters give the same results in Jekyll 4, but not in Jekyll 3. It seems that this was overlooked because the sanity-testing of v0.4.0.rc3 was inadvertently using Jekyll 4.2.2.
Fix external links and collections
The navigation should only display the external links once, after the links to pages that are not in collections.
The test for PR #876 at https://just-the-docs.github.io/just-the-docs-tests/navigation/external-links fails with v0.4.0.rc3,
and succeeds when the updated `nav.html` is added locally.
The docs need updating to clarify how the interaction between the collections feature and the external links feature is resolved.
* Optimize simple navigation cases
Fix inefficiency reported in feedback on v0.4.0.rc2 (see discussion #958).
This PR:
* essentially reverts `_includes/nav.html` to v0.4.0.rc1
* preserves the ARIA labels added by #950
* adds a test to optimize builds of sites that rely on `title` fields to order pages.
Building the `endoflife.date` site (130 pages) now takes only about 7 seconds.
Building the `machinetranslate.org` site ( 350 pages) takes about 7 minutes. (Without the added test, it takes just over 5 minutes: the condition of the test is merely to compare the size of two arrays, but that is apparently enough to prevent Jekyll from applying some optimization).
A warning is added to the docs about the need for numbers to be in quotes when used as title values.
* Update navigation-structure.md
A clarification is added to the docs about the need for numbers to be in quotes when used as title values.
* Simplify the control and data flow
- Defer concatenation of `string_order_pages` with `title_order_pages` until needed.
- Replace tests on size with tests for `empty`.
- Rename variables accordingly.
* Fix child nav order
This PR started from the navigation in RC1. Some cosmetic improvements had been made in RC2. This commit adds some of those changes to this PR.
It also fixes a bug (revealed by a new regression test) due to a reference to `node.child_nav_order` instead of `child.child_nav_order`, which prevented reversal of the order in children of children. Presumably a top-level reversal should apply only to direct children, and not to grandchildren. The latter interpretation would be very confusing in a deep multi-level hierarchy.
* Allow pages with numeric titles
An omitted `nav_order` value should default to the `title` value, regardless of its type. Jekyll 3 gives build errors when numbers and strings are sorted together. This commit drops the assumption that `title` values are always strings – a 404 page naturally has a numeric title. It updates the docs page accordingly.
The extra code does not affect the build time for the `endoflife.date` site (7 seconds). For the `machinetranslate` site, changing the title of the 404 page to a number increases the build time from 7 minutes to 9 minutes – the `nav_order` numbers on that site are program-generated in the range 1..1000, which might be atypical.
This commit has not yet been checked using the regression tests.
The gemspec used for testing specifies `spec.add_runtime_dependency "jekyll", "~> 3.8.5"`, and `Gemfile.lock` shows `jekyll (3.8.7)`.
* Update nav.html
Add comment about an optimization that will be possible in Jekyll 4.
* Update nav.html
- Update the comment about optimization possibility.
- TEMPORARILY add Jekyll 3 code for conditionally optimizing.
* Update nav.html
Minor improvements and cosmetic changes.
* Major revision
This update is based on extensive experimentation and profiling with alternative versions of the Liquid code used to build the main navigation panel.
Due to the fragility of Jekyll's optimizations, combining alternative approaches with conditionals turned out to be too expensive: merely adding a condition to check whether some array of pages is empty can add about 20% to the build time!
The current code avoids sorting pages on `nav_order` and `title` fields together. The standard way of doing that in Jekyll is to use the `group_by` filter; but extracting the sorted pages from the groups turned out to be too inefficient (as seen in RC1), as was generating links directly from the groups (in RC2).
Making all pages with `nav_order` values come before all those ordered by their `title` values is not ideal (it doesn't support tweaking the relative order of two pages in a list of pages ordered by their titles) but it appears to be necessary for efficient builds on large sites.
This version has not yet been fully tested for regression, but otherwise seems to give the expected navigation on the endoflife.date and machinetranslate websites. (I'm unable to install the Python-based how2data repository on my laptop, due to package version issues on Apple silicon).
Co-authored-by: Peter Mosses <18308236+pdmosses@users.noreply.github.com>
An occurrence of `grandchild` was miss-spelled `grand_child` (!).
That caused omission of the `active` class in the link in the nav panel for all grandchildren.
This bug was discovered using `diff` to compare the generated sites built using v0.4.0.rc1 and commit 4396b6b1c836f354bb7aa7edc1a06a49f137d615 on the fix-nav-performance PR branch.
Fix#863.
The current Liquid code to generate the navigation panel involves the inefficient extraction of a list of pages from a list of page groups (identified by @captn3m0 in his original [explanation of the performance issue](https://github.com/just-the-docs/just-the-docs/issues/863)).
The optimisation implemented by this PR generates navigation links directly from the list of page groups, thereby avoiding the extraction of a list of pages from it. The Liquid code is now a bit tedious, but I don't see a simpler solution.
The need for grouping pages arises because Jekyll doesn't provide a filter to sort a list of pages on the value of an arbitrary expression.
Using Jekyll v4.2.2 (macOS 12.5, M2 MacBook Air, 16 GB memory), building https://github.com/endoflife-date/endoflife.date using https://github.com/pdmosses/just-the-docs/blob/fix-nav-performance/_includes/nav.html produced the following profile extract:
Filename | Count | Bytes | Time
------------------------------------------------------------|-------|----------|-------
| just-the-docs-0.4.0.rc1/_layouts/default.html | 130 | 3792.04K | 5.160 |
| _includes/nav.html | 130 | 1405.20K | 4.054 |
| just-the-docs-0.4.0.rc1/_includes/head.html | 130 | 617.82K | 0.495 |
| _layouts/product.html | 127 | 1014.38K | 0.413 |
| _includes/head_custom.html | 130 | 427.83K | 0.393 |
| assets/js/zzzz-search-data.json | 1 | 149.31K | 0.050 |
@nathancarter has tried adding the new `nav.html` to [a site with over 300 pages](https://nathancarter.github.io/how2data/site/), and reported that it improved the build time of more than 3 minutes to about 30 seconds.
Further optimisation of navigation might be possible (e.g., using [Jekyll include caching](https://github.com/benbalter/jekyll-include-cache)), but the current optimisation should be sufficient for v0.4.0.
To test that this PR does not appear to affect the navigation panel generated by v0.3.3:
1. Clone https://github.com/just-the-docs/just-the-docs-tests.
2. Update `_config.yml` and `Gemfile` to use this PR branch.
3. Run `bundle update`.
4. Inspect the rendering of the entire collection of navigation tests.
(Many of the differences reported in the GitHub visualisation of the changes are due to shifting much of the code 2 spaces to the left, in connection with moving the first `ul` element to be close to its first item.)
Adds accessible nav elements for nested pages
Why are these changes being introduced:
* The current links to show/hide the nested pages use a visual only
svg image with no accessible affordance provided so screenreaders
will not be able to provide appropriate context for users as to what
they should expect when clicking these links
* You can see the problem by running a tool like ANDI on the current
main branch of this repository and then running it again on this
branch. ANDI shows what a screenreader would read.
* You can also use a tool like Voiceover to hear the importance of what
this introduces to users that use that technology. Before this change,
Voiceover would read all of these navigation links as
"link image just-the-docs" but with this change it will read
`link image toggle links in <categoryName> category`
Relevant ticket(s):
* This was discussed as part of the larger WCAG compliance ticket
https://github.com/just-the-docs/just-the-docs/issues/566
How does this address that need:
* This adds an `aria-label` to the link https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-label
Document any side effects to this change:
It appears it might be prefereable to use `aria-labelledby` whenever
possible, but from what I can tell these links are just the visual cue
of the svg with no other affordance given to users to understand what
they'll do so there is nothing to point `aria-labelledby` at.
An `svg` title was considered, but in reading more about it it seemed
like `aria-label` was more appropriate as it puts the label on the `a`
rather than the `svg` which feels more accurate.
* https://developer.mozilla.org/en-US/docs/Web/SVG/Element/title
* https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-labelledby
Co-authored-by: Matt Wang <matt@matthewwang.me>
- add an extra filter to children-list to fix#854
- strengthen condition for assignment to first_level_url
Tested with Jekyll 3.9.2 and 4.2.2
Note: Jekyll 3.9.2 doesn't support `| where: "grand_parent: nil"`
This PR combines (and resolves conflicts between) #448, #463, #466, #494, #495, #496, #498, and #572.
The main aim is to facilitate use of several of the implemented features _together_, when using the fork as a remote theme. It should also simplify merging the included PRs into a future release.
The branch [combination-rec-nav](https://github.com/pdmosses/just-the-docs/tree/combination-rec-nav) adds [multi-level navigation](https://github.com/pmarsceill/just-the-docs/pull/462) and (NEW:) [sibling links](https://github.com/pmarsceill/just-the-docs/pull/394) to the branch used for this PR. It includes updated [documentation for the navigation structure](https://pdmosses.github.io/just-the-docs/docs/navigation-structure/), and reorganised and extended [navigation tests](https://pdmosses.github.io/just-the-docs/tests/navigation/). The documentation and the tests can be browsed at the (temporary) [website published from the combination-rec-nav branch](https://pdmosses.github.io/just-the-docs/).
_Caveat:_ The changes to v0.3.3 in this PR and #462 have not yet been reviewed or approved, and may need updating before merging into a release of the theme. If you use a branch from a PR as a remote theme, there is a risk of such updates affecting your website. Moreover, these branches are likely to be deleted after they have been merged. To avoid such problems, you could copy the branch that you want to use to your own fork of the theme.
Co-authored-by: Matt Wang <matt@matthewwang.me>
- Limit the effect of `nav_exclude: true` to the main navigation.
- Include links to excluded pages in auto-generating lists of child pages
and in breadcrumbs.
- Refactor implementation by moving assignment of `first_level_url` and `second_level_url` from `_includes/nav.html` to `_layouts/default.html`.
- Clarify the effect of `nav_exclude` in the documentation.
The values of `title` and `nav_order` can be numbers or strings.
Jekyll gives build failures when sorting on mixtures of different types,
so numbers and strings need to be sorted separately.
Here, numbers are sorted by their values, and come before all strings.
An omitted `nav_order` value is equivalent to the page's `title` value
(except that a numerical `title` value is treated as a string).
The case-sensitivity of string sorting is determined by `site.nav_sort`.
Pages with `nav_exclude: true` were included when sorting on `title` or `nav_order`. That could cause build failures when the type of value of the field differs from that on other pages, as reported in https://github.com/pmarsceill/just-the-docs/issues/406.
Pages with `nav_exclude: true` or no `title` are never displayed in the navigation, so removing them from `pages_list` cannot break existing sites. This change also allows the removal of some tests in the code. (The indentation of the code should now be adjusted, but that has been deferred, to restrict the size of the diff for review.)
For testing, the title of `404.html` has been changed to the number `404`, the page `docs/untitled-test.md` has been added, and `nav_sort_order` has been set to `case_sensitive`. Those updates give build failures with the current version of `_includes/nav.html`, but not after the suggested changes.
It will still be possible for build failures to occur due to sorting fields of *non-excluded* pages with differing types of values (e.g., `nav_order`a mixture of numbers and strings). To make the code completely safe will require relatively complicated changes,.
Added a configuration option to determine whether the sort order is case-sensitive.
The default is case-insensitive.
To test:
- open `/just-the-docs/docs/utilities/` in the browser,
and check that the navigation links in `Utilities` are sorted alphabetically;
- in `docs/utilities/layout.md', change the preamble to `title: layout`,
and check that the links in `Utilities` are still sorted alphabetically;
- add `nav_sort: case_sensitive` in the configuration file,
and check that the link to `layout` is now listed last under `Utilities`.
It appears nav_exclude only works on top level navigation items. I needed it to work at the child level as well. I believe these changes accomplish that for the child and grand_child levels.
Love this theme. I've used it a few times. Apologies if this pull request is not according to convention. This is the first time I've done it on someone else's code. Thanks!
When `nav_order` is omitted, the order of nodes at each menu level (and in the auto-generated TOC) is alphabetical by `title`, instead of random.
Any nodes with a specified `nav_order` precede all nodes at that level where it is omitted.
Note that `nav_order` fields must have a uniform site-ide type: integers and strings cannot be mixed, otherwise Jekyll reports errors.
The implementation filters the ordered and unordered pages from `site.html_pages`, sorts them separately, and concatenates the resulting arrays.
- Trimmed whitespace between html elements
This reduced one of my files from 850KB to 115KB
- Do not sort the whole list of pages on every iteration
This made build 3x faster
