Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple Sidebar Nav Lists #2843

Merged
merged 2 commits into from
May 5, 2024
Merged

Conversation

tsmanner
Copy link
Contributor

This is an enhancement or feature.

Summary

This implements a small change to the nav_bar include that iterates over elements in the page.sidebar.nav variable, creating nav links for each of them. Existing behavior is unchanged, the new behavior allows the following YAML as well.

sidebar:
  nav:
    - main
    - docs

Context

This is not related to any issues. It is a small change to the include file that allows for more configuration reuse for common navigation elements.

Let me know what you think. I have one thought already which is that I'm not sure if the nav_bar loop should be inside or outside the <ul class="nav__items"> tag, or possibly even outside the <nav class="nav__list"> tag with a sidebar.nav.[nav].title key or something like that. Doing it that way seemed to be introducing a lot more complexity without gaining much usefulness, so I went for the simplest solution here.

@iBug
Copy link
Collaborator

iBug commented Feb 24, 2021

Have you tested this? I'm afraid it breaks existing setup.

@tsmanner
Copy link
Contributor Author

Huh, I tested it, but with something very simple, I'll take another look today with some more complexity. I'm okay with closing this in the meantime if you'd like to unclutter the list a bit, I can always reopen it later.

@iBug
Copy link
Collaborator

iBug commented Feb 24, 2021

Looks OK to me.

The point I missed was that looping over a string in Liquid is like looping over an array with just that string.

@tsmanner
Copy link
Contributor Author

Ah, got it. It's a pretty handy feature. I did some more testing on a larger page and it seems to be holding up there.

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity.

This pull request will automatically be closed in 7 days if no further activity occurs. Thank you for all your contributions.

iBug referenced this pull request May 13, 2021
@github-actions github-actions bot closed this May 21, 2021
@iBug iBug reopened this Apr 23, 2024
@iBug iBug merged commit 64b1d42 into mmistakes:master May 5, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants