🐛 Fix missing useCardLoadingState params across 3 cards#4330
🐛 Fix missing useCardLoadingState params across 3 cards#4330clubanderson merged 1 commit intomainfrom
Conversation
- EtcdStatus: add missing isRefreshing to useCachedPods destructuring and useCardLoadingState call - AdmissionWebhooks: add isRefreshing state tracking to useAdmissionWebhooks hook and pass it to useCardLoadingState - QuotaHeatmap: add useCardLoadingState call entirely (was missing), replacing hand-rolled loading check with showSkeleton, and wire up isRefreshing, isDemoFallback, isFailed, consecutiveFailures Fixes #4322, Fixes #4310, Fixes #4305 Signed-off-by: Andrew Anderson <andy@clubanderson.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR aligns three dashboard cards (EtcdStatus, AdmissionWebhooks, QuotaHeatmap) with the shared useCardLoadingState / CardWrapper state-reporting contract so header-level UX (refresh animation, demo/failure indicators) stays in sync with each card’s underlying data lifecycle.
Changes:
- Wire
isRefreshingintouseCardLoadingStatefor EtcdStatus and AdmissionWebhooks so the CardWrapper header refresh indicator can animate during background refresh. - Add
isRefreshingtracking to the useAdmissionWebhooks hook API/result. - Add a missing
useCardLoadingStateintegration to QuotaHeatmap, replacing a hand-rolled skeleton condition withshowSkeletonand reporting demo/failure/refresh state.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| web/src/hooks/useAdmissionWebhooks.ts | Adds isRefreshing state to the hook result and toggles it during silent/background refetches. |
| web/src/components/cards/AdmissionWebhooks.tsx | Passes isRefreshing into useCardLoadingState so CardWrapper can animate refresh and show correct status badges. |
| web/src/components/cards/EtcdStatus.tsx | Destructures and forwards isRefreshing from useCachedPods() into useCardLoadingState. |
| web/src/components/cards/QuotaHeatmap.tsx | Integrates useCardLoadingState and uses showSkeleton to connect the card to CardWrapper’s unified state system. |
| const refetch = useCallback(async (silent = false) => { | ||
| if (!silent && !initialLoadDone.current) { | ||
| setIsLoading(true) | ||
| } | ||
| if (silent) { | ||
| setIsRefreshing(true) | ||
| } | ||
|
|
There was a problem hiding this comment.
isRefreshing was added and is now used to drive CardWrapper refresh animation, but there’s no unit test asserting the new behavior (e.g., refetch(true) sets isRefreshing true while the request is in-flight and resets it afterward). Since this hook already has a dedicated test file, please add a test to prevent regressions in the refresh-indicator wiring.
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 0 code suggestion(s) and 1 general comment(s). Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
Summary
isRefreshingtouseCachedPods()destructuring anduseCardLoadingStatecall — the refresh icon in the CardWrapper header never animated during background data updates.isRefreshingstate tracking to theuseAdmissionWebhookshook (interface, state, set/clear in refetch) and pass it through touseCardLoadingState— same refresh icon animation bug.useCardLoadingStatecall entirely (was completely missing), replacing the hand-rolledisLoading && pods.length === 0check withshowSkeleton. Wire upisRefreshing,isDemoFallback,isFailed, andconsecutiveFailures— card was disconnected from the CardWrapper state system (no demo badge, no yellow outline, no refresh icon, no failure badge).Note: Issues #4314, #4307, #4303, and #4300 reference WarningEvents, NodeConditions, DNSHealth, and NetworkPolicyCoverage respectively. These cards already have the correct wiring on
main— the fixes were likely applied in prior PRs. Only 3 cards (EtcdStatus, AdmissionWebhooks, QuotaHeatmap) still needed fixes.Fixes #4322, Fixes #4310, Fixes #4305
Test plan
npm run build)