-
Notifications
You must be signed in to change notification settings - Fork 51
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
👷 Deploy request for kleros-v2-testnet pending review.Visit the deploys page to approve it
|
👷 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
|
👷 Deploy request for kleros-v2-neo pending review.Visit the deploys page to approve it
|
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
StatsContent
with 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
Stats
is correctly handled, please add React Testing Library tests covering:
- Spinner is displayed when either
useCourtDetails
oruseCoinPrice
is 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.tsx
I’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
isLoading
anderror
aligns the hook with the PR goal and enables the consumer to render proper UI states.
29-31
: Return shape extension looks good.Returning
isLoading
anderror
preserves 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
/error
from both hooks avoids collisions and reads well at call sites. No changes needed.
54-62
: No fallback needed fortheme.error
Verified that
theme.error
is 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.tsx
currently 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
isLoading
anderror
fromuseCourtDetails
hook (standard React Query)useCoinPrice
hook to returnisLoading
anderror
properties<Spinner />
component during data fetch for both sources<ErrorMessage>
component with theme-consistent stylingStatsContent
when both data sources are successfully loadedFiles Changed
web/src/hooks/useCoinPrice.tsx
- Enhanced to returnisLoading
anderror
web/src/pages/Courts/CourtDetails/Stats/index.tsx
- Added loading/error handlingPR-Codex overview
This PR enhances the
Stats
component by adding loading and error handling states, improving user experience during data fetching. It also refines thefetchCoinPrices
function to ensure proper handling of coin IDs and response errors.Detailed summary
Spinner
component for loading state inStats
.ErrorMessage
styled component for error display.useCourtDetails
to include loading and error states.useCoinPrice
to handle coin ID filtering and error handling.useCoinPrice
.Summary by CodeRabbit