Skip to content

Conversation

@VahantSharma
Copy link
Contributor

Note: This PR replaces #3263 (closed due to branch naming issue causing CI failure).

Summary

This PR prevents accidental row navigation in AppTable and VariantsTable when users interact with checkboxes, dropdown menus, and other interactive elements inside table rows.

It applies the shared shouldIgnoreRowClick guard to these tables to ensure row-level navigation only triggers when the user intentionally clicks the row body.

Context

Several tables in the UI implement row-level navigation (onRow / onClick) alongside interactive child elements such as checkboxes and action menus.

In AppTable and VariantsTable, this resulted in the UX issue described in #3254: clicks near or on interactive elements could unintentionally trigger row navigation.

Both components already use stopPropagation() defensively on individual actions; however, this PR adds a row-level guard as a final safety net, following the same pattern previously used in InfiniteVirtualTable.

What changed

AppTable

  • Guarded the row click handler using shouldIgnoreRowClick
  • Prevents accidental navigation when interacting with the dropdown menu or nearby areas

VariantsTable

  • Guarded the row click handler using shouldIgnoreRowClick
  • Prevents conflicts between row navigation and:
    • Selection checkboxes
    • Dropdown action menus
    • Commit notes tooltip interactions
  • Applies consistently across all contexts where VariantsTable is used (overview page, deployment modals, evaluation configuration)

No other logic or behavior was changed.

Why this approach

  • Keeps the fix small, targeted, and low-risk
  • Preserves existing navigation behavior while eliminating accidental triggers
  • Aligns with existing defensive event handling (stopPropagation) already present in these components

Testing

Manually verified the following scenarios:

AppTable

  • ✅ Clicking the row body navigates as before
  • ✅ Clicking the dropdown menu opens the menu without navigation
  • ✅ Clicking near the dropdown does not trigger navigation

VariantsTable

  • ✅ Clicking checkboxes toggles selection only
  • ✅ Clicking dropdown menus opens menus without triggering row actions
  • ✅ Clicking commit notes tooltip behaves correctly
  • ✅ Clicking empty row areas still triggers the expected row behavior
  • ✅ Verified behavior in:
    • Overview / Variants page
    • Deployment modals
    • Evaluation configuration flow

Related


Changes since #3263: Branch renamed from ui/fix-row-click-app-variants to fix-row-click-tables to resolve GitHub Actions checkout refspec error.

- Add shouldIgnoreRowClick guard to AppTable row click handler
- Add shouldIgnoreRowClick guard to VariantsTable row click handler
- Fixes issue where clicking checkboxes or dropdown menus triggers row navigation
- Affects app management page and all variant table contexts (overview, deployment, evaluation)

Builds on shared utility from PR Agenta-AI#1
Copilot AI review requested due to automatic review settings January 1, 2026 00:57
@vercel
Copy link

vercel bot commented Jan 1, 2026

@VahantSharma is attempting to deploy a commit to the agenta projects Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jan 1, 2026
@dosubot dosubot bot added UI UX labels Jan 1, 2026
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 prevents accidental row navigation in AppTable and VariantsTable by extracting a shared shouldIgnoreRowClick helper function that guards against clicks on interactive elements within table rows.

  • Extracts the shouldIgnoreRowClick helper from InfiniteVirtualTable to a shared utility module
  • Applies the helper to AppTable and VariantsTable row click handlers
  • Ensures row navigation only triggers on intentional row body clicks, not on interactive elements

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
web/oss/src/lib/tableRowClick.ts New shared utility function with comprehensive JSDoc that checks if a click originated from interactive elements (buttons, links, checkboxes, dropdowns, inputs)
web/oss/src/components/pages/app-management/components/AppTable.tsx Applies shouldIgnoreRowClick guard to prevent accidental navigation when clicking dropdown menus
web/oss/src/components/VariantsComponents/Table/index.tsx Applies shouldIgnoreRowClick guard to prevent conflicts between row navigation and checkboxes/dropdowns/tooltips
web/oss/src/components/InfiniteVirtualTable/hooks/useTableManager.tsx Refactored to import shouldIgnoreRowClick from the new shared location instead of defining it locally

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@VahantSharma
Copy link
Contributor Author

VahantSharma commented Jan 1, 2026

@mmabrouk This PR replaces #3263 (closed due to branch naming issue causing CI failure).

@mmabrouk mmabrouk requested a review from ardaerzin January 2, 2026 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files. UI UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant