Prioritize Internal SPA Navigation for Research Tools

[google-labs-jules]

Problem Statement

The current implementation of the ResearchAnalytics dashboard components does not prioritize internal Single Page Application (SPA) navigation over external links, leading to potential confusion for users.

Goal

Refactor the ResearchAnalytics dashboard components to ensure that card clicks prioritize internal navigation while still providing access to external links when necessary.

Non-Goals

None.

Proposed Approach

1. ToolCard Refactoring:

   • Replace the card-level <a> tag with an <article> container using a stretched NavLink overlay to prioritize internal navigation.
   • Add a secondary GitHub icon link (with a "Source" label for clarity) that opens in a new tab without triggering the card's main action.

2. FlagshipCard Refactoring:

   • Update the button logic to always show a primary "View Assets" button pointing to the tool's canonicalPath.
   • Render external live links and source repository links as secondary action buttons.

3. Cleanup:

   • Remove the unused navigate prop from ToolCard and its call sites.

Alternatives Considered

None.

Architectural Impact

These modifications will ensure a consistent user experience where research tools link to their internal detail pages or live assets by default while providing easy access to their source code on GitHub.

Scope

The changes will affect the ToolCard and FlagshipCard components within the ResearchAnalytics dashboard.

UNDERSTAND THE ISSUE

The issue is that the current dashboard components do not effectively guide users to navigate within the application, leading to a less intuitive user experience.

DETERMINE APPROACH

The proposed approach focuses on maximizing internal navigation while still offering access to external resources, thereby improving user experience and clarity.

SPECIFY SCOPE

The refactoring will only involve the ToolCard and FlagshipCard components, specifically addressing their navigation and linking behavior.

DEFINITION OF DONE

• The ToolCard and FlagshipCard components are refactored as described.
• Internal navigation is prioritized over external links.
• Secondary links are clearly labeled and open in a new tab.
• The unused navigate prop is removed from ToolCard.
• All relevant tests are updated or created to ensure the new functionality works as intended.

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[github-actions]

🚀 Deployment Details (Last updated: Jun 19, 2026, 6:45 PM PST)

🚀 Pushed to gh-pages; publish in progress

• Preview URL: https://arii.github.io/tech-dancer/fix/research-routing-13197735050478246489/
• Preview Dashboard: https://arii.github.io/tech-dancer/previews/index.html
• Commit SHA: f30dd64
• Marker File: verify here

[github-actions]

🐙 GitHub Models Code Review

Powered by GitHub Models

Reviewing: PR #2654

Model: gpt-4.1

Code Review Feedback

High Severity Review

1. Blocking Bug: Removal of navigate Prop Without Updating Navigation Logic

Evidence

• In the previous implementation of ToolCard, navigation was handled via the navigate prop:

  function ToolCard({ tool, navigate }: {
    tool: ResearchTool;
    navigate: (path: string) => void;
  }) {
    ...
    const handleClick = (e: React.MouseEvent<HTMLAnchorElement>) => {
      if (!isLink) {
        e.preventDefault();
        navigate(href);
      }
    };
    ...
    <Stack
      as="a"
      href={href}
      target={isLink ? "_blank" : undefined}
      rel={isLink ? "noopener noreferrer" : undefined}
      onClick={handleClick}
      ...
    >
    ...
  }

• In the new implementation, the navigate prop is removed:

  function ToolCard({ tool }: {
    tool: ResearchTool;
  }) {
    ...
    <BaseCard
      key={tool.id}
      to={internalPath}
      ariaLabel={`View details for ${tool.title}`}
      height="full"
      padding={0}
      className="group/card"
    >
    ...
  }

• The call sites in ResearchAnalytics are updated to remove the navigate prop:

  {engineeringTools.map((tool) => (
    <ToolCard key={tool.id} tool={tool} />
  ))}

Problem

Blocking bug: The new implementation assumes that BaseCard supports SPA navigation via a to prop (presumably using React Router's <NavLink> or similar). However, the diff does not show the implementation of BaseCard. If BaseCard does not support SPA navigation via the to prop, or if it does not render a stretched overlay link, then clicking the card will not trigger internal navigation as intended. This is a concrete contradiction: the navigation logic is removed from ToolCard, but the new navigation mechanism is not shown or guaranteed to work.

If BaseCard does not support SPA navigation via to, this will break navigation for all tool cards.

Blocking Severity Justification

• The diff removes the only navigation logic from ToolCard and its call sites.
• The new navigation mechanism is not shown in the diff.
• If BaseCard does not implement SPA navigation, clicking a card will not navigate anywhere.
• This is a direct contradiction in the diff: navigation logic is removed, but not replaced with a proven working alternative.

---

2. Blocking Bug: Missing Required Prop Types

Evidence

• The ToolCard and FlagshipCard components are typed as:

  function ToolCard({ tool }: {
    tool: ResearchTool;
  }) { ... }

  function FlagshipCard({
    tool,
    baseUrl,
    onImageClick
  }: {
    tool: ResearchTool;
    baseUrl: string;
    onImageClick?: (src: string) => void;
  }) { ... }

• The ResearchTool type is not shown in the diff, but the code assumes properties like tool.canonicalPath, tool.id, tool.sourceUrl, tool.tags, etc.

Problem

Blocking bug: If ResearchTool does not include all the properties used in the code (e.g., canonicalPath, sourceUrl, tags, subtitle, description, category, status), TypeScript will throw errors at compile time. This is a concrete contradiction: the code uses properties that may not exist on the type.

If ResearchTool is missing any of these properties, the code will not compile.

Blocking Severity Justification

• The code uses properties on tool that are not guaranteed to exist.
• If the type definition is missing these properties, this is a blocking type error.
• This is a direct contradiction: the code uses properties that may not exist.

---

3. Blocking Bug: onClick={handleSourceClick} Does Not Prevent Default on External Links

Evidence

• The handleSourceClick handler is:

  const handleSourceClick = (e: React.MouseEvent) => {
    e.stopPropagation();
  };

• It is used on external links:

  <Box
    as="a"
    href={tool.sourceUrl}
    target="_blank"
    rel="noopener noreferrer"
    onClick={handleSourceClick}
    ...
  >
    ...
  </Box>

Problem

Blocking bug: The handler only calls e.stopPropagation(), but does not call e.preventDefault(). If the link is rendered as <a>, clicking it will still open the link in a new tab, but if the parent card is a <NavLink> overlay, the click may also trigger the internal navigation, depending on event bubbling and how the overlay is implemented. The intention is to allow the external link to open in a new tab without triggering the card's main action. If preventDefault is not called, the parent navigation may still occur.

If the overlay link is implemented as a stretched <NavLink>, the click may bubble and trigger internal navigation, causing both internal and external navigation.

Blocking Severity Justification

• The handler does not prevent default browser behavior.
• This can cause both internal and external navigation to occur.
• This is a direct contradiction: the handler is intended to prevent parent navigation, but does not do so reliably.

---

Summary Table

Issue ID	File	Line	Snippet	Issue	Status	Fix Summary
finding-1	src/features/research/ResearchAnalytics.tsx	ToolCard, FlagshipCard	<ToolCard key={tool.id} tool={tool} />	Navigation logic removed, not replaced with proven SPA navigation	unresolved	Ensure BaseCard implements SPA navigation via to prop and renders a stretched overlay link
finding-2	src/features/research/ResearchAnalytics.tsx	ToolCard, FlagshipCard	tool.canonicalPath, tool.tags, etc.	Missing required prop types for ResearchTool	unresolved	Update ResearchTool type to include all used properties
finding-3	src/features/research/ResearchAnalytics.tsx	handleSourceClick	onClick={handleSourceClick}	Does not call preventDefault, may trigger both internal and external navigation	unresolved	Update handler to call e.preventDefault() as well as e.stopPropagation()

---

Final Verdict

[VERDICT: FAIL]

---

{ "findings": [ { "id": "finding-1", "file": "src/features/research/ResearchAnalytics.tsx", "line": "ToolCard, FlagshipCard", "snippet": "", "issue": "Navigation logic removed, not replaced with proven SPA navigation", "status": "unresolved", "fixSummary": "Ensure BaseCard implements SPA navigation via to prop and renders a stretched overlay link" }, { "id": "finding-2", "file": "src/features/research/ResearchAnalytics.tsx", "line": "ToolCard, FlagshipCard", "snippet": "tool.canonicalPath, tool.tags, etc.", "issue": "Missing required prop types for ResearchTool", "status": "unresolved", "fixSummary": "Update ResearchTool type to include all used properties" }, { "id": "finding-3", "file": "src/features/research/ResearchAnalytics.tsx", "line": "handleSourceClick", "snippet": "onClick={handleSourceClick}", "issue": "Does not call preventDefault, may trigger both internal and external navigation", "status": "unresolved", "fixSummary": "Update handler to call e.preventDefault() as well as e.stopPropagation()" } ] }

---

Generated by github-models-code-review

[github-actions]

🐙 GitHub Models Visual Review

Powered by GitHub Models Vision + Blast-Radius Analyzer

Summary: 🔴 1 high · 🟡 1 medium · 🟢 0 low
Reviewing: PR #2654

🔴 /research (mobile)

Pixel diff: 12.91%

• ✅ INTENTIONAL: The previous round’s issues (layout shifts, broken spacing, clipping/overlap, and footer problems) appear to be resolved. The AFTER screenshot shows consistent card spacing, no visible clipping, and a properly rendered footer.
• No new unrelated issues observed. All major red regions in the DIFF correspond to card content and layout adjustments, but spacing and alignment are now visually consistent.
• Visual quality is acceptable, but some cards and sections still look dense and could benefit from further refinement.

Recommendations for Improvement:

• Consider increasing vertical spacing between cards by 4–8px for improved readability and touch accessibility.
• Evaluate the footer’s padding and contrast; ensure it is visually distinct and not cramped, especially on smaller screens.

---

{ "findings": [ { "id": "finding-1", "route": "/research (mobile)", "issue": "Major visual regressions: layout shifts, broken spacing, possible clipping/overlap, and footer issues.", "status": "resolved", "fixSummary": "Spacing, clipping, and footer rendering issues have been addressed; layout is now consistent and visually stable." } ] }

---

🟡 /research

Pixel diff: 3.71%

• ✅ INTENTIONAL: The visual diff shows significant changes to card layouts, section spacing, and footer alignment, matching the previous review round’s findings.
• The spacing between cards, sections, and footer is now consistent and visually balanced. No obvious clipping or broken layouts are present.
• Button placement and alignment have been normalized across cards and sections.
• No new layout, spacing, or accessibility regressions are visible. The previous issue appears fully resolved.

Recommendations for Improvement:

• Consider increasing vertical spacing between section headers and their content by 4px for improved visual separation.
• Review button contrast and hover states for accessibility; ensure all buttons meet WCAG AA contrast requirements.

---

{ "findings": [ { "id": "finding-1", "route": "/research", "issue": "Significant layout and spacing changes across cards, sections, and footer; inconsistent spacing and potential clipping.", "status": "resolved", "fixSummary": "Spacing and layout have been normalized across cards, sections, and footer. No clipping or broken layouts observed." } ] }

---

Generated by impact-github-models-review — Blast-Radius Analyzer

[arii]

🤖 AI Technical Audit

ANTI-AI-SLOP

• The code changes correctly simplify component interfaces by removing the redundant navigate prop.
• The use of BaseCard with an internal path is a significant architectural improvement over manual <a> tag management.
• Prop drilling was correctly identified and mitigated.
• I observed no significant over-engineering; the logic remains concise.

FINAL RECOMMENDATION

Approved

Review automatically published via RepoAuditor.

[arii]

🤖 AI Technical Audit

ANTI-AI-SLOP

• Replaced manual onClick navigation logic with declarative BaseCard components, significantly reducing cyclomatic complexity.
• Standardized event handling for nested links, improving robustness.
• Removed unused navigate prop, reducing component prop drilling.
• Unified spacing and layout constants across FlagshipCard and ToolCard.

FINAL RECOMMENDATION

Approved

Review automatically published via RepoAuditor.