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

When using sphinx-hoverxref together with this theme, links with tooltips are hidden #577

Closed
humitos opened this issue Jun 23, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@humitos
Copy link

humitos commented Jun 23, 2022

Describe the bug

When using sphinx-hoverxref on a Jupyter Book, the links do not appear when the HTML is rendered. However, the HTML tags are there. It seems there is a clash between CSS classes that produces this problem.

An issue was open in the sphinx-hoverxref repository (readthedocs/sphinx-hoverxref#180) about this problem, but I'm not sure if the problem is there, here, or somewhere else 😅 . I'm copying (and modifying it a bit) and pasting the content of it here.

I'm the author of the sphinx-hoverxref extension. Let me know if I can be useful somehow here.

Reproduce the bug

  1. in a clean virtualenv, install the dependencies pip install sphinx-hoverxref jupyter-book matplotlib
  2. create a new book, jupyter-book create mynewbook/
  3. edit the mynewbook/_config.yml file to install the sphinx-hoverxref extension
# _config.yml

sphinx:
  config:
    hoverxref_roles:
      - term
    hoverxref_role_types:
      term: modal
  extra_extensions:
    - hoverxref.extension
  1. edit the mynewbook/intro.md to make usage of the extension
```{glossary}
Term one
  An indented explanation of term 1
```

Here is my term!

{term}`Term one`

Is it shown here?
  1. build the book, jupyter-book build mynewbook

When you open mynewbook/_build/html/index.html you will see that the Term one is not shown. However, if you inspect the DOM, the HTML element is there. It seems there is a problem with CSS where Jupyter Book's theme interferes with the .tooltip (opacity: 0) and .modal (display: none) classes making them invisible.

171012026-1a53e5b4-3d18-4f0a-a7c2-6f4a5afbd483

List your environment

▶ jupyter-book --version       

Jupyter Book      : 0.13.0
External ToC      : 0.2.4
MyST-Parser       : 0.15.2
MyST-NB           : 0.13.2
Sphinx Book Theme : 0.3.2
Jupyter-Cache     : 0.4.3
NbClient          : 0.5.13
@humitos humitos added the bug Something isn't working label Jun 23, 2022
@welcome
Copy link

welcome bot commented Jun 23, 2022

Thanks for opening your first issue here! Engagement like this is essential for open source projects! 🤗

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! 🎉

@choldgraf
Copy link
Member

@humitos - thanks for opening this one up. Hmmm, I can think of two potential ways around this:

  1. Special-case sphinx hoverxref by adding an SCSS file that follows similar patterns as the others in this folder and making the behavior be what we want (I'm happy to enable sphinx-hoverxref on these docs if that would help test it out). Note that we might consider doing this in the PyData theme instead, if we imagine the same problem might occur there (it similarly has an extensions folder for special-case CSS).
  2. Remove the bootstrap tooltips. We may decide that these bootstrap tooltips are not be worth all of the trouble they are causing 😅. I wonder if there's a way that we could still nicely-style the tooltips with our own CSS rather than resorting to the Bootstrap over-rides, which tend to be a bit aggressive.

Any thoughts or guidance on either of these options?

@humitos
Copy link
Author

humitos commented Jun 27, 2022

@choldgraf

Special-case sphinx hoverxref by adding an SCSS file that follows similar patterns as the others in this folder and making the behavior be what we want (I'm happy to enable sphinx-hoverxref on these docs if that would help test it out). Note that we might consider doing this in the PyData theme instead, if we imagine the same problem might occur there (it similarly has an extensions folder for special-case CSS).

I've done something similar in sphinx-hoverxref as a quick fix for some other themes. However, I think this case, in particular, is a little more complex since there is a clash between the CSS classes that sphinx-hoverxref uses with the ones coming from bootstrap. So, I think there should be a way to use a "namespace" to not collide? I'm not good at CSS and I'm not sure if that's a thing or not, but if both, sphinx hoverxref and bootstrap, use .tooltip; how do we distinguish them when writing CSS rules?

Should I prefix all sphinx-hoverxref with hxr-, for example? Liike, hxr-tooltip, hxr-hoverxref, hxr-modal, etc

Remove the bootstrap tooltips. We may decide that these bootstrap tooltips are not be worth all of the trouble they are causing sweat_smile. I wonder if there's a way that we could still nicely-style the tooltips with our own CSS rather than resorting to the Bootstrap over-rides, which tend to be a bit aggressive.

I'm not aware of the troubles these have been causing, but if they are not being used anymore, removing these is the best option 😄

humitos added a commit to readthedocs/sphinx-hoverxref that referenced this issue Jun 27, 2022
Avoid collisioning with other CSS frameworks/themes/etc.

See executablebooks/sphinx-book-theme#577
Closes #180
@humitos
Copy link
Author

humitos commented Jun 27, 2022

Should I prefix all sphinx-hoverxref with hxr-, for example? Liike, hxr-tooltip, hxr-hoverxref, hxr-modal, etc

Prefixing all the classes is not a lot of work and solves the immediate problem, at least. See readthedocs/sphinx-hoverxref#205

humitos added a commit to readthedocs/sphinx-hoverxref that referenced this issue Jun 27, 2022
Avoid collisioning with other CSS frameworks/themes/etc.

See executablebooks/sphinx-book-theme#577
Closes #180
@choldgraf
Copy link
Member

I do agree that prefixing things in hoverxref is the easiest path forward and is a good practice. It is similar to what sphinx design uses to avoid the same problem (and annoying that bootstrap doesn't do it)

humitos added a commit to readthedocs/sphinx-hoverxref that referenced this issue Jun 28, 2022
Avoid collisioning with other CSS frameworks/themes/etc.

See executablebooks/sphinx-book-theme#577
Closes #180
humitos added a commit to readthedocs/sphinx-hoverxref that referenced this issue Jul 6, 2022
* Prefix all CSS classes with `hxr-`

Avoid collisioning with other CSS frameworks/themes/etc.

See executablebooks/sphinx-book-theme#577
Closes #180

* Update tests to use the prefix `hxr-`

* Refactor code to use CSS prefix variable
@humitos
Copy link
Author

humitos commented Jul 7, 2022

Hi! I just released a new version of sphinx-hoverxref extension that prefixes all the CSS classes with hxr-. This should solve the collision problem that we had. I'm closing this issue now. Feel free to re-open, ping me or anything if you consider that's not working as expected. Thanks!

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

2 participants