OceanGlidersCommunity / Oxygen_SOP

Oxygen Standard Operating Procedure (SOP)
https://oceangliderscommunity.github.io/Oxygen_SOP/README.html
Other
6 stars 21 forks source link

utterance commentary function does not work?! #234

Open soerenthomsen opened 2 years ago

soerenthomsen commented 2 years ago

Hi @jbusecke,

during a workshop we just realised that the commentary function below our beloved SOPs is gone? Any idea what happened?

Cheers

callumrollo commented 2 years ago

I've done a bit of digging. Looks like the culprit is the div name of sections. UItterances identifies sections presumably to create a seperate set of comments for each one.

Inspect element shows this block is throwing an error

    sections = document.querySelectorAll("div.section");
    if (sections !== null) {
        section = sections[sections.length-1];
        section.appendChild(script);
    }

It's looking for divs with class "section" In the Nitrate SOP, where comments are working, we do see these divs, e.g.

<div class="section" id="questions">

meanwhile, over in the Oxygen SOP, the same object is called a section, rather than being a div with class section

<section id="questions">

I'm sure there's an easy way to solve this and I'm having a go, but my html-fu is rudimentary at best.

callumrollo commented 2 years ago

Even changing the JS to search for section, it still throws an error later on. Can't figure out why jupyter-books changed to using section rather than div.section :/

soerenthomsen commented 2 years ago

Thanks @callumrollo for digging!

It is strange but maybe helps to find the error that indeed the commentary function works for the nitrate SOP but not for the oxygen and salinity. And even more confusing not for the Depth Average Current SOP as we have not changed anything there for a long time.

@jbusecke and myself pushed quite some PR's lately to the Oxygen SOP to create the pdf (just for Oxygen SOP so far) and improve the citation style (both for oxygen and salinity).

@jbusecke any idea what might have happened?

I cannot say since when exactly the commentary function is off unfortunately. Realised it yesterday during the workshop!

Here some info: https://utteranc.es

soerenthomsen commented 2 years ago

In the

oxygen

_toc.yml @jbusecke changed to "jb-book" and "chapters" to build the release v1.0.0 pdf. Don't remember why.

`format: jb-book root: README options: # The options key will be applied to all chapters, but not sub-sections numbered: true chapters:

Nitrate

it is called "sections" in the _toc.yml, which works

`format: jb-article root: README options: # The options key will be applied to all chapters, but not sub-sections numbered: True sections:

Salinity

_toc.yml, which does not work...

`format: jb-article root: README options: # The options key will be applied to all chapters, but not sub-sections numbered: True sections:

callumrollo commented 2 years ago

Seeing as the _toc files for nitrate and salinity are identical, bar section names, I don't think it's the source. That was the first thing I tested. My suspicion is that something in the build process has changed. nitrate SOP hasn't been updated since May. Perhaps we need to pin some earlier package versions if the behavior of the jupyter books build has changed since then

callumrollo commented 2 years ago

Nitrate was last built with Jupyter-Book v0.12.3, the others are using v0.13. Coud've been a breaking change. I will try to pin the environment and build locally with the earlier jupyter-books

soerenthomsen commented 2 years ago

Here @callumrollo fixed it quickly for the salinity SOP ahead of the workshop starting now. https://github.com/OceanGlidersCommunity/Salinity_SOP/pull/139

jbusecke commented 2 years ago

Sorry for the late comments here.

I am having a bit of trouble identifying the issue here.

I initially thought the same as @soerenthomsen , but the _toc.yml and and the _config.yml are also different on the Salinity SOP (which is currently down it seems 😜).

Perhaps we need to pin some earlier package versions if the behavior of the jupyter books build has changed since then

If this ultimately fixes the issue this should be definitely brought to the attention of the jupyter-book folks (maybehere?)

Ill try the version pin on the oxygen SOP right now, lets see if that works

jbusecke commented 2 years ago

Even changing the JS to search for section, it still throws an error later on. Can't figure out why jupyter-books changed to using section rather than div.section :/

This would be an excellent question to ask over there! Great job digging out the reason for the failure!

It might be useful to make a minimally reproducible example (maybe some dummy 1-page repo?) to check if this persists in a less complex case.

callumrollo commented 2 years ago

I've filed an Issue with jupyter-book. It's late in the day here, but I'll try making a minimum reproducible example tomorrow

jbusecke commented 2 years ago

Amazing. Thanks for the work. Glad there was an intermediate fix!

soerenthomsen commented 2 years ago

Thanks @callumrollo and @jbusecke for your support on this!

soerenthomsen commented 2 years ago

So we keep this open for now until we find a long term fix?

callumrollo commented 2 years ago

Yeah, let's see what comes out of the jupyter-books Issue. Pinning on a previous version isn't ideal and could create problems down the line. It's the best fix we've got for now though.

https://github.com/executablebooks/jupyter-book/issues/1762