Corrects logic for enabling ToC

Hanse00 commented 5 months ago

In the existing version, the ToC partial checks if .Params "toc" is set, which will never be set as the value must be accessed as .Site.Params, therefore the default value of true was always used.

This change rearranges the logic so that we correctly check if toc is set, and respect tocWordCount if that is set.

netlify[bot] commented 5 months ago

Deploy Preview for hugo-theme-anubis2 ready!

Name	Link
Latest commit	5b402c4b5d74b30fe331c43f48d1916ded8d61cf
Latest deploy log	https://app.netlify.com/sites/hugo-theme-anubis2/deploys/6632b0b744dcde0009429b2c
Deploy Preview	https://deploy-preview-44--hugo-theme-anubis2.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 100 Accessibility: 96 Best Practices: 92 SEO: 91 PWA: - View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Junyi-99 commented 5 months ago

Thank you for submitting! Anubis2 does have this problem.

However, we should allow a page to set their own toc alone, so .Params.toc should have a higher priority than Site.Params.toc. In your commit, there are only a Site-level setting.

I think it could be improved a bit more, would it be possible to add toc settings for the page? Thus it could be merged into the main branch.

Hanse00 commented 5 months ago

I see what you're saying. I've written down a truth table to capture the interactions between the three values: Page ToC setting, Site ToC setting, and the ToCWordCount setting.

Do you agree with my logic here about how the three settings should interact with each other? If yes, I can implement it this way.

Display TOC?	Page ToC setting	Site ToC setting	Site word count setting	Note
true	true	unset	unset	If the page ToC value is true, display ToC no matter what
true	true	true	unset
true	true	false	unset
true	true	unset	set unmet
true	true	true	set unmet
true	true	false	set unmet
true	true	unset	set met
true	true	true	set met
true	true	false	set met
false	false	unset	unset	If the page ToC value is false, do not display ToC no matter what
false	false	true	unset
false	false	false	unset
false	false	unset	set unmet
false	false	true	set unmet
false	false	false	set unmet
false	false	unset	set met
false	false	true	set met
false	false	false	set met
true	unset	unset	unset	If page ToC value is not set, fall back to site value - Unset defaults to true
true	unset	true	unset
false	unset	false	unset
false	unset	unset	set unmet	If word count is set, respect it
false	unset	true	set unmet
false	unset	false	set unmet
true	unset	unset	set met
true	unset	true	set met
false	unset	false	set met	If word count is set, but ToC is explicitly set to false - Ignore word count and don't show ToC

Junyi-99 commented 5 months ago

Excellent clear truth table! I agree 100% with your logic, you are really a very responsible person, please go ahead!

Hanse00 commented 5 months ago

As agreed, the logic is now:

If the page has a toc parameter, use that exclusively to decide what to do.

If not: If the page word count is above tocWordCount (the function assumes 0 if the parameter is not explicitly set), then enable the ToC.

If however the toc site parameter is set to false, don't display the toc, no matter the word count.

I think that satisfies all the requirements.

Junyi-99 commented 5 months ago

Thank you for your contribution!

Junyi-99 / hugo-theme-anubis2