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

Redirecting to homepage broken for version switcher #1343

Closed
lagru opened this issue Jun 7, 2023 · 10 comments · Fixed by #1350
Closed

Redirecting to homepage broken for version switcher #1343

lagru opened this issue Jun 7, 2023 · 10 comments · Fixed by #1350
Labels
kind: bug Something isn't working

Comments

@lagru
Copy link

lagru commented Jun 7, 2023

The documentation for the version switcher states

When clicked, the switcher will check for the existence of that page, and if it doesn’t exist, redirect to the homepage of that docs version instead.

I think I discovered at least two cases where this doesn't seem to work:

We might have some wrong configuration but finding this issue on your website as well leads me to believe that this may actually be a bug?

@lagru
Copy link
Author

lagru commented Jun 7, 2023

cc @stefanv

@12rambau
Copy link
Collaborator

12rambau commented Jun 7, 2023

is it related to #932 ?

@12rambau
Copy link
Collaborator

12rambau commented Jun 7, 2023

note that for our theme it's expected as there were no "example" section in v0.9 (see: https://pydata-sphinx-theme.readthedocs.io/en/v0.9.0/index.html)

@lagru
Copy link
Author

lagru commented Jun 7, 2023

I don't think this is a duplicate of #932, though it might be related. #932 describes behavior that happens on the root of v0.8.0, while this issue pertains to the switchers behavior on subpages.

note that for our theme it's expected as there were no "example" section in v0.9

Precisely. The fact that v0.9 doesn't have an example section is why we can see the supposed bug in action. The cited part from the doc pertains to the behavior of the version switcher if the switched-to version doesn't have a corresponding equivalent of the current subpage.

@12rambau 12rambau added kind: bug Something isn't working impact: block-release Should block a release from happening. Only use if this is a critical problem we don't want to ship labels Jun 7, 2023
@12rambau
Copy link
Collaborator

12rambau commented Jun 7, 2023

the code dealing with this is store here:

function checkPageExistsAndRedirect(event) {

I tried to reproduce the configuration in of v0.9 in a codepen (https://codepen.io/12rambau/pen/RwqwdvV) and it should return the correct homepage.

My best guess is that the location is not updated and the event returns the original url whatever the reult of the fetch test. As I'm no js expert I need to continue investigate.

@12rambau
Copy link
Collaborator

12rambau commented Jun 8, 2023

related to #844

@lagru
Copy link
Author

lagru commented Jun 21, 2023

Hey, thanks for trying to addressing this. I just tried to test the fix with the approach described below, and it seems like it still doesn't work?

  1. Setup scikit-image (assumes a clean Python environment outside the project directory)
git clone https://github.com/scikit-image/scikit-image.git
cd scikit-image
pip install -r requirements.txt -r requirements/build.txt
  1. Apply the following patch
Subject: [PATCH] Update pydata-sphinx-theme to rev e8d1e68
---
Index: pyproject.toml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/pyproject.toml b/pyproject.toml
--- a/pyproject.toml	(revision a20870ee582f844482096e2edff65fa9e511213c)
+++ b/pyproject.toml	(date 1687369797918)
@@ -101,7 +101,7 @@
     'kaleido',
     'scikit-learn>=0.24.0',
     'sphinx_design>=0.3',
-    'pydata-sphinx-theme>=0.13',
+    'pydata-sphinx-theme @ https://github.com/pydata/pydata-sphinx-theme/archive/e8d1e689105dd135762a021821f8348845c38172.zip',
 ]
 optional = [
     'SimpleITK',
Index: requirements/docs.txt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/requirements/docs.txt b/requirements/docs.txt
--- a/requirements/docs.txt	(revision a20870ee582f844482096e2edff65fa9e511213c)
+++ b/requirements/docs.txt	(date 1687369796044)
@@ -17,4 +17,4 @@
 kaleido
 scikit-learn>=0.24.0
 sphinx_design>=0.3
-pydata-sphinx-theme>=0.13
+pydata-sphinx-theme @ https://github.com/pydata/pydata-sphinx-theme/archive/e8d1e689105dd135762a021821f8348845c38172.zip
  1. Build documentation
spin docs
  1. Open the resulting file doc/build/html/index.html inside a browser and navigate to the "About" section in navigation bar, try to switch to version 0.20. Which attempts to access the non-existent https://scikit-image.org/docs/0.20.x/about/index.html.

I am not sure whether there it makes a difference that I am accessing the site on the local file system instead of some server.

@drammock
Copy link
Collaborator

darn, yeah, I was afraid that #1350 would not be enough.

@drammock drammock reopened this Jun 21, 2023
@12rambau
Copy link
Collaborator

Note that I'm also super sade ;-)
but TBH I really don't understand why it's not working. if I check the header of the 404 page it should work. Looking more closely to the inspector of my browser, it seems the link is followed before the try/catch statment, maybe the preventdefault method should be called at the begining of the function and not at the end ?

(sorry I'm no javascripts expert, it's try and error process here)

@12rambau 12rambau removed the impact: block-release Should block a release from happening. Only use if this is a critical problem we don't want to ship label Jul 22, 2023
@12rambau
Copy link
Collaborator

Removing the block-release impact as it's not preventing core functionality and we need to release the latest layout. I'm still trying to understand why links are followed, but I'm afraid I need more JS skills for that.

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