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.)
This is the minimum necessary change to make `back_to_top` work when there is no custom footer, last edit timestamp, or GitHub edit link.
Two immediate thoughts. First, we have a pair of variables: `back_to_top` and `back_to_top_text`. In my opinion, this seems a bit unnecessary; we could just use a `back_to_top`, and treat any non-`nil`/`false` value as the text. We could make this backwards compatible (i.e. support but deprecate `back_to_top_text`). Any thoughts here?
Secondly, some of these conditions are weak:
a251382b7a/_includes/components/footer.html (L7-L9)
Here, this conditional *should* also check for `back_to_top_text`, and presumably, this should "bubble up" to the overall `if` statement on line 4 (similar things for the `gh_*` variables - the line 4 condition only checks for `gh_edit_link`). Is this a reasonable concern/take? If so, I can approach this some time (either in this PR, in another PR, etc.).
---
After we decide the correct behaviour of `back_to_top`, I'll add documentation to the "Configuration" section!
Separately, @pdmosses mentioned:
> The obvious fix is to always render the footer when `site.back_to_top` is set. However, it would improve the usability of the back-to-top feature if individual pages could override the site setting (to suppress the link on some short pages, or to show it on some long pages).
Happy to do that too - for organizational purposes, I'll punt that to another PR (that I can merge into the same release).
---
Fixes#1443.
* Allow unlimited multi-level navigation
This PR supersedes #462.
The only user-level difference from #462 is that disambiguation of parent pages has to use either `grand_parent` or `ancestor` titles: the somewhat unnatural `section_id` and `in_section` fields are not supported.
The implementation has been significantly simplified by the changes introduced in v0.7.0 of the theme.
* Detect cyclic parenthood
A page should not have a parent or ancestor with the same title. If it does, the location of the repeated link is marked by ∞, to facilitate debugging the navigation (and an unbounded loop leading to a build exception is avoided).
* Add nav_error_report warning in main navigation
When activated by `nav_error_report: true` in `_config.yml`, displays warnings about pages with the same title as their parent page or an ancestral page.
* Cache site-nav with links to all pages
The extra cached site-nav is used for determining breadcrumbs and children navigation, which may involve pages that are excluded from the main navigation.
* Replace code for determining children by inclusion of components/nav/children.html
* Update CHANGELOG.md
---------
Co-authored-by: Matt Wang <matt@matthewwang.me>
We are running into an issue upgrading from 0.7 to 0.8, suggesting that `trim` is not a method.
```
1.963 Liquid Exception: Liquid error (/usr/gem/gems/just-the-docs-0.8.0/_includes/components/breadcrumbs.html line 59): undefined filter trim included in /_layouts/default.html
1.968 /usr/gem/gems/liquid-4.0.4/lib/liquid/strainer.rb:58:in `invoke': Liquid error (/usr/gem/gems/just-the-docs-0.8.0/_includes/components/breadcrumbs.html line 59): undefined filter trim included (Liquid::UndefinedFilter)
```
Looking at the liquid docs, we're probably looking for `strip` and not `trim`
https://shopify.github.io/liquid/filters/strip/https://github.com/Shopify/liquid/blob/main/History.md#300--2014-11-12, I don't see a trim so it may have been an extension? This does work normally on just-the-docs itself running from scratch, so it might also be a bug in our application (or combination of different versions in plugins).
Rectify and simplify CSS rules used in the head element. The rules now avoid the invalid use of nested `:not()` pseudo-classes.
- Update CHANGELOG.md
- Remove ignore statements re CSS parse errors
- Add rules for excluded pages and simplify
- Add `activation_no_nav_link` with the rules for excluded pages
- Insert `activation_no_nav_link` when no other rules are generated
- Replace `:nth-child(1)` by `:first-child`
- Correct `:nth-child(n + {{ activation_index | plus: 1 }})` to `:not(:first-child)`
- Eliminate `activation_collection_index`
- Generate `.nav-category-list` selectors only for sites with collections
---------
Co-authored-by: Matt Wang <matt@matthewwang.me>
I've whipped up a solution that solves #1103. I've added a config option `nav_external_links_new_tab`, which is off by default. When turned on, it'll pop external nav links into a new tab. The idea was borrowed from how [aux_nav.html](https://github.com/just-the-docs/just-the-docs/blob/main/_includes/components/aux_nav.html) does it with `aux_links_new_tab`.
---------
Co-authored-by: Matt Wang <matt@matthewwang.me>
* Remove "passive" toggle
PR #1244 introduced the "passive" toggle, but just-the-docs.js subsequently disabled the only styling that used it, so it became redundant.
This removes it.
* Reduce build time for page-dependent CSS
Fix#1323
- Remove `_includes/head_nav.html`.
- Generate page-independent SCSS in `assets/css/just-the-docs-head-nav.css`.
- Link to `/assets/css/just-the-docs-head-nav.css` in `head.html`.
- Disable the above stylesheet in `assets/js/just-the-docs.js`.
- Generate page-dependent CSS in `_includes/css/activation.scss.liquid` and include in `head.html`.
* No override svg rotate
* Disable both stylesheets safely
* Move the site nav to a new include
- Fix the complete site nav
- Move the site nav to `_includes/site_nav.html`
- Cache the site nav
- Uncache `nav.html`
* Move nav and site_nav to _includes/components
* Replace id prefix
* Update breadcrumbs.html
Replace several filters by a single loop through all the pages,
but breaking as soon as possible.
Profiling indicates that this saves up to 50% of the breadcrumbs build time for the filters.
* Update just-the-docs-head-nav.css
Adjust the number of lines to keep
* Update head.html
Remove superflous type.
* Update activation.scss.liquid
Remove a superfluous closing brace.
Adjust layout.
* Use `scssify` to remove nesting
Preliminary profiling indicates that using `scssify` on the small number of nested CSS rules produced by `activation.scss.liquid` is quick enough.
* Update head.scss
Manual attempt at prettier (pending installation in Atom).
* Avoid generation of nested CSS
Local profiling indicated that using `scssify` on each page takes about 1% of the build time.
- Update `_includes/css/activation.scss.liquid` to generate non-nested CSS.
- Remove use of `scssify` from `_includes/head.html`.
* Ignore false positives from validator
Ignores: `:1.810-1.823: error: CSS: Parse Error.` and `:1.811-1.824: error: CSS: Parse Error.`; had to shift things around since the local config overrides the CI flag.
* Inline `_sass/head.css`
* Update CHANGELOG.md
---------
Co-authored-by: Matthew Wang <matt@matthewwang.me>
* Remove "passive" toggle
PR #1244 introduced the "passive" toggle, but just-the-docs.js subsequently disabled the only styling that used it, so it became redundant.
This removes it.
* Update CHANGELOG.md
* Update CHANGELOG.md
Co-authored-by: Matt Wang <matt@matthewwang.me>
* Update CHANGELOG.md
Co-authored-by: Matt Wang <matt@matthewwang.me>
---------
Co-authored-by: Matt Wang <matt@matthewwang.me>
* 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 "<!DOCTYPE html>"
* 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 <matt@matthewwang.me>
* 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 `<style>` element for pages excluded from the navigation.
* Add a footnote about `.site-nav`
* Make setTheme remove background images from nav links
With a fixed nav panel, a <style> in the <head> sets a background image to highlight the nav list link to the current page. The image color depends on site.color_scheme.
Ideally, setTheme(theme) would change the image color to match the theme/scheme. Here, for simplicity, we merely remove the image.
* Explain `nav_fold` in collections.
* Refactor
Attempt at cleaning up the duplicated nav links code and simplifying removal of the background image:
* Add function `navLink()`
* Replace `removeNavBackgroundImages()` by `removeNavBackgroundImage()`
* Replace var `siteNav` by `document.getElementById('site-nav')`
* Replace code in `scrollNav` and `activateNav` by `navLink()`
* Replace a (non-local!) reference to `siteNav` by `document`
* Disable the page-specific stylesheet when JS is enabled
The page-specific `<style>` in the `<head>` is needed only when JS is disabled. Moreover, it incorrectly overrides dynamic stylesheets set by setTheme(theme).
The page-specific stylesheet is assumed to have index 1 in the list of stylesheets. It would be safer to select it by `id`, but adding an `id` can break existing sites.
* Avoid constraint on use of `.site-nav`, and refactor
Avoid the constraint on use of `.site-nav` by determining how many occurrences are produced by `css/activation.scss.liquid` when custom color schemes are ignored.
Move the Liquid code used for generating the page-dependent style element to a new include `head_nav.html`, to simplify `head.html`.
Remove the footnote about `.site-nav` in `docs/customization.md`.
Test the styling with JS disabled, since the resulting style element is disabled by JS.
* Revert "Avoid constraint on use of `.site-nav`, and refactor"
This reverts commit 5284892a7486ef9d2af9929c8a509b89731bb233.
* Avoid constraint on use of `.site-nav`, and refactor
(This corrects a bug in the previous reverted commit for excluded pages such as 404.html.)
Avoid the constraint on use of `.site-nav` by determining how many occurrences are produced by `css/activation.scss.liquid` when custom color schemes are ignored.
Move the Liquid code used for generating the page-dependent style element to a new include `head_nav.html`, to simplify `head.html`.
Remove the footnote about `.site-nav` in `docs/customization.md`.
Test the styling with JS disabled, since the resulting style element is disabled by JS.
* Update comment
* Fix duplicate plugins setting
@mattxwang noticed that the second `plugins` setting was apparently overriding the first, leading to a missing plugin when using `fixtures/Gemfile-github-pages`.
* Avoid double inclusion of activation file
The previous changes to remove the constraint re ".site-nav" duplicated the inclusion of `css/activation.scss.liquid`.
That caused significant extra build time, which the current changes to `head_nav.html` avoid (without affecting the built site).
* Clarify collection configuration docs
* Update and clarify the CHANGELOG
---------
Co-authored-by: Matt Wang <matt@matthewwang.me>
This follows up from #1259 and closes#1261. Basically, this PR accomplishes the two items discussed in the issue:
1. for all anchors that are *actually* buttons (i.e., have `href="#"`), I've replaced them with a semantic `<button>`
- under the hood, I've made a `.btn-reset` class pulling out the reset from #1259, so there's no visual change
2. for anchors that are ["toggle buttons"](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/button_role#toggle_buttons) (the mobile menu nav, sidebar children/grandchildren toggles), I've added an `aria-pressed` property that is updated as the button is clicked
I've also slightly modified some of the `aria-label`s to make them more consistent. Observe that we *shouldn't* update these as the button is clicked; screen readers use the `aria-pressed` property to add an annotation to each button.
To test this,
- the sidebar children and grandchildren can be done on the deploy preview:
- open an arbitrary page; observe that the sidebar children/grandchildren dropdown ticks now have a proper `aria-label` and `aria-pressed`, as well as otherwise work as intended
- toggle one of the buttons; observe the `aria-pressed` role changing as this is done
- open a grandchild page; observe that the `aria-pressed` has a correct default wrt whether or not the page is active
- the mobile menu can be done on the deploy preview; on a smaller viewport, observe the correct `aria-pressed`
- two features require local changes to test:
- the `site.search.button` needs to be enabled in the `_config.yml`. To test this, locally clone the repo, change the flag, and observe that the button still works as intended + has no visual regressions.
- the collections feature is a bit more complicated. To test this, locally clone the repo, add an arbitrary collection and changes to `_config.yml`, and observe the same behaviour for the sidebar children/grandchildren above
This PR fixes several Axe errors I found while working on https://viewcomponent.org/. I did not see any visual regressions on my deploy, but I'd encourage testing with other sites.
---------
Co-authored-by: Lindsey Wild <35239154+lindseywild@users.noreply.github.com>
* 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.
* Fix order of breadcrumbs
* Update breadcrumbs.html
Revert inclusion of single breadcrumb for top-level pages.
* Update breadcrumbs.html
* Update children_nav.html
Revert to the previous layout in the HTML, to allow the use of `diff` to check the built site.
* Update minimal.html
Remove the previously required workaround involving `nav.html`.
* Add docs pages about layouts
The aim of the initial version of these docs pages is to illustrate the difference between the default and minimal layouts.
* Update CHANGELOG.md
This PR does two things:
1. fixes using mermaid version `>= 10` from the CDN, by importing the ESM module instead
2. moves script loading code from `head.html` to the mermaid include
I've also added some light documentation to clarify how using mermaid with local paths should work (users should specify a version, and they should only use fully-minified bundles with no local references).
The nice thing about this approach is that it's a breaking change for nobody, and only adds functionality (v10 support). Eventually, we should remove support for mermaid <10, which should make this much easier!
Closes#1206.
## Context
In v10, Mermaid has implemented a few (admittedly, very frustrating to deal with) breaking changes:
1. they've removed CJS support, which is fine, *but*
2. that means that the `dist` they publish to JSDelivr now has a **different URL**: for versions `10.0.0` - `10.0.2`, **they do not have a minified bundle -- you have to load the ESM version with relative imports**
3. and, separately the `init` function has been deprecated
2 is really the issue, and so I've had to go into the code to now load mermaid by ESM by default when the user is on mermaid > v10.
I've tested this with:
- CDN version < 10 (v9)
- CDN version 10
- local path with version < 10 (v9)
- local path with version 10 (new: also loaded as an ESM module)
Separately, I chose to put all the mermaid stuff in one include because:
- I think @pdmosses requested something like this - it's a bit confusing that some mermaid code is *not* in the include, and this makes modular components ... more modular
- from a developer perspective, it's more clear what's happening with mermaid
- mermaid is not render-blocking, so it shouldn't be in the `head` anyways
---------
Co-authored-by: Peter Mosses <18308236+pdmosses@users.noreply.github.com>
In the conversation for #1166, I noticed that the import order for `setup.scss` disagrees with our docs.
> In particular, the [docs for `setup.scss`](https://just-the-docs.com/docs/customization/#override-and-define-new-variables) reads:
>
> > To define new SCSS variables, functions, or override existing theme variables, place SCSS code in `_sass/custom/setup.scss`
>
> But, this is not true - `setup.scss` is loaded *before* all of the themes, so it doesn't override existing theme variables.
>
> In my opinion, the solution here is to move `setup.scss` after all the `color_scheme` SCSS. This way, `setup.scss` properly allows theme overrides. In addition, users who previously defined variables in `setup.scss` and then used them in their custom color schemes can shift those declarations to be entirely in the theme code.
This is a one-liner that fixes the behaviour to be in-line with what our docs state.
This is a deceptively simple PR that stops the double import of `color_schemes`. With @pdmosses' stellar suggestion, it's a simple two-liner!
## interaction with #1166
This is a clean merge!
## path forward for default syntax highlighting
However, this leaves an interesting question: if the user doesn't provide syntax highlighting as part of their color scheme, should we include a default set (in this case, the light theme)?
Broadly, I see a few arguments:
1. if we don't provide defaults, we'll break color schemes that don't define their own syntax highlighting
2. if we do provide defaults, we could unnecessarily bloat the file size
I think 1 is more pernicious than 2. Thus, my suggested path forward is:
- for now, merge this
- in `v1.0`, separate these concerns properly, and force each color scheme to provide its own syntax highlighting CSS. Provide a default file for users to import with `@import` or `@use`[^1].
[^1]: Separately, we're using `!default` wrong; [looking at the docs](https://sass-lang.com/documentation/variables#default-values), we need to be using it with `@use` for the defaults to take effect. Since we're not doing that, `!default` isn't actually doing anything! This is why variable overrides aren't propagating the way they should be (and thus, users need to do a lot of duplication). Fixing this is probably a v1 item, though I'll have to think about it more.
I introduced a bug in #1172 --- there's an implicit requirement that the line in `mermaid_config.js` has either an expression or the start of a code block; a comment (and then a newline) is invalid.
(to maintain the ignore behaviour, I've instead moved it to the `.prettierignore`)
This one's on me -- I hadn't realized that the `prettier-ignore` affected this file. Given the relative urgency of this, going to merge this in.
Closes#1188.
After some discussion in #1166, I realized that our prettier workflow is somewhat broken! This PR fixes it, by:
- separating Stylelint & Prettier (and removing `stylelint-prettier`) to clearly separate linting versus formatting
- this is also helpful for the upcoming upgrading of Stylelint
- having Prettier ignore `assets/js/zzzz-search-data.json`; on `main`, `npm run format` fails because of this file (Prettier doesn't know how to interpret the front matter)
- change `npm test` to run both Stylelint and Prettier on CI (uses `npm-run-all` utility
- had to add one `// prettier-ignore` in `_includes/mermaid_config.js`, which I think is reasonably placed
- removing an unused `script` (that doesn't work!)
This is a no-op on the site itself, just for our dev environment/CI.
Ref: https://github.com/just-the-docs/just-the-docs/pull/1166#discussion_r1110397592.
---
Separately, upgrading to Stylelint 15 will come with another interaction - the [deprecation of stylistic rules](https://stylelint.io/migration-guide/to-15#deprecated-stylistic-rules) + shift to Prettier should be nice!
Hi there!
Thank you for the great theme! I am a happy user and was delighted to see that mermaid support has landed.
In some cases the usage of jsDelivr might not be possible for technical or compliance reasons.
This commit adds a second way to include the mermaid lib by specifying a path in the mermaid config. This way a local version of the lib can be used.
It should be fully backwards compatible, not requiring any action by users that already include the lib from the CDN.
I already added some documentation, but I am also happy to extend this, if this change is generally well-received.
Cheers,
Christian
This is an alternative PR that resolves#1011. Unlike #1013, this PR defines a *new* SASS file, `_sass/custom/setup.scss`, specifically designed for new custom variables (and other SASS-only constructs). It's imported after our `support` SASS files are (functions, variables), but otherwise is imported before all other files (ex, when CSS is emitted).
So, custom callout colors can now be defined in this file. I also move the custom callout colors present in `custom.scss` to the right location.
I've added some docs that briefly explain how to use the feature. Feedback is welcome!
---
As an aside, I chose not to add a `_includes/css` file that imports this, and then import that file. I think that's only necessary if we're trying to render liquid somehow in the SASS file; since we're not trying to do that for `setup.scss`, I've opted to not include it. If we think this is relevant, I can re-add it.
Co-authored-by: Peter Mosses <18308236+pdmosses@users.noreply.github.com>
This is a prototype for review and discussion. My use and testing of this PR is on top of 6d9d41359c46882d9b64a446d5a83fac5b3e20a7. The changes are trival to rebase to `main` and I'm happy to do so if this prototype moves forward.
* Feature request details in linked issue, fixesjust-the-docs/just-the-docs#1067
* I welcome feedback and all discussion
* A draft doc site of mine using this PR is at https://docs.hidale.com/
To use the prototype, the two include files need to be customized. Here are mine from the draft website 9c0d836408
Co-authored-by: Matt Wang <matt@matthewwang.me>
Hi everyone, this is a large refactoring PR that looks to **modularize site components** following the discussion in #959. At the top-level, it:
- moves icons, the sidebar, header (navbar, search, aux links), footer, and mermaid components of the `default` layout into their own `_includes`
- creates a new `minimal` layout that does not render the header or sidebar as a proof-of-concept for the composability of components
- documents all existing and new layouts (including vendor code) in the "Customization" section
An important goal of this PR is for it to be **just code motion and flexibility**: there should be **zero impact** on the average end user that only consumes the `default` theme.
The next few sections go in-depth on each of the listed changes.
### new components
The `default` layout contains a "list" of all relevant components. Importantly, some of these components have sub-components:
- the header is split into the search bar, custom code, and aux links
- the icons include imports different icon components, some of which are conditionally imported by feature guards
There are also candidates for future splits and joins:
- the sidebar could be split into navigation, collections, external link, and header/footer code
- the "search footer" could be joined with other search code, which would make it easier to "include search" in one go; *however, this is a markup change*
- @kevinlin1 has pointed out that there is some leakage between the sidebar (which computes parents/grandparents) and the breadcrumbs (which needs them to render). He's graciously added a bandaid fix to `minimal` (which does not render the sidebar). However, in the long term, we should either:
- calculate this in a parent and pass the information to both components
- change how this works entirely (which may happen with multi-level navigation)
@pdmosses has done a great job outlining this and more in his [Modular Layouts test site](https://pdmosses.github.io/modular-layouts/docs/main/).
### minimal layout
Based on @kevinlin1's use-case in just-the-class (see: his [Winter 2023 CSE 373 site](https://courses.cs.washington.edu/courses/cse373/23wi/)), we've created a first-class `minimal` layout that does not render the sidebar or header.
In a [comment](https://github.com/just-the-docs/just-the-docs/pull/1058#discussion_r1057015039), Kevin has indicated that we can re-add the search bar in the minimal layout; however, it seems like this would be a code change. I think we should punt this to a future issue/PR.
@pdmosses has also discussed the confusion of `minimal` as a layout and its meaning in inheritance. I've added a note in documentation to clarify the (lack of) inheritance relationship.
### documentation
I've written documentation in the "Customization" page / [Custom layouts and includes](https://deploy-preview-1058--just-the-docs.netlify.app/docs/customization/#custom-layouts-and-includes) section explaining:
- generally, that we use includes/layouts (and pointing to docs)
- the `default` layout and its constituent components (with a warning about name collisions)
- creating alternative layouts with `minimal` as an example
- the inheritance chain of layouts and the vendor layouts that we consume
I've also created (and linked to) a [minimal layout test](https://deploy-preview-1058--just-the-docs.netlify.app/docs/minimal-test/) that is currently a copy of the markdown kitchen sink but with the minimal layout. I think there's room to improve this in the future.
### future work
I think there's a lot we can do. Let me break this into various sections.
Potential follow-ups before `v0.4.0`:
- re-including search in `minimal` (anticipating a minor code change)
- fixing the leakage of parent/grandparent information between the sidebar and breadcrumbs (anticipating no end-user code change, but good to evaluate separately and discuss)
- heavily document this in the migration guide (#1059) and in our RC4 release docs
- improve semantic markup for components (ex `main`, `nav`)
Related work in later minor versions:
- split up components into smaller components
- allow users to easily customize new layouts using frontmatter (see @kevinlin1's [comment in #959](https://github.com/just-the-docs/just-the-docs/issues/959#issuecomment-1249755249))
Related work for `v1.0` (i.e. a major breaking change):
- rename and better categorize existing includes
- standardizing the "custom" includes
- moving other components to the `components/` folder (ex `head`, `nav`)
- potentially: less confusing naming for various components
- potentially separate the search and header as components, so that they are completely independent
Tangentially related work:
- more flexible grid (see @JPrevost's [comment in this PR thread](https://github.com/just-the-docs/just-the-docs/pull/1058#issuecomment-1363314610))
- a formal [feature model](https://en.wikipedia.org/wiki/Feature_model) of JTD, documenting feature dependence (see @pdmosses's [comment in this PR thread](https://github.com/just-the-docs/just-the-docs/pull/1058#issuecomment-1365414023))
- better annotate new features (motivated by writing these docs)
- we should add "New" to new features :)
- we should note when a feature was introduced (I think this is a core part of most software documentation)
- we should annotate things that are "Advanced" in so far as the average Just the Docs user will not use them / they require significant Jekyll knowledge
---
Closes#959.
Avoid the need to add a link to favicon,ico when editing `_includes/head_custom.html`, and avoid creating an invalid favicon link
- Remove the content of `_includes/head_custom.html`
- Add code to `_includes/head.html` to create a link to an existing favicon,ico
- Add an explanation of favicon_ico to docs/configuration.md
- Remove the example of `includes/head_custom.html` and add an explanation of what the `<head>` element automatically includes
Avoid Liquid failure when no pages with titles
Fix issue #1085
The user's config specified collections (incorrectly). Trying to build the site resulted in Jekyll failing due to a Liquid error. The error report did not suggest the cause of the error.
Liquid fails with division by 0 when title_pages_size is 0. This fix guards that code by checking that title_pages is non-empty.
To test:
1. Specify a Jekyll collection with no pages, and specify it as a JTD collection.
2. Build the site.
3. Check that the specified collection has no nav links to pages.
Closes#1070.
This PR:
- updates `jekyll-anchor-headings` to `1.0.12`; this was a simple copy-paste
- updates `lunr.js` to `2.3.9`; this was a bit more involved:
- I didn't see a minified build in the repo, so I ran it through [DigitalOcean's minifier](https://www.digitalocean.com/community/tools/minify)
- copyright notices weren't properly included in the previous minified build, so I:
- include an actual copy of the original MIT License for `lunr.js`
- includes proper attribution for other functions, which include derivative works
There's a tiny bundle size increase in `lunr.js` due to the comments, but I think that's reasonable given that it's related to licensing; still trivial in the grand scheme of things.
As an aside: it would be neat if we could include minification as part of the build pipeline instead!
* Issue #1023 - note that GA4 properties are supported
* Issue #1023 - parameterize Google Analytics property script
* Issue #1023 - support a list of multiple Google Analytics tracking IDs in config
* Issue #1023 - update Google Analytics configuration doc
* Fix configuration of multiple Google Analytics properties and simplify type checking
* simplify unnecessary code repetition
* tweak Google Analytics config documentation wording
* Add 'reversed' as the preferred keyword, with 'desc' as a deprecated alternate
* Doc updates
* Add the test for 'reversed' to the toc_list
Add also a comment about this.
Co-authored-by: Peter Mosses <18308236+pdmosses@users.noreply.github.com>
* Removes `favicon.html`, shifts content to `head_custom.html`
* explicit callout for custom favicon in customization docs
* Cleaner and more consistent documentation (review from @pdmosses)
Co-authored-by: Peter Mosses <18308236+pdmosses@users.noreply.github.com>
In case 'just-the-docs' is used as a theme, and a user does not
eplicitly include 'jekyll-seo-tag' inside the plugins list in
_config.yml, two 'title' elements were rendered. Since
jekyll-seo-tag is always available, because it's set in the plugins
list of the theme itself, the 'seo' tag will aways work, making
the seperate title and description elements obsolete.
Fixes#1008
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>
* Fix default highlighting in custom schemes
Fix#982
The variable settings for highlighting in the default `light` scheme are currently (v0.4.0.rc2`) in `_sass/color_schemes/light.scss`.
This PR ensures that custom schemes are based on the `light` scheme.
It also adds a note explaining the default to the customization docs,
and gives an example of how to define a custom scheme based on the `dark` scheme
* Prettier
* Deleted test file
Co-authored-by: Matt Wang <matt@matthewwang.me>
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>
This PR has a bit of scope creep! This PR now:
- changes the mermaid opt-in logic to only check for the existence of a `mermaid` key instead of `mermaid != false`: this resolves the follow-up in #857
- changes the behaviour of mermaid configuration
- instead of using `mermaid_init.html` with default settings, makes the include `mermaid_config.js`
- the include is loaded directly into the contents of `mermaid_initialize`
- by default, it is an empty object (i.e. `{}`), triggering the defaults
- updates docs
- adds an example to the markdown kitchen sink
It does significantly change the interface provided in #857, and I apologize for the confusion. However, given the discussion in this PR, I think it's the best move forward!
