-
Notifications
You must be signed in to change notification settings - Fork 157
feat: improve request table #218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances the request table by adding visual database type indicators and improving client information display. The changes focus on better UX through database logos in filter buttons and human-readable client names.
Key changes:
- Added parseUserAgent utility to display readable browser/client names instead of raw user agent strings
- Integrated database type logos into source filter buttons for better visual identification
- Reordered table columns to move Client information to the end for better information hierarchy
| </div> | ||
| </td> | ||
| <td className="px-4 py-2 text-sm text-muted-foreground whitespace-nowrap"> | ||
| {parseUserAgent(request.client)} |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parseUserAgent function is called for every row during each render. With up to 100 requests per source and potential re-renders when filtering or other state changes occur, this could result in unnecessary repeated parsing. Consider using useMemo to cache the parsed user agent values based on the requests array to avoid redundant regex operations.
| Promise.all([fetchRequests(), fetchSources()]) | ||
| .then(([requestsData, sourcesData]) => { | ||
| setRequests(requestsData.requests); | ||
| const typeMap: Record<string, DatabaseType> = {}; | ||
| for (const source of sourcesData) { | ||
| typeMap[source.id] = source.type; | ||
| } | ||
| setSourceTypes(typeMap); | ||
| setIsLoading(false); | ||
| }) | ||
| .catch((err) => { | ||
| console.error('Failed to fetch requests:', err); | ||
| const message = err instanceof ApiError ? err.message : 'Failed to load requests'; | ||
| console.error('Failed to fetch data:', err); | ||
| const message = err instanceof ApiError ? err.message : 'Failed to load data'; | ||
| setError(message); | ||
| setIsLoading(false); | ||
| }); |
Copilot
AI
Dec 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using Promise.all means that if either fetchRequests or fetchSources fails, the entire data load fails and the user sees an error screen even if requests data could have been loaded successfully. Consider handling these API calls separately or implementing partial failure handling so users can still view request data even if source type information fails to load.
No description provided.