Evaluator Listing UI and Interaction Improvements#602
Conversation
…List and MonitorTable components
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adjusts evaluator page navigation and card rendering, fixes monitor LLM provider config sanitization, adds aria-labels to icon buttons, and expands monitor search to include Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (2 warnings, 2 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
console/workspaces/pages/eval/src/EvalEvaluators.Component.tsx (2)
459-459: UserowsPerPagefor paginator visibility instead of a fixed threshold.Tying this condition to
6can show pagination when only one page exists under the current page size. Consider basing it ontotalItems > rowsPerPageto stay consistent with user-selected page size.Suggested change
- {totalItems > 6 && ( + {totalItems > rowsPerPage && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/pages/eval/src/EvalEvaluators.Component.tsx` at line 459, The paginator visibility currently uses a hardcoded threshold (totalItems > 6); change the condition to use the user-selected page size by replacing that check with totalItems > rowsPerPage so the paginator (where totalItems is evaluated) only appears when there are more items than the current rowsPerPage value; update the JSX conditional that renders the paginator in EvalEvaluators.Component (look for the expression using totalItems > 6) to totalItems > rowsPerPage.
271-327: Replace hardcoded card/layout dimensions with theme tokens or shared constants.The newly added fixed values (
260px,300px,224) make this layout harder to keep consistent with theme-driven spacing and sizing.As per coding guidelines: "Use theme tokens via the
sxprop instead of hardcoded colors and spacing values".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/pages/eval/src/EvalEvaluators.Component.tsx` around lines 271 - 327, Replace the hardcoded sizing in the evaluators list with theme tokens/constants: update the gridTemplateColumns minmax values (currently "minmax(260px, 1fr)" and "minmax(300px, 1fr)") and the Form.CardButton fixed height (currently 224) to use theme spacing/size tokens or shared constants; locate the evaluators.map render block and change the sx props on the surrounding container (gridTemplateColumns) and on Form.CardButton (height, width if needed) to reference theme.breakpoints, theme.spacing/theme.shape values or exported constants so sizing follows the app theme instead of literal px values.console/workspaces/pages/eval/src/CreateEvaluator.Component.tsx (1)
84-84: Remove the now-deadbackHrefprop pass-through toEvaluatorForm.Line 84 still passes
backHref, but the form no longer uses it. Dropping this from the form API will reduce prop surface and avoid stale wiring.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/pages/eval/src/CreateEvaluator.Component.tsx` at line 84, Remove the now-unused backHref prop passed into EvaluatorForm from CreateEvaluator.Component.tsx: delete the backHref attribute in the JSX where EvaluatorForm is rendered (the pass-through named backHref) and, if present in this file, stop reading backHref from the parent component props; also update any local references or prop-type/interface usage in this file related to backHref so the EvaluatorForm invocation matches its current API (reference: EvaluatorForm and the backHref prop name).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@console/workspaces/pages/eval/src/CreateEvaluator.Component.tsx`:
- Line 84: Remove the now-unused backHref prop passed into EvaluatorForm from
CreateEvaluator.Component.tsx: delete the backHref attribute in the JSX where
EvaluatorForm is rendered (the pass-through named backHref) and, if present in
this file, stop reading backHref from the parent component props; also update
any local references or prop-type/interface usage in this file related to
backHref so the EvaluatorForm invocation matches its current API (reference:
EvaluatorForm and the backHref prop name).
In `@console/workspaces/pages/eval/src/EvalEvaluators.Component.tsx`:
- Line 459: The paginator visibility currently uses a hardcoded threshold
(totalItems > 6); change the condition to use the user-selected page size by
replacing that check with totalItems > rowsPerPage so the paginator (where
totalItems is evaluated) only appears when there are more items than the current
rowsPerPage value; update the JSX conditional that renders the paginator in
EvalEvaluators.Component (look for the expression using totalItems > 6) to
totalItems > rowsPerPage.
- Around line 271-327: Replace the hardcoded sizing in the evaluators list with
theme tokens/constants: update the gridTemplateColumns minmax values (currently
"minmax(260px, 1fr)" and "minmax(300px, 1fr)") and the Form.CardButton fixed
height (currently 224) to use theme spacing/size tokens or shared constants;
locate the evaluators.map render block and change the sx props on the
surrounding container (gridTemplateColumns) and on Form.CardButton (height,
width if needed) to reference theme.breakpoints, theme.spacing/theme.shape
values or exported constants so sizing follows the app theme instead of literal
px values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8108e8cd-7175-4edf-9491-c520106260c7
📒 Files selected for processing (7)
console/workspaces/pages/eval/src/CreateEvaluator.Component.tsxconsole/workspaces/pages/eval/src/EditMonitor.Component.tsxconsole/workspaces/pages/eval/src/EvalEvaluators.Component.tsxconsole/workspaces/pages/eval/src/ViewEvaluator.Component.tsxconsole/workspaces/pages/eval/src/subComponents/EvaluatorForm.tsxconsole/workspaces/pages/eval/src/subComponents/MonitorRunList.tsxconsole/workspaces/pages/eval/src/subComponents/MonitorTable.tsx
Purpose
Fixes: #483
This pull request introduces several UI and code quality improvements to the evaluator and monitor management pages. The main focus is on refactoring the evaluator listing and card UI, improving accessibility with ARIA labels, and clarifying the handling of secrets in monitor editing. There are also some minor stylistic and usability adjustments to buttons and navigation.
Evaluator Listing and Card UI Refactor:
EvalEvaluatorsComponentto use newForm.CardButton,Form.CardHeader, and related components for a more consistent and accessible card UI, including improved handling of description truncation, tags, and card actions. The edit and delete buttons now use event handlers that prevent event bubbling, and the card's structure is simplified for readability and maintainability. [1] [2] [3] [4]Form,Form.Stack, and removed unused ones for better code organization.Accessibility and Usability Improvements:
IconButtoncomponents in monitor-related tables and lists to improve accessibility for screen readers. [1] [2] [3] [4]Evaluator Creation and Editing Flows:
ViewEvaluatorComponentto use theEditicon and updated button text for clarity. [1] [2]Monitor Editing and Filtering Logic:
"****") are now correctly interpreted as "preserve existing secret," ensuring the backend receives the right intent.monitor.displayNamein the search haystack, making search results more comprehensive.Goals
Approach
User stories
Release note
Documentation
Training
Fixes: #483
Certification
This pull request updates the Evaluators UI in several ways to improve consistency, usability, and code structure. The main changes include a redesign of the evaluator listing cards, updates to navigation and button behaviors, and improvements to the edit and form actions. These changes enhance the user experience when creating, viewing, and managing evaluators.
Evaluator Listing UI and Interaction Improvements:
EvalEvaluators.Component.tsxto use newForm.CardButton,Form.CardHeader, and related components for a more consistent and accessible UI. Added truncated descriptions with tooltips, improved tag display, and moved edit/delete actions into card actions with better event handling. [1] [2] [3] [4]rowsPerPage.Navigation and Back Button Updates:
CreateEvaluatorComponentto provide a "Back to Evaluators" label and link in the page layout, improving navigation clarity.EvaluatorForm, replaced the "Back to Evaluators" link button with a disabled "Previous" button for step navigation, and updated the variant of the page navigation button for consistency. [1] [2]Edit and Delete Button Consistency:
Editinstead ofPencil) and updated the edit button label to "Edit Evaluator" in the evaluator view. Changed the cancel edit button to use theoutlinedvariant for visual clarity. [1] [2]Code Cleanup and Imports:
EvalEvaluators.Component.tsx, grouping and formatting them for readability, and removed unused imports.Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
Bug Fixes
Accessibility
UI Improvements