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

Adjust the "Edit this page" button to always point to versioned docs #4371

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Abbondanzo
Copy link
Contributor

@Abbondanzo Abbondanzo commented Nov 23, 2024

Per a recent comment here, it was pointed out that the "Edit this page" button in the footer of all our pages do not actually point at the versioned docs but at the latest copy of the docs.

This PR changes the URL to match the respective doc location, not the most recent copy of docs. So, if I'm looking at docs for 0.76, the edit button will link me to the exact 0.76 copies in the repository.

I am hesitant to merge it because it might be more desirable to point people at latest by default. It will also result in broken URLs when the older versions are archived and no longer in the repository.

Copy link

netlify bot commented Nov 23, 2024

Deploy Preview for react-native ready!

Name Link
🔨 Latest commit 7902da8
🔍 Latest deploy log https://app.netlify.com/sites/react-native/deploys/675063f0d3622b0008573772
😎 Deploy Preview https://deploy-preview-4371--react-native.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@yungsters
Copy link
Contributor

Hmm, yeah… I'm not sure that this is ideal.

I think there's certainly a problem of helping contributors understand documentation versioning, but I think most people would reasonably expect that their changes will make it into at least next and all future versions of the docs.

They probably also want it to be in the currently viewed version (most commonly, the latest versioned docs).

What if we had two buttons? One that leads to the current version, and one the leads to master / next?

@Abbondanzo
Copy link
Contributor Author

What if we had two buttons? One that leads to the current version, and one the leads to master / next?

We have a custom footer component swizzled in there and I wonder if we can return an array of URLs from the editUrl function. At worst, we have piles of context we can tap into to build both URLs. I'll give this a try tomorrow if it sounds reasonable. The naming portion is hard, what do you think?

[Edit this page] [Edit latest page]

[Edit this version] [Edit latest version]

[Edit version XYZ] [Edit latest version]

@yungsters
Copy link
Contributor

Hmm… if we can determine the current version being viewed (ideally without parsing the current URL, but including that as an option), maybe we could do something like…

  • If viewing next (master)…
    • Edit page for next release
    • (Just one button.)
  • If viewing latest version (latest versioned)…
    • Edit page for next release
    • Edit page for current release
  • If viewing an older version…
    • Edit page for next release
    • Edit page for 0.XY release

Maybe something like that? It's more work, so of course don't feel obligated to do all of this. We can still make incremental improvements. 😁

@Abbondanzo
Copy link
Contributor Author

Abbondanzo commented Nov 26, 2024

Hmm… if we can determine the current version being viewed (ideally without parsing the current URL, but including that as an option), maybe we could do something like…

I got something like this working in b9261b6. Had to hack the editUrl to be a stringified JSON blob so I could unpack it in our footer. I don't see the editUrl used anywhere else so this should be okay but it's a bit wonky

Here's "Next":
Screenshot 2024-11-26 at 11 50 37 AM
Here's 0.76 (latest):
Screenshot 2024-11-26 at 11 50 31 AM
Here's 0.75:
Screenshot 2024-11-26 at 11 50 43 AM

@slorber
Copy link
Contributor

slorber commented Dec 4, 2024

I guess I should add support for edit url arrays in Docusaurus 😄

I already wanted to add support for multiple edit variants (GitHub, stackblitz, codesandbox). I thought about presenting the edit options in some kind of modal, possibly explaining the tradeoffs of each option. In your case, it probably doesn't make sense though, but you could still disable that modal through swizzle.

But your use-case also makes sense and probably should be the default for versioned docs sites? 🤔

@Abbondanzo
Copy link
Contributor Author

But your use-case also makes sense and probably should be the default for versioned docs sites? 🤔

Maybe it could be useful to add support for multiple edit URLs with labels, but this change isn't very intelligent in the sense that does not check if a version of the page you're looking at exists for both the "next" version and that version. So, if I introduce a page that is only present in an old version, clicking "Edit page for next release" can very well lead to a broken URL.

@slorber
Copy link
Contributor

slorber commented Dec 5, 2024

That's true. The edit url is a plugin-specific feature (not Docusaurus core) and each plugin can have different opinions (docs has versioning for example). Supporting arrays in docs/blog might be a good first step, but I'll need to design the feature a bit more for docs.

@hichemfantar
Copy link
Contributor

hichemfantar commented Dec 6, 2024

this pr increases the lint task duration from 3m to 38mins btw

my pr did the same as well #4384

@hichemfantar
Copy link
Contributor

hichemfantar commented Dec 6, 2024

it seems that the cause in my pr was upgrading to eslint-plugin-mdx v3 which i believe means the check for broken external urls lint rule wasn't actually doing anything before i created my pr

@hichemfantar
Copy link
Contributor

actually nevermind it seems the latest commit in this pr was ran after my pr was merged so the cause is 100% #4384

like i said previously, i think the rule started working in my pr but i'm investigating

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants