- Notifications
You must be signed in to change notification settings - Fork 50
feat: add loading and error states to stats component #2104
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
base: dev
Are you sure you want to change the base?
feat: add loading and error states to stats component #2104
Conversation
|
| Name | Link |
|---|---|
| 🔨 Latest commit | ea70e31 |
👷 Deploy request for kleros-v2-university pending review.Visit the deploys page to approve it
|
👷 Deploy request for kleros-v2-testnet-devtools pending review.Visit the deploys page to approve it
|
|
| Name | Link |
|---|---|
| 🔨 Latest commit | 70628fb |
WalkthroughAdds loading and error propagation to the coin-price hook and updates the Court Details Stats page to use those states, rendering a Spinner while loading and an ErrorMessage on failure; core rendering logic for stats remains unchanged. Changes
Sequence Diagram(s)sequenceDiagram autonumber participant U as User participant Stats as CourtDetails/Stats participant CD as useCourtDetails participant CP as useCoinPrice participant API as Backend U->>Stats: Open Court Details (Stats) activate Stats Stats->>CD: request court details (id) Stats->>CP: request coin prices (coinIds) par Fetch court details CD->>API: GET /court/:id API-->>CD: court data / error and Fetch coin prices CP->>API: GET /prices?ids=... API-->>CP: prices / error end CD-->>Stats: { data?, isLoading?, error? } CP-->>Stats: { prices?, isLoading?, error? } alt any isLoading == true Stats-->>U: Render Spinner else any error present Stats-->>U: Render ErrorMessage "Failed to load statistics." else Stats-->>U: Render StatsContent (court data + prices) end deactivate Stats Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File ( |
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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
web/src/pages/Courts/CourtDetails/Stats/index.tsx (4)
71-77: Prefer showing errors before the spinner to avoid masking failures while a sibling query is still loading.If prices fail fast but court details are still loading, the current order keeps users on a perpetual spinner until the other query settles. Show the error as soon as any query errors.
Apply this diff:
- if (isLoadingCourt || isLoadingPrices) { - return <Spinner />; - } - - if (courtError || pricesError) { - return <ErrorMessage>Failed to load statistics</ErrorMessage>; - } + if (courtError || pricesError) { + return <ErrorMessage role="alert" aria-live="assertive">Failed to load statistics.</ErrorMessage>; + } + + if (isLoadingCourt || isLoadingPrices) { + return <Spinner />; + }
17-17: Optional: consider Skeletons for perceived performance.If the rest of the app leans on Skeletons, swapping or augmenting
<Spinner />with a skeleton state can reduce layout shift and better match brand UX.
79-94: Follow-up: consider partial render on prices failure.Today any failure blocks the entire stats view. If court details succeed but prices fail, you could still render court stats and only degrade price-dependent fields. This is optional and product-driven.
If you’d like, I can sketch a version that conditionally renders
StatsContentwith price fields gated and an inline “Retry” for prices only.
1-97: Add RTL tests for loading, error, and success statesTo ensure the new conditional rendering in
Statsis correctly handled, please add React Testing Library tests covering:
- Spinner is displayed when either
useCourtDetailsoruseCoinPriceis loading- ErrorMessage is displayed when either hook returns an error
- The main content (
StatsContent) is rendered when both hooks succeed- (Optional) Verify that error state takes precedence over loading, if both occur simultaneously
Suggested location for tests:
•web/src/pages/Courts/CourtDetails/Stats/__tests__/index.test.tsxI’m happy to help set up mocks for
useCourtDetails,useCoinPrice, anduseIsDesktop—just let me know!
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
web/src/hooks/useCoinPrice.tsx(1 hunks)web/src/pages/Courts/CourtDetails/Stats/index.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
web/src/pages/Courts/CourtDetails/Stats/index.tsx (3)
web-devtools/src/styles/Theme.tsx (1)
theme(3-10)web/src/hooks/queries/useCourtDetails.ts (1)
useCourtDetails(33-45)web/src/hooks/useCoinPrice.tsx (1)
useCoinPrice(13-32)
🔇 Additional comments (4)
web/src/hooks/useCoinPrice.tsx (2)
16-21: Good addition: surface loading and error from React Query.Exposing
isLoadinganderroraligns the hook with the PR goal and enables the consumer to render proper UI states.
29-31: Return shape extension looks good.Returning
isLoadinganderrorpreserves backward compatibility while enabling consumers to branch on fetch state.web/src/pages/Courts/CourtDetails/Stats/index.tsx (2)
66-69: State naming and usage are clear.Aliasing
isLoading/errorfrom both hooks avoids collisions and reads well at call sites. No changes needed.
54-62: No fallback needed fortheme.errorVerified that
theme.erroris defined in both light (#F60C36) and dark (#FF5A78) themes inweb/src/styles/themes.ts, so relying on it is safe.
• Optional: consider using a shared Alert/Error component for consistency if the codebase provides one.
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
web/src/hooks/useCoinPrice.tsx (4)
16-26: Normalize coinIds in the queryKey to avoid cache misses when the array order changes.Right now the fetch normalizes IDs but the queryKey uses the raw array. If callers pass the same IDs in a different order, you’ll get unnecessary cache misses and refetches. Compute normalizedIds once and use it for queryKey, enabled, and queryFn.
Apply this diff:
export const useCoinPrice = (coinIds: string[]) => { - const { + const normalizedIds = Array.from(new Set(coinIds)).filter(Boolean).sort(); + const { data: prices, isError, isLoading, error, } = useQuery<Prices>({ - queryKey: ["coinPrice", coinIds], - enabled: Array.isArray(coinIds) && coinIds.length > 0, - queryFn: () => fetchCoinPrices(coinIds), + queryKey: ["coinPrice", normalizedIds], + enabled: normalizedIds.length > 0, + queryFn: () => fetchCoinPrices(normalizedIds), });
3-7: Pass AbortSignal through to fetch so queries cancel correctly.React Query provides an AbortSignal to the queryFn. Wiring it to fetch avoids wasted requests and race conditions when inputs change rapidly.
Apply this diff:
-const fetchCoinPrices = async (coinIds: readonly string[]): Promise<Prices> => { +const fetchCoinPrices = async ( + coinIds: readonly string[], + signal?: AbortSignal +): Promise<Prices> => { const ids = Array.from(new Set(coinIds)).filter(Boolean).sort().map(encodeURIComponent).join(","); if (!ids) return {}; - const response = await fetch(`https://coins.llama.fi/prices/current/${ids}?searchWidth=1h`); + const response = await fetch( + `https://coins.llama.fi/prices/current/${ids}?searchWidth=1h`, + { signal } + ); if (!response.ok) throw new Error(`Failed to fetch coin prices (${response.status})`); const data = await response.json(); return (data?.coins ?? {}) as Prices; };And in the hook:
- queryFn: () => fetchCoinPrices(coinIds), + queryFn: ({ signal }) => fetchCoinPrices(coinIds, signal),If you adopt the normalizedIds change, pass that instead of coinIds.
Also applies to: 23-25
16-33: Annotate the hook’s signature for API stability and readonly input.Make the input readonly and the return type explicit so consumers see that prices can be undefined and you keep a stable contract.
-export const useCoinPrice = (coinIds: string[]) => { +export const useCoinPrice = ( + coinIds: readonly string[] +): { + prices: Prices | undefined; + isError: boolean; + isLoading: boolean; + error: unknown; +} => {
23-26: Consider a short staleTime to reduce refetch churn.Prices don’t need millisecond freshness in most UIs. A 30–60s staleTime cuts refetches on focus/re-render without sacrificing UX. Tune as needed.
} = useQuery<Prices>({ queryKey: ["coinPrice", coinIds], enabled: Array.isArray(coinIds) && coinIds.length > 0, - queryFn: () => fetchCoinPrices(coinIds), + queryFn: () => fetchCoinPrices(coinIds), + staleTime: 60_000, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
web/src/hooks/useCoinPrice.tsx(1 hunks)
🔇 Additional comments (1)
web/src/hooks/useCoinPrice.tsx (1)
3-9: Solid fetch helper: normalization + HTTP error surfacing — aligns with the new error UI.Dedupe/sort/encode + early return prevents bad URLs; explicit response.ok check ensures React Query’s error path triggers. Looks good.
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.
lgtm
Background
The Stats component in
web/src/pages/Courts/CourtDetails/Stats/index.tsxcurrently does not handle loading and error states when fetching data from multiple sources (court details and coin prices). This improvement will help users understand when data is being loaded or if there are any issues with data fetching.Changes Made
Technical Implementation
isLoadinganderrorfromuseCourtDetailshook (standard React Query)useCoinPricehook to returnisLoadinganderrorproperties<Spinner />component during data fetch for both sources<ErrorMessage>component with theme-consistent stylingStatsContentwhen both data sources are successfully loadedFiles Changed
web/src/hooks/useCoinPrice.tsx- Enhanced to returnisLoadinganderrorweb/src/pages/Courts/CourtDetails/Stats/index.tsx- Added loading/error handlingPR-Codex overview
This PR focuses on enhancing the
Statscomponent by adding loading and error handling features, as well as improving thefetchCoinPricesfunction for better error management.Detailed summary
Spinnercomponent for loading state inStats.ErrorMessagestyled component for error display.useCourtDetailsanduseCoinPricehooks to manage loading and error states.fetchCoinPricesfunction for improved error handling and data fetching.Summary by CodeRabbit