Add nav_enabled variables for more customizable and feature-complete minimal layouts

[kevinlin1]

For a while now, we have implemented and maintained a minimal layout #959 that removes the sidebar, search, and auxiliary navigation links (buttons next to the search bar). It turns out that there's actually a relatively simple way to enable search and aux links features. Here's what I did:

1. Replaced minimal.html with the latest copy of default.html.
2. Deleted line 12, {% include components/sidebar.html %}, to avoid rendering the sidebar in the minimal.html layout.
3. Added the mx-auto utility class to the main div to horizontally-center the main div.

The result is depicted in the following mockup. (Due to a dependency chain issues, I wasn't able to fully test the site on my local machine.)

Here is the old (current) version of the minimal layout for reference.

Benefits
This change should allow the configuration options like search_enabled and aux_links to work on minimal layouts.

I think this approach also brings improved maintainability: although we'll still need to contend with managing two different layout files, at least the diff between the two layout files is now much smaller.

-  {% include components/sidebar.html %}
-  <div class="main" id="top">
+  <div class="main mx-auto" id="top">

Can you test this change and let me know what you think? Thanks in advance!

[kevinlin1]

Alternatively, this could be implemented as a top-level _config.yml toggle to globally enable or disable the sidebar. This would have the further benefit of easing maintenance between these two otherwise identical files, but comes at the cost of reduced flexibility as website designers won't be able to as easily specify the layout per page. But I wonder how often people would actually want to intermix "default" and "minimal" layout pages on a website since it would be confusing.

So perhaps a top-level toggle for default.html is the better approach?

[kevinlin1]

I was able to mock-test this on a live site. I think the approach of adding a site-level toggle will be better just-the-docs.js currently assumes that the site will have a side/mobile navigation menu, and the lack of that menu breaks some of the JavaScript. I'll revise this pull request accordingly.

[kevinlin1]

Here is a sample commit that I assembled for testing purposes on a template site. If this sounds good, I can adjust this pull request accordingly and push it to my fork so that you can see.

[pdmosses]

Alternatively, this could be implemented as a top-level _config.yml toggle to globally enable or disable the sidebar.

Presumably the default would be to include the sidebar, so to exclude it you would specify nav_enabled: false.

This would have the further benefit of easing maintenance between these two otherwise identical files, but comes at the cost of reduced flexibility as website designers won't be able to as easily specify the layout per page.

You might support enabling and disabling the sidebar with a front-matter variable as well as a site configuration variable, with the former overriding the latter:

---
title: ...
layout: default
nav_enabled: true
---
# ...

Alternatively, the minimal layout could perhaps itself set a layout variable to exclude the sidebar:

---
layout: default
nav_enabled: false
---

{{ content }}

Then users would specify a minimal layout page as usual:

---
title: ...
layout: minimal
---
# ...

[kevinlin1]

Thanks for the suggestion @pdmosses! I've implemented your idea to handle site.nav_enabled, layout.nav_enabled, and page.nav_enabled variables. Here is how it appears on my local test site when you set any of the nav_enabled variables to false.

Here are two more screenshots: first showing the effect of setting search_enabled to false and then showing that option plus removing aux_links.

Note that this approach is now more backwards-compatible as it keeps the minimal layout working. However, the appearance of the header is a difference: perhaps a separate patch should consider hiding the components/header.html if: (1) search is disabled, (2) there is no custom header code, and (3) there are no aux links.

Note that disabling site.nav_enabled but enabling layout.nav_enabled or page.nav_enabled won't work as just-the-docs.js only includes mobile nav handling code if site.nav_enabled is not false.

We will also need to update the documentation later.

[pdmosses]

Thanks for the updates to the code!

Based on a quick look, this seems almost ready for review. To facilitate reviewing, please:

1. Update the PR description to match the current proposal.
2. Add instructions for exhaustively exercising the various options.
3. Add a draft CHANGELOG entry.

Note that disabling site.nav_enabled but enabling layout.nav_enabled or page.nav_enabled won't work as just-the-docs.js only includes mobile nav handling code if site.nav_enabled is not false.

Wouldn't it work if you change just-the-docs.js to test if (document.getElementById('site-nav')) { ... } instead of {%- if site.nav_enabled != false %} ... {%- endif %}? (Caveat: My level of JS expertise is very low!)

[kevinlin1]

Yes, that sounds good. We should be able to keep the JavaScript function definitions for navigation so long as they aren't called if the element is not found: this does make a slightly heavier than necessary JavaScript payload for sites that never use the side navigation, but it will perhaps make the configuration more predictable.

[mattxwang]

So sorry for the delay @kevinlin1 - finally feel like I'm catching up to the start of spring quarter. Left one documentation note and one correctness question, but otherwise this seems pretty solid to me.

I'm not too worried about backwards compatibility with minimal! However, this change does have downstream impacts on users who have heavily modified the sidebar CSS logic. I'll make a brief note in the migration guide for this feature that those users should be careful when upgrading.

[mattxwang]

This is well-written! After some thought, I'm not sure if this should be in the layout docs - rather, this feels like something that should be in configuration (since this is includes a config.yml variable) and in customization (where we already discuss the minimal layout - and also update that example). @pdmosses, what are your thoughts? I think you've always had a great eye for the organization of our documentation.

[pdmosses]

I think the change to _config.yml proposed by the PR should be accompanied by adding a corresponding section in docs/configuration.md. ¹

However, I'd prefer to keep the proposed explanation in docs/layout/layout.md.

The (simplified) examples of layouts in docs/customization.md indeed need to be updated to be consistent with the (current and proposed) code.

Footnotes

1. I also think the configuration docs page is long overdue for splitting into child pages, but that obviously shouldn't be in this PR. ↩

[kevinlin1]

I pushed some changes to the documentation pages: docs/customization.md and docs/configuration.md as I saw fit. Feel free to suggest further changes or push commits directly on top of my branch.

[mattxwang]

Thanks for the quick turnaround @kevinlin1! This looks solid to me.

I quickly gave this PR branch a spin on some of our other client sites and they work out-of-the-box. When I draft the migration guide for the next release, I'll make a note that

• we've updated the minimal layout: users who have overriden just that layout should take a look, but users who inherit from it may see different behaviour
• we've updated the sidebar styling, so those with custom sidebars may see different behaviour

@pdmosses, any thoughts?

---

I may be a bit ambitious and try to also merge in #1440, which gives us a pretty solidly-scoped minor release.

[pdmosses]

@mattxwang:

When I draft the migration guide for the next release, I'll make a note that

• we've updated the minimal layout: users who have overriden just that layout should take a look, but users who inherit from it may see different behaviour
• we've updated the sidebar styling, so those with custom sidebars may see different behaviour

It would be helpful to indicate in what ways the behaviour might differ.

---

I may be a bit ambitious and try to also merge in #1440, which gives us a pretty solidly-scoped minor release.

#1440 includes a major update to the docs for the main navigation in connection with #1431, so #1440 needs to be merged in the same release as #1431 (which hasn't yet been reviewed).

In fact some of the new docs pages in #1440 are more than 3 levels deep, and they won't appear correctly in the navigation until the theme supports multi-level.

Although the present PR and #1431 both affect the default layout, I don't see a significant advantage of merging them in the same release, as they address completely different issues.

[mattxwang]

I may be a bit ambitious and try to also merge in #1440, which gives us a pretty solidly-scoped minor release.

#1440 includes a major update to the docs for the main navigation in connection with #1431, so #1440 needs to be merged in the same release as #1431 (which hasn't yet been reviewed).

In fact some of the new docs pages in #1440 are more than 3 levels deep, and they won't appear correctly in the navigation until the theme supports multi-level.

Ah! Apologies - I had meant to tag #1431, not #1440. 100% my fault - sorry for the confusion. Definitely agree with all the analysis here!

Although the present PR and #1431 both affect the default layout, I don't see a significant advantage of merging them in the same release, as they address completely different issues.

Certainly not required. I can go ahead and release this soon then!

---

Will merge this in now. Re:

It would be helpful to indicate in what ways the behaviour might differ.

I will be sure to do that in the migration guide.

Thanks for all your work in drafting + implementing this feature @kevinlin1!