-
Notifications
You must be signed in to change notification settings - Fork 6
fix(web): contract arbitrators in wagmi hook generation #111
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
WalkthroughThis set of changes updates the Changes
Sequence Diagram(s)sequenceDiagram
participant TransactionDetails (Component)
participant GeneratedHook (useReadKlerosCoreArbitrationCost)
participant KlerosCoreContract
TransactionDetails->>GeneratedHook: Call useReadKlerosCoreArbitrationCost(arbitratorExtraData)
GeneratedHook->>KlerosCoreContract: Query arbitrationCost(bytes)
KlerosCoreContract-->>GeneratedHook: Return arbitration cost
GeneratedHook-->>TransactionDetails: Provide arbitration cost result
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
🪧 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-escrow-v2 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: 0
🧹 Nitpick comments (1)
web/src/components/ScrollTop.tsx (1)
1-31: Well-implemented scroll management component with possible dependency issueThe component correctly implements scroll management with section targeting. However, the useEffect dependency array is empty while the effect uses several variables from component scope (search, pathname, navigate, scrollTop).
While this might be intentional to only trigger the scroll once, it can lead to stale closures if these values change between renders. Consider whether these dependencies should be added to the array or if there should be a comment explaining why they're intentionally omitted.
- }, []); + }, [pathname, search, navigate, scrollTop]); // Re-run if any of these values changeOr if the intention is to only run once regardless of dependency changes:
- }, []); + }, []); // Intentionally empty - we only want to scroll once on initial mount
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
web/src/components/ScrollTop.tsx(1 hunks)web/src/hooks/useScrollTop.ts(1 hunks)web/src/pages/AttachmentDisplay/index.tsx(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
web/src/hooks/useScrollTop.ts (1)
web/src/context/OverlayScrollContext.tsx (1)
OverlayScrollContext(3-3)
web/src/pages/AttachmentDisplay/index.tsx (3)
web/src/styles/landscapeStyle.ts (2)
MAX_WIDTH_LANDSCAPE(3-3)landscapeStyle(7-11)web/src/styles/responsiveSize.ts (1)
responsiveSize(9-12)web/src/components/ExternalLink.tsx (1)
ExternalLink(5-9)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
🔇 Additional comments (7)
web/src/hooks/useScrollTop.ts (1)
1-15: Well-implemented scrolling utility hookThe hook provides a clean implementation for scrolling to the top of a page using the OverlayScrollContext. The optional chaining is a good approach for safely accessing nested properties, preventing runtime errors if any part of the chain is undefined.
The optional
smoothparameter provides flexibility in controlling the scrolling behavior.web/src/pages/AttachmentDisplay/index.tsx (6)
2-4: Good use of styled-components with responsive utilitiesThe imports for styled-components and responsive utilities are well organized and provide access to the necessary styling tools.
11-12: Component integrationGood integration of the new ScrollTop component and using the ExternalLink component for consistency across the application.
18-32: Enhanced container styling with responsive designThe updated Container styling improves visual appearance with background color, proper padding, and a maximum width constraint. The use of landscapeStyle for responsive adjustments is a good practice for maintaining consistent layout across different screen sizes.
40-46: Refactored external link with improved stylingThe styled ExternalLink component provides a cleaner implementation than the previous direct styled anchor. The alignment and spacing adjustments improve the visual layout.
63-65: Security improvement in external linkThe change from "noreferrer" to "noopener noreferrer" in the rel attribute is an important security enhancement. The "noopener" prevents the opened page from accessing the window.opener property, which can be a security risk.
77-77: Good UX enhancement with automatic scrollingAdding the ScrollTop component improves user experience by managing scroll position when viewing attachments.
PR-Codex overview
This PR primarily focuses on updating the
@kleros/kleros-v2-contractsdependency version, removing unused files, and introducing new hooks and components to improve scrolling behavior and contract interactions within the application.Detailed summary
@kleros/kleros-v2-contractsfrom^0.9.2to^0.9.3in multiple files.web/src/consts/arbitration.tsandweb/src/hooks/queries/useArbitrationCostFromKlerosCore.ts.useScrollTophook for smooth scrolling to the top.ScrollTopcomponent to manage scroll behavior based on URL parameters.useArbitrationCostwithuseReadKlerosCoreArbitrationCostinTransactionDetails.AttachmentDisplayto includeScrollTopand modified styles.wagmi.config.tsto accommodate new arbitrator contracts structure.Summary by CodeRabbit
Chores
Refactor
Cleanup
New Features
Style