feat: UX & UI impros in various places (evidence cards, voting cards, case overview, top jurors) #1754
Conversation
…tached, transaction hash
WalkthroughThe changes in this pull request introduce a new field, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 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-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ 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-university ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
web/src/hooks/queries/useEvidences.ts (1)
Line range hint
41-57: Consider caching optimization for search queries.The implementation is solid, but search queries might benefit from a shorter cache lifetime to prevent stale results.
Consider adding a separate staleTime configuration for search queries:
return useQuery<{ evidences: EvidenceDetailsFragment[] }>({ queryKey: [keywords ? `evidenceSearchQuery${evidenceGroup}-${keywords}` : `evidencesQuery${evidenceGroup}`], enabled: isEnabled, refetchInterval: REFETCH_INTERVAL, + staleTime: keywords ? 0 : REFETCH_INTERVAL, // Immediate refetch for search queries queryFn: async () => {web/src/pages/Cases/CaseDetails/Evidence/index.tsx (2)
98-107: LGTM! Consider adding type safety.The addition of
transactionHashto the evidence mapping is correct and aligns with the PR objectives. However, consider adding TypeScript types for the evidence object to ensure type safety.interface Evidence { evidence: string; sender: { id: string }; timestamp: string; transactionHash: string; name: string; description: string; fileURI: string; evidenceIndex: string; }
116-121: Consider extracting the evidence card rendering logic to reduce duplication.The evidence card rendering logic is duplicated between real and spam evidences. Consider extracting this into a reusable function to improve maintainability.
const renderEvidenceCard = ({ evidence, sender, timestamp, transactionHash, name, description, fileURI, evidenceIndex, }) => ( <EvidenceCard key={timestamp} index={parseInt(evidenceIndex)} sender={sender?.id} {...{ evidence, timestamp, transactionHash, name, description, fileURI }} /> );Then use it like:
evidences?.spamEvidences.map(renderEvidenceCard)web/src/components/EvidenceCard.tsx (1)
179-185: Enhance accessibility for clickable elementsConsider adding:
- Visual indicators for clickable elements in their default state
- Descriptive aria-labels for screen readers
- <StyledA href={addressExplorerLink} rel="noreferrer" target="_blank"> + <StyledA + href={addressExplorerLink} + rel="noreferrer" + target="_blank" + aria-label={`View address ${sender} on block explorer`} + > <p>{shortenAddress(sender)}</p> </StyledA> - <StyledA href={transactionExplorerLink} rel="noreferrer" target="_blank"> + <StyledA + href={transactionExplorerLink} + rel="noreferrer" + target="_blank" + aria-label={`View transaction from ${formatDate(Number(timestamp), true)} on block explorer`} + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
subgraph/core/schema.graphql(1 hunks)subgraph/core/src/EvidenceModule.ts(1 hunks)web/src/components/EvidenceCard.tsx(5 hunks)web/src/hooks/queries/useEvidences.ts(1 hunks)web/src/pages/Cases/CaseDetails/Evidence/index.tsx(2 hunks)
🔇 Additional comments (6)
web/src/hooks/queries/useEvidences.ts (1)
18-18: LGTM! Verify fragment usage across components.
The addition of transactionHash to the fragment is correct and well-positioned. This field enables the new transaction explorer linking feature.
Let's verify the fragment usage across components:
✅ Verification successful
LGTM! Fragment usage is consistent and well-integrated
The verification shows that:
- The
EvidenceDetailsfragment is properly defined inuseEvidences.tsand includes thetransactionHashfield - The fragment is consistently used in evidence queries within the same file
- The
transactionHashfield is correctly consumed inEvidenceCard.tsxandEvidence/index.tsxcomponents for displaying transaction links - No direct queries of
transactionHashoutside the fragment were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the EvidenceDetails fragment is consistently used across components
# and that no component is directly querying transactionHash without using the fragment
# Check for fragment usage
rg "\.\.\.EvidenceDetails" -A 3
# Check for direct transactionHash queries (should not find any)
rg "transactionHash[^.]" --type ts --type tsx
Length of output: 494
Script:
#!/bin/bash
# Let's try a more comprehensive search approach
# 1. Find all TypeScript files that might use the fragment
fd "\.(ts|tsx)$" web/src
# 2. Check for EvidenceDetails fragment usage with better context
rg "\.\.\.EvidenceDetails" -B 2 -A 5 --type-add "typescript:*.{ts,tsx}"
# 3. Check for transactionHash usage in TypeScript files
rg "transactionHash" --type-add "typescript:*.{ts,tsx}" --type typescript
# 4. Look for GraphQL fragment definition
rg "fragment EvidenceDetails" --type-add "typescript:*.{ts,tsx}" --type typescript -B 2 -A 10
Length of output: 19970
subgraph/core/src/EvidenceModule.ts (2)
Line range hint 28-53: LGTM! Robust error handling preserved.
The error handling ensures the evidence record (including the new transaction hash) is saved in all scenarios, with proper logging of failure cases.
20-20: LGTM! Verify schema compatibility.
The transaction hash assignment is correctly placed with other metadata assignments and will be properly saved regardless of JSON parsing outcomes.
Let's verify the schema definition matches the implementation:
✅ Verification successful
Schema compatibility verified, implementation is correct
The transactionHash field is properly defined in the schema as Bytes! (non-nullable), which matches the implementation where we assign event.transaction.hash. The field placement and type are consistent across both the schema and the event handler.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the ClassicEvidence entity schema definition
# Expected: Should find transactionHash field of type Bytes! in schema.graphql
rg -A 1 'type ClassicEvidence' subgraph/core/schema.graphql
Length of output: 257
Script:
#!/bin/bash
# Let's get more context around the ClassicEvidence entity definition
rg -A 15 'type ClassicEvidence implements Evidence' subgraph/core/schema.graphql
Length of output: 514
web/src/components/EvidenceCard.tsx (2)
1-1: LGTM: Clean styled-component implementation
The new StyledA component follows consistent styling patterns and properly handles hover states using theme colors.
Also applies to: 12-12, 120-131
179-185: LGTM: Clean implementation of clickable elements
The implementation:
- Makes addresses and timestamps clickable as per PR objectives
- Follows security best practices with rel="noreferrer"
- Maintains existing functionality while adding new features
subgraph/core/schema.graphql (1)
321-321: Verify data migration strategy for existing evidence entries.
Since ClassicEvidence is marked as immutable and transactionHash is a required field, ensure there's a migration strategy to handle existing evidence entries.
Run the following script to check for existing evidence entries:
…nding court when clicking on it
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
web/src/components/DisputeView/DisputeInfo/DisputeInfoCard.tsx (1)
70-77: Consider accessibility improvements for the styled linkWhile the hover state is well implemented, consider adding the following improvements:
const StyledLink = styled(Link)` + text-decoration: none; :hover { label { &.value { cursor: pointer; color: ${({ theme }) => theme.primaryBlue}; text-decoration: underline; } } } + :focus-visible { + outline: 2px solid ${({ theme }) => theme.primaryBlue}; + outline-offset: 2px; + } `;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
web/src/components/DisputeView/DisputeInfo/DisputeInfoCard.tsx(5 hunks)web/src/hooks/useScrollTop.ts(1 hunks)
🔇 Additional comments (4)
web/src/hooks/useScrollTop.ts (1)
8-8: Verify scroll behavior across different browsers
The scroll behavior might vary across different browsers. Consider adding smooth scrolling for better user experience.
- osInstanceRef?.current?.osInstance().elements().viewport.scroll({ top: 0 });
+ osInstanceRef?.current?.osInstance().elements().viewport.scroll({
+ top: 0,
+ behavior: 'smooth'
+ });web/src/components/DisputeView/DisputeInfo/DisputeInfoCard.tsx (3)
4-8: LGTM: Import statements are properly organized
The new imports for Link and useScrollTop are correctly placed and necessary for the new functionality.
65-68: Consider browser compatibility for text-wrap property
The text-wrap: auto property might not be supported in all browsers. Consider adding a fallback or using word-break: break-word for broader compatibility.
106-108: Verify scroll behavior on navigation
The implementation of scrollTop on link click aligns with the PR objectives. However:
- Ensure this doesn't interfere with the browser's default scroll restoration on navigation
- Consider adding a smooth scroll behavior for better UX
You might want to update the scroll behavior:
- onClick={() => scrollTop()}
+ onClick={() => scrollTop({ behavior: 'smooth' })}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (7)
web/src/pages/Home/TopJurors/JurorCard/JurorTitle.tsx (1)
38-40: Consider error handling for chain configurationWhile the
useMemoimplementation is correct, consider adding error handling in case the chain configuration is undefined.const addressExplorerLink = useMemo(() => { - return `${getChain(DEFAULT_CHAIN)?.blockExplorers?.default.url}/address/${address}`; + const chain = getChain(DEFAULT_CHAIN); + if (!chain?.blockExplorers?.default.url) { + console.warn(`Block explorer URL not found for chain ${DEFAULT_CHAIN}`); + return '#'; + } + return `${chain.blockExplorers.default.url}/address/${address}`; }, [address]);web/src/pages/Cases/CaseDetails/Voting/VotesDetails/AccordionTitle.tsx (3)
44-52: Consider adding transition for smoother hover effects.The styled anchor component implementation looks good, but the hover effect could be smoother.
const StyledA = styled.a` + transition: all 0.2s ease-in-out; :hover { text-decoration: underline; label { cursor: pointer; color: ${({ theme }) => theme.primaryBlue}; } } `;
99-101: Consider adding aria-label for better accessibility.While the link implementation is correct, adding an aria-label would improve accessibility for screen readers.
- <StyledA href={addressExplorerLink} rel="noopener noreferrer" target="_blank"> + <StyledA + href={addressExplorerLink} + rel="noopener noreferrer" + target="_blank" + aria-label={`View juror ${juror} on block explorer`} + >
99-101: Consider adding visual indicator for external link.Since this is an external link that opens in a new tab, it would be helpful to add a visual indicator.
Consider adding a small external link icon after the address to indicate that clicking will open a new tab. This could be implemented using an icon from your existing icon library.
web/src/components/EvidenceCard.tsx (3)
120-131: Enhance accessibility of styled linksConsider adding focus styles and improving link visibility:
const StyledA = styled.a` + text-decoration-thickness: 1px; + &:focus-visible { + outline: 2px solid ${({ theme }) => theme.primaryBlue}; + outline-offset: 2px; + } :hover { text-decoration: underline; + text-decoration-thickness: 1px; p { color: ${({ theme }) => theme.primaryBlue}; }
155-161: Optimize explorer link memoizationConsider extracting the base explorer URL to avoid redundant calculations:
+const useExplorerBaseUrl = () => { + return useMemo(() => { + const chainData = getChain(DEFAULT_CHAIN); + return chainData?.blockExplorers?.default?.url ?? '#'; + }, []); +}; const EvidenceCard: React.FC<IEvidenceCard> = ({ ... }) => { + const baseExplorerUrl = useExplorerBaseUrl(); const addressExplorerLink = useMemo(() => { - return `${getChain(DEFAULT_CHAIN)?.blockExplorers?.default.url}/address/${sender}`; + return `${baseExplorerUrl}/address/${sender}`; }, [sender]); const transactionExplorerLink = useMemo(() => { - return `${getChain(DEFAULT_CHAIN)?.blockExplorers?.default.url}/tx/${transactionHash}`; + return `${baseExplorerUrl}/tx/${transactionHash}`; }, [transactionHash]);
179-191: Enhance semantic HTML for better accessibilityConsider adding semantic HTML elements and ARIA labels:
- <StyledA href={addressExplorerLink} rel="noopener noreferrer" target="_blank"> + <StyledA + href={addressExplorerLink} + rel="noopener noreferrer" + target="_blank" + aria-label={`View address ${sender} on block explorer`} + > <p>{shortenAddress(sender)}</p> </StyledA> </AccountContainer> - <StyledA href={transactionExplorerLink} rel="noopener noreferrer" target="_blank"> - <Timestamp>{formatDate(Number(timestamp), true)}</Timestamp> + <StyledA + href={transactionExplorerLink} + rel="noopener noreferrer" + target="_blank" + aria-label={`View transaction from ${formatDate(Number(timestamp), true)} on block explorer`} + > + <Timestamp> + <time dateTime={new Date(Number(timestamp)).toISOString()}> + {formatDate(Number(timestamp), true)} + </time> + </Timestamp> </StyledA>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
web/src/components/EvidenceCard.tsx(5 hunks)web/src/pages/Cases/CaseDetails/Voting/VotesDetails/AccordionTitle.tsx(4 hunks)web/src/pages/Home/TopJurors/JurorCard/JurorTitle.tsx(2 hunks)
🔇 Additional comments (7)
web/src/pages/Home/TopJurors/JurorCard/JurorTitle.tsx (3)
1-1: LGTM: Proper imports for optimization and chain utilities
The addition of useMemo and chain utilities is appropriate for optimizing the explorer link generation.
Also applies to: 5-5
23-31: LGTM: Well-implemented styled component for link hover effects
The StyledA component properly implements hover effects and maintains consistency with the theme system. The cursor and color changes provide good visual feedback for interactive elements.
45-47: LGTM: Secure external link implementation
The link implementation correctly includes security attributes (rel="noopener noreferrer") and opens in a new tab. This aligns with the PR objective of making addresses clickable.
web/src/pages/Cases/CaseDetails/Voting/VotesDetails/AccordionTitle.tsx (2)
1-1: LGTM! Imports are properly organized.
The addition of useMemo and chain-related imports aligns well with the new functionality for creating clickable address links.
Also applies to: 7-7
91-93: LGTM! Proper memoization of explorer link.
The useMemo hook is correctly used to optimize the address explorer link generation, preventing unnecessary recalculations when other props change.
web/src/components/EvidenceCard.tsx (2)
1-1: LGTM: Import changes are appropriate
The new imports for useMemo and chain-related utilities are necessary for the added functionality.
Also applies to: 12-12
145-154: Previous interface comment still applies
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
web/src/components/EvidenceCard.tsx (2)
120-131: Consider enhancing accessibility of styled linksThe hover styles are good, but consider adding focus styles for keyboard navigation and ensuring sufficient color contrast for accessibility.
const StyledA = styled.a` + &:focus { + outline: 2px solid ${({ theme }) => theme.primaryBlue}; + outline-offset: 2px; + } :hover { text-decoration: underline; p { color: ${({ theme }) => theme.primaryBlue}; } label { cursor: pointer; color: ${({ theme }) => theme.primaryBlue}; } } `;
180-186: Enhance security for external linksWhile
rel="noopener noreferrer"is correctly used, consider adding additional security attributes for external links.-<StyledA href={addressExplorerLink} rel="noopener noreferrer" target="_blank"> +<StyledA href={addressExplorerLink} rel="noopener noreferrer nofollow" target="_blank"> -<StyledA href={transactionExplorerLink} rel="noopener noreferrer" target="_blank"> +<StyledA href={transactionExplorerLink} rel="noopener noreferrer nofollow" target="_blank">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
subgraph/core/schema.graphql(2 hunks)subgraph/package.json(1 hunks)web/src/components/EvidenceCard.tsx(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- subgraph/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- subgraph/core/schema.graphql
🔇 Additional comments (1)
web/src/components/EvidenceCard.tsx (1)
Line range hint 146-195: Implementation successfully meets PR objectives
The changes effectively enhance the UX by:
- Making addresses clickable and linking to blockchain explorers
- Adding transaction hash links
- Improving the overall navigation and interaction design
The implementation is solid with the suggested improvements for error handling and security.
kemuru
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
web/src/layout/Header/navbar/DappList.tsx (1)
128-130: Consider reordering the POH entry for better UX.Currently, the POH V2 entry is placed between "Kleros Scout" and "Governor". Consider moving it to be grouped with other V2 services at the beginning of the array for better organization and user experience.
const ITEMS = [ { text: "Court V2", Icon: Court, url: "https://v2.kleros.builders/", }, { text: "Curate V2", Icon: Curate, url: "https://curate-v2.netlify.app/", }, { text: "Resolver V2", Icon: Resolver, url: "https://v2.kleros.builders/#/resolver", }, { text: "Escrow V2", Icon: Escrow, url: "https://escrow-v2.kleros.builders/", }, + { + text: "POH V2", + Icon: POH, + url: "https://v2.proofofhumanity.id", + }, { text: "Court V1", Icon: Court, url: "https://court.kleros.io/", }, // ... other V1 entries ... { text: "Vea", Icon: Vea, url: "https://veascan.io", }, { text: "Kleros Scout", Icon: Curate, url: "https://klerosscout.eth.limo", }, - { - text: "POH V2", - Icon: POH, - url: "https://v2.proofofhumanity.id", - }, { text: "Governor", Icon: Governor, url: "https://governor.kleros.io", },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
web/src/layout/Header/navbar/DappList.tsx(1 hunks)
🔇 Additional comments (1)
web/src/layout/Header/navbar/DappList.tsx (1)
128-130: LGTM! POH V2 update looks good.
The update to POH V2 and its corresponding URL is consistent with the versioning pattern used for other Kleros services.
PR-Codex overview
This PR focuses on updating the evidence handling and display functionality, enhancing the user interface, and improving code organization. Key updates include version changes, the addition of
transactionHash, and adjustments to component names and imports.Detailed summary
transactionHashto the evidence structure.DappListfrom "POH V1" to "POH V2" and changed its URL.EvidenceAttachmentDisplaytoAttachmentDisplay.AttachmentDisplay.DisputeInfoCardwith enhanced link handling.useScrollTophook for scrolling functionality.EvidenceCardto include transaction links.Summary by CodeRabbit
Summary by CodeRabbit
New Features
transactionHashfield to theEvidenceandClassicEvidenceentities, enhancing evidence tracking.EvidenceCardcomponent to display links to blockchain explorers for the sender's address and transaction.useScrollTophook for improved navigation within the application.DisputeInfoCardcomponent to allow navigation to court details.AccordionTitleandJurorTitlecomponents with links to juror addresses on blockchain explorers.EvidenceAttachmentDisplaycomponent toAttachmentDisplayand added a new route for policy attachments.Bug Fixes
Chores
package.jsonfile to reflect minor improvements.