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

utterance commentary function does not work?! #234

Open
soerenthomsen opened this issue Jun 20, 2022 · 14 comments
Open

utterance commentary function does not work?! #234

soerenthomsen opened this issue Jun 20, 2022 · 14 comments
Assignees
Labels
bug Something isn't working

Comments

@soerenthomsen
Copy link
Member

Hi @jbusecke,

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

Cheers

@callumrollo
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

soerenthomsen commented Jun 21, 2022

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
Copy link
Member Author

soerenthomsen commented Jun 21, 2022

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:

  • file: sections/authors_SOP_development_process`

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:

  • file: sections/authors_SOP_development_process`

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:

  • file: sections/authors_SOP_development_process`

@soerenthomsen soerenthomsen added the bug Something isn't working label Jun 21, 2022
@callumrollo
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

Here @callumrollo fixed it quickly for the salinity SOP ahead of the workshop starting now.
OceanGlidersCommunity/Salinity_SOP#139

@jbusecke
Copy link
Member

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
Copy link
Member

jbusecke commented Jun 21, 2022

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
Copy link
Member

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
Copy link
Member

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

@soerenthomsen
Copy link
Member Author

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

@soerenthomsen
Copy link
Member Author

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

@callumrollo
Copy link
Member

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.

jupyter-book/jupyter-book#1762

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants