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

broken reference in API builds #1435

Open
12rambau opened this issue Sep 14, 2023 · 3 comments
Open

broken reference in API builds #1435

12rambau opened this issue Sep 14, 2023 · 3 comments
Labels
help wanted Extra attention is needed impact: high Something that is relevant to nearly all users kind: bug Something isn't working

Comments

@12rambau
Copy link
Collaborator

12rambau commented Sep 14, 2023

I wanted to work on the last item preventing us to release the latest version of the theme: #1317

As it's not link to any issue I fail to understand what we are trying to solve here. I checked the current build of our own API and:

  • scroll_spy finally work as expected
  • clicking on the secondary sidebar links is leading to the one in the page
  • search is broken (it doesn't find API items anymore)

@drammock as you have already spend some time working on this could you explain me what is wrong with the links and I'll try to work on it before monday, I really want to show the amazing new collor scheme to the users.

In adition I created a pre-build of one of my repo with the theme and not only it looks amazing, I see no issues with the anchors: https://sepal-ui--876.org.readthedocs.build/en/876/modules/sepal_ui.html

@drammock
Copy link
Collaborator

drammock commented Sep 14, 2023

I also have this on my list but I am super swamped this month. If I am correctly understanding the situation:

  • #1208 tried to make our theme work more nicely with sphinx-apidoc by changing ids and hrefs on sidebar TOC entries for the documented package's API pages. Specifically, it converts . to _ in id and href.
  • Meanwhile, #1208 seems to have broken other links
  • #1317 attempts to fix what #1208 broke. It is stalled; I did some refactoring of it to make it more DRY and try to understand what it was doing, but the massive searchtools file that was added I haven't even looked at yet.

IMO the best course of action is to revert #1208 and do what we can to help make an upstream fix happen. If you want, we could consider re-merging something like #1208 after cutting the release, but only if it gets some additional work (especially tests) to make sure it's not creating breakages like @tpoint75 discovered. If we do that, it would be helpful if @bheberlein and/or @tpoint75 could help out on the testing side (by helping us craft test(s) that are reflective of their build situations).

@12rambau
Copy link
Collaborator Author

My problem is that I did other documentations recently using other templates like this one with immaterial and there are no issues with the scrollspy in their seconday sidebar. nor does furo in urlib.

My conclusion is maybe it's actually the scrollspy method that should be changed instead.

In any case would you agree to revert changes introduced by #1208 and publish a release and then try to understand what we should change from our javascript side ?

@drammock
Copy link
Collaborator

ok after looking a bit more closely I think maybe you're right that the fix should be in scrollspy code. Specifically:

So my conclusion is still: we should revert #1208, then cut a release, then the third step is try to fix #1026 in a different way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed impact: high Something that is relevant to nearly all users kind: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants