-
-
Notifications
You must be signed in to change notification settings - Fork 377
feat: support breadcrumbs for single pages that are neither docs or blogs #743
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
Conversation
✅ Deploy Preview for hugo-hextra ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate more on the necessity of this breaking change?
Curious about the use cases this change enables.
|
I have found myself either having to set the content layout of different archetypes of content to be docs or blog, or copy and pasting the blog single.html layout and adding this line to get breadcrumbs across my website. I am not a huge fan of setting the content layout in front matter because the content is neither a blog or docs Additionally, I thought it would be good to have uniformity across the functionality of all single pages that this theme provides. |
…er than docs and blog docs: added docs and examples for enabling breadcrumbs for specific content types
|
Added a new site parameter, which now stops this from being a breaking change. Essentially, |
thanks for this, definitely on the right track! I wonder if instead of a list targeting per content type, we could make |
Ah. That already exists, with hiding breadcrumbs. The way breadcrumbs work without this PR, is that breadcrumbs are enabled by default for docs and blog but they can be disabled via front matter on a per-page basis. This PR will allow other content types to opt in to the same functionality. |
My thinking is we can keep the breadcrumb implementation in If we do so, we need to slightly modify the existing breadcrumbs enablement logic by making it default to |
…front matter field. docs: updated docs to reflect that there is no site parameter driving breadcrumbs feat: enabled breadcrumbs for lists as well as singles for uniformity
Yup this is definitely a much nicer way of doing it. The code is cleaner, and it is more easily configurable. Also enabled breadcrumbs in lists as well as singles since the docs lists.html gets breadcrumbs. I felt all lists.html should also have the option :) |
| @@ -0,0 +1,7 @@ | |||
| {{ $ret := false }} | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this helper function partial is not needed
layouts/_partials/breadcrumb.html
Outdated
| @@ -1,4 +1,4 @@ | |||
| {{- if (default true .Params.breadcrumbs) }} | |||
| {{- if (default (partial "utils/default-breadcrumbs-enabled" .) .Params.breadcrumbs) }} | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can make the partial accept parameters
so for docs/blogs we can pass in true and for other, we can pass in .Params.breadcrumbs and falls back to false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Though rather than passing in .Params.breadcrumbs I pass in a dict which contains the page and a bool called enabledByDefault. We need to pass in the page for the logic of the partial, and I didn't want callers of breadcrumb.html to be the ones to do the logic of combining the default and front matter parameters. So I left that logic inside the breadcrumb.html partial.
I can easily hoist that logic out, if you would prefer :)
…f extra parameterization of _partials/breadcrumb.html
… to remain consistent with the rest of the theme
…ky/hextra into BreadcrumbsInDefaultSingle
|
@KStocky thank you, great work! |
BREAKING CHANGE: since
layouts/_partials/breadcumb.htmlwill enable breadcrumbs by default, this is a breaking change. Sites that have pages that uselayouts/single.htmlwill get breadcrumbs by default on their pages with this change. They can disable the breadcrumbs with frontmatter, however.