Skip to content

fix(pages): add dedicated features route#496

Open
oozan wants to merge 1 commit into
alibaba:mainfrom
oozan:fix/features-navigation-1784910862
Open

fix(pages): add dedicated features route#496
oozan wants to merge 1 commit into
alibaba:mainfrom
oozan:fix/features-navigation-1784910862

Conversation

@oozan

@oozan oozan commented Jul 24, 2026

Copy link
Copy Markdown

Summary

  • add a dedicated /features route
  • make the Features navigation tab open that route
  • simplify active-tab detection now that every tab has a distinct path

Testing

  • npm run typecheck
  • npm run build

Closes #495

AI assistance was used to prepare the initial patch and automation. I reviewed the change and validated it with the repository frontend checks.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
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.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

🔍 OpenCodeReview found 1 issue(s) in this PR.

  • ✅ Successfully posted inline: 1 comment(s)

<div style={{ display: 'flex', alignItems: 'center', gap: 4 }}>
{navTabs.map((tab) => {
const isActive = tab.path === '/' ? currentPath === '/' : currentPath.startsWith(tab.path);
const isActive = currentPath.startsWith(tab.path);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Potential regression: When the user visits the root path /, no nav tab will be highlighted as active. Previously, the special-case logic (tab.path === '/' ? currentPath === '/' : ...) ensured the features tab was active on /. Since App.tsx still serves FeaturesPage at both / and /features, users landing on / will see no active tab indicator.

Consider adding back a check for the root path, e.g.:

const isActive = tab.path === '/features' ? (currentPath === '/' || currentPath.startsWith('/features')) : currentPath.startsWith(tab.path);

@di3go04

di3go04 commented Jul 24, 2026

Copy link
Copy Markdown

Hi @oozan,

Thanks for working on this — the dedicated /features route is exactly what issue #495 needs.

While reviewing your changes, I noticed a small regression: when visiting the home page (/), no nav tab is highlighted as active. This happens because the simplified check (currentPath.startsWith(tab.path)) doesn't match / against /features.

I've tested a fix locally that preserves your approach while addressing the regression.

Changes

1. App.tsx (line 24): add the /features route alongside the existing / route:

<Route path="/" element={} />
<Route path="/features" element={} />

2. Navbar.tsx (line 17): change the Features tab path:

{ path: '/features', labelKey: 'navbar.features' },

3. Navbar.tsx (line 83): update active detection:

const isActive = tab.path === '/features'
? (currentPath === '/' || currentPath.startsWith('/features'))
: currentPath.startsWith(tab.path);

This way the Features tab highlights on both / and /features, while the simplified startsWith logic is retained for all other tabs.

Would you like me to push these changes to your branch, or would you prefer to apply them yourself? If you're busy, I can open a separate PR building on yours.

Thanks again for the contribution!

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