Skip to content

Conversation

@Raubzeug
Copy link
Contributor

@Raubzeug Raubzeug commented Dec 5, 2025

Greptile Overview

Greptile Summary

This PR standardizes cluster navigation behavior in the clusters table by removing conditional target="_blank" attributes from cluster links. Previously, cluster links would open in a new tab only when the cluster had a clusterDomain, creating inconsistent user experience. Now all cluster links (both main cluster name and versions bar) consistently open in the same tab. This change affects the ExternalLink components in the clusters table columns, removing the conditional target={row.clusterDomain ? '_blank' : undefined} logic and simplifying the navigation behavior to be more predictable for users navigating within the YDB monitoring interface.

Important Files Changed

Filename Score Overview
src/containers/Clusters/columns.tsx 5/5 Removes conditional target="_blank" from cluster links to standardize same-tab navigation

Confidence score: 4/5

  • This PR is safe to merge with minimal risk
  • Score reflects simple, straightforward UI behavior change with no breaking logic
  • No files require special attention

Sequence Diagram

sequenceDiagram
    participant User
    participant ClustersTable
    participant ClusterName
    participant calculateClusterPath
    participant Browser

    User->>ClustersTable: "clicks cluster name link"
    ClustersTable->>ClusterName: "render cluster name with link"
    ClusterName->>calculateClusterPath: "call calculateClusterPath(row)"
    calculateClusterPath-->>ClusterName: "return cluster path URL"
    ClusterName->>Browser: "navigate to cluster path in same tab"
    Browser-->>User: "display cluster page"

    User->>ClustersTable: "clicks cluster versions link"
    ClustersTable->>Versions: "render versions with link"
    Versions->>calculateClusterPath: "call calculateClusterPath(row, clusterTabsIds.versions)"
    calculateClusterPath-->>Versions: "return cluster versions path URL"
    Versions->>Browser: "navigate to cluster versions in same tab"
    Browser-->>User: "display cluster versions page"
Loading

CI Results

Test Status: ⚠️ FLAKY

📊 Full Report

Total Passed Failed Flaky Skipped
378 374 0 2 2
Test Changes Summary ⏭️2

⏭️ Skipped Tests (2)

  1. Scroll to row, get shareable link, navigate to URL and verify row is scrolled into view (tenant/diagnostics/tabs/queries.test.ts)
  2. Copy result button copies to clipboard (tenant/queryEditor/queryEditor.test.ts)

Bundle Size: ✅

Current: 62.34 MB | Main: 62.34 MB
Diff: 0.32 KB (-0.00%)

✅ Bundle size unchanged.

ℹ️ CI Information
  • Test recordings for failed tests are available in the full report.
  • Bundle size is measured for the entire 'dist' directory.
  • 📊 indicates links to detailed reports.
  • 🔺 indicates increase, 🔽 decrease, and ✅ no change in bundle size.

Copilot AI review requested due to automatic review settings December 5, 2025 08:54
@Raubzeug Raubzeug enabled auto-merge December 5, 2025 08:54
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the conditional target="_blank" attribute from cluster links in the clusters table, ensuring all cluster links (both internal and external) now open in the same tab instead of opening external cluster links in a new tab.

Key Changes

  • Removed conditional target prop from cluster name links that previously opened external clusters in a new tab
  • Removed conditional target prop from cluster versions links with the same behavior
  • Simplified the ExternalLink component usage by removing the conditional logic based on row.clusterDomain

<ExternalLink href={clusterPath} target={row.clusterDomain ? '_blank' : undefined}>
{row.title || row.name}
</ExternalLink>
<ExternalLink href={clusterPath}>{row.title || row.name}</ExternalLink>
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

When row.clusterDomain is present, calculateClusterPath() returns an absolute URL pointing to a different domain (see line 30 in utils.ts where clusterDomain is passed as the domain parameter). Opening external cluster links in the same tab will navigate the user away from the current clusters page, potentially losing their context and requiring them to use browser back/forward buttons to return.

Consider:

  1. If this is intentional, verify that this behavior aligns with user expectations
  2. Alternatively, consider using InternalLink component for internal cluster navigation and keeping target="_blank" for external domains to prevent navigation away from the clusters page

Copilot uses AI. Check for mistakes.
@Raubzeug Raubzeug added this pull request to the merge queue Dec 5, 2025
Merged via the queue into main with commit e5b243a Dec 5, 2025
17 of 19 checks passed
@Raubzeug Raubzeug deleted the path-cluster branch December 5, 2025 09:36
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.

3 participants