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

Link-redirect-fix #1187

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

Conversation

Yashwanth1906
Copy link

What kind of change does this PR introduce?
Change to correct wrong redirecting URLs
Issue Number:
closes: #1038

Screenshots/videos:

If relevant, did you update the documentation?

Summary

This pull request addresses an issue related to incorrect URL redirection in the JSON Schema website. Users were being redirected to incorrect or unintended pages when clicking on certain links.

Changes Made:
Corrected Redirecting URLs: Updated the wrong redirecting URLs in various components to ensure they point to the correct destinations. This change enhances navigation and provides users with access to the intended content.

Refactored JSX Components: Adjusted the formatting of JSX components to comply with the project's style guide. Specifically, I ensured that there were proper spaces before closing brackets in the JSX tags, addressing the errors raised by the reviewers.

Code Consistency: Ensured consistency in the codebase by adhering to Prettier and ESLint rules, which included correcting formatting issues to enhance code readability and maintainability.

These changes improve user experience by ensuring that links function correctly, reducing frustration caused by misdirected navigation.

Does this PR introduce a breaking change?

No

@Yashwanth1906 Yashwanth1906 requested a review from a team as a code owner December 20, 2024 06:50
Copy link
Member

@DhairyaMajmudar DhairyaMajmudar left a comment

Choose a reason for hiding this comment

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

The lint and formating workflows are failing pls. fix them

Copy link

github-actions bot commented Dec 20, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
website ✅ Ready (View Log) Visit Preview 336c747

Copy link

codecov bot commented Dec 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (5114356) to head (336c747).

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1187   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          373       384   +11     
  Branches        94        99    +5     
=========================================
+ Hits           373       384   +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Yashwanth1906
Copy link
Author

@DhairyaMajmudar Fixed the linting issue

@Yashwanth1906
Copy link
Author

@DhairyaMajmudar Can u review this pr?

@benjagm benjagm requested a review from DarhkVoyd December 27, 2024 09:25
@benjagm benjagm mentioned this pull request Dec 27, 2024
@Yashwanth1906
Copy link
Author

@DhairyaMajmudar Resolved the merge conflict

Copy link
Member

@DarhkVoyd DarhkVoyd left a comment

Choose a reason for hiding this comment

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

Thank you so much for this good work. Almost there, I have suggested more accurate fileRenderType for some cases, and we have to do something about inconsistent behavior in case of submodule pages and better handle cases with custom markdown file names.

pages/learn/index.page.tsx Show resolved Hide resolved
@@ -29,7 +29,7 @@ export default function Content({
content: any;
}) {
const newTitle = 'Code of Conduct';

const fileRenderType = 'tsx';
Copy link
Member

Choose a reason for hiding this comment

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

We should probably redirect to the code of conduct instead of this tsx.

Copy link
Author

Choose a reason for hiding this comment

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

@DarhkVoyd Do I need to redirect to the code of conduct submodule github page?

Copy link
Author

Choose a reason for hiding this comment

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

@DarhkVoyd Resolved this issue

Copy link
Member

Choose a reason for hiding this comment

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

Hard coding these values inside fileRenderType doesn't seem like a good idea, instead let's allow this component to accept urls as strings for these cases where we can simply pass custom links.
So, instead of code_of_conduct we should instead be passing something like: https://github.com/json-schema-org/.github/blob/main/CODE_OF_CONDUCT.md

@@ -21,7 +21,7 @@ export async function getStaticProps() {

export default function ContentExample({ blocks }: { blocks: any[] }) {
const newTitle = 'Sponsors';

const fileRenderType = '_indexmd';
Copy link
Member

Choose a reason for hiding this comment

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

In case of Code of Conduct submodule, the changes point to tsx but here for sponsors submodule we are pointing to the _indexmd, this behavior is not consistent.

Copy link
Author

Choose a reason for hiding this comment

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

@DarhkVoyd Resolved the issue

Copy link
Member

Choose a reason for hiding this comment

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

Hard coding these values inside fileRenderType doesn't seem like a good idea, instead let's allow this component to accept urls as strings for these cases where we can simply pass custom links.
So, instead of sponsors we should instead be passing something like: https://github.com/json-schema-org/community/blob/main/programs/sponsors/sponsors.md

Copy link
Author

Choose a reason for hiding this comment

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

@DarhkVoyd Got it I'll work on it and make the changes asap.

pages/understanding-json-schema/reference/index.page.tsx Outdated Show resolved Hide resolved
@Yashwanth1906
Copy link
Author

@DarhkVoyd Done with the changes , Review the pr.

Copy link
Member

@DarhkVoyd DarhkVoyd left a comment

Choose a reason for hiding this comment

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

Hey, thanks for these changes. Let's modify the behavior of the DocsHelp component to accept manual/custom links too instead of hard coding certain redirects, this will make the component more flexible and also fixes,
#1114

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: Mis-linked page (Again)
3 participants