Skip to content

fix(frontend): unify shouldIgnoreRowClick to shared utility in useTableManager#4391

Draft
Quantum0uasar wants to merge 6 commits into
Agenta-AI:release/v0.100.2from
Quantum0uasar:fix/unify-shouldIgnoreRowClick-utility
Draft

fix(frontend): unify shouldIgnoreRowClick to shared utility in useTableManager#4391
Quantum0uasar wants to merge 6 commits into
Agenta-AI:release/v0.100.2from
Quantum0uasar:fix/unify-shouldIgnoreRowClick-utility

Conversation

@Quantum0uasar
Copy link
Copy Markdown

Summary

Closes #3254

This PR removes the duplicate shouldIgnoreRowClick helper from EvaluationRunsTablePOC/actions/navigationActions.ts and updates the import in EvaluationRunsTable/index.tsx to use the canonical shared utility already defined in InfiniteVirtualTable/hooks/useTableManager.tsx.

Changes

  • EvaluationRunsTable/index.tsx: Updated import of shouldIgnoreRowClick from ../../actions/navigationActions../../../../InfiniteVirtualTable/hooks/useTableManager
  • navigationActions.ts: Removed the duplicate shouldIgnoreRowClick function (lines 26–32) and the now-unused import type {MouseEvent} from "react" on line 1

Why

shouldIgnoreRowClick was defined in two places with slightly different implementations. The canonical version in useTableManager.tsx is more thorough (handles Ant Design-specific selectors like .ant-dropdown-trigger, .ant-checkbox-wrapper, .ant-select). All tables should use the one shared utility to stay consistent.

Testing

  • Verified the import path resolves correctly relative to the file location
  • No logic was changed — only the import source was updated
  • The canonical implementation in useTableManager.tsx is unchanged

@vercel
Copy link
Copy Markdown

vercel Bot commented May 20, 2026

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

A member of the Team first needs to authorize it.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 20, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

Review Change Stack

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 14c202cb-5a30-4a46-9e18-d1d778585837

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR consolidates the shouldIgnoreRowClick row click handler utility by moving it from a local navigationActions module to a shared utility location in InfiniteVirtualTable/hooks/useTableManager. The import in EvaluationRunsTable is updated to reference the new location, removing the local dependency on navigationActions.

Changes

Row click handler consolidation

Layer / File(s) Summary
Consolidate row click handler to shared utility
web/oss/src/components/EvaluationRunsTablePOC/actions/navigationActions.ts, web/oss/src/components/EvaluationRunsTablePOC/components/EvaluationRunsTable/index.tsx
The shouldIgnoreRowClick function and its MouseEvent import are removed from navigationActions.ts, and the table component is updated to import the function from the centralized InfiniteVirtualTable/hooks/useTableManager module instead.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: moving a duplicate shouldIgnoreRowClick helper to use a shared utility.
Description check ✅ Passed The description is well-detailed, explains the rationale for consolidation, and references the linked issue #3254.
Linked Issues check ✅ Passed The PR successfully addresses issue #3254 by consolidating duplicate click-handling logic onto the shared utility, ensuring consistent behavior across tables.
Out of Scope Changes check ✅ Passed All changes are directly related to consolidating the shouldIgnoreRowClick helper; no unrelated modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Quantum0uasar Quantum0uasar marked this pull request as ready for review May 21, 2026 03:17
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. Frontend refactoring A code change that neither fixes a bug nor adds a feature labels May 21, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 766f6668-bb60-4914-96cc-05d28e8b3a80

📥 Commits

Reviewing files that changed from the base of the PR and between 5eef689 and cd25bce.

📒 Files selected for processing (2)
  • web/oss/src/components/EvaluationRunsTablePOC/actions/navigationActions.ts
  • web/oss/src/components/EvaluationRunsTablePOC/components/EvaluationRunsTable/index.tsx
💤 Files with no reviewable changes (1)
  • web/oss/src/components/EvaluationRunsTablePOC/actions/navigationActions.ts

import {useQueryParamState} from "@/oss/state/appState"

import {shouldIgnoreRowClick} from "../../actions/navigationActions"
import {shouldIgnoreRowClick} from "../../../../InfiniteVirtualTable/hooks/useTableManager"
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 21, 2026

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use @/oss/* alias instead of a deep relative import.

This import should use the OSS alias to avoid brittle relative traversal and to stay consistent with module-boundary rules.

♻️ Proposed fix
-import {shouldIgnoreRowClick} from "../../../../InfiniteVirtualTable/hooks/useTableManager"
+import {shouldIgnoreRowClick} from "`@/oss/components/InfiniteVirtualTable/hooks/useTableManager`"

As per coding guidelines, "Use @/oss/* path alias for shared utilities, helpers, types, hooks, and state that work the same in both EE and OSS" and "Never use relative paths for cross-package imports; use explicit path aliases instead".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import {shouldIgnoreRowClick} from "../../../../InfiniteVirtualTable/hooks/useTableManager"
import {shouldIgnoreRowClick} from "`@/oss/components/InfiniteVirtualTable/hooks/useTableManager`"

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.

@Quantum0uasar let's tackle this feedback 🙏

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

@junaway junaway changed the base branch from main to release/v0.100.1 May 21, 2026 06:10
Copy link
Copy Markdown
Contributor

@ardaerzin ardaerzin left a comment

Choose a reason for hiding this comment

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

one small change is required to improve import paths in the EvaluationRunsTable/index.tsx file changes

@ardaerzin ardaerzin self-requested a review May 25, 2026 08:31
@ardaerzin ardaerzin changed the base branch from release/v0.100.1 to release/v0.100.2 May 25, 2026 08:31
Copy link
Copy Markdown
Contributor

@ardaerzin ardaerzin left a comment

Choose a reason for hiding this comment

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

thanks for the PR @Quantum0uasar 👍

@Quantum0uasar
Copy link
Copy Markdown
Author

Hey maintainers 👋 — the CI failures on this PR are not caused by any code issues.

All 6 build jobs are failing with:

denied: installation not allowed to Write organization package

This is because the PR comes from a fork (Quantum0uasar/agenta), and GitHub restricts the GITHUB_TOKEN for fork PRs to read-only access — so the workflow cannot push Docker images to ghcr.io/agenta-ai/.

The actual code change in this PR (removing the duplicate shouldIgnoreRowClick helper and importing from the shared utility in useTableManager) is clean and correct.

Could a maintainer please re-run the failed jobs from your side (with an org-scoped token), or approve the workflow run under Settings > Actions > Fork pull request workflows? That should unblock the merge. Thanks!

@Quantum0uasar
Copy link
Copy Markdown
Author

Thanks so much @ardaerzin for the review and approval! Really appreciate the feedback on the import path — glad we got it sorted. 🙏

@junaway junaway marked this pull request as draft May 29, 2026 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Frontend refactoring A code change that neither fixes a bug nor adds a feature size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[UI/UX] Improve click behavior in tables

3 participants