Skip to content

fix(pages): add dedicated /features route with correct active-tab highlighting - #548

Merged
lizhengfeng101 merged 3 commits into
alibaba:mainfrom
varunsahni18:fix/issue-495-features-nav-link
Jul 28, 2026
Merged

fix(pages): add dedicated /features route with correct active-tab highlighting#548
lizhengfeng101 merged 3 commits into
alibaba:mainfrom
varunsahni18:fix/issue-495-features-nav-link

Conversation

@varunsahni18

Copy link
Copy Markdown
Contributor

Summary

Fixes #495

The Features nav link previously navigated to / which was indistinguishable from the home page. This adds a dedicated /features route and updates the navbar to use it, with correct active-tab highlighting for both / and /features.

Changes

  • pages/src/App.tsx: Added /features route pointing to FeaturesPage
  • pages/src/components/Navbar.tsx: Changed Features nav path from / to /features
  • pages/src/components/Navbar.tsx: Updated active tab logic so Features is highlighted on both / and /features (the home page IS the features page)

Why this over existing PRs

This PR is the minimal correct fix: 2 files, 5 lines added, 2 removed, and the Features tab stays highlighted on the home page.

Testing

  • npm run typecheck: passes with zero errors
  • npm run build: compiles successfully

Closes #495

@CLAassistant

CLAassistant commented Jul 28, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions

Copy link
Copy Markdown
Contributor

OpenCodeReview: No comments generated. Looks good to me.

@varunsahni18

Copy link
Copy Markdown
Contributor Author

How this compares to the existing PRs

Two other PRs are open for this issue:

PR #496 (oozan) takes the same approach (dedicated /features route) but has a bug in the active-tab logic. Their change simplifies the check to currentPath.startsWith(tab.path) for all tabs, which means the Features tab is not highlighted when the user is on / (the home page). Since the home page IS the features page, this is a regression: the active tab indicator disappears when you land on or navigate to /.

PR #537 (Sunil56224972) uses a hash-based scroll approach (/#features) with 48 additions and 8 deletions across 2 files. It adds useCallback, scroll-to-anchor logic, and a wrapper <div id="features"> in FeaturesPage. This is over-engineered for what should be a simple routing fix. It also introduces scroll behavior that may conflict with the existing ScrollToTop component and the FadeInSection animations.

This PR (#548) is the minimal correct fix:

  • 2 files, 5 lines added, 2 removed
  • Adds a dedicated /features route (same as fix(pages): add dedicated features route #496)
  • Correctly handles active-tab highlighting: the Features tab is highlighted on both / and /features because the home page IS the features page
  • No scroll hacks, no new dependencies, no wrapper divs
  • npm run typecheck and npm run build both pass cleanly

The Features nav link previously navigated to '/' which was
indistinguishable from the home page. This adds a dedicated
/features route and updates the navbar to use it.

Closes alibaba#495

- pages/src/App.tsx: Added /features route pointing to FeaturesPage
- pages/src/components/Navbar.tsx: Changed Features nav path from '/' to '/features'
- pages/src/components/Navbar.tsx: Updated active tab logic to highlight Features for both '/' and '/features'

- Build passes: npm run build compiles successfully
- Active tab highlighting: Features tab is active on both '/' and '/features'
- All other nav links unchanged and continue to work
@varunsahni18
varunsahni18 force-pushed the fix/issue-495-features-nav-link branch from b60c70f to 2c9d39f Compare July 28, 2026 05:00
@lizhengfeng101

Copy link
Copy Markdown
Collaborator

Review Feedback

The approach of adding a dedicated /features route is correct, but the current implementation has an issue: the /features route renders the full FeaturesPage component (Hero + Highlights + UseCases + FeaturesSection + Benchmark + QuickStart + Footer), which is identical to the home page /.

This means clicking "Features" in the navbar still takes the user to the top of the landing page (HeroSection), rather than showing the features content directly — the same UX problem the issue describes.

How /benchmark works for comparison

/benchmark has its own dedicated page component (BenchmarkPage.tsx) that renders only BenchmarkSection + Footer with a top padding, so users see the benchmark content immediately:

// pages/src/pages/BenchmarkPage.tsx
const BenchmarkPage: React.FC = () => {
  return (
    <div style={{ paddingTop: 72 }}>
      <FadeInSection><BenchmarkSection /></FadeInSection>
      <FadeInSection><Footer /></FadeInSection>
    </div>
  );
};

Suggested fix

To be consistent with /benchmark, /features should have its own page component that renders FeaturesSection + Footer directly, e.g.:

// pages/src/pages/FeaturesRoutePage.tsx (or similar name)
const FeaturesRoutePage: React.FC = () => {
  return (
    <div style={{ paddingTop: 72 }}>
      <FadeInSection><FeaturesSection /></FadeInSection>
      <FadeInSection><Footer /></FadeInSection>
    </div>
  );
};

Then use this component in the /features route instead of the full FeaturesPage.

The navbar active-tab logic change looks correct and can stay as-is.

The /features route now renders FeaturesSection + Footer directly
(instead of the full landing page with Hero), matching the pattern
used by /benchmark. This means clicking 'Features' in the navbar
shows the features content immediately rather than the Hero section.

- pages/src/pages/FeaturesRoutePage.tsx: New page component following
  the BenchmarkPage pattern (FeaturesSection + Footer with paddingTop)
- pages/src/App.tsx: /features route now uses FeaturesRoutePage
@varunsahni18

Copy link
Copy Markdown
Contributor Author

Good catch, that is a fair point. Fixed in ef7e2de.

Created FeaturesRoutePage.tsx following the same pattern as BenchmarkPage: renders FeaturesSection + Footer directly with paddingTop: 72, so clicking "Features" in the navbar now shows the features content immediately instead of the Hero section.

// pages/src/pages/FeaturesRoutePage.tsx
const FeaturesRoutePage: React.FC = () => {
  return (
    <div style={{ paddingTop: 72 }}>
      <FadeInSection><FeaturesSection /></FadeInSection>
      <FadeInSection><Footer /></FadeInSection>
    </div>
  );
};

The home page (/) still renders the full FeaturesPage (Hero + Highlights + UseCases + FeaturesSection + Benchmark + QuickStart + Footer) as before. The /features route now shows just the features content, consistent with how /benchmark works.

npm run typecheck and npm run build both pass cleanly.

@lizhengfeng101

Copy link
Copy Markdown
Collaborator

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

@varunsahni18

varunsahni18 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Hi @lizhengfeng101
I've already signed the CLA (confirmed at cla-assistant.io), but the status check is stuck in "waiting" and hasn't reported back despite pushing new commits and requesting a recheck. This looks like a CLA Assistant webhook issue. Could you trigger a manual recheck from the CLA Assistant dashboard, or override the check on your end?

image

@lizhengfeng101

Copy link
Copy Markdown
Collaborator

@varunsahni18 The CLA check is failing because your commits are authored by a different GitHub account than the one that opened this PR.

  • PR opened by: varunsahni18
  • Commit author (linked via varunsahni18@gmail.com): VarunOfficialAcc

CLA Assistant checks the commit author's GitHub account, so even though varunsahni18 has signed the CLA, the commits are attributed to VarunOfficialAcc, which hasn't signed it.

To fix this, you can either:

  1. Sign the CLA with the VarunOfficialAcc account as well, or
  2. Update your git config to use an email associated with the varunsahni18 account and amend/rebase your commits

@varunsahni18
varunsahni18 force-pushed the fix/issue-495-features-nav-link branch from 8de5f14 to 4a775d6 Compare July 28, 2026 11:27
@varunsahni18

Copy link
Copy Markdown
Contributor Author

Thanks @lizhengfeng101
Reauthored them to use the email associated with varunsahni18 and the CLA check is passing now.

@lizhengfeng101 lizhengfeng101 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@lizhengfeng101
lizhengfeng101 merged commit f93d1a4 into alibaba:main Jul 28, 2026
8 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.

fix(pages): Features nav link navigates to "/" making it indistinguishable from Home

3 participants