-
Notifications
You must be signed in to change notification settings - Fork 49
feat(web): jurors staked in this court section #1969
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
Conversation
…ases page, fix tiny padding iss
❌ Deploy Preview for kleros-v2-university failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
❌ Deploy Preview for kleros-v2-neo failed. Why did it fail? →
|
WalkthroughThis update introduces a new "Jurors Staked by Court" feature, including paginated, searchable, and sortable juror lists for specific courts with infinite scrolling. It also refines navigation and link rendering in various components, adjusts UI styling for wallet/account displays, and improves tab navigation by preserving query parameters. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CourtDetailsPage
participant JurorsStakedByCourt
participant Search
participant DisplayJurors
participant useTopStakedJurorsByCourt
User->>CourtDetailsPage: Navigates to Court Details
CourtDetailsPage->>JurorsStakedByCourt: Renders with courtName
JurorsStakedByCourt->>Search: Renders search input
JurorsStakedByCourt->>DisplayJurors: Renders jurors list
DisplayJurors->>useTopStakedJurorsByCourt: Fetch jurors (courtId, page, search, order)
useTopStakedJurorsByCourt-->>DisplayJurors: Returns jurors data
DisplayJurors->>User: Shows jurors list
User->>Search: Types in search input
Search->>DisplayJurors: Updates search param (debounced)
DisplayJurors->>useTopStakedJurorsByCourt: Refetch jurors with new search
User->>DisplayJurors: Scrolls to bottom
DisplayJurors->>useTopStakedJurorsByCourt: Fetch next page
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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: 4
🧹 Nitpick comments (11)
web/src/pages/Courts/CourtDetails/Description.tsx (1)
122-122
: Consider preserving search parameters in the catch-all route.The catch-all route doesn't append the search parameter suffix, which may lead to inconsistent behavior compared to the other navigation paths.
- <Route path="*" element={<Navigate to={filteredTabs.length > 0 ? filteredTabs[0].path : ""} replace />} /> + <Route path="*" element={<Navigate to={filteredTabs.length > 0 ? `${filteredTabs[0].path}${suffix}` : ""} replace />} />web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/Search.tsx (2)
8-15
: Remove unnecessary z-index propertyThe
z-index: 0
property in the Container styled component doesn't seem to serve a purpose since it's the default stacking context value.const Container = styled.div` width: 100%; display: flex; flex-wrap: wrap; gap: 8px; margin-bottom: 5px; - z-index: 0; `;
47-57
: Consider adding accessibility attributes to search inputWhile the component renders a functional search input, it could benefit from additional accessibility attributes.
<StyledSearchbar dir="auto" type="text" placeholder="Search by address" value={value} onChange={(e) => setValue(e.target.value)} + aria-label="Search jurors by address" + role="search" />web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/DisplayJurors/JurorCard.tsx (1)
20-22
: Use rgba() for background transparency instead of hex alphaUsing
BB
as a suffix for color transparency is non-standard and may cause confusion.:hover { - background-color: ${({ theme }) => theme.lightGrey}BB; + background-color: ${({ theme }) => `rgba(${theme.lightGrey}, 0.73)`}; }Alternatively, if the theme's lightGrey is already a hex color:
:hover { - background-color: ${({ theme }) => theme.lightGrey}BB; + background-color: ${({ theme }) => theme.lightGrey + "BB"}; }web/src/hooks/queries/useTopStakedJurorsByCourt.ts (1)
38-39
: Consider typing orderBy parameter to be more specific.The
orderBy
parameter is typed as string, but it should be restricted to valid field names for better type safety.- orderBy: string, + orderBy: "effectiveStake" | "id", // Add other valid field names as neededweb/src/components/ConnectWallet/AccountDisplay.tsx (2)
103-104
: Avoid using !important in CSS.Using
!important
flags makes styles harder to override and maintain. Consider restructuring your CSS selectors to achieve the same specificity without!important
.- width: ${({ size }) => size + "px"} !important; - height: ${({ size }) => size + "px"} !important; + width: ${({ size }) => size + "px"}; + height: ${({ size }) => size + "px"};
115-117
: Avoid using !important in new StyledSmallLabel component.Similarly, the new
StyledSmallLabel
component uses!important
which should be avoided for the same reasons.- font-size: 14px !important; + font-size: 14px;web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/DisplayJurors/index.tsx (4)
80-80
: Unnecessary useMemo.The
useMemo
hook here just returnsacc
without any transformation, making it redundant.- const jurors = useMemo(() => acc, [acc]); + const jurors = acc;
48-48
: Consider defining an explicit interface for juror data.The type for the accumulated jurors is inferred from the useState initialization. It would be better to have an explicit interface for clarity and type safety.
+interface Juror { + id: string; + userAddress: string; + effectiveStake: string; +} - const [acc, setAcc] = useState<{ id: string; userAddress: string; effectiveStake: string }[]>([]); + const [acc, setAcc] = useState<Juror[]>([]);
18-19
: Fixed height may limit responsiveness.The
CardsWrapper
has a fixed max-height of 520px, which might not be responsive for all screen sizes. Consider using a relative unit or a responsive approach.- max-height: 520px; + max-height: 80vh; // or use responsiveSize() utility
50-63
: Consider combining related useEffect hooks.The two useEffect hooks both deal with managing the juror data and could potentially be combined for better readability.
useEffect(() => { setPage(0); setAcc([]); }, [searchValue, courtId, order]); - useEffect(() => { - const chunk = - data?.jurorTokensPerCourts?.map((j) => ({ - id: j.juror.id, - userAddress: j.juror.userAddress, - effectiveStake: j.effectiveStake, - })) ?? []; - if (chunk.length) setAcc((prev) => [...prev, ...chunk]); - }, [data]); + useEffect(() => { + if (!data) return; + + const chunk = + data.jurorTokensPerCourts?.map((j) => ({ + id: j.juror.id, + userAddress: j.juror.userAddress, + effectiveStake: j.effectiveStake, + })) ?? []; + + if (chunk.length) setAcc((prev) => [...prev, ...chunk]); + }, [data]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (11)
web/src/components/ConnectWallet/AccountDisplay.tsx
(5 hunks)web/src/hooks/queries/useTopStakedJurorsByCourt.ts
(1 hunks)web/src/pages/Courts/CourtDetails/Description.tsx
(2 hunks)web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/DisplayJurors/Header.tsx
(1 hunks)web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/DisplayJurors/JurorCard.tsx
(1 hunks)web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/DisplayJurors/Stake.tsx
(1 hunks)web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/DisplayJurors/index.tsx
(1 hunks)web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/Search.tsx
(1 hunks)web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/index.tsx
(1 hunks)web/src/pages/Courts/CourtDetails/index.tsx
(2 hunks)web/src/pages/Home/TopJurors/JurorCard/JurorTitle.tsx
(4 hunks)
✅ Files skipped from review due to trivial changes (3)
- web/src/pages/Courts/CourtDetails/index.tsx
- web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/DisplayJurors/Header.tsx
- web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/index.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/DisplayJurors/JurorCard.tsx (1)
web/src/styles/commonStyles.ts (1)
hoverShortTransitionTiming
(3-5)
web/src/pages/Home/TopJurors/JurorCard/JurorTitle.tsx (2)
web/src/components/StyledArrowLink.tsx (1)
StyledArrowLink
(5-27)web/src/components/ConnectWallet/AccountDisplay.tsx (2)
IdenticonOrAvatar
(124-142)AddressOrName
(149-161)
⏰ 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-university
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: SonarCloud
- GitHub Check: contracts-testing
- GitHub Check: Analyze (javascript)
- GitHub Check: Mend Security Check
🔇 Additional comments (14)
web/src/pages/Courts/CourtDetails/Description.tsx (1)
99-113
: LGTM: Well-implemented search parameter preservation.Your implementation correctly preserves URL search parameters during tab navigation, ensuring a consistent user experience when switching between tabs. The use of a suffix string and its inclusion in both navigation functions is a clean approach.
web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/DisplayJurors/Stake.tsx (1)
10-13
: LGTM: Well-structured styled componentThe styled component correctly uses theme colors and has appropriate font sizing.
web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/Search.tsx (1)
34-46
: Well-implemented debounce pattern for searchThe debounce implementation is well done, providing a good balance between responsiveness and performance by delaying URL updates until the user pauses typing.
web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/DisplayJurors/JurorCard.tsx (1)
30-37
: LGTM: Clean component implementationThe component correctly uses prop spreading and composing smaller components (JurorTitle and Stake) to create a clean, maintainable implementation.
web/src/pages/Home/TopJurors/JurorCard/JurorTitle.tsx (3)
4-4
: LGTM: Updated import name to be more descriptiveRenaming the import from ArrowIcon to ArrowSvg makes the code more self-documenting.
26-46
: LGTM: Well-implemented conditional styling for smallDisplayThe conditional styling for the smallDisplay prop is cleanly implemented using a styled-component's prop-based styling pattern.
62-65
: LGTM: Consistent prop spreading patternUsing the prop spreading syntax provides a cleaner and more maintainable way to pass props to child components.
web/src/hooks/queries/useTopStakedJurorsByCourt.ts (2)
16-16
: GraphQL query filtering could be optimized.The query filters by
effectiveStake_gt: 0
which is good but consider if additional filters might be needed for performance with large datasets.
56-56
: String normalization for search is a good practice.Converting the search string to lowercase before sending to the GraphQL query ensures case-insensitive matching, which is user-friendly.
web/src/components/ConnectWallet/AccountDisplay.tsx (3)
124-124
: Consistent size changes for improved UI.The default size change from "16" to "20" for
IdenticonOrAvatar
and size reduction from "32" to "20" inAccountDisplay
creates better UI consistency.Also applies to: 173-173
146-146
: Good addition of smallDisplay prop for flexible UI.The new
smallDisplay
prop and conditional rendering inAddressOrName
improves component flexibility and reusability.Also applies to: 149-149, 160-160
158-159
: Cleaner content variable extraction.Extracting the content to a variable before rendering makes the code more readable and maintainable.
web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/DisplayJurors/index.tsx (2)
72-72
: Potential issue with infinite scrolling condition.The condition
acc.length % PER_PAGE === 0
assumes each fetch will always return exactlyPER_PAGE
items, which might not be true for the last page. Consider checking if there's more data available instead.- if (e.isIntersecting && !isFetching && acc.length % PER_PAGE === 0) setPage((p) => p + 1); + if (e.isIntersecting && !isFetching && data?.jurorTokensPerCourts?.length === PER_PAGE) setPage((p) => p + 1);
88-96
: Well-structured UI rendering with appropriate loading states.The component handles loading states and empty results well, providing good user feedback.
web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/DisplayJurors/Stake.tsx
Outdated
Show resolved
Hide resolved
web/src/pages/Courts/CourtDetails/TopJurorsStakedByCourt/DisplayJurors/index.tsx
Outdated
Show resolved
Hide resolved
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: 4
🧹 Nitpick comments (7)
web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/DisplayJurors/Stake.tsx (1)
10-13
: Consider adding RTL language support to the styled labelSince this component displays numerical values that might appear in various language contexts, consider adding RTL (right-to-left) language support.
const StyledLabel = styled.label` font-size: 14px; color: ${({ theme }) => theme.primaryText}; + direction: ltr; + text-align: start; `;web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/index.tsx (2)
18-29
: Extract court name logic to a helper functionThe conditional logic for handling the court name presentation is complex and could be extracted to a separate function for better readability and maintenance.
+const formatCourtName = (courtName?: string) => { + if (!courtName) return ""; + const lowerCaseName = courtName.toLowerCase(); + const needsSuffix = !lowerCaseName.endsWith("court") && !lowerCaseName.startsWith("corte"); + return `${courtName}${needsSuffix ? " Court" : ""}`; +}; const JurorsStakedByCourt: React.FC<{ courtName: string | undefined }> = ({ courtName }) => { return ( <Container> <Title> - Jurors Staked in {courtName} - {courtName?.toLowerCase().endsWith("court") || courtName?.toLowerCase().startsWith("corte") ? null : " Court"} + Jurors Staked in {formatCourtName(courtName)} </Title> <Search /> <DisplayJurors /> </Container> ); };
8-11
: Consider responsive max-width for better mobile supportThe container has a fixed max-width which might limit responsiveness on smaller screens. Consider using responsive sizing for the max-width as well.
const Container = styled.div` margin-top: ${responsiveSize(28, 48)}; - max-width: 578px; + max-width: ${responsiveSize(350, 578)}; + width: 100%; `;web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/Search.tsx (2)
34-46
: Extract URL parameter logic to a custom hookThe debounce and URL parameter handling logic is complex and could be extracted to a custom hook for better reusability and testability.
+const useSearchQueryParam = (paramName: string, debounceTime = 500) => { + const [searchParams] = useSearchParams(); + const navigate = useNavigate(); + const { pathname } = useLocation(); + const initialValue = searchParams.get(paramName) ?? ""; + const [value, setValue] = useState(initialValue); + + useDebounce( + () => { + const params = new URLSearchParams(searchParams); + if (isEmpty(value)) { + params.delete(paramName); + } else { + params.set(paramName, value); + } + navigate(`${pathname}?${params.toString()}`, { replace: true }); + }, + debounceTime, + [value, searchParams, pathname] + ); + + return [value, setValue] as const; +}; const Search: React.FC = () => { - const [searchParams] = useSearchParams(); - const navigate = useNavigate(); - const { pathname } = useLocation(); - const initial = searchParams.get("jurorStakedSearch") ?? ""; - const [value, setValue] = useState(initial); - useDebounce( - () => { - const params = new URLSearchParams(searchParams); - if (isEmpty(value)) { - params.delete("jurorStakedSearch"); - } else { - params.set("jurorStakedSearch", value); - } - navigate(`${pathname}?${params.toString()}`, { replace: true }); - }, - 500, - [value] - ); + const [value, setValue] = useSearchQueryParam("jurorStakedSearch");
8-15
: Remove z-index property if not necessaryThe container sets
z-index: 0
, which is the default value. Unless there's a specific need for this z-index, it can be removed to simplify the styling.const Container = styled.div` width: 100%; display: flex; flex-wrap: wrap; gap: 8px; margin-bottom: 5px; - z-index: 0; `;
web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/DisplayJurors/index.tsx (2)
80-80
: Remove unnecessary useMemoThis
useMemo
doesn't provide any performance benefit since it just returns theacc
state variable without any computation. It can be safely removed.- const jurors = useMemo(() => acc, [acc]); + const jurors = acc;
18-18
: Use responsive sizing for max-heightThe component uses a fixed max-height for the scrollable container, which might not be ideal for all screen sizes. Consider using responsive sizing.
- max-height: 520px; + max-height: ${responsiveSize(400, 520)};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (7)
web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/DisplayJurors/Header.tsx
(1 hunks)web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/DisplayJurors/JurorCard.tsx
(1 hunks)web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/DisplayJurors/Stake.tsx
(1 hunks)web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/DisplayJurors/index.tsx
(1 hunks)web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/Search.tsx
(1 hunks)web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/index.tsx
(1 hunks)web/src/pages/Courts/CourtDetails/index.tsx
(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/DisplayJurors/Header.tsx
- web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/DisplayJurors/JurorCard.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/pages/Courts/CourtDetails/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-university
- 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
- 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: SonarCloud
- GitHub Check: Analyze (javascript)
- GitHub Check: contracts-testing
- GitHub Check: Mend Security Check
web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/DisplayJurors/Stake.tsx
Show resolved
Hide resolved
web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/DisplayJurors/index.tsx
Show resolved
Hide resolved
web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/DisplayJurors/index.tsx
Show resolved
Hide resolved
web/src/pages/Courts/CourtDetails/JurorsStakedByCourt/DisplayJurors/index.tsx
Show resolved
Hide resolved
Code Climate has analyzed commit 9e0a7bd and detected 8 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
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
PR-Codex overview
This PR introduces a new component,
JurorsStakedByCourt
, to display jurors and their staked amounts for a specific court. It also enhances the UI and functionality across several components related to courts and jurors.Detailed summary
JurorsStakedByCourt
component.Stake
andHeader
components for displaying juror stakes.CourtDetails
to includeJurorsStakedByCourt
.Summary by CodeRabbit
New Features
Enhancements