feat(ui): runs & robots loader #358
Conversation
WalkthroughThe pull request introduces loading indicators to two table components: Changes
Sequence DiagramsequenceDiagram
participant User
participant Component
participant DataSource
User->>Component: Loads Page
Component->>DataSource: Fetch Data
alt Data Loading
Component->>User: Display CircularProgress
else Data Loaded
Component->>User: Render Table
end
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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/robot/RecordingsTable.tsx (1)
Line range hint
108-110: Align error handling with RunsTable.The error case only logs to console, while RunsTable uses the notify system. Consider using consistent error handling across components.
} else { - console.log('No recordings found.'); + notify('error', t('recordingtable.notifications.no_recordings')); }
🧹 Nitpick comments (3)
src/components/run/RunsTable.tsx (2)
163-167: Consider improving the loading state handling.The current implementation might lead to a brief flash of the loading indicator during initial load, and doesn't distinguish between "loading" and "no data" states.
Consider tracking the loading state explicitly:
+const [isLoading, setIsLoading] = useState(true); const fetchRuns = async () => { + setIsLoading(true); const runs = await getStoredRuns(); if (runs) { const parsedRows: Data[] = runs.map((run: any, index: number) => ({ id: index, ...run, })); setRows(parsedRows); } else { notify('error', t('runstable.notifications.no_runs')); } + setIsLoading(false); }; -{rows.length === 0 ? ( +{isLoading ? ( <Box display="flex" justifyContent="center" alignItems="center" height="50%"> <CircularProgress /> </Box> +) : rows.length === 0 ? ( + <Box display="flex" justifyContent="center" alignItems="center" height="50%"> + <Typography color="textSecondary"> + {t('runstable.no_data', 'No runs available')} + </Typography> </Box> ) : (
164-164: Improve vertical centering consistency.Using a percentage height might not provide consistent centering across different viewport sizes.
Consider using a fixed height or viewport units:
-<Box display="flex" justifyContent="center" alignItems="center" height="50%"> +<Box display="flex" justifyContent="center" alignItems="center" minHeight="400px">src/components/robot/RecordingsTable.tsx (1)
203-207: Apply consistent loading state improvements.Similar to RunsTable, consider improving the loading state handling to distinguish between "loading" and "no data" states.
Apply the same improvements suggested for RunsTable:
+const [isLoading, setIsLoading] = useState(true); const fetchRecordings = async () => { + setIsLoading(true); const recordings = await getStoredRecordings(); if (recordings) { const parsedRows: Data[] = []; recordings.map((recording: any, index: number) => { if (recording && recording.recording_meta) { parsedRows.push({ id: index, ...recording.recording_meta, content: recording.recording }); } }); setRecordings(parsedRows.map((recording) => recording.name)); setRows(parsedRows); } else { console.log('No recordings found.'); } + setIsLoading(false); } -{rows.length === 0 ? ( +{isLoading ? ( <Box display="flex" justifyContent="center" alignItems="center" height="50%"> <CircularProgress /> </Box> +) : rows.length === 0 ? ( + <Box display="flex" justifyContent="center" alignItems="center" height="50%"> + <Typography color="textSecondary"> + {t('recordingtable.no_data', 'No recordings available')} + </Typography> </Box> ) : (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/robot/RecordingsTable.tsx(2 hunks)src/components/run/RunsTable.tsx(2 hunks)
🔇 Additional comments (2)
src/components/run/RunsTable.tsx (1)
12-12: LGTM! Clean import organization.The CircularProgress component is correctly imported alongside other Material-UI components.
src/components/robot/RecordingsTable.tsx (1)
14-14: LGTM! Clean import organization.The CircularProgress component is correctly imported alongside other Material-UI components.
No description provided.