Add Auto-Refresh on Page Focus and Manual Refresh Button#204
Add Auto-Refresh on Page Focus and Manual Refresh Button#204codybrouwers wants to merge 5 commits intobenvinegar:mainfrom
Conversation
…eric support - Changed import statements for `AppLoadContext` and `PlatformProxy` to use type-only imports. - Updated `Cloudflare` type to be exported. - Enhanced `PaginatedTableCard` to accept generic types for better type safety. - Modified `TableCard` to handle `CountByProperty` more robustly. - Adjusted various route components to utilize the updated `PaginatedTableCard` and loader types, ensuring proper type definitions for `analyticsEngine` and `cloudflare` in loaders.
codybrouwers
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR implements auto-refresh functionality for the dashboard when users return to the page, along with a manual refresh button. The changes include refactoring the data fetching architecture by moving useFetcher into the PaginatedTableCard component and improving type safety throughout.
- Auto-refresh on page focus with debouncing to prevent excessive API calls
- Manual refresh button with loading state indication
- Refactored data fetching to be self-contained within
PaginatedTableCardwith improved type safety
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/server/app/routes/dashboard.tsx |
Added auto-refresh on focus/visibility change and manual refresh button with proper event handling |
packages/server/app/components/PaginatedTableCard.tsx |
Refactored to manage its own data fetching with generic typing and parameter change detection |
packages/server/app/components/TableCard.tsx |
Enhanced type handling for CountByProperty to support string/number values and improved type safety |
packages/server/app/load-context.ts |
Exported Cloudflare type for better type sharing |
| Resource route files | Updated to use new type-safe PaginatedTableCard interface, removing dataFetcher prop |
Comments suppressed due to low confidence (1)
packages/server/app/components/PaginatedTableCard.tsx:19
- [nitpick] The generic type parameter
Tis not descriptive. Consider renaming it toTLoaderorLoaderFunctionto better indicate its purpose as a loader function type.
const PaginatedTableCard = <T extends (...args: any[]) => Promise<{ countsByProperty: CountByProperty }>>({
| export async function loader({ context, request }: LoaderFunctionArgs<{ | ||
| analyticsEngine: AnalyticsEngineAPI; | ||
| }>) { | ||
| const { analyticsEngine } = context || {}; |
There was a problem hiding this comment.
Using optional chaining on context could lead to undefined behavior. The destructuring assignment will fail if context is null/undefined, making the null check ineffective. Consider checking context first: if (!context) throw new Error("Context is not defined"); const { analyticsEngine } = context;
| const { analyticsEngine } = context || {}; | |
| if (!context) throw new Error("Context is not defined"); | |
| const { analyticsEngine } = context; |
| lastParamsRef.current = fullUrl; | ||
| fetcher.load(fullUrl); | ||
| } | ||
| }, [fetcher.load, loaderUrl, siteId, interval, filters, timezone, page]); |
There was a problem hiding this comment.
Including fetcher.load in the dependency array can cause infinite re-renders since fetcher.load is not a stable reference and changes on each render. Remove fetcher.load from the dependency array as it's not needed for the callback's functionality.
| }, [fetcher.load, loaderUrl, siteId, interval, filters, timezone, page]); | |
| }, [loaderUrl, siteId, interval, filters, timezone, page]); |
Overview
This PR adds automatic data refreshing when users return to the dashboard tab/window, along with a manual refresh button for on-demand updates. To implement this cleanly, the data fetching architecture was refactored to move
useFetcherinto thePaginatedTableCardcomponent with improved type safety.Features Added
🔄 Auto-Refresh on Page Focus
visibilitychangeandfocusevents for comprehensive coverage🔃 Manual Refresh Button
Technical Changes
Architecture Refactoring
useFetchernow lives insidePaginatedTableCardinstead of being passed as a propPaginatedTableCardthat ensures type-safe data fetchingdataFetcherprop, making the component more self-containedComponent Updates
CountByPropertyto support both string and number valuesPaginatedTableCardinterfaceType System Improvements
CountByPropertytype for better type sharingImplementation Details