Fix dokieli unreliability

executablebooks / sphinx-comments

hypothes.is interaction layer with Sphinx

https://sphinx-comments.readthedocs.io

MIT License

20 stars 8 forks source link

Fix dokieli unreliability #12

Open choldgraf opened 3 years ago

choldgraf commented 3 years ago

It seems that sometimes Dokieli works as expected and other times it doesn't. For example, when dokieli is activated via the extension, some page loads it will work and others it won't, I'm not sure exactly why this is but there seems to be some kind of race condition going on.

Moreover, when building with Jupyter Book there is some kind of error that pops up on the JS side, resulting in this message:

csarven commented 3 years ago

Just checking in.. how can this be reproduced? Can you share links - that'd help a lot. Last I looked was here: https://github.com/executablebooks/jupyter-book/issues/861#issuecomment-672097889 . Can we isolate the issue so we know its actual origin? I'm not yet convinced that this is a dokieli issue but happy to look into it with you.

choldgraf commented 3 years ago

Hey @csarven - I've made a PR that activates Dokieli in the documentation here: https://github.com/executablebooks/sphinx-comments/pull/17

and we can preview the built site here:

https://sphinx-comments--17.org.readthedocs.build/en/17/

it seems like Dokieli loads the first time that the page is loaded, but a hard refresh makes it not show up anymore? I'm not really sure, but any fixes to make this reliably show up would be appreciated!

csarven commented 3 years ago

Thank you for putting this together @choldgraf !

dokieli.js appears to be included twice in https://sphinx-comments--17.org.readthedocs.build/en/17/dokieli.html . In this case, after DOM loaded, two <menu>s are getting inserted - first with event handler and second doesn't. Clicking on the menu right now doesn't do anything as you'd expect. So, might want to simply remove the second dokieli.js in the HTML ie. the one in <body>. Ditto dokieli.css. That should make the menu button working again.

Aside: dokieli shouldn't have loaded the second time as there is a check to see if DO already exists. I can test that further though.

I'm not seeing an issue with refresh right but perhaps the above quick fix will move things forward.

firasm commented 3 years ago

I am trying to get dokieli working on my jupyter book, and the public version here: https://firasm.github.io/jupyterbook_course_template/about/syllabus.html

Based on the info above, I know I need to add this to my _config.yml file:

  comments:
    dokieli: true
    hypothesis: false
    utterances:
      repo: "firasm/jupyterbook_course_template"

However I don't see the button at the top left to activate it. I don't see the .js file appearing twice. In the <head> I see:

<link media="all" rel="stylesheet" type="text/css" href="https://dokie.li/media/css/dokieli.css" />
<script async="async" kind="dokieli" src="https://dokie.li/scripts/dokieli.js"></script>

It's seems clear to me that a js file is added because it seems to overwrite some of the CSS because some of my admonitions look different.

With dokieli: false I see

and with it set to true, I see:

@csarven Were you able to get dokieli working on a JupyterBook ?