executablebooks / sphinx-thebe

A Sphinx extension to convert static code into interactive code cells with Jupyter, Thebe, and Binder.
https://sphinx-thebe.readthedocs.io/en/latest/
MIT License
28 stars 15 forks source link

Thebe clashes with hide-cell tag in Jupyter-book #26

Open fortierq opened 3 years ago

fortierq commented 3 years ago

Describe the bug

Thebe prevents cells with hide-cell tag from opening in Jupyter-book. "Click to show" does nothing. This is because the ID of the div is overwritten by shpinx-thebe, and the "click to show" button uses the original ID to toggle visibility.

To Reproduce

Steps to reproduce the behavior:

  1. Define a code cell for Jupyter-book:
    ```{code-cell} ipython3
    :tags: [hide-cell]
    (1 + 5**0.5)/2
  2. Launch Thebe inside Jupyter-book
  3. "Click to show" does not show hidden cell.

Expected behavior

"Click to show" should show the hidden cell.

Environment

Additional context

The following line of code change the id of an hidden cell, preventing to toggle its hidden status: https://github.com/executablebooks/sphinx-thebe/blob/53c098b0aba0a64a21d7c842c6dead140164bd31/sphinx_thebe/_static/sphinx-thebe.js#L62-L65 Why do we need to change this codeCell.id? Can we modify only the id of non-hidden cells?

I would happily try to submit a PR if needed :)

PS: in https://jupyterbook.org/interactive/thebe.html, an example is provided with a hide-input cell. This works because the corresponding HTML element has class cell-input and not class cell (like hide-cell tag), and thus its id is not modified.

welcome[bot] commented 3 years ago

Thanks for opening your first issue here! Engagement like this is essential for open source projects! :hugs:
If you haven't done so already, check out EBP's Code of Conduct. Also, please try to follow the issue template as it helps other community members to contribute more effectively.
If your issue is a feature request, others may react to it, to raise its prominence (see Feature Voting).
Welcome to the EBP community! :tada:

choldgraf commented 3 years ago

@fortierq thanks for opening this one up - I think that your diagnosis is correct that we are overwriting and ID. I think that we could try:

If a cell already has an ID, then we don't assign it a new one. If we can confirm that the behavior still works the same way, then maybe we are fine to stop there.

Want to give that a shot?

fortierq commented 3 years ago

Thanks @choldgraf! I'm afraid I'm too busy at the moment to work on this at the moment. Maybe later.