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: dynamic editUrl path for documentation subfolder structure #247

Merged
merged 4 commits into from
Oct 10, 2024

Conversation

Codexnever
Copy link
Contributor

Description:

This pull request addresses an issue with the editUrl configuration in the docusaurus.config.js file, where the editUrl was incorrectly concatenating the paths for editing the documentation. The issue was observed when the documentation structure was split into multiple subfolders, but the correct path was not reflected in the edit links.

Problem:

When attempting to edit documentation from the site, the generated edit URL was appending paths incorrectly. For example, it was redirecting users to:

https://github.com/open-sauced/intro/blob/main/docs/intro-to-oss/README.md/docs/intro-to-oss/README.md

This resulted in a broken link because of the duplicate path segments.

Solution:

  1. Dynamic Path Handling: Updated the editUrl on line 47 of the docusaurus.config.js file to handle the subfolder structure dynamically. The editUrl now accurately reflects the path by properly concatenating the correct segments of the URL.

  2. Folder Structure: The course documentation was split into two subfolders, which wasn't properly handled in the editUrl path. The issue has now been resolved by making the editUrl match the actual structure of the docs subfolder.

Changes Made:

  • Modified the editUrl on line 47 of the docusaurus.config.js to dynamically handle the subfolder structure.
  • Ensured that the URL correctly redirects to the main branch of the GitHub repository for the documentation:
    https://github.com/open-sauced/intro/blob/main/docs/intro-to-oss/README.md
    

Verification:

  • Verified that the changes work locally by testing the documentation edit links in the Docusaurus site. The edit links now correctly redirect to the respective documentation files in the GitHub repository.

Affected Files:

  • docusaurus.config.js: Corrected the editUrl configuration to prevent path duplication and broken links.

Related Issues:

Close #241

@Codexnever Codexnever requested a review from a team as a code owner October 5, 2024 06:25
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

PR Compliance Checks Passed!

Copy link

netlify bot commented Oct 5, 2024

Deploy Preview for learn-opensauced ready!

Name Link
🔨 Latest commit 544cc25
🔍 Latest deploy log https://app.netlify.com/sites/learn-opensauced/deploys/6706a1a7f161140007601c31
😎 Deploy Preview https://deploy-preview-247--learn-opensauced.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.

@Codexnever Codexnever changed the title Fix dynamic editUrl path for documentation subfolder structure Fix: dynamic editUrl path for documentation subfolder structure Oct 5, 2024
@Codexnever Codexnever changed the title Fix: dynamic editUrl path for documentation subfolder structure fix: dynamic editUrl path for documentation subfolder structure Oct 5, 2024
Copy link
Member

@adiati98 adiati98 left a comment

Choose a reason for hiding this comment

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

Hey @Codexnever,
Thank you for your PR! ✨

I've tested this out locally, and it works well. 👍🏼
As @BekahHW mentioned:

You can create the PR if it's working locally.

let's stick to that.

I see that you made some other changes here that are not required in the issue. Is there any reason for making these changes?

@adiati98
Copy link
Member

adiati98 commented Oct 7, 2024

@BekahHW, I just have some thoughts here after taking a closer look, and would love your feedback.

When folks click the "Edit this page" link, they will be redirected to the page where they can edit their changes directly on the main branch of the original repo. And we don't want them to make any changes directly in the original repo and main branch.

edit page on github

Some docs have this link on their website. However, most of them redirect folks to a page asking contributors to fork the repo to make changes. Let's take Algolia DocSearch as an example. The link redirects to this page (see the screenshot below).

Screenshot 2024-10-07 120107

I think we can adapt this method for this intro repo to encourage folks to improve our website while preventing any direct changes.

@BekahHW
Copy link
Member

BekahHW commented Oct 7, 2024

@BekahHW, I just have some thoughts here after taking a closer look, and would love your feedback.

When folks click the "Edit this page" link, they will be redirected to the page where they can edit their changes directly on the main branch of the original repo. And we don't want them to make any changes directly in the original repo and main branch.

edit page on github

Some docs have this link on their website. However, most of them redirect folks to a page asking contributors to fork the repo to make changes. Let's take Algolia DocSearch as an example. The link redirects to this page (see the screenshot below).

Screenshot 2024-10-07 120107

I think we can adapt this method for this intro repo to encourage folks to improve our website while preventing any direct changes.

I think once the user completes the changes, this is what they should see bc it's a protected branch. So this shouldn't be a problem:

image

@Codexnever
Copy link
Contributor Author

Hi @BekahHW ,

After reflecting on @adiati98 comments, I think we can improve the "Edit this page" functionality. Here’s my proposal:

  1. Redirect Users to Their Fork: For users who have already forked the repository, clicking "Edit this page" should take them directly to edit the README in their fork.
  2. Guide New Users to Fork: For users who haven’t forked yet, the link can redirect them to the main repo to encourage them to fork it before making any edits.

This way, we ensure that edits are made in the appropriate context while helping new contributors understand the process better. What are your thoughts on this approach?

@BekahHW
Copy link
Member

BekahHW commented Oct 8, 2024

@Codexnever I think it should automatically tell contributors that if we have the permissions set up properly. For example, when I try to edit a repo I don't have permission to directly in GH, this is what comes up:

image

Can you try to edit the Readme or one of the files in this project directly to see what happens?

@Codexnever
Copy link
Contributor Author

Hey @BekahHW,
Thanks for your feedback! I just tested the edit functionality by trying to make direct changes to the README, and it prompts the expected behavior, asking users to fork the repo if they don’t have direct permissions. I also reverted the unnecessary changes I made earlier, and you can check those in the latest commit.

Copy link
Member

@adiati98 adiati98 left a comment

Choose a reason for hiding this comment

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

Hey @Codexnever,
You haven't revert the changes in the package-lock.json. Can you please revert to the original state? Thanks.

Copy link
Member

@adiati98 adiati98 left a comment

Choose a reason for hiding this comment

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

Well done and thank you for your contribution, @Codexnever! ✨🚢

If you haven't, you can join our Community. 🍕

applause GIF

@adiati98 adiati98 requested a review from BekahHW October 9, 2024 18:02
@adiati98 adiati98 merged commit 8916859 into open-sauced:main Oct 10, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug : Broken "Edit the Page" Link on Intro to OSS Page
3 participants