-
Notifications
You must be signed in to change notification settings - Fork 46
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
feat: jurors page #1840
base: dev
Are you sure you want to change the base?
feat: jurors page #1840
Conversation
…g tweak, dashboard to profile
WalkthroughThe pull request introduces significant changes to the web application's routing and navigation structure. The primary modification involves replacing the Changes
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
❌ Deploy Preview for kleros-v2-neo failed. Why did it fail? →
|
❌ Deploy Preview for kleros-v2-university failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
❌ Deploy Preview for kleros-v2-testnet failed. Why did it fail? →
|
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: 3
🧹 Nitpick comments (10)
web/src/pages/Jurors/StatsAndFilters.tsx (2)
7-14
: Consider using semantic HTML for the container.While the styling is good, consider using
<section>
instead ofdiv
for better semantic meaning since this represents a distinct section of the page.-const Container = styled.div` +const Container = styled.section` display: flex; flex-wrap: wrap; gap: 8px; margin-top: ${responsiveSize(4, 8)}; margin-bottom: ${responsiveSize(16, 32)}; justify-content: space-between; `;
16-20
: Consider making props spreading more explicit.While props spreading works, it's better to be explicit about which props are being passed for better maintainability and type safety.
-const StatsAndFilters: React.FC<IStats> = ({ totalJurors }) => ( +const StatsAndFilters: React.FC<IStats> = ({ totalJurors }) => ( <Container> - <Stats {...{ totalJurors }} /> + <Stats totalJurors={totalJurors} /> </Container> );web/src/pages/Jurors/Stats.tsx (2)
13-18
: Enhance Field component reusability and accessibility.The Field component could be more reusable and accessible with these improvements:
- Add
htmlFor
attribute to label- Add ARIA attributes
- Make the value styling more flexible
-const Field: React.FC<{ label: string; value: string }> = ({ label, value }) => ( +interface FieldProps { + label: string; + value: string; + id?: string; + valueClassName?: string; +} + +const Field: React.FC<FieldProps> = ({ label, value, id = label.toLowerCase(), valueClassName }) => ( <FieldWrapper> - <StyledLabel>{label}</StyledLabel> - <small>{value}</small> + <StyledLabel htmlFor={id}>{label}</StyledLabel> + <small id={id} className={valueClassName} aria-label={`${label}: ${value}`}> + {value} + </small> </FieldWrapper> );
24-26
: Consider adding prop types validation.Add runtime validation for the totalJurors prop to ensure it's a positive number.
const Stats: React.FC<IStats> = ({ totalJurors }) => { + if (totalJurors < 0) { + console.warn('totalJurors should be a positive number'); + return null; + } return <Field label="Total" value={`${totalJurors} Jurors`} />; };web/src/pages/Home/TopJurors/JurorCard/JurorTitle.tsx (1)
37-37
: Consider extracting route constants.The profile link structure should be defined in a central location for better maintainability.
+const ROUTES = { + PROFILE: (address: string) => `/profile/1/desc/all?address=${address}`, +} as const; - const profileLink = `/profile/1/desc/all?address=${address}`; + const profileLink = ROUTES.PROFILE(address);web/src/pages/Home/TopJurors/index.tsx (2)
Line range hint
25-37
: Consider adding media query breakpoints for grid columns.The grid layout could be more responsive with multiple column breakpoints.
export const ListContainer = styled.div` display: flex; flex-direction: column; justify-content: center; ${landscapeStyle( () => css` display: grid; - grid-template-columns: 1fr; + grid-template-columns: repeat(auto-fit, minmax(300px, 1fr)); + gap: ${responsiveSize(16, 24)}; ` )} `;
38-40
: Consider adding text ellipsis for long labels.Add text overflow handling for better UI consistency.
export const StyledLabel = styled.label` font-size: 16px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; `;web/src/pages/Jurors/index.tsx (2)
38-38
: Consider making the jurors limit configurable.The hard-coded limit of 1000 jurors should be moved to a configuration constant for better maintainability and flexibility.
- const { data: queryJurors } = useTopUsersByCoherenceScore(1000); + const JURORS_LIMIT = 1000; // Move to config file + const { data: queryJurors } = useTopUsersByCoherenceScore(JURORS_LIMIT);
40-43
: Extract rank calculation to a utility function.The rank calculation logic should be moved to a separate utility function for reusability and better separation of concerns.
+const addRankToJurors = (jurors) => + jurors?.map((juror, index) => ({ + ...juror, + rank: index + 1, + })); - const topJurors = queryJurors?.users?.map((juror, index) => ({ - ...juror, - rank: index + 1, - })); + const topJurors = addRankToJurors(queryJurors?.users);web/src/pages/Profile/index.tsx (1)
100-102
: Consider improving the wallet connection message.The current message is quite basic. Consider providing more context about the benefits of connecting.
- To see your profile, connect first + Connect your wallet to view your profile and manage your juror activities
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
web/src/app.tsx
(2 hunks)web/src/components/EvidenceCard.tsx
(2 hunks)web/src/components/Popup/MiniGuides/JurorLevels.tsx
(1 hunks)web/src/layout/Header/navbar/Explore.tsx
(1 hunks)web/src/pages/Cases/CaseDetails/Voting/VotesDetails/AccordionTitle.tsx
(1 hunks)web/src/pages/Courts/CourtDetails/TopSearch.tsx
(2 hunks)web/src/pages/Home/TopJurors/JurorCard/JurorLevel.tsx
(1 hunks)web/src/pages/Home/TopJurors/JurorCard/JurorTitle.tsx
(1 hunks)web/src/pages/Home/TopJurors/index.tsx
(2 hunks)web/src/pages/Jurors/Search.tsx
(1 hunks)web/src/pages/Jurors/Stats.tsx
(1 hunks)web/src/pages/Jurors/StatsAndFilters.tsx
(1 hunks)web/src/pages/Jurors/index.tsx
(1 hunks)web/src/pages/Profile/JurorInfo/Header.tsx
(1 hunks)web/src/pages/Profile/index.tsx
(3 hunks)
✅ Files skipped from review due to trivial changes (4)
- web/src/pages/Profile/JurorInfo/Header.tsx
- web/src/components/Popup/MiniGuides/JurorLevels.tsx
- web/src/pages/Courts/CourtDetails/TopSearch.tsx
- web/src/pages/Home/TopJurors/JurorCard/JurorLevel.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
🔇 Additional comments (6)
web/src/layout/Header/navbar/Explore.tsx (1)
57-57
: LGTM! Navigation link addition aligns with new jurors page.The addition of the jurors link is well-placed in the navigation structure and correctly points to the new jurors page route.
web/src/pages/Cases/CaseDetails/Voting/VotesDetails/AccordionTitle.tsx (1)
91-91
: LGTM! Profile link update aligns with navigation restructuring.The replacement of dashboard link with profile link is consistent with the broader architectural changes in the PR.
Also applies to: 97-97
web/src/pages/Profile/index.tsx (1)
Line range hint
53-111
: LGTM! Component transition from Dashboard to Profile is well-implemented.The component maintains its core functionality while appropriately reflecting the new Profile context. The implementation correctly handles:
- Wallet connection states
- URL parameters and navigation
- Data fetching with proper pagination
- Conditional rendering based on user state
web/src/app.tsx (2)
17-19
: LGTM! Proper lazy loading implementation.The lazy loading of new components follows the established pattern and maintains consistency with other route components.
76-81
: LGTM! Profile route parameters are well-defined.The route pattern
profile/:page/:order/:filter
maintains consistency with the component's expectations and provides necessary navigation flexibility.web/src/components/EvidenceCard.tsx (1)
226-226
: LGTM! Profile link implementation is consistent.The transition from dashboard to profile link maintains the expected URL structure and properly handles address parameters.
However, let's verify that all dashboard links have been updated across the codebase:
Also applies to: 251-253
✅ Verification successful
Dashboard to profile link transition is complete
The codebase shows no remaining dashboard navigation links or routes, confirming that the transition to profile links is consistent. The only dashboard references found are in image asset paths which don't affect navigation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining dashboard references in routes or links rg -i "dashboard.*link|/dashboard" src/Length of output: 86
Script:
#!/bin/bash # Search for dashboard references in the web source directory rg -i "dashboard.*link|/dashboard|route.*dashboard" web/src/ # Also search specifically for route configurations ast-grep --pattern 'path = "/dashboard"' web/src/ ast-grep --pattern 'path: "/dashboard"' web/src/Length of output: 714
web/src/pages/Jurors/index.tsx
Outdated
<ListContainer> | ||
<Header /> | ||
{!isUndefined(topJurors) | ||
? topJurors.map((juror) => <JurorCard key={juror.rank} address={juror.id} {...juror} />) | ||
: [...Array(5)].map((_, i) => <SkeletonDisputeListItem key={i} />)} | ||
</ListContainer> |
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.
🛠️ Refactor suggestion
Consider implementing virtualization for better performance.
The current implementation renders all jurors at once, which could impact performance with a large dataset. Consider using a virtualization library like react-window
or react-virtualized
for better performance.
web/src/app.tsx
Outdated
path="jurors/*" | ||
element={ | ||
<Suspense fallback={<Loader width={"48px"} height={"48px"} />}> | ||
<Dashboard /> | ||
<Jurors /> | ||
</Suspense> | ||
} | ||
/> |
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.
💡 Codebase verification
Remove the wildcard pattern from the jurors route
The jurors/*
wildcard pattern is unnecessary as the Jurors component implements a single view without any nested routes or dynamic segments. Replace it with the exact path path="jurors"
.
🔗 Analysis chain
Verify the wildcard route pattern for jurors.
The jurors/*
wildcard route might be too permissive. Consider explicitly defining the expected subroutes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential jurors route definitions
rg -l "jurors/[^\*]" src/
Length of output: 72
Script:
#!/bin/bash
# Search for jurors-related routes and navigation
rg -l "jurors" web/src/
rg "to=\"/jurors" web/src/
ast-grep --pattern 'path="jurors' web/src/
Length of output: 1503
Script:
#!/bin/bash
# Check Jurors component implementation
cat web/src/pages/Jurors/index.tsx
# Look for any route definitions within Jurors
ast-grep --pattern 'Route' web/src/pages/Jurors/index.tsx
Length of output: 1960
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 (6)
web/src/pages/Home/CourtOverview/Stats.tsx (3)
40-41
: Consider adding input validation.The function export is a good change for reusability. However, consider adding input validation for undefined/null values to make it more robust.
export const getLastOrZero = (src: HomePageQuery["counters"], stat: HomePageQueryDataPoints) => - src.length > 0 ? src.at(-1)?.[stat] : 0n.toString(); + !src ? "0" : src.length > 0 ? src.at(-1)?.[stat] ?? "0" : "0";
Line range hint
52-89
: Consider performance optimizations.The stats configuration array could be memoized since it's static. Additionally, price calculations could benefit from memoization to prevent unnecessary recalculations on re-renders.
+import { useMemo } from "react"; + -const stats: IStat[] = [ +const useStats = (): IStat[] => useMemo(() => [ { title: "PNK Staked", coinId: 0, getText: (counters) => formatPNK(getLastOrZero(counters, "stakedPNK")), getSubtext: (counters, coinPrice) => formatUSD(Number(formatUnitsWei(getLastOrZero(counters, "stakedPNK"))) * (coinPrice ?? 0)), color: "purple", icon: PNKIcon, }, // ... other stats -]; +], []); const Stats = () => { + const stats = useStats(); // ... rest of the component };
Line range hint
90-109
: Well-structured implementation with good error handling.The component handles loading states and responsiveness well. Consider using a more specific type for the map index parameter.
- {stats.map(({ title, coinId, getText, getSubtext, color, icon }, i) => { + {stats.map(({ title, coinId, getText, getSubtext, color, icon }, i: number) => {web/src/hooks/queries/useTopUsersByCoherenceScore.ts (1)
Line range hint
20-39
: Consider making the hook more flexible and review caching strategy.The implementation is solid, but consider these improvements:
- Make
orderBy
andorderDirection
configurable through hook parameters to support different sorting options.- Review if
staleTime: Infinity
is appropriate - if coherence scores update frequently, you might want to reduce this value.Example implementation:
export const useTopUsersByCoherenceScore = ( first = 5, orderBy: "coherenceScore" | "totalResolvedVotes" = "coherenceScore", orderDirection: "asc" | "desc" = "desc", staleTime = Infinity ) => { // ... rest of the implementation }web/src/pages/Home/TopJurors/JurorCard/Coherence.tsx (2)
14-17
: Consider adding input validationThe
getPercent
function assumes valid number conversions. Consider adding validation for the string-to-number conversion.const getPercent = (num: number, den: number): string => { + if (isNaN(num) || isNaN(den)) return "0%"; if (den === 0) return "0%"; return `${Math.floor((num * 100) / den)}%`; };
29-29
: Consider moving type conversionMove the
Number()
conversions closer to the component's input to maintain consistent types throughout the component.- <Tooltip text={coherenceRatio}>{getPercent(Number(totalCoherentVotes), Number(totalResolvedVotes))}</Tooltip> + const numCoherentVotes = Number(totalCoherentVotes); + const numResolvedVotes = Number(totalResolvedVotes); + <Tooltip text={coherenceRatio}>{getPercent(numCoherentVotes, numResolvedVotes)}</Tooltip>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
web/src/hooks/queries/useTopUsersByCoherenceScore.ts
(1 hunks)web/src/pages/Home/CourtOverview/Stats.tsx
(1 hunks)web/src/pages/Home/TopJurors/JurorCard/Coherence.tsx
(1 hunks)web/src/pages/Home/TopJurors/JurorCard/DesktopCard.tsx
(1 hunks)web/src/pages/Home/TopJurors/JurorCard/JurorLevel.tsx
(2 hunks)web/src/pages/Home/TopJurors/JurorCard/MobileCard.tsx
(1 hunks)web/src/pages/Home/TopJurors/JurorCard/index.tsx
(1 hunks)web/src/pages/Jurors/index.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/pages/Jurors/index.tsx
- web/src/pages/Home/TopJurors/JurorCard/JurorLevel.tsx
🧰 Additional context used
📓 Learnings (1)
web/src/pages/Home/TopJurors/JurorCard/Coherence.tsx (1)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1739
File: web/src/pages/Home/TopJurors/JurorCard/Coherency.tsx:22-26
Timestamp: 2024-11-12T04:49:43.234Z
Learning: In the `Coherency` component (`web/src/pages/Home/TopJurors/JurorCard/Coherency.tsx`), `totalResolvedVotes` is always greater than or equal to `totalCoherentVotes`. When both are zero, `0/0` results in `NaN`, which is acceptable in this context.
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: dependency-review
- GitHub Check: SonarCloud
- GitHub Check: contracts-testing
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
web/src/hooks/queries/useTopUsersByCoherenceScore.ts (1)
12-12
: LGTM! Good addition of the filter condition.The addition of
where: { totalResolvedVotes_gt: 0 }
is a good improvement as it ensures the leaderboard only shows active jurors who have participated in voting. This makes the data more meaningful and relevant for the Jurors page.Let's verify if there are any other queries that might need similar filtering:
✅ Verification successful
The filter is correctly applied only where needed
The
totalResolvedVotes_gt: 0
filter is specific to the jurors leaderboard use case and doesn't need to be applied elsewhere. Other user-related queries in the codebase serve different purposes (authentication, user management, single user fetches) and don't require this filtering.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar GraphQL queries that might benefit from the same filter ast-grep --pattern 'query $_($first: Int!, $_) { users(first: $first, $_) { $$$ } }'Length of output: 94
Script:
#!/bin/bash # Search with a more flexible pattern for GraphQL queries ast-grep --pattern 'query $_ { users($_) { $$$ } }' # Backup search with ripgrep rg -l "query.*users.*\{" --type ts --type graphqlLength of output: 145
Script:
#!/bin/bash # Search for GraphQL queries in TypeScript files with context rg -A 5 -B 5 "gql|graphql\`.*users.*\`" --type ts # Specific search for the query structure rg -A 5 -B 5 "users\s*\([^)]*first:" --type tsLength of output: 9163
web/src/pages/Home/TopJurors/JurorCard/index.tsx (1)
9-12
: Verify data type conversion pointThe change from
number
tostring
types suggests these values are received as strings from an external source. Verify where the type conversion occurs (API, GraphQL, etc.) to ensure consistency and prevent multiple conversions.✅ Verification successful
Type conversion pattern is correct
The string types are appropriate as these metrics originate as BigInt in the subgraph, are transmitted through GraphQL as strings, and are only converted to numbers when numerical operations are needed (e.g., in Profile/JurorInfo components).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for GraphQL queries or API calls fetching these metrics rg -t typescript -t javascript "query.*\b(coherenceScore|totalCoherentVotes|totalResolvedVotes|totalResolvedDisputes)\b"Length of output: 157
Script:
#!/bin/bash # Search for all occurrences of these metrics without file type restriction rg "coherenceScore|totalCoherentVotes|totalResolvedVotes|totalResolvedDisputes" # Look for potential GraphQL query definitions ast-grep --pattern 'const $_ = gql` query $_ { $$$ } `' # Look for type definitions related to these metrics ast-grep --pattern 'type $_ { $$$ coherenceScore $$$ }'Length of output: 9935
web/src/pages/Home/TopJurors/JurorCard/DesktopCard.tsx (1)
42-45
: LGTM!The type changes are consistent with the parent component and the values are properly passed to child components.
web/src/pages/Home/TopJurors/JurorCard/MobileCard.tsx (1)
83-86
: LGTM!The type changes are consistent with the parent component and the desktop variant.
…on, levels miniguides tweak etc
Code Climate has analyzed commit c95ee8a and detected 62 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
Quality Gate passedIssues Measures |
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: 8
♻️ Duplicate comments (1)
web/src/pages/Jurors/Search.tsx (1)
32-45
:⚠️ Potential issueAdd input sanitization before URL encoding.
The search input should be sanitized before being encoded in the URL to prevent potential XSS attacks.
🧹 Nitpick comments (20)
web/src/pages/Jurors/Stats.tsx (3)
8-20
: Consider using theme constants for spacing values.While the styled components are well-structured, consider moving the gap values (8px, 4px) to theme constants for better maintainability and consistency across the application.
+// In your theme file +const theme = { + // ... other theme values + spacing: { + xs: '4px', + sm: '8px', + // ... other spacing values + } +} const FieldWrapper = styled.div` display: inline-flex; - gap: 8px; + gap: ${({ theme }) => theme.spacing.sm}; `; const ValueAndExtraLabel = styled.div` display: flex; - gap: 4px; + gap: ${({ theme }) => theme.spacing.xs}; `;
22-30
: Extract props interface and skeleton width constant.Consider these improvements for better maintainability:
- Extract the props interface
- Move the skeleton width to a constant
+interface FieldProps { + label: string; + value?: number; + extraLabel?: string; +} + +const SKELETON_WIDTH = 16; + -const Field: React.FC<{ label: string; value?: number; extraLabel?: string }> = ({ label, value, extraLabel }) => ( +const Field: React.FC<FieldProps> = ({ label, value, extraLabel }) => ( <FieldWrapper> <StyledLabel>{label}</StyledLabel> <ValueAndExtraLabel> - <small>{!isUndefined(value) ? value : <Skeleton width={16} />}</small> + <small>{!isUndefined(value) ? value : <Skeleton width={SKELETON_WIDTH} />}</small> {extraLabel ? <small>{extraLabel}</small> : null} </ValueAndExtraLabel> </FieldWrapper> );
32-40
: Simplify value handling and add prop documentation.The component logic can be simplified, and the interface would benefit from JSDoc documentation.
+/** + * Interface for Stats component props + * @property {number} [totalJurors] - The total number of jurors to display + */ export interface IStats { totalJurors?: number; } const Stats: React.FC<IStats> = ({ totalJurors }) => { - const value = !isUndefined(totalJurors) ? totalJurors : undefined; - return <Field label="Total" value={value} extraLabel="Jurors" />; + return <Field label="Total" value={totalJurors} extraLabel="Jurors" />; };web/src/layout/Header/navbar/Menu/Settings/General/WalletAndProfile.tsx (1)
41-53
: Consider adding prop types validation.The component accepts
toggleIsSettingsOpen
fromISettings
interface but doesn't validate it. Consider adding PropTypes or making the prop required in TypeScript.-const WalletAndProfile: React.FC<ISettings> = ({ toggleIsSettingsOpen }) => { +const WalletAndProfile: React.FC<Required<ISettings>> = ({ toggleIsSettingsOpen }) => {web/src/layout/Header/navbar/Menu/Settings/General/index.tsx (1)
73-73
: Consider destructuring props explicitly.The spread operator
{...{ toggleIsSettingsOpen }}
is less readable than explicit prop passing.-<WalletAndProfile {...{ toggleIsSettingsOpen }} /> +<WalletAndProfile toggleIsSettingsOpen={toggleIsSettingsOpen} />web/src/layout/Header/navbar/Menu/Settings/index.tsx (2)
94-98
: Refactor tab content rendering for better maintainability.The current implementation uses conditional rendering which could become unwieldy with more tabs. Consider using an object map or switch statement.
- {currentTab === 0 ? ( - <General {...{ toggleIsSettingsOpen }} /> - ) : ( - <NotificationSettings {...{ toggleIsSettingsOpen }} /> - )} + { + { + 0: <General toggleIsSettingsOpen={toggleIsSettingsOpen} />, + 1: <NotificationSettings toggleIsSettingsOpen={toggleIsSettingsOpen} /> + }[currentTab] + }Or better yet, map the tabs configuration to components:
const TAB_COMPONENTS = { GENERAL: { text: "General", value: 0, component: General }, NOTIFICATIONS: { text: "Notifications", value: 1, component: NotificationSettings } } as const; const TABS = Object.values(TAB_COMPONENTS).map(({ text, value }) => ({ text, value }));Then render:
const TabComponent = Object.values(TAB_COMPONENTS).find( tab => tab.value === currentTab )?.component; return TabComponent && <TabComponent toggleIsSettingsOpen={toggleIsSettingsOpen} />;
Line range hint
65-71
: Consider memoizing the click away handler.The click away handler creates a new function on each render that navigates and calls
toggleIsSettingsOpen
.+const handleClickAway = useCallback(() => { + toggleIsSettingsOpen(); + if (location.hash.includes("#notifications")) { + navigate("#", { replace: true }); + } +}, [toggleIsSettingsOpen, location.hash, navigate]); -useClickAway(containerRef, () => { - toggleIsSettingsOpen(); - if (location.hash.includes("#notifications")) navigate("#", { replace: true }); -}); +useClickAway(containerRef, handleClickAway);web/src/pages/Home/TopJurors/JurorCard/JurorTitle.tsx (2)
26-31
: Check hover color consistency.
You're overriding color in the label on hover. Verify this styling is consistent with the rest of the app theme and doesn't conflict with other link states (focus, visited, etc.).
53-56
: Add alt or accessibility text to the arrow icon.
If feasible, implement accessible text or aria-label for<ArrowIcon />
, so screen readers interpret the icon meaningfully.- <ArrowIcon /> + <ArrowIcon aria-label="View Profile" role="img" />web/src/hooks/queries/useTotalLeaderboardJurors.ts (1)
17-32
: Ensure robust handling of missing data.
WhilestaleTime
is set toInfinity
to minimize refetching, ensure there's a strategy for manual invalidation or updates if the data changes. Handling potential fetch failures might also be beneficial.web/src/utils/userLevelCalculation.ts (1)
24-25
: Edge-case handling for boundary conditions.
Ensure that boundary checks for>= minCoherencePercentage
and<= maxCoherencePercentage
do not accidentally place borderline users in multiple categories. Add tests for exactly 70%, 80%, etc.web/src/hooks/queries/useJurorsByCoherenceScore.ts (2)
10-26
: Consider adding a minimum threshold for coherence score.The query filters users with
totalResolvedVotes_gt: 0
but might benefit from a minimum coherence score threshold to ensure quality data in the leaderboard.- where: { totalResolvedVotes_gt: 0 } + where: { totalResolvedVotes_gt: 0, coherenceScore_gt: "0.5" }
28-50
: Consider caching strategy for real-time updates.The current implementation uses
staleTime: Infinity
which means the data will never be considered stale. This might not be ideal for a leaderboard that should reflect real-time changes.- staleTime: Infinity, + staleTime: 60 * 1000, // Stale after 1 minute + cacheTime: 5 * 60 * 1000, // Cache for 5 minutesweb/src/pages/Jurors/DisplayJurors.tsx (2)
44-51
: Consider memoizing rank calculation.The rank calculation could be optimized by using a separate memoized value for the base rank.
+ const baseRank = useMemo(() => jurorSkip + 1, [jurorSkip]); const jurors = useMemo( () => queryJurors?.users?.map((juror, index) => ({ ...juror, - rank: jurorSkip + index + 1, + rank: baseRank + index, })), - [queryJurors, jurorSkip] + [queryJurors, baseRank] );
70-72
: Add loading state handling.The component could benefit from a proper loading state indicator rather than just skeleton items.
+ const isLoading = !isUndefined(jurors); {!isUndefined(jurors) ? jurors.map((juror) => <JurorCard key={juror.rank} address={juror.id} {...juror} />) - : [...Array(jurorsPerPage)].map((_, i) => <SkeletonDisputeListItem key={i} />)} + : <LoadingState> + {[...Array(jurorsPerPage)].map((_, i) => ( + <SkeletonDisputeListItem key={i} /> + ))} + </LoadingState>}web/src/pages/Home/TopJurors/JurorCard/Coherence.tsx (2)
14-17
: Consider usingMath.round
for more accurate percentage display.The current implementation uses
Math.floor
which always rounds down. For percentages,Math.round
would provide more accurate representation (e.g., 74.6% → 75% instead of 74%).- return `${Math.floor((num * 100) / den)}%`; + return `${Math.round((num * 100) / den)}%`;
20-21
: Consider using branded types for vote counts.Using string type for vote counts loses type safety. Consider using branded types to ensure type safety while maintaining string representation.
type VoteCount = string & { readonly __brand: unique symbol }; interface ICoherence { totalCoherentVotes: VoteCount; totalResolvedVotes: VoteCount; }web/src/pages/Home/TopJurors/JurorCard/JurorLevel.tsx (1)
48-50
: Consider moving string to number conversion to a utility function.The conversion from string to number is done multiple times. Consider extracting it to a utility function to ensure consistent handling across the codebase.
+const toNumber = (value: string) => { + const num = Number(value); + return isNaN(num) ? 0 : num; +}; + const JurorLevel: React.FC<IJurorLevel> = ({ totalCoherentVotes, totalResolvedVotes, totalResolvedDisputes }) => { - const coherencePercentage = getPercent(Number(totalCoherentVotes), Number(totalResolvedVotes)); - const userLevelData = getUserLevelData(coherencePercentage, Number(totalResolvedDisputes)); + const coherencePercentage = getPercent(toNumber(totalCoherentVotes), toNumber(totalResolvedVotes)); + const userLevelData = getUserLevelData(coherencePercentage, toNumber(totalResolvedDisputes));web/src/pages/Jurors/index.tsx (1)
72-72
: Avoid hardcoding skeleton height.The skeleton height is hardcoded to 1000px which might not match the actual content height. Consider using a dynamic height based on the viewport or expected content size.
-<Skeleton height={1000} /> +<Skeleton height={`${Math.min(window.innerHeight * 0.8, 1000)}px`} count={3} />web/src/pages/Home/TopJurors/JurorCard/MobileCard.tsx (1)
104-104
: Avoid props spreading for better maintainability.Props spreading can make it harder to track which props are being used. Consider explicitly passing the required props.
-<JurorLevel {...{ totalCoherentVotes, totalResolvedVotes, totalResolvedDisputes }} /> +<JurorLevel + totalCoherentVotes={totalCoherentVotes} + totalResolvedVotes={totalResolvedVotes} + totalResolvedDisputes={totalResolvedDisputes} +/>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
subgraph/core/schema.graphql
(1 hunks)subgraph/core/src/KlerosCore.ts
(3 hunks)subgraph/core/src/datapoint.ts
(3 hunks)web/src/app.tsx
(2 hunks)web/src/components/Popup/MiniGuides/JurorLevels.tsx
(2 hunks)web/src/hooks/queries/useJurorsByCoherenceScore.ts
(1 hunks)web/src/hooks/queries/useTopUsersByCoherenceScore.ts
(0 hunks)web/src/hooks/queries/useTotalLeaderboardJurors.ts
(1 hunks)web/src/layout/Header/navbar/Explore.tsx
(1 hunks)web/src/layout/Header/navbar/Menu/Settings/General/WalletAndProfile.tsx
(1 hunks)web/src/layout/Header/navbar/Menu/Settings/General/index.tsx
(2 hunks)web/src/layout/Header/navbar/Menu/Settings/index.tsx
(1 hunks)web/src/pages/Home/TopJurors/JurorCard/Coherence.tsx
(1 hunks)web/src/pages/Home/TopJurors/JurorCard/DesktopCard.tsx
(2 hunks)web/src/pages/Home/TopJurors/JurorCard/JurorLevel.tsx
(2 hunks)web/src/pages/Home/TopJurors/JurorCard/JurorTitle.tsx
(3 hunks)web/src/pages/Home/TopJurors/JurorCard/MobileCard.tsx
(2 hunks)web/src/pages/Home/TopJurors/index.tsx
(3 hunks)web/src/pages/Jurors/DisplayJurors.tsx
(1 hunks)web/src/pages/Jurors/Search.tsx
(1 hunks)web/src/pages/Jurors/Stats.tsx
(1 hunks)web/src/pages/Jurors/StatsAndFilters.tsx
(1 hunks)web/src/pages/Jurors/index.tsx
(1 hunks)web/src/pages/Profile/JurorInfo/index.tsx
(2 hunks)web/src/utils/userLevelCalculation.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- web/src/hooks/queries/useTopUsersByCoherenceScore.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- web/src/layout/Header/navbar/Explore.tsx
- web/src/pages/Jurors/StatsAndFilters.tsx
- web/src/components/Popup/MiniGuides/JurorLevels.tsx
- web/src/pages/Home/TopJurors/index.tsx
- subgraph/core/src/KlerosCore.ts
- web/src/pages/Home/TopJurors/JurorCard/DesktopCard.tsx
🧰 Additional context used
📓 Learnings (1)
web/src/pages/Home/TopJurors/JurorCard/Coherence.tsx (1)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1739
File: web/src/pages/Home/TopJurors/JurorCard/Coherency.tsx:22-26
Timestamp: 2024-11-12T04:49:43.234Z
Learning: In the `Coherency` component (`web/src/pages/Home/TopJurors/JurorCard/Coherency.tsx`), `totalResolvedVotes` is always greater than or equal to `totalCoherentVotes`. When both are zero, `0/0` results in `NaN`, which is acceptable in this context.
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Analyze (javascript)
- GitHub Check: dependency-review
- GitHub Check: SonarCloud
- GitHub Check: contracts-testing
🔇 Additional comments (19)
web/src/pages/Jurors/Stats.tsx (1)
1-6
: LGTM! Clean and well-organized imports.The imports are properly organized, following the pattern of external dependencies first, followed by internal utilities.
web/src/layout/Header/navbar/Menu/Settings/General/index.tsx (1)
61-62
: Add error handling for disconnected state.The component uses
address
fromuseAccount
but doesn't handle the loading or error states from the hook.web/src/pages/Home/TopJurors/JurorCard/JurorTitle.tsx (3)
4-5
: Import for Arrow Icon is appropriate.
No issues here; the introduced SVG icon is consistent with the new link styling.
7-8
: Confirm usage ofStyledArrowLink
anduseAccount
.
The usage ofStyledArrowLink
fromcomponents/StyledArrowLink
and thewagmi
hook appears correct, but ensure all related dependencies and their versions are aligned in your package.
44-48
: Conditional link logic looks good.
The approach for matching the connected address and adjusting the route accordingly is clear. Consider whether we need additional checks for edge cases (e.g., empty addresses).web/src/hooks/queries/useTotalLeaderboardJurors.ts (4)
1-2
: ImportinguseQuery
from React Query is correct.
This is a good approach for data caching and refetch. Ensure pinned package versions for consistency.
3-4
: Use of GraphqlBatcher context.
You are correctly leveraging the GraphQL batcher for composable queries. Confirm that this context is provided at a top-level to avoid runtime errors.
5-8
: GraphQL definitions are well-structured.
Exporting the query type ensures type safety. Watch for automatic code generation if you rely on a codegen process.
9-15
: Check for error handling in the query.
The query shape looks correct. Consider verifying ifcounter
can benull
or iftotalLeaderboardJurors
can be missing, and gracefully handle that if needed.web/src/utils/userLevelCalculation.ts (2)
5-6
: Coherence-based thresholds.
Switching tominCoherencePercentage
andmaxCoherencePercentage
makes sense but double-check boundary overlaps across multiple levels.
17-20
: Parsing string percentage is fine.
ThepercentageToNumber
approach is straightforward. Confirm that all callers reliably provide the string in a “XX%” format to avoid parsing errors.web/src/hooks/queries/useJurorsByCoherenceScore.ts (1)
1-8
: LGTM! Clean imports and type exports.The imports are well-organized and the type export is properly defined.
subgraph/core/src/datapoint.ts (1)
18-18
: LGTM! Consistent variable handling.The totalLeaderboardJurors variable is properly integrated into the existing variables array and initialization logic.
Also applies to: 47-47
web/src/pages/Home/TopJurors/JurorCard/JurorLevel.tsx (1)
8-8
: Verify the new import path.The import path has changed from
Dashboard
toProfile
. Ensure all references to this component have been updated accordingly.✅ Verification successful
Import path change is correctly implemented
All references to the PixelArt component are using the new path from the Profile directory. No instances of the old Dashboard path remain in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to the old path rg -l "pages/Dashboard/JurorInfo/PixelArt"Length of output: 42
Script:
#!/bin/bash # Check if the new file exists fd PixelArt.tsx # Find all imports of PixelArt rg -l "import.*PixelArt" # Search for any variations of the path rg -l "JurorInfo/PixelArt"Length of output: 370
Script:
#!/bin/bash # Check the actual import statements in all files rg "import.*PixelArt" -A 1 -B 1Length of output: 849
web/src/pages/Home/TopJurors/JurorCard/MobileCard.tsx (1)
83-86
: LGTM: Type changes align with other components.The interface changes from
number
tostring
types align with the changes in other components, maintaining consistency across the codebase.web/src/app.tsx (3)
17-17
: LGTM! Components are correctly lazy loaded.The Profile and Jurors components are properly imported using lazy loading, which helps with code splitting and initial load performance.
Also applies to: 19-19
68-74
: LGTM! Route parameters are well-structured.The jurors route with path parameters
:page/:order/:filter
provides good flexibility for pagination, sorting, and filtering functionality.
75-81
: LGTM! Profile route maintains consistency.The profile route follows the same parameter structure as the jurors route, maintaining consistency in the routing pattern.
subgraph/core/schema.graphql (1)
240-240
: LGTM! Field addition is well-defined.The
totalLeaderboardJurors
field is appropriately typed as non-nullableBigInt!
and follows the existing pattern of counter metrics in the entity.Let's verify that the field is being properly updated in the codebase:
✅ Verification successful
Field is properly implemented and integrated
The
totalLeaderboardJurors
field is correctly implemented with:
- Initialization and update logic in subgraph mappings
- Proper frontend integration with type safety
- Complete data flow from contract events to UI
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to totalLeaderboardJurors in the codebase echo "Searching for field updates in the subgraph mappings..." rg "totalLeaderboardJurors" subgraph/ echo "Searching for field usage in GraphQL queries..." rg "totalLeaderboardJurors" web/src/Length of output: 1839
<IdenticonOrAvatar /> | ||
<AddressOrName /> | ||
</AvatarAndAddressContainer> | ||
<ReStyledArrowLink to={"/profile/1/desc/all"} onClick={toggleIsSettingsOpen}> |
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.
💡 Codebase verification
The hardcoded profile route contradicts the router configuration
The route /profile/1/desc/all
uses hardcoded values, but the router in app.tsx
defines this as a dynamic route: profile/:page/:order/:filter
. This suggests these parameters should be configurable:
:page
instead of hardcoded "1":order
instead of hardcoded "desc":filter
instead of hardcoded "all"
🔗 Analysis chain
Verify the hardcoded profile route.
The profile route /profile/1/desc/all
appears to be hardcoded. This could be problematic if:
- The route structure changes
- The user ID (
1
) should be dynamic - The sort order (
desc
) or filter (all
) should be configurable
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other occurrences of this route pattern
rg -g '*.{ts,tsx,js,jsx}' '/profile/\d+/(desc|asc)/(all|[a-z]+)'
# Search for route definitions to verify the structure
ast-grep --pattern 'Route { path: "/profile/$_" }'
Length of output: 781
Script:
#!/bin/bash
# Search for router configuration files
fd -e tsx -e ts -e js router
# Search for profile-related route definitions
rg -g '*.{ts,tsx,js,jsx}' -B 2 -A 2 'path.*profile'
# Check for any constants or utilities related to profile routes
rg -g '*.{ts,tsx,js,jsx}' 'const.*profile.*path|const.*PROFILE.*PATH'
Length of output: 484
{ level: 4, title: "Aristotle", minDisputes: 10, minCoherencePercentage: 90, maxCoherencePercentage: 100 }, | ||
{ level: 3, title: "Plato", minDisputes: 7, minCoherencePercentage: 80, maxCoherencePercentage: 100 }, | ||
{ level: 2, title: "Socrates", minDisputes: 3, minCoherencePercentage: 70, maxCoherencePercentage: 100 }, | ||
{ level: 1, title: "Pythagoras", minDisputes: 1, minCoherencePercentage: 0, maxCoherencePercentage: 70 }, | ||
{ level: 0, title: "Diogenes", minDisputes: 3, minCoherencePercentage: 0, maxCoherencePercentage: 49 }, |
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.
Validate range overlaps.
Your definitions for e.g., “Plato” (80-100) and “Socrates” (70-100) overlap at 80-100. Verify whether this is intentional. You may want distinct ranges for each level.
const coherencePercentage = getPercent(Number(totalCoherentVotes), Number(totalResolvedVotes)); | ||
const userLevelData = getUserLevelData(coherencePercentage, totalResolvedDisputes); |
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.
🛠️ Refactor suggestion
Ensure safe number conversion for coherence calculation.
The current implementation might throw if the string values can't be converted to numbers. Consider adding error handling.
- const coherencePercentage = getPercent(Number(totalCoherentVotes), Number(totalResolvedVotes));
+ const coherencePercentage = (() => {
+ try {
+ return getPercent(Number(totalCoherentVotes), Number(totalResolvedVotes));
+ } catch (error) {
+ console.error("Error calculating coherence percentage:", error);
+ return 0;
+ }
+ })();
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const coherencePercentage = getPercent(Number(totalCoherentVotes), Number(totalResolvedVotes)); | |
const userLevelData = getUserLevelData(coherencePercentage, totalResolvedDisputes); | |
const coherencePercentage = (() => { | |
try { | |
return getPercent(Number(totalCoherentVotes), Number(totalResolvedVotes)); | |
} catch (error) { | |
console.error("Error calculating coherence percentage:", error); | |
return 0; | |
} | |
})(); | |
const userLevelData = getUserLevelData(coherencePercentage, totalResolvedDisputes); |
const jurorsPerPage = isDesktop ? 20 : 10; | ||
const currentPage = parseInt(page ?? "1"); | ||
|
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.
Add validation for page parameter.
The current implementation assumes the page parameter is always a valid number. Add validation to handle invalid inputs.
- const currentPage = parseInt(page ?? "1");
+ const currentPage = (() => {
+ const parsed = parseInt(page ?? "1");
+ return isNaN(parsed) || parsed < 1 ? 1 : parsed;
+ })();
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const jurorsPerPage = isDesktop ? 20 : 10; | |
const currentPage = parseInt(page ?? "1"); | |
const jurorsPerPage = isDesktop ? 20 : 10; | |
const currentPage = (() => { | |
const parsed = parseInt(page ?? "1"); | |
return isNaN(parsed) || parsed < 1 ? 1 : parsed; | |
})(); |
export function updateTotalLeaderboardJurors(delta: BigInt, timestamp: BigInt): void { | ||
updateDataPoint(delta, timestamp, "totalLeaderboardJurors"); | ||
} |
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.
🛠️ Refactor suggestion
Consider adding validation for negative deltas.
The updateTotalLeaderboardJurors function should validate that the total doesn't become negative.
export function updateTotalLeaderboardJurors(delta: BigInt, timestamp: BigInt): void {
+ const counter = Counter.load("0");
+ if (counter && delta.lt(ZERO)) {
+ const currentTotal = counter.totalLeaderboardJurors;
+ if (currentTotal.plus(delta).lt(ZERO)) {
+ return;
+ }
+ }
updateDataPoint(delta, timestamp, "totalLeaderboardJurors");
}
Committable suggestion skipped: line range outside the PR's diff.
export const getPercent = (num: number, den: number): string => { | ||
if (den === 0) return "0%"; | ||
return `${Math.floor((num * 100) / den)}%`; | ||
}; |
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.
🛠️ Refactor suggestion
Add input validation for negative numbers.
The function should validate that both inputs are non-negative numbers, as negative votes don't make sense in this context.
export const getPercent = (num: number, den: number): string => {
+ if (num < 0 || den < 0) return "0%";
if (den === 0) return "0%";
+ if (num > den) num = den; // Ensure numerator doesn't exceed denominator
return `${Math.floor((num * 100) / den)}%`;
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const getPercent = (num: number, den: number): string => { | |
if (den === 0) return "0%"; | |
return `${Math.floor((num * 100) / den)}%`; | |
}; | |
export const getPercent = (num: number, den: number): string => { | |
if (num < 0 || den < 0) return "0%"; | |
if (den === 0) return "0%"; | |
if (num > den) num = den; // Ensure numerator doesn't exceed denominator | |
return `${Math.floor((num * 100) / den)}%`; | |
}; |
useDebounce( | ||
() => { | ||
if (initialRenderRef.current && isEmpty(search)) { | ||
initialRenderRef.current = false; | ||
return; | ||
} | ||
initialRenderRef.current = false; | ||
const newFilters = isEmpty(search) ? { ...filterObject } : { ...filterObject, id: search }; | ||
const encodedFilter = encodeURIFilter(newFilters); | ||
navigate(`${location}/${page}/${order}/${encodedFilter}?${searchParams.toString()}`); | ||
}, | ||
500, | ||
[search] | ||
); |
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.
🛠️ Refactor suggestion
Consider using AbortController with useDebounce.
The current debounce implementation doesn't handle component unmounting or rapid navigation changes. Consider using AbortController to cancel pending operations.
+const abortController = useRef<AbortController>();
useDebounce(
() => {
+ if (abortController.current) {
+ abortController.current.abort();
+ }
+ abortController.current = new AbortController();
+
if (initialRenderRef.current && isEmpty(search)) {
initialRenderRef.current = false;
return;
}
initialRenderRef.current = false;
const newFilters = isEmpty(search) ? { ...filterObject } : { ...filterObject, id: search };
const encodedFilter = encodeURIFilter(newFilters);
- navigate(`${location}/${page}/${order}/${encodedFilter}?${searchParams.toString()}`);
+ navigate(`${location}/${page}/${order}/${encodedFilter}?${searchParams.toString()}`, {
+ signal: abortController.current.signal
+ });
},
500,
[search]
);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
useDebounce( | |
() => { | |
if (initialRenderRef.current && isEmpty(search)) { | |
initialRenderRef.current = false; | |
return; | |
} | |
initialRenderRef.current = false; | |
const newFilters = isEmpty(search) ? { ...filterObject } : { ...filterObject, id: search }; | |
const encodedFilter = encodeURIFilter(newFilters); | |
navigate(`${location}/${page}/${order}/${encodedFilter}?${searchParams.toString()}`); | |
}, | |
500, | |
[search] | |
); | |
const abortController = useRef<AbortController>(); | |
useDebounce( | |
() => { | |
if (abortController.current) { | |
abortController.current.abort(); | |
} | |
abortController.current = new AbortController(); | |
if (initialRenderRef.current && isEmpty(search)) { | |
initialRenderRef.current = false; | |
return; | |
} | |
initialRenderRef.current = false; | |
const newFilters = isEmpty(search) ? { ...filterObject } : { ...filterObject, id: search }; | |
const encodedFilter = encodeURIFilter(newFilters); | |
navigate(`${location}/${page}/${order}/${encodedFilter}?${searchParams.toString()}`, { | |
signal: abortController.current.signal | |
}); | |
}, | |
500, | |
[search] | |
); |
{!isUndefined(totalLeaderboardJurors) && Number(totalLeaderboardJurors) > 0 ? ( | ||
<DisplayJurors totalLeaderboardJurors={Number(totalLeaderboardJurors)} /> | ||
) : ( | ||
<Skeleton height={1000} /> | ||
)} |
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.
🛠️ Refactor suggestion
Improve loading state handling.
The current implementation shows a skeleton only when totalLeaderboardJurors
is undefined or zero. Consider showing a loading state while the query is in progress.
-{!isUndefined(totalLeaderboardJurors) && Number(totalLeaderboardJurors) > 0 ? (
+{queryTotalLeaderBoardJurors.isLoading ? (
+ <Skeleton height={`${Math.min(window.innerHeight * 0.8, 1000)}px`} count={3} />
+) : !isUndefined(totalLeaderboardJurors) && Number(totalLeaderboardJurors) > 0 ? (
<DisplayJurors totalLeaderboardJurors={Number(totalLeaderboardJurors)} />
) : (
- <Skeleton height={1000} />
+ <div>No jurors found</div>
)}
Committable suggestion skipped: line range outside the PR's diff.
PR-Codex overview
This PR focuses on refactoring the dashboard into a profile section, updating various components and queries to reflect this change. It also introduces new functionalities related to juror statistics and leaderboard management.
Detailed summary
Dashboard
toProfile
in multiple files.totalLeaderboardJurors
field to the GraphQL schema.useTotalLeaderboardJurors
anduseJurorsByCoherenceScore
hooks.number
tostring
.Summary by CodeRabbit
Release Notes
New Features
Navigation Changes
UI Improvements
Performance