Skip to content

Tabs: let <Tab href> link tabs carry Next navigation props (scroll/replace/prefetch) #133

@hb-agent

Description

@hb-agent

Context

The <Tabs> primitive (src/components/ui/tabs.tsx) supports rendering a tab as a real Next <Link> via <Tab href> — with full role="tab", aria-selected, roving tabIndex, and arrow/Home/End keyboard nav. This is the right shape for URL-router tab strips (middle-click / open-in-new-tab works, the tab is a real anchor).

But TabProps only extends React.ButtonHTMLAttributes<HTMLButtonElement>, so it does not type-accept Next <Link>'s navigation props (scroll, replace, prefetch). In the link path the extra props are even cast to AnchorHTMLAttributes, so those props can't flow through to <Link>. The in-page query-param tab strips need replace + scroll={false} semantics, so PR #130 migrated them to button tabs whose onChange calls router.replace/push(href, { scroll: false }) instead of using real link tabs.

As a result, every migrated strip already computes an href but throws away the anchor benefits.

Files involved

  • src/components/ui/tabs.tsxTabProps (extends ButtonHTMLAttributes only); Tab link branch (the isLink <Link> render that spreads props as AnchorHTMLAttributes).
  • src/components/layout/desktop-top-bar.tsx — three button-tab strips that already build an href but use onChange + router.replace/push(..., { scroll: false }):
    • Explore ?kind= strip — router.replace('/explore?…', { scroll: false })
    • Profile ?tab= strip — router.push(tabHref(tab), { scroll: false }) (tabHref helper, +line ~326)
    • Cert/Project detail strip — router.replace(hrefFor(t), { scroll: false }) (hrefFor helper)

These are the canonical consumers; if other migrations followed the same pattern they should be covered too.

Proposed approach

Widen the link-tab surface so an href tab can forward Next navigation props, without changing the button-tab path:

  • Add an optional linkProps?: Pick<LinkProps, "scroll" | "replace" | "prefetch"> (import LinkProps from next/link) to TabProps, applied only when isLink. A scoped linkProps keeps the button-tab type surface unchanged and avoids leaking anchor-only props onto button tabs.
  • In the isLink branch, spread {...linkProps} onto <Link> (before/after the existing prop spread as appropriate) so scroll={false} replace reach the router. Keep the existing onClick that calls ctx.onChange(value) so controlled value stays in sync.
  • Migrate at least one desktop-top-bar.tsx strip (e.g. the detail strip, which already has hrefFor) to <Tab href={hrefFor(t)} linkProps={{ replace: true, scroll: false }}>, dropping the onChange→router workaround, as the reference pattern.

Acceptance criteria

  • A <Tab href> can render <Link href replace scroll={false}> (and optionally prefetch) and these reach Next's router on activation and on middle-click/open-in-new-tab.
  • The rendered link tab keeps role="tab", aria-selected, aria-controls, roving tabIndex, and arrow/Home/End keyboard navigation (no regression vs. current link path).
  • Button tabs are unaffected: their type surface does not gain anchor-only props, and existing button-tab call sites compile and behave unchanged.
  • At least one desktop-top-bar.tsx query-param strip is migrated to real link tabs with replace + scroll={false} as a worked example.
  • npx tsc --noEmit is clean and npm run lint shows no new warnings vs. baseline; existing tabs tests still pass (add a test asserting an href tab forwards replace/scroll).

Auto-filed follow-up from the component-library completion (Draft PR #130). The library itself is gate-green; these are tracked enhancements/cleanups, not blockers.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions