Skip to content

refactor: replace legacy programmatic routing with semantic Link components#311

Open
itvi-1234 wants to merge 1 commit into
kmesh-net:mainfrom
itvi-1234:fix/migrate-usehistory-to-usenavigate
Open

refactor: replace legacy programmatic routing with semantic Link components#311
itvi-1234 wants to merge 1 commit into
kmesh-net:mainfrom
itvi-1234:fix/migrate-usehistory-to-usenavigate

Conversation

@itvi-1234

Copy link
Copy Markdown

Description

This PR refactors the website's programmatic routing to use standard semantic <Link> components from @docusaurus/Link instead of useHistory or legacy <a> tags with custom onClick click-handlers. Aligned with Docusaurus Core Link Best Practices to leverage build-time broken link detection and smart client-side prefetching instead of fragile programmatic handlers

The Problems Solved:

  1. Accessibility (a11y) Violations: Custom <a> anchors used as buttons (e.g. "View All" buttons) lacked href attributes. These links were not focusable via keyboard (Tab navigation) and invisible to screen readers.
  2. SEO & Search Indexability: Crawlers like Googlebot only follow links containing real href attributes. The click handlers on the homepage blog title cards and navigation links were invisible to SEO indexing.
  3. Robust Version-Compatibility: Programmatic routing hooks are prone to version mismatch between React Router V5/V6 during Docusaurus updates (e.g. useHistory deprecation). Moving to declarative semantic <Link> wraps completely avoids runtime hook dependencies.

Changes:

  • src/components/slider/index.js:
    • Replaced custom <a onClick={() => history.push(...)}> button wrap with a proper <Link to={...}> component.
    • Added secure sandbox and deferred loading="lazy" attributes to the GitHub Star button Iframe to eliminate initial render blockage.
    • Resolved key={index} anti-pattern to stable strings.
  • src/components/Blogs/index.js:
    • Converted the "View All" legacy click-handler to a standard accessible <Link to="/blog">.
    • Refactored <h3> heading blog card elements so that the title string itself is safely wrapped inside a <Link> tag (restores native browser clickable behavior).
    • Added strict safe guards against potential missing/unresolved blog plugin data arrays to avoid runtime build breaks.
image

Copilot AI review requested due to automatic review settings May 18, 2026 04:12
@netlify

netlify Bot commented May 18, 2026

Copy link
Copy Markdown

Deploy Preview for kmesh-net ready!

Name Link
🔨 Latest commit 5a98947
🔍 Latest deploy log https://app.netlify.com/projects/kmesh-net/deploys/6a0a945f1695a600089f01e6
😎 Deploy Preview https://deploy-preview-311--kmesh-net.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 project configuration.

@kmesh-bot

Copy link
Copy Markdown
Collaborator

Welcome @itvi-1234! It looks like this is your first PR to kmesh-net/website 🎉

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request replaces useHistory with the Docusaurus Link component across the Blogs and Slider components, adds safety checks for blog data, and introduces security attributes to the GitHub Stars iframe. Review feedback identifies a bug in the slider's key generation logic that would cause duplicate keys and notes that the iframe's sandbox attribute requires additional permissions to allow the GitHub widget to function correctly.

Comment thread src/components/slider/index.js Outdated
Comment thread src/components/slider/index.js Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Replaces programmatic routing (useHistory) and bare <a onClick> handlers in the homepage slider and Blogs section with declarative @docusaurus/Link components, improves keyboard/SEO accessibility, hardens the GitHub-star iframe and external author links, and adds defensive handling for missing blog plugin data.

Changes:

  • Migrate useHistory().push(...) click handlers to <Link to={...}> in both slider and Blogs components, and wrap blog titles inside the <Link> so the title text itself is the clickable target.
  • Add loading="lazy" and sandbox="allow-scripts allow-same-origin" to the GitHub stars iframe, and add rel="noopener noreferrer" to external author links.
  • Replace key={index} with derived keys and guard against missing blogGlobalData.blogPosts (early return null when empty).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/components/slider/index.js Swaps useHistory for <Link>, hardens the GitHub stars iframe, and replaces index-based keys with a derived keyString.
src/components/Blogs/index.js Swaps useHistory for <Link> (wrapping titles), adds null-safety around plugin data, and adds rel="noopener noreferrer" to external author links.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/slider/index.js Outdated
Comment thread src/components/Blogs/index.js Outdated
Comment thread src/components/Blogs/index.js Outdated
@itvi-1234 itvi-1234 force-pushed the fix/migrate-usehistory-to-usenavigate branch from ad3231d to 8ba5440 Compare May 18, 2026 04:18
@itvi-1234 itvi-1234 requested a review from Copilot May 18, 2026 04:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread src/components/Blogs/index.js Outdated
Comment thread src/components/slider/index.js Outdated
…onents

Signed-off-by: itvi-1234 <rjsumit71@gmail.com>
@itvi-1234 itvi-1234 force-pushed the fix/migrate-usehistory-to-usenavigate branch from 8ba5440 to 5a98947 Compare May 18, 2026 04:23
@itvi-1234 itvi-1234 requested a review from Copilot May 18, 2026 04:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@itvi-1234

itvi-1234 commented May 18, 2026

Copy link
Copy Markdown
Author

/cc @yashisrani
/cc @LiZhenCheng9527
/cc @jayesh9747

@kmesh-bot kmesh-bot requested a review from yashisrani May 18, 2026 04:28
@kmesh-bot

Copy link
Copy Markdown
Collaborator

@itvi-1234: GitHub didn't allow me to request PR reviews from the following users: Jayesh0167.

Note that only kmesh-net members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @yashisrani
/cc @LiZhenCheng9527
/cc @Jayesh0167

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kmesh-bot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yashisrani
Once this PR has been reviewed and has the lgtm label, please assign lizhencheng9527 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@yashisrani

Copy link
Copy Markdown
Contributor

/lgtm

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.

4 participants