docsy icon indicating copy to clipboard operation
docsy copied to clipboard

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

Open sarahmaddox opened this issue 6 years ago • 15 comments

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 https://github.com/google/docsy/issues/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 avatar Oct 08 '19 02:10 sarahmaddox

Perhaps it should be:

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

sarahmaddox avatar Oct 08 '19 18:10 sarahmaddox

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 avatar Oct 09 '19 16:10 LisaFC

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 avatar Oct 11 '19 15:10 LisaFC

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!

sarahmaddox avatar Oct 12 '19 12:10 sarahmaddox

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 avatar Oct 21 '19 15:10 LisaFC

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.

LisaFC avatar Oct 25 '19 12:10 LisaFC

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.

sarahmaddox avatar Oct 25 '19 13:10 sarahmaddox

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?

  • https://piweatherrock.technicalissues.us/docs/
  • https://www.docsy.dev/docs/adding-content/repository-links/

genebean avatar Jan 31 '20 03:01 genebean

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

LisaFC avatar Jan 31 '20 15:01 LisaFC

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.

genebean avatar Feb 01 '20 04:02 genebean

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?

sarahmaddox avatar Feb 01 '20 15:02 sarahmaddox

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 avatar Feb 01 '20 15:02 genebean

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?

genebean avatar Feb 01 '20 17:02 genebean

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!

LisaFC avatar Feb 05 '20 00:02 LisaFC

@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.

dend avatar Sep 01 '22 16:09 dend

Running into the same issue

ManuelRauber avatar Oct 25 '22 10:10 ManuelRauber

#1744 should finally fix this...

LisaFC avatar Nov 16 '23 16:11 LisaFC

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

  • https://github.com/kubeflow/website/pull/3625

chalin avatar Nov 16 '23 18:11 chalin

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

  • Have a dup of this issue: #981
  • Probably have a fix, TBC soon! #1744

chalin avatar Nov 27 '23 20:11 chalin