No JavaScript regression (BookEnableJS option)

[Roboe]

Commit dda0a0e introduced a new JS-based search and, at the same time, removed the BookEnableJS option, which was false by default.

In configs with explicit BookEnableJS = false, this introduced a regression which makes it load JS scripts without the awareness of the site owner (for the search and for the menu scroll).

Since Hugo Books was one of the few pretty and made-with-care JS-free themes for Hugo, is there a possibility to restore that functionality for site owners not wanting any JavaScript at all?

I can look into it and prepare a PR (edit: look at the progress here). If I'm not mistaken, there are only three scripts: one for the scrolling menu, one for the search widget, and one for Google Analytics (if googleAnalytics option is set).

[alex-shpak]

Hi!
I see your point.

The reason why I removed BookEnableJS is because while implementing search I came to conclusion that it's quite vague parameter. For example: how user expect it to affect search? would Search = true be suppressed by EnableJS = false, will search box show anyway?

Also 2 shortcodes were introduced that use JS: mermaid and katex, would it also prevent these shortcodes from loading?

So at that point I decided to remove BookEnableJS but goal is still keep website working if JS is disabled in browser.
Though I'm open to adding it back if we find some clear way to communicate that to end user.

[Roboe]

Hmm, I see there's a lack of clarity on the option, too. In my opinion, there are two different ways to achieve this:

• Implement a "feature flag" for each JS feature. BookSearch is already there. There are just 3 left. Show a message when JS-based shortcodes are used.
• Re-introduce the generic option BookEnableJS and specify in the example configs which features it will disable. Templates could follow this conditional:

{{ if and (.Site.Params.BookEnableJS) (default true .Site.Params.Book<Feature>) }}

I did a proof-of-concept implementation here.

Maybe things could be easier to understand and use if all feature flags were negative, since not specifying them won't disable anything. That way we can avoid the default true part in templates, too.

{{ if or (.Site.Params.BookDisableJS) (.Site.Params.BookDisableSearch) }}

What do you think?

[alex-shpak]

Sorry for late reply, I have read your comment some time ago and gave some time to thought it
through.

I think generic BookEnableJS will have to go, because it can't guarantee that JS script is not included in some way.
Maybe there can be

...
params:
  BookJS:
    - scrollReset
    - otherFeature

that can be emptied with BookJS: [] config.

Though, there is now only one feature that will stay there, which is scroll reset.
Other features controlled already by some params:

• Google Analytics is not included if you don't have config param
• SearchJS is not included without BookSearch
• Katex/Mermaid included only if shortcode is used.

Also perhaps this list can be extended https://gohugo.io/about/hugo-and-gdpr/#all-privacy-settings

[alex-shpak]

I think to add BookMenuRestorePosition configuration param, so same as you mentioned in you case number 1. Defaulting to true.
Would work for you?

[Roboe]

Yes! That would work perfectly, 🤗
Please, include a new option for each new JS-based feature in the future. I'll do my part and check their config before each update.

Thank you so much for your work, and excuse my absence!

(This issue can be closed when 6877700 reaches master)

[alex-shpak]

Hi!
I kinda feel unsure about what is best way for future, here is another idea:

have script in assets/menu-reset.js (and keep other scripts as file in assets)

And add this to partial template.

{{ $script := resources.Get "menu-reset.js" | resources.Minify }}
{{ with $script.Content }}
  <script>{{ . | safeJS }}</script>
{{ end }}

So in order to disable it you will overwrite assets/menu-reset.js to empty content, so it would not render on page.
WDYT?

[Roboe]

Hmm! That's a pretty clever idea, certainly! I didn't knew about the .Content property for Hugo resources. This is a quite interesting and flexible option that would serve well, IMO.

Maybe group script assets in a script or js (or better, book-scripts/book-js) folder, to avoid misunderstanding and overpopulating the user assets folder?

[alex-shpak]

Hi!
I put it into assets folder, since theme scss files are also there, just to keep it simple.

Because it is important to you please keep an eye on these files, in case if new added. I will try to not break it without reason.

[Roboe]

Great! I'll test this sortly and close this issue as solved accordingly.

Thank you so much again for your consideration and caring, Alex.

[alex-shpak]

Hi!
I do a bit of cleanup, I will close this issue, just write here if some more changes needed - I will reopen.

[alex-shpak]

@Roboe Hi there!
Just want to notify you, there is new JS script added:assets/sw.js is a Service Worker registration script.
It is disabled but might become on by default after some testing.

[Roboe]

Hey, Alex! Thank you for the notification!
I will ensure we override that file.

Thanks again for your consideration!