refactor: simplify mobile data fetching with useOfflineData hook - #29
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors mobile dashboard pages to use a shared useOfflineData hook for cache-first data loading, reducing repeated localStorage + fetch-with-timeout logic across screens.
Changes:
- Introduces
src/hooks/use-offline-data.tsto centralize cache read/write + timed fetching and expose a consistent{ data, loading, error, retry }API. - Updates multiple mobile dashboard pages to replace local state +
useEffectfetch flows with the new hook. - Standardizes offline / skeleton / error rendering conditions around the hook outputs.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hooks/use-offline-data.ts | Adds the reusable cache-first data fetching hook used by mobile dashboard pages. |
| src/pages-mobile/dashboard/index.tsx | Migrates dashboard home aggregated fetching (CGPA/feedback/profile/trend) to useOfflineData. |
| src/pages-mobile/dashboard/timetable.tsx | Migrates timetable + attendance fetching to useOfflineData and consolidates retry logic. |
| src/pages-mobile/dashboard/profile.tsx | Migrates student profile fetching/caching to useOfflineData with custom emptiness check. |
| src/pages-mobile/dashboard/payment-receipts.tsx | Migrates payment receipts fetching to useOfflineData and preserves list cleanup via transform. |
| src/pages-mobile/dashboard/marks.tsx | Migrates marks fetching to useOfflineData and simplifies offline/error gating. |
| src/pages-mobile/dashboard/hod-dean.tsx | Migrates HOD/Dean details fetching/caching to useOfflineData. |
| src/pages-mobile/dashboard/grades.tsx | Migrates semester grades + history fetching to useOfflineData and syncs derived semester state. |
| src/pages-mobile/dashboard/exams.tsx | Migrates exam schedule fetching/caching to useOfflineData. |
| src/pages-mobile/dashboard/courses.tsx | Migrates registered courses fetching/caching to useOfflineData. |
| src/pages-mobile/dashboard/contact.tsx | Migrates contacts fetching/caching to useOfflineData. |
| src/pages-mobile/dashboard/attendance.tsx | Migrates attendance overview fetching/caching to useOfflineData. |
| src/pages-mobile/dashboard/attendance/[classId].tsx | Migrates attendance detail fetching/caching to useOfflineData. |
| src/pages-mobile/dashboard/academic-calendar.tsx | Migrates calendar options + monthly view fetching/caching to useOfflineData. |
| src/pages-mobile/dashboard/curriculum/index.tsx | Migrates curriculum categories fetching/caching to useOfflineData. |
| src/pages-mobile/dashboard/curriculum/[categoryId].tsx | Migrates curriculum category courses fetching/caching to useOfflineData. |
Suppressed comments (2)
src/hooks/use-offline-data.ts:90
- After introducing refs for
fetcher/transform,fetchDatashould call the ref-held functions (otherwise it still uses the potentially-stale closures).
const res = await fetchWithTimeout(fetcher(), timeout);
if (res?.success && res.data !== undefined && res.data !== null) {
const processed = transform ? transform(res.data) : res.data;
setData(processed);
writeCache(cacheKey, processed);
src/hooks/use-offline-data.ts:122
retryshould use the forced-loading refresh mode so callers can disable Retry buttons / show progress even when cached data is present.
const retry = useCallback(() => {
fetchData();
}, [fetchData]);
Comment on lines
+71
to
+74
| // Use ref to avoid stale closure issues in fetch | ||
| const dataRef = useRef(data); | ||
| dataRef.current = data; | ||
|
|
Comment on lines
+75
to
+83
| const fetchData = useCallback(async () => { | ||
| if (!enabled) return; | ||
|
|
||
| const currentData = dataRef.current; | ||
| const hasCache = hasData(currentData); | ||
|
|
||
| setError(null); | ||
| setLoading(!hasCache); | ||
|
|
Comment on lines
+165
to
+173
| // Update selected tab if groups change and current tab is not in new groups | ||
| useMemo(() => { | ||
| if (groups.length > 0) { | ||
| const tabNames = groups.map(g => g.examType); | ||
| if (!selectedTab || !tabNames.includes(selectedTab)) { | ||
| setSelectedTab(groups[0].examType); | ||
| } | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| } | ||
|
|
||
| useEffect(() => { | ||
| if (isLoggedIn) { | ||
| load(); | ||
| } | ||
| }, [isLoggedIn, authLoading]); | ||
| }, [groups, selectedTab]); |
Comment on lines
+114
to
+118
| useEffect(() => { | ||
| if (enabled) { | ||
| fetchData(); | ||
| } | ||
| }, [enabled]); |
…key bug in useOfflineData hook
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR extracts the repetitive cache-first SWR pattern from all mobile dashboard pages into a clean, reusable useOfflineData custom hook. Closes #28.