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

fix version-switcher with dirhtml builder #1795

Merged
merged 3 commits into from
May 20, 2024

Conversation

casperdcl
Copy link
Contributor

Problem: version-switcher suffixes .html even if sphinx was used with the dirhtml builder.

Solution: suffix / instead where necessary.

@casperdcl casperdcl force-pushed the fix-version-switcher-dirhtml branch from c65fdb4 to c31b7e1 Compare April 30, 2024 13:47
@trallard trallard added kind: enhancement New feature or request tag: component Issues or improvements associated with a given component in the theme labels May 8, 2024
@azeey
Copy link

azeey commented May 10, 2024

I was having the same issue and this PR does fix it for me. Thanks @casperdcl!

@casperdcl casperdcl force-pushed the fix-version-switcher-dirhtml branch from c31b7e1 to f614def Compare May 14, 2024 15:03
Copy link

Coverage report

This PR does not seem to contain any modification to coverable code.

var getCurrentUrlPath = () => {
if (DOCUMENTATION_OPTIONS.BUILDER == "dirhtml") {
return DOCUMENTATION_OPTIONS.pagename == "index"
? `/`
Copy link
Contributor Author

@casperdcl casperdcl May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw I'm unsure if this would suffice (avoids duplicate // suffix, but maybe breaks in some unknown edge cases? I haven't tested it thoroughly)

Suggested change
? `/`
? ""

Copy link
Collaborator

@Carreau Carreau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

@Carreau
Copy link
Collaborator

Carreau commented May 20, 2024

Ok, let's try that. It seem simple enough that I believe it is safe to merge.

@Carreau Carreau merged commit cad0c74 into pydata:main May 20, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement New feature or request tag: component Issues or improvements associated with a given component in the theme
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants