feat: placeholder message for empty robots and runs #761
Conversation
WalkthroughAdds isLoading states and spinner-driven loading UI to RecordingsTable and RunsTable; introduces debouncedSearchTerm for search-aware empty-state placeholders; always-fetch on mount; preserves existing table/accordion rendering and pagination for non-empty results; adds localized placeholder keys across multiple locale files and a minor RunsTable header hover tweak. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant SearchInput as Search/Input (debounced)
participant RecordingsTable
participant RunsTable
Note right of SearchInput: useDebounce -> debouncedSearchTerm
User->>SearchInput: type query
SearchInput-->>RecordingsTable: debouncedSearchTerm
SearchInput-->>RunsTable: debouncedSearchTerm
alt Loading
RecordingsTable-->>User: show CircularProgress
RunsTable-->>User: show CircularProgress
else Not loading
alt No results (recordings)
RecordingsTable-->>User: show centered placeholder (search-aware)
else Results exist (recordings)
RecordingsTable-->>User: render table + pagination
end
alt No results (runs)
RunsTable-->>User: show centered placeholder (search-aware)
else Results exist (runs)
RunsTable-->>User: render accordions -> nested tables -> per-robot pagination + outer pagination
User->>RunsTable: hover header
RunsTable-->>User: reveal sort icon
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/run/RunsTable.tsx (1)
198-211: Debounce is ineffective; a new debounced fn is recreated per keystroke.This schedules multiple timeouts instead of coalescing them.
- const handleSearchChange = useCallback((event: React.ChangeEvent<HTMLInputElement>) => { - const debouncedSetSearch = debouncedSearch((value: string) => { - setSearchTerm(value); - setAccordionPage(0); - setPaginationStates(prev => { - const reset = Object.keys(prev).reduce((acc, robotId) => ({ - ...acc, - [robotId]: { ...prev[robotId], page: 0 } - }), {}); - return reset; - }); - }, 300); - debouncedSetSearch(event.target.value); - }, [debouncedSearch]); + const debouncedSetSearch = useMemo(() => debouncedSearch((value: string) => { + setSearchTerm(value); + setAccordionPage(0); + setPaginationStates(prev => { + const reset = Object.keys(prev).reduce((acc, robotId) => ({ + ...acc, + [robotId]: { ...prev[robotId], page: 0 } + }), {}); + return reset; + }); + }, 300), [debouncedSearch]); + + const handleSearchChange = useCallback((event: React.ChangeEvent<HTMLInputElement>) => { + debouncedSetSearch(event.target.value); + }, [debouncedSetSearch]);Optionally make the TextField controlled: add
value={searchTerm}.
🧹 Nitpick comments (2)
src/components/robot/RecordingsTable.tsx (2)
523-528: Match empty-state spacing with non-empty table spacing.Add top margin so the placeholder aligns visually with the table path (which has marginTop: 15px).
<Box display="flex" flexDirection="column" alignItems="center" justifyContent="center" sx={{ minHeight: 300, textAlign: 'center', - color: 'text.secondary' + color: 'text.secondary', + marginTop: '15px' }} >
529-538: Optional: Add a CTA to create a new recording directly from the empty state.Improves discoverability and reduces clicks.
<Typography variant="body2" color="text.secondary"> {debouncedSearchTerm ? t('recordingtable.search_criteria') : t('recordingtable.placeholder.body') } </Typography> + <Button variant="contained" sx={{ mt: 2 }} onClick={handleNewRecording}> + {t('recordingtable.new')} + </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/components/robot/RecordingsTable.tsx(1 hunks)src/components/run/RunsTable.tsx(1 hunks)
🔇 Additional comments (2)
src/components/robot/RecordingsTable.tsx (1)
516-578: Empty-state handling is clear and consistent.Conditional render + removal of pagination when empty looks good.
src/components/run/RunsTable.tsx (1)
381-403: Empty-state rendering reads well.Clear messaging with search-aware text is good.
src/components/run/RunsTable.tsx
Outdated
| {Object.keys(groupedRows).length === 0 ? ( | ||
| <Box | ||
| display="flex" | ||
| flexDirection="column" | ||
| alignItems="center" | ||
| justifyContent="center" | ||
| sx={{ | ||
| minHeight: 300, | ||
| textAlign: 'center', | ||
| color: 'text.secondary' | ||
| }} | ||
| > | ||
| <Typography variant="h6" gutterBottom> | ||
| {searchTerm ? t('runstable.placeholder.search') : t('runstable.placeholder.title')} | ||
| </Typography> | ||
| <Typography variant="body2" color="text.secondary"> | ||
| {searchTerm | ||
| ? t('recordingtable.search_criteria') | ||
| : t('runstable.placeholder.body') | ||
| } | ||
| </Typography> | ||
| </Box> | ||
| ) : ( |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify i18n keys exist for new runstable strings.
Ensure these keys exist: runstable.placeholder.title/body/search, runstable.search_criteria, runstable.sort_tooltip.
Also applies to: 424-468, 492-500
🏁 Script executed:
#!/bin/bash
# Check runstable translation keys
rg -n --glob '!**/node_modules/**' \
-e '"runstable\.placeholder\.title"' \
-e '"runstable\.placeholder\.body"' \
-e '"runstable\.placeholder\.search"' \
-e '"runstable\.search_criteria"' \
-e '"runstable\.sort_tooltip"' \
-- **/*.{json,ts,tsx}Length of output: 368
🏁 Script executed:
#!/bin/bash
# List JSON translation files in src
find src -type f -name '*.json' ! -path '*/node_modules/*'
# Show first 200 lines of src/i18n.ts to locate translation resource loading
echo "----- src/i18n.ts -----"
head -n 200 src/i18n.tsLength of output: 695
🏁 Script executed:
#!/bin/bash
# List all JSON files in the repo (excluding node_modules)
find . -type f -name '*.json' ! -path '*/node_modules/*'
# Find any 'locales' directories in the repo
find . -type d -name 'locales' ! -path '*/node_modules/*'
# If a public/locales directory exists, list its contents
if [ -d "./public/locales" ]; then
echo "---- public/locales ----"
ls -1 ./public/locales
fiLength of output: 639
🏁 Script executed:
#!/bin/bash
# Verify translation keys in locale JSON files
python3 - << 'EOF'
import json, os
languages = ['en','es','ja','zh','de','tr']
keys = [
'runstable.placeholder.title',
'runstable.placeholder.body',
'runstable.placeholder.search',
'runstable.search_criteria',
'runstable.sort_tooltip'
]
for lang in languages:
path = os.path.join('public','locales', f'{lang}.json')
try:
data = json.load(open(path, 'r', encoding='utf-8'))
except Exception as e:
print(f"{lang}.json: ERROR loading file ({e})")
continue
def flatten(d, prefix='', out=None):
out = {} if out is None else out
if isinstance(d, dict):
for k,v in d.items():
new_prefix = f"{prefix}.{k}" if prefix else k
flatten(v, new_prefix, out)
else:
out[prefix] = d
return out
flat = flatten(data)
missing = [key for key in keys if key not in flat]
status = "None" if not missing else ", ".join(missing)
print(f"{lang}.json missing: {status}")
EOFLength of output: 791
Add missing i18n keys for runstable placeholders and search criteria
Missing in all locale JSONs:
- runstable.placeholder.title
- runstable.placeholder.body
- runstable.placeholder.search
- runstable.search_criteria
(The runstable.sort_tooltip key is already present.)
🤖 Prompt for AI Agents
In src/components/run/RunsTable.tsx lines 381-403, the i18n keys used for the
empty-state and search copy are missing from the locale JSONs; add the following
keys to every locale file: "runstable.placeholder.title",
"runstable.placeholder.body", "runstable.placeholder.search", and
"runstable.search_criteria" (ensure the translations are supplied for each
locale). Also verify the component uses the intended key (it references
recordingtable.search_criteria in one place) and either add that key to locales
or change the component to use runstable.search_criteria so keys and code match.
| {searchTerm | ||
| ? t('recordingtable.search_criteria') | ||
| : t('runstable.placeholder.body') |
There was a problem hiding this comment.
Fix wrong i18n namespace for search criteria.
This references recordingtable.* inside RunsTable; should use runstable.*.
- ? t('recordingtable.search_criteria')
+ ? t('runstable.search_criteria')📝 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.
| {searchTerm | |
| ? t('recordingtable.search_criteria') | |
| : t('runstable.placeholder.body') | |
| {searchTerm | |
| ? t('runstable.search_criteria') | |
| : t('runstable.placeholder.body')} |
🤖 Prompt for AI Agents
In src/components/run/RunsTable.tsx around lines 397 to 399, the i18n key used
for the search criteria message is incorrectly namespaced as
recordingtable.search_criteria; change it to use the runstable namespace
(runstable.search_criteria) so it matches this component's translation keys and
returns the correct localized string.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
public/locales/de.json(2 hunks)public/locales/en.json(2 hunks)public/locales/es.json(2 hunks)public/locales/ja.json(2 hunks)public/locales/tr.json(2 hunks)public/locales/zh.json(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- public/locales/zh.json
- public/locales/en.json
🔇 Additional comments (7)
public/locales/de.json (1)
51-56: LGTM: Added empty-state strings for recordingsWording and punctuation look natural. Keys align with likely usage in RecordingsTable.
public/locales/ja.json (2)
51-56: LGTM: 録画テーブルの空状態テキスト自然な日本語で、コンポーネント用途にも合致しています。
99-103: LGTM: 実行テーブルの空状態テキスト用語とトーンが既存文言と一貫しています。
public/locales/es.json (2)
51-56: LGTM: Mensajes de estado vacío para robotsRedacción clara y acorde al resto del archivo.
99-103: LGTM: Placeholders de RunsTerminología “ejecuciones” consistente con el resto de la locale.
public/locales/tr.json (2)
51-56: LGTM: Kayıtlar için boş durum metinleriDoğal ve arayüzle uyumlu.
99-103: LGTM: Çalıştırmalar için boş durum metinleriTerim kullanımı tutarlı.
| "placeholder": { | ||
| "title": "Keine Durchläufe gefunden", | ||
| "body": "Hier werden alle Ihre Roboter-Durchläufe angezeigt. Sobald ein Roboter aktiv ist, werden seine Durchläufe hier protokolliert.", | ||
| "search": "Keine Durchläufe entsprechen Ihrer Suche" | ||
| }, |
There was a problem hiding this comment.
Use “Ausführungen” consistently instead of “Durchläufe”
Elsewhere in this locale you use “Ausführungen”. Aligning avoids mixed terminology in the Runs UI.
Apply this diff:
- "title": "Keine Durchläufe gefunden",
- "body": "Hier werden alle Ihre Roboter-Durchläufe angezeigt. Sobald ein Roboter aktiv ist, werden seine Durchläufe hier protokolliert.",
- "search": "Keine Durchläufe entsprechen Ihrer Suche"
+ "title": "Keine Ausführungen gefunden",
+ "body": "Hier werden alle Ihre Roboter-Ausführungen angezeigt. Sobald ein Roboter aktiv ist, werden seine Ausführungen hier protokolliert.",
+ "search": "Keine Ausführungen entsprechen Ihrer Suche"📝 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.
| "placeholder": { | |
| "title": "Keine Durchläufe gefunden", | |
| "body": "Hier werden alle Ihre Roboter-Durchläufe angezeigt. Sobald ein Roboter aktiv ist, werden seine Durchläufe hier protokolliert.", | |
| "search": "Keine Durchläufe entsprechen Ihrer Suche" | |
| }, | |
| "placeholder": { | |
| "title": "Keine Ausführungen gefunden", | |
| "body": "Hier werden alle Ihre Roboter-Ausführungen angezeigt. Sobald ein Roboter aktiv ist, werden seine Ausführungen hier protokolliert.", | |
| "search": "Keine Ausführungen entsprechen Ihrer Suche" | |
| }, |
🤖 Prompt for AI Agents
In public/locales/de.json around lines 99 to 103, replace the word "Durchläufe"
with "Ausführungen" in the three placeholder strings so the localization uses
"Ausführungen" consistently; update "title" to "Keine Ausführungen gefunden",
"body" to "Hier werden alle Ihre Roboter-Ausführungen angezeigt. Sobald ein
Roboter aktiv ist, werden seine Ausführungen hier protokolliert.", and "search"
to "Keine Ausführungen entsprechen Ihrer Suche".
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/robot/RecordingsTable.tsx (1)
252-271: Use persisted recording ID instead of array index (breaks delete/run/retrain + type mismatch)Index-based IDs won’t match storage APIs (e.g., checkRunsForRecording/deleteRecordingFromStorage) and violate Data.id:string. Use the meta ID and drop rows without an ID.
- const parsedRows = recordings - .map((recording: any, index: number) => { - if (recording?.recording_meta) { - const parsedDate = parseDateString(recording.recording_meta.updatedAt); - - return { - id: index, - ...recording.recording_meta, - content: recording.recording, - parsedDate - }; - } - return null; - }) - .filter(Boolean) - .sort((a, b) => b.parsedDate.getTime() - a.parsedDate.getTime()); + const parsedRows = recordings + .map((recording: any) => { + const meta = recording?.recording_meta; + if (!meta?.id) return null; + const parsedDate = parseDateString(meta.updatedAt); + return { + id: String(meta.id), + ...meta, + content: recording.recording, + parsedDate + }; + }) + .filter(Boolean) + .sort((a: any, b: any) => b.parsedDate.getTime() - a.parsedDate.getTime()) + .map(({ parsedDate, ...row }: any) => row);src/components/run/RunsTable.tsx (1)
191-198: Debounce is ineffective: a new debounced function is created per keystroke (multiple timeouts fire)
Make the debounced setter stable and fix the Timeout type for the browser.- const debouncedSearch = useCallback((fn: Function, delay: number) => { - let timeoutId: NodeJS.Timeout; - return (...args: any[]) => { - clearTimeout(timeoutId); - timeoutId = setTimeout(() => fn(...args), delay); - }; - }, []); + const debouncedSearch = useCallback((fn: Function, delay: number) => { + let timeoutId: ReturnType<typeof setTimeout>; + return (...args: any[]) => { + clearTimeout(timeoutId); + timeoutId = setTimeout(() => fn(...args), delay); + }; + }, []); - const handleSearchChange = useCallback((event: React.ChangeEvent<HTMLInputElement>) => { - const debouncedSetSearch = debouncedSearch((value: string) => { - setSearchTerm(value); - setAccordionPage(0); - setPaginationStates(prev => { - const reset = Object.keys(prev).reduce((acc, robotId) => ({ - ...acc, - [robotId]: { ...prev[robotId], page: 0 } - }), {}); - return reset; - }); - }, 300); - debouncedSetSearch(event.target.value); - }, [debouncedSearch]); + const debouncedSetSearch = React.useMemo( + () => + debouncedSearch((value: string) => { + setSearchTerm(value); + setAccordionPage(0); + setPaginationStates(prev => + Object.fromEntries( + Object.entries(prev).map(([robotId, s]: any) => [robotId, { ...s, page: 0 }]) + ) + ); + }, 300), + [debouncedSearch, setAccordionPage, setPaginationStates] + ); + + const handleSearchChange = useCallback( + (event: React.ChangeEvent<HTMLInputElement>) => { + debouncedSetSearch(event.target.value); + }, + [debouncedSetSearch] + );Also applies to: 199-213
♻️ Duplicate comments (3)
src/components/run/RunsTable.tsx (3)
432-474: Prefer MUI TableSortLabel for accessible sortable headers
Removes custom hover logic and adds keyboard/a11y.- <TableCell + <TableCell key={column.id} align={column.align} - style={{ - minWidth: column.minWidth, - cursor: column.id === 'startedAt' || column.id === 'finishedAt' ? 'pointer' : 'default' - }} - onClick={() => { - if (column.id === 'startedAt' || column.id === 'finishedAt') { - handleSort(column.id, robotMetaId); - } - }} + style={{ minWidth: column.minWidth }} > - <Tooltip - title={ - (column.id === 'startedAt' || column.id === 'finishedAt') - ? t('runstable.sort_tooltip') - : '' - } - > - <Box sx={{ - display: 'flex', - alignItems: 'center', - gap: 1, - '&:hover': { - '& .sort-icon': { - opacity: 1 - } - } - }}> - {column.label} - <Box className="sort-icon" sx={{ - display: 'flex', - alignItems: 'center', - opacity: accordionSortConfigs[robotMetaId]?.field === column.id ? 1 : 0.3, - transition: 'opacity 0.2s' - }}> - {renderSortIcon(column, robotMetaId)} - </Box> - </Box> - </Tooltip> + {(column.id === 'startedAt' || column.id === 'finishedAt') ? ( + <Tooltip title={t('runstable.sort_tooltip')}> + <Box> + <TableSortLabel + active={accordionSortConfigs[robotMetaId]?.field === column.id} + direction={accordionSortConfigs[robotMetaId]?.direction === 'desc' ? 'desc' : 'asc'} + onClick={() => handleSort(column.id as keyof Data, robotMetaId)} + > + {column.label} + </TableSortLabel> + </Box> + </Tooltip> + ) : ( + column.label + )} </TableCell>Add near imports:
+import TableSortLabel from '@mui/material/TableSortLabel';
385-407: Verify i18n keys exist for the new empty-state strings#!/bin/bash # Verify required runstable keys exist in locale JSONs rg -n --glob '!**/node_modules/**' \ -e '"runstable\.placeholder\.title"' \ -e '"runstable\.placeholder\.body"' \ -e '"runstable\.placeholder\.search"' \ -e '"runstable\.search_criteria"' \ public/locales/*.json
403-405: Wrong i18n key namespace: use runstable.search_criteria
This will fall back or display the wrong string at runtime.- ? t('recordingtable.search_criteria') + ? t('runstable.search_criteria')
🧹 Nitpick comments (5)
src/components/robot/RecordingsTable.tsx (2)
519-521: Center the loading spinner for consistent UX with the empty-state block- {isLoading ? ( - <CircularProgress size={32} /> - ) : filteredRows.length === 0 ? ( + {isLoading ? ( + <Box display="flex" alignItems="center" justifyContent="center" sx={{ minHeight: 300 }}> + <CircularProgress size={32} /> + </Box> + ) : filteredRows.length === 0 ? (
423-437: Don’t re-implement useDebounce here; import the shared hook to avoid duplication- function useDebounce<T>(value: T, delay: number): T { - const [debouncedValue, setDebouncedValue] = React.useState<T>(value); - - useEffect(() => { - const handler = setTimeout(() => { - setDebouncedValue(value); - }, delay); - - return () => { - clearTimeout(handler); - }; - }, [value, delay]); - - return debouncedValue; - } + // Prefer: import useDebounce from '../../hooks/useDebounce';If you want, I can submit a follow-up PR wiring the shared hook.
src/components/run/RunsTable.tsx (3)
385-387: Center the spinner to match the empty-state layout- {isLoading? ( - <CircularProgress size={32} /> - ) : Object.keys(groupedRows).length === 0 ? ( + {isLoading? ( + <Box display="flex" alignItems="center" justifyContent="center" sx={{ minHeight: 300 }}> + <CircularProgress size={32} /> + </Box> + ) : Object.keys(groupedRows).length === 0 ? (
445-452: Avoid rendering Tooltip with an empty title on non-sortable columns
Small cleanup; conditionally render Tooltip only when needed.
224-225: “No runs” shouldn’t be an error-level notification
Use info/warning or rely on the empty-state UI only.- notify('error', t('runstable.notifications.no_runs')); + notify('info', t('runstable.notifications.no_runs'));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/components/robot/RecordingsTable.tsx(5 hunks)src/components/run/RunsTable.tsx(4 hunks)
🔇 Additional comments (1)
src/components/robot/RecordingsTable.tsx (1)
26-26: Loading-state wiring looks goodAlso applies to: 158-159, 275-277, 411-413
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/robot/RecordingsTable.tsx (1)
248-271: Bug: row.id is a number but type and downstream APIs expect a string ID.id is set from the array index; this breaks type consistency (Data.id is string) and may cause mismatches for delete/check APIs.
- return { - id: index, - ...recording.recording_meta, - content: recording.recording, - parsedDate - }; + return { + id: String(recording.recording_meta.id), + ...recording.recording_meta, + content: recording.recording, + parsedDate + };Optional TS fix for the type after filter(Boolean):
- .filter(Boolean) + .filter((r): r is { id: string; name: string; parsedDate: Date; recording?: any; recording_meta?: any } => Boolean(r))
♻️ Duplicate comments (4)
src/components/robot/RecordingsTable.tsx (1)
531-552: Verify i18n keys exist for the new placeholders.Run to confirm across locales:
#!/bin/bash rg -n --glob '!**/node_modules/**' \ -e '"recordingtable\.placeholder\.title"' \ -e '"recordingtable\.placeholder\.body"' \ -e '"recordingtable\.placeholder\.search"' \ -e '"recordingtable\.search_criteria"' \ public/locales/*.jsonsrc/components/run/RunsTable.tsx (3)
441-483: Prefer MUI TableSortLabel for accessible sortable headers (keyboard + a11y).- <TableCell + <TableCell key={column.id} align={column.align} - style={{ - minWidth: column.minWidth, - cursor: column.id === 'startedAt' || column.id === 'finishedAt' ? 'pointer' : 'default' - }} - onClick={() => { - if (column.id === 'startedAt' || column.id === 'finishedAt') { - handleSort(column.id, robotMetaId); - } - }} + style={{ minWidth: column.minWidth }} > - <Tooltip - title={ - (column.id === 'startedAt' || column.id === 'finishedAt') - ? t('runstable.sort_tooltip') - : '' - } - > - <Box sx={{ - display: 'flex', - alignItems: 'center', - gap: 1, - '&:hover': { - '& .sort-icon': { - opacity: 1 - } - } - }}> - {column.label} - <Box className="sort-icon" sx={{ - display: 'flex', - alignItems: 'center', - opacity: accordionSortConfigs[robotMetaId]?.field === column.id ? 1 : 0.3, - transition: 'opacity 0.2s' - }}> - {renderSortIcon(column, robotMetaId)} - </Box> - </Box> - </Tooltip> + {column.id === 'startedAt' || column.id === 'finishedAt' ? ( + <TableSortLabel + active={accordionSortConfigs[robotMetaId]?.field === column.id} + direction={accordionSortConfigs[robotMetaId]?.direction === 'desc' ? 'desc' : 'asc'} + onClick={() => handleSort(column.id as keyof Data, robotMetaId)} + title={t('runstable.sort_tooltip')} + > + {column.label} + </TableSortLabel> + ) : ( + column.label + )} </TableCell>Add import:
+import TableSortLabel from '@mui/material/TableSortLabel';
398-419: Verify i18n keys exist for runstable placeholders and guidance.#!/bin/bash rg -n --glob '!**/node_modules/**' \ -e '"runstable\.placeholder\.title"' \ -e '"runstable\.placeholder\.body"' \ -e '"runstable\.placeholder\.search"' \ -e '"runstable\.search_criteria"' \ public/locales/*.json
413-416: Wrong i18n namespace in empty-state body (uses recordingtable.*).Switch to runstable.search_criteria so it resolves to the correct string.
- ? t('recordingtable.search_criteria') + ? t('runstable.search_criteria')
🧹 Nitpick comments (5)
src/components/robot/RecordingsTable.tsx (3)
158-159: Ensure spinner shows during all fetches (set isLoading(true) before each fetch).Currently isLoading starts true and is reset in finally, but rerender-triggered fetches won’t show the spinner. Set isLoading(true) at the start of fetchRecordings and before rerender-driven fetches.
const fetchRecordings = useCallback(async () => { - try { + try { + setIsLoading(true); const recordings = await getStoredRecordings(); @@ } catch (error) { console.error('Error fetching recordings:', error); notify('error', t('recordingtable.notifications.fetch_error')); } finally { setIsLoading(false); } }, [setRecordings, notify, t]); @@ useEffect(() => { if (rerenderRobots) { - fetchRecordings().then(() => { + setIsLoading(true); + fetchRecordings().then(() => { setRerenderRobots(false); }); } }, [rerenderRobots, fetchRecordings, setRerenderRobots]);Also applies to: 275-277, 411-413
519-531: Minor a11y: label the progress indicator.- <CircularProgress size={60} /> + <CircularProgress size={60} aria-label={t('common.loading', 'Loading')} />
495-516: Use Button instead of IconButton for a text button.Improves semantics/a11y; keep styles via sx and use startIcon.
- <IconButton - aria-label="new" - size={"small"} + <Button onClick={handleNewRecording} sx={{ width: '140px', borderRadius: '5px', padding: '8px', background: '#ff00c3', color: 'white', marginRight: '10px', fontFamily: '"Roboto","Helvetica","Arial",sans-serif', fontWeight: '500', fontSize: '0.875rem', lineHeight: '1.75', letterSpacing: '0.02857em', '&:hover': { color: 'white', backgroundColor: '#ff00c3' } }} > - <Add sx={{ marginRight: '5px' }} /> {t('recordingtable.new')} - </IconButton> + <Add sx={{ mr: 1 }} /> {t('recordingtable.new')} + </Button>src/components/run/RunsTable.tsx (2)
385-397: Minor a11y: label the progress indicator.- <CircularProgress size={60} /> + <CircularProgress size={60} aria-label={t('common.loading', 'Loading')} />
191-213: Debounce is ineffective; new debounced function is created on every keystroke.Timers aren’t cleared across events. Keep a stable debounced function or use a debounce hook.
- const handleSearchChange = useCallback((event: React.ChangeEvent<HTMLInputElement>) => { - const debouncedSetSearch = debouncedSearch((value: string) => { - setSearchTerm(value); - setAccordionPage(0); - setPaginationStates(prev => { - const reset = Object.keys(prev).reduce((acc, robotId) => ({ - ...acc, - [robotId]: { ...prev[robotId], page: 0 } - }), {}); - return reset; - }); - }, 300); - debouncedSetSearch(event.target.value); - }, [debouncedSearch]); + const debouncedSetSearch = React.useMemo( + () => + debouncedSearch((value: string) => { + setSearchTerm(value); + setAccordionPage(0); + setPaginationStates(prev => + Object.fromEntries(Object.entries(prev).map(([id, s]) => [id, { ...s, page: 0 }])) + ); + }, 300), + [debouncedSearch] + ); + + const handleSearchChange = useCallback((event: React.ChangeEvent<HTMLInputElement>) => { + debouncedSetSearch(event.target.value); + }, [debouncedSetSearch]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/components/robot/RecordingsTable.tsx(5 hunks)src/components/run/RunsTable.tsx(4 hunks)
🔇 Additional comments (3)
src/components/robot/RecordingsTable.tsx (1)
26-27: LGTM: import for spinner is correct and used.src/components/run/RunsTable.tsx (2)
12-12: LGTM: spinner import is correct.
137-138: Tri-state loading/empty/data flow looks good.isLoading gating with finally reset is correctly applied and shows a centered spinner on initial and rerender fetches.
Also applies to: 228-230, 236-243, 385-397
2 participants
Closes: #747
Summary by CodeRabbit
New Features
Style
Localization