feat: improve table result UX #208
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the user experience for table results by adding search/filter functionality, execution time tracking, and row numbering to make the data more navigable and informative.
Key changes:
- Added client-side search/filter functionality with text highlighting for table results
- Implemented execution time measurement and display
- Added row numbers to the results table for easier reference
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
frontend/src/components/views/ToolDetailView.tsx |
Adds state management and timing logic to track execution time and SQL query, passes new props to ResultsTable |
frontend/src/components/tool/ResultsTable.tsx |
Implements search/filter UI with highlighting, row numbers column, and execution metadata display footer |
| <mark key={index} className="bg-amber-200/50 text-foreground"> | ||
| {text.substring(index, index + searchTerm.length)} | ||
| </mark> |
There was a problem hiding this comment.
The highlighted text uses a key based on the index position, which could cause React rendering issues when the same search term appears multiple times at different positions in the text. Since index represents the position in the string, multiple matches could theoretically have the same key if not handled properly. Consider using a combination of index and a counter, or use the React fragment key pattern with a unique identifier like ${index}-${parts.length}.
| </div> | ||
| {executedSql && executionTimeMs !== undefined && ( | ||
| <div className="flex justify-between items-center text-sm text-muted-foreground"> | ||
| <span>{executedSql}</span> |
There was a problem hiding this comment.
The executed SQL query is displayed without any length limit or truncation. For long or complex SQL queries, this could cause layout issues or make the footer unwieldy. Consider truncating long queries with an ellipsis or displaying them in a collapsible section.
| {executedSql && executionTimeMs !== undefined && ( | ||
| <div className="flex justify-between items-center text-sm text-muted-foreground"> | ||
| <span>{executedSql}</span> |
There was a problem hiding this comment.
The condition checks if executedSql is truthy, but executedSql is initialized as an empty string. If getSqlPreview() or sql returns an empty string, this footer won't display even though executionTimeMs might be valid. This could hide execution time information for empty queries. Consider checking executionTimeMs !== undefined as the primary condition, or handle the empty SQL case explicitly.
| {executedSql && executionTimeMs !== undefined && ( | |
| <div className="flex justify-between items-center text-sm text-muted-foreground"> | |
| <span>{executedSql}</span> | |
| {executionTimeMs !== undefined && ( | |
| <div className="flex justify-between items-center text-sm text-muted-foreground"> | |
| {executedSql !== undefined && executedSql !== '' && ( | |
| <span>{executedSql}</span> | |
| )} |
| <div className="border border-border rounded-lg overflow-hidden"> | ||
| {/* Header */} | ||
| <div className="flex bg-muted/50 border-b border-border"> | ||
| <div className="w-16 px-3 py-2 text-sm font-medium text-foreground flex-shrink-0"> |
There was a problem hiding this comment.
This empty div creates a blank column header for the row numbers column, but it contains no content or accessible label. While visually it may be intentional to leave it blank, consider adding a visually-hidden label like "Row" or "#" for screen reader users to understand the purpose of this column.
| <div className="w-16 px-3 py-2 text-sm font-medium text-foreground flex-shrink-0"> | |
| <div className="w-16 px-3 py-2 text-sm font-medium text-foreground flex-shrink-0"> | |
| <span className="sr-only">Row</span> |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.