Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Broken logic for building GitHub URL for "Edit this page" link #138

Closed
sarahmaddox opened this issue Oct 8, 2019 · 19 comments · Fixed by #1744 or kubeflow/website#3625
Closed

Broken logic for building GitHub URL for "Edit this page" link #138

sarahmaddox opened this issue Oct 8, 2019 · 19 comments · Fixed by #1744 or kubeflow/website#3625
Assignees
Milestone

Comments

@sarahmaddox
Copy link
Contributor

Hallo to the Docsy theme team :)

The page-meta-links partial layout (at line 8) contains the following code, which doesn't look right:

{{ if and ($gh_subdir) (.Site.Language.Lang) }}

I tried removing the and, and I also tried moving the and to between the two bracketed clauses, but neither of those updates works as expected. So I've decided to hand this problem over to people who have more background into the recent updates to that file.

I found the above code while investigating a problem when upgrading Kubeflow to the latest Docsy. Kubeflow does not use a language directory, and therefore doesn't need the /en inserted into the GitHub path. On testing the upgrade, we found that the latest version of Docsy insists on including /en in the file path. (Thank you to @joeliedtke for pointing out that the "Edit this page" link was broken!)

For Kubeflow, we'll to continue using our custom layout, which checks .Site.IsMultiLingual to determine whether to add the /en or not. We need the custom layout anyway at this stage, because it contains the code to inject the page URL into the issue body when creating an issue. See feature request #91. At some stage, though, it'll be good to remove our custom layout and use the standard Docsy one, when both these issues are fixed.

@sarahmaddox
Copy link
Contributor Author

Perhaps it should be:

{{ if .Site.IsMultiLingual and ($gh_subdir) (.Site.Language.Lang) }}

@LisaFC
Copy link
Collaborator

LisaFC commented Oct 9, 2019

Thanks for spotting that! I'll take a look, I remember having to do a lot of tweaking to that bit of template to cover various permutations and combinations of "is multilingual" and "is in a subdirectory" (the original version broke our own user guide site) and I will admit that I may not have got it quite right.....

@LisaFC
Copy link
Collaborator

LisaFC commented Oct 11, 2019

Just to clarify, it's sticking the "/en" into the path even though your content isn't in an "/en" subdir? Going to make a copy of your site and start further poking at it.

@LisaFC LisaFC self-assigned this Oct 11, 2019
@sarahmaddox
Copy link
Contributor Author

Just to clarify, it's sticking the "/en" into the path even though your content isn't in an "/en" subdir?

Yes, that's right. Thanks Lisa!

@LisaFC
Copy link
Collaborator

LisaFC commented Oct 21, 2019

Aha, I think I see what's going on. I'm checking on Site.Language.Lang and Site.isMultiLingual as if they're the same thing, and they're not - you can have a specified language setting without a language-specific subdir. Should be able to fix!

@LisaFC
Copy link
Collaborator

LisaFC commented Oct 25, 2019

So this is turning out to be an annoying one - basically unless you haven't specified any supported languages you always have a Site.Language.Lang (ergo not a very useful thing to check on), and IsMultiLingual only checks if you have more than one language, not whether you have anything in a language-specific subdirectory. What I actually need to check on is if your content directory is just "content" rather than "content/en" or "content/no" or whatever, but that is not exported as a site param from config.toml that I can see.

Pending me finding some lovely elegant solution (hem hem) you have two options:

  • keep your own override version of this partial
  • export the fact that your content root is just "content" in your config.toml and I can check that in the partial.

I may refactor this partial anyway as right now it seems to be checking on something that will return true for most sites if they have typical setup.

@sarahmaddox
Copy link
Contributor Author

Thanks @LisaFC We'll hang on to our override for the moment. We're planning to move to support multiple languages at some time anyway. But it may be worth pursuing your lovely elegant solution (it's gotta be out there somewhere heh heh) for other sites that offer only one language.

@genebean
Copy link

Not sure if this is the same issue or not, but my site nor the docsy site seem to have the “edit this page” links. Is there anything I can do to help debug this?

@LisaFC
Copy link
Collaborator

LisaFC commented Jan 31, 2020

I can see the Edit This Page links now, and they work on both sites. May have been just a transient issue with GitHub?

@genebean
Copy link

genebean commented Feb 1, 2020

What am I missing or doing wrong then (honest question)? I don’t see the link on my phone in Safari or Firefox nor do I see it on my iPad in Safari, Firefox, or Brave. The iPad appears to be showing the desktop site too.

@sarahmaddox
Copy link
Contributor Author

Hallo @genebean Do you see any of the links in the right-hand panel (edit this page, create doc issue, create project issue)? I've noticed that the right-hand panel disappears when the screen width is narrower than a certain width, even on desktop. This also happens if you crank up the zoom on the browser (as I do). Do the right-hand links appear if you reduce the zoom level?

@genebean
Copy link

genebean commented Feb 1, 2020

Okay... so I do see them on my laptop. I guess my question becomes what needs to change so that these exist on non-wide screens (and would you like a separate issue for that)? It seems like they should flow up into another location when that bar goes away or become a an overlay like the "Fork me on GitHub" banners I have seen over the years. I'm really interested in this because so many people only see our sites from small or smallish screens.

@genebean
Copy link

genebean commented Feb 1, 2020

I was just thinking more about this... maybe near the feedback section at the bottom of each page would be a good place either for the to flow to or to just live all the time. Would moving them there just be a case of updating the partials?

@LisaFC
Copy link
Collaborator

LisaFC commented Feb 5, 2020

Aha, right! Yes the entire right menu on docs pages doesn't appear in the mobile/smaller version of the site but I can see how the github links would be useful. Just trying to figure out where to put them on smaller screens and how to implement it (currently the github links partial is included in the toc partial)

I think having them at the top right for bigger screens is the right place to put them rather than moving them down the bottom for all users, as especially for longer pages users may not want to scroll all the way down to the bottom to find the edit or feedback links.

And yes, please start a new issue!

@dend
Copy link

dend commented Sep 1, 2022

@LisaFC was there ever a solution to this? In a newly-deployed site (see example) seems like the edit links are still expecting the /en suffix for content, even though the content directory in the configuration file is different.

@ManuelRauber
Copy link

Running into the same issue

LisaFC added a commit that referenced this issue Nov 16, 2023
Fixes #981
Fixes #138

Requires Hugo 0.112.0
@LisaFC
Copy link
Collaborator

LisaFC commented Nov 16, 2023

#1744 should finally fix this...

@chalin
Copy link
Collaborator

chalin commented Nov 16, 2023

The original issue (well, at least in the current version of the website), is now fixed by:

@chalin
Copy link
Collaborator

chalin commented Nov 27, 2023

Kubeflow's original issue is fixed, so I'm going to close this now because we:

@chalin chalin closed this as completed Nov 27, 2023
chalin pushed a commit that referenced this issue Jan 12, 2024
Fixes #981
Fixes #138

Requires Hugo 0.112.0
chalin pushed a commit that referenced this issue Feb 1, 2024
Fixes #981
Fixes #138

Requires Hugo 0.112.0
chalin pushed a commit that referenced this issue Feb 1, 2024
Fixes #981
Fixes #138

Requires Hugo 0.112.0
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
6 participants