feat: sort functionality for runs table data #407
Conversation
WalkthroughThis pull request introduces sorting functionality to the RunsTable component across multiple localization files. The changes include adding a new Changes
Sequence DiagramsequenceDiagram
participant User
participant RunsTable
participant SortState
User->>RunsTable: Click column header
RunsTable->>SortState: Update sort configuration
SortState-->>RunsTable: Return new sort direction
RunsTable->>RunsTable: Re-render sorted rows
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/components/run/RunsTable.tsx (1)
Line range hint
209-242: Improve date handling robustness.The current date string manipulation assumes a specific format and might fail with different locale formats. Consider using a date parsing library like
date-fnsormoment.jsfor more robust date handling.Apply this diff to improve date handling:
-const dateA = new Date(a[sortConfig.field!].replace(/(\d+)\/(\d+)\//, '$2/$1/')); -const dateB = new Date(b[sortConfig.field!].replace(/(\d+)\/(\d+)\//, '$2/$1/')); +import { parse } from 'date-fns'; + +const parseDate = (dateStr: string) => { + try { + return parse(dateStr, 'dd/MM/yyyy HH:mm:ss', new Date()); + } catch (error) { + console.error(`Failed to parse date: ${dateStr}`, error); + return new Date(0); // fallback to epoch + } +}; + +const dateA = parseDate(a[sortConfig.field!]); +const dateB = parseDate(b[sortConfig.field!]);
🧹 Nitpick comments (2)
src/components/run/RunsTable.tsx (2)
314-351: Enhance accessibility for sortable columns.While the implementation is good, consider adding ARIA attributes to improve accessibility for screen readers.
Apply this diff to enhance accessibility:
<TableCell key={column.id} align={column.align} style={{ minWidth: column.minWidth, cursor: column.id === 'startedAt' || column.id === 'finishedAt' ? 'pointer' : 'default' }} + aria-sort={ + column.id === 'startedAt' || column.id === 'finishedAt' + ? accordionSortConfigs[robotMetaId]?.field === column.id + ? accordionSortConfigs[robotMetaId].direction + : 'none' + : undefined + } + role="columnheader" onClick={() => { if (column.id === 'startedAt' || column.id === 'finishedAt') { handleSort(column.id, robotMetaId); } }}
190-195: Optimize search filtering performance.Consider memoizing the search term transformation to avoid unnecessary string operations on every render.
Apply this diff to optimize performance:
const filteredRows = useMemo(() => { + const searchLower = searchTerm.toLowerCase(); let result = rows.filter((row) => - row.name.toLowerCase().includes(searchTerm.toLowerCase()) + row.name.toLowerCase().includes(searchLower) ); return result; -}, [rows, searchTerm]); +}, [rows, searchTerm]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locales/de.json(1 hunks)public/locales/en.json(1 hunks)public/locales/es.json(1 hunks)public/locales/ja.json(1 hunks)public/locales/zh.json(1 hunks)src/components/run/RunsTable.tsx(8 hunks)
🔇 Additional comments (8)
src/components/run/RunsTable.tsx (3)
31-38: Well-structured type definitions for sorting functionality!The
SortDirectiontype andAccordionSortConfiginterface provide a clean and type-safe way to manage sorting states independently for each robot meta ID.
82-100: Clean implementation of sort state management!The sort handler effectively manages state with proper immutability and includes memoization for performance optimization. The sort direction cycles logically through none -> asc -> desc -> none.
244-268: Excellent implementation of sort icons with visual feedback!The sort icon rendering provides clear visual feedback with proper opacity transitions and follows Material-UI best practices.
public/locales/zh.json (1)
66-66: Appropriate Chinese translation for sort tooltip!The translation "点击排序" is concise and accurately conveys the sorting action to Chinese users.
public/locales/ja.json (1)
66-66: Appropriate Japanese translation for sort tooltip!The translation "クリックして並べ替え" is natural and follows Japanese language conventions.
public/locales/en.json (1)
66-66: Clear and concise English tooltip text!The text "Click to sort" effectively communicates the action to users.
public/locales/es.json (1)
66-66: LGTM! Spanish translation is accurate and natural.The tooltip message "Haga clic para ordenar" is grammatically correct and effectively conveys the sorting functionality to Spanish users.
public/locales/de.json (1)
65-65: LGTM! German translation is accurate and natural.The tooltip message "Zum Sortieren klicken" is grammatically correct and effectively conveys the sorting functionality to German users.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/run/RunsTable.tsx (1)
Line range hint
221-254: Optimize sorting performance.The current implementation sorts the data on every render. Consider memoizing the sorted data to improve performance.
- const renderTableRows = useCallback((data: Data[], robotMetaId: string) => { + const getSortedData = useCallback((data: Data[], robotMetaId: string) => { const sortConfig = accordionSortConfigs[robotMetaId]; - let sortedData = [...data]; if (sortConfig?.field === 'startedAt' || sortConfig?.field === 'finishedAt') { if (sortConfig.direction !== 'none') { - sortedData.sort((a, b) => { + return [...data].sort((a, b) => { const dateA = parseDateString(a[sortConfig.field!]); const dateB = parseDateString(b[sortConfig.field!]); return sortConfig.direction === 'asc' ? dateA.getTime() - dateB.getTime() : dateB.getTime() - dateA.getTime(); }); } } + return data; + }, [accordionSortConfigs, parseDateString]); + + const sortedDataMap = useMemo(() => { + const map = new Map<string, Data[]>(); + Object.entries(groupedRows).forEach(([robotMetaId, data]) => { + map.set(robotMetaId, getSortedData(data, robotMetaId)); + }); + return map; + }, [groupedRows, getSortedData]); + + const renderTableRows = useCallback((data: Data[], robotMetaId: string) => { const start = page * rowsPerPage; const end = start + rowsPerPage; - return sortedData + return sortedDataMap.get(robotMetaId) .slice(start, end) .map((row) => ( <CollapsibleRow key={`row-${row.id}`} row={row} handleDelete={handleDelete} isOpen={runId === row.runId && runningRecordingName === row.name} currentLog={currentInterpretationLog} abortRunHandler={abortRunHandler} runningRecordingName={runningRecordingName} /> )); - }, [page, rowsPerPage, runId, runningRecordingName, currentInterpretationLog, abortRunHandler, handleDelete, accordionSortConfigs]); + }, [page, rowsPerPage, runId, runningRecordingName, currentInterpretationLog, abortRunHandler, handleDelete, sortedDataMap]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/run/RunsTable.tsx(8 hunks)
🔇 Additional comments (5)
src/components/run/RunsTable.tsx (5)
31-38: LGTM! Well-structured type definitions for sorting.The
SortDirectiontype andAccordionSortConfiginterface provide a robust foundation for managing sort states per robot meta ID.
82-82: LGTM! Efficient sort state management.Using a separate sort configuration per robot meta ID allows for independent sorting of each accordion section.
84-100: LGTM! Well-implemented sort direction cycling.The
handleSortfunction correctly implements the sort direction cycle (none → asc → desc → none) while maintaining independent sort states for each robot meta ID.
326-363: LGTM! Excellent UI/UX implementation for sorting.The implementation includes:
- Clear visual feedback with sort icons
- Smooth hover transitions
- Proper accessibility with tooltips
- Cursor changes only for sortable columns
339-339: Verify sort tooltip translations.Ensure that the
sort_tooltiptranslation key is present in all language files.✅ Verification successful
Translation key
sort_tooltipis present in all locale files🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for missing sort_tooltip translations for locale in public/locales/*.json; do echo "Checking $locale..." if ! grep -q '"sort_tooltip"' "$locale"; then echo "Warning: Missing sort_tooltip in $locale" fi doneLength of output: 834
| const parseDateString = (dateStr: string): Date => { | ||
| try { | ||
| if (dateStr.includes('PM') || dateStr.includes('AM')) { | ||
| return new Date(dateStr); | ||
| } | ||
|
|
||
| return new Date(dateStr.replace(/(\d+)\/(\d+)\//, '$2/$1/')) | ||
| } catch { | ||
| return new Date(0); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance date parsing robustness.
The current date parsing implementation has potential issues:
- The date format detection is fragile
- Silently falling back to epoch time (Date(0)) could hide parsing errors
Consider using a robust date parsing library:
- const parseDateString = (dateStr: string): Date => {
- try {
- if (dateStr.includes('PM') || dateStr.includes('AM')) {
- return new Date(dateStr);
- }
-
- return new Date(dateStr.replace(/(\d+)\/(\d+)\//, '$2/$1/'))
- } catch {
- return new Date(0);
- }
- };
+ import { parseISO, parse } from 'date-fns';
+
+ const parseDateString = (dateStr: string): Date => {
+ try {
+ // First try ISO format
+ const isoDate = parseISO(dateStr);
+ if (!isNaN(isoDate.getTime())) return isoDate;
+
+ // Then try common formats
+ const formats = [
+ 'MM/dd/yyyy h:mm a',
+ 'dd/MM/yyyy HH:mm',
+ ];
+
+ for (const format of formats) {
+ const parsedDate = parse(dateStr, format, new Date());
+ if (!isNaN(parsedDate.getTime())) return parsedDate;
+ }
+
+ throw new Error(`Unable to parse date: ${dateStr}`);
+ } catch (error) {
+ console.error(`Date parsing error: ${error}`);
+ throw error; // Let the caller handle the error
+ }
+ };Committable suggestion skipped: line range outside the PR's diff.
Closes: #404
Maxun._.Open.Source.No.Code.Web.Data.Extraction.Platform.-.Brave.2025-01-28.00-50-51.mp4
Summary by CodeRabbit
New Features
Localization