feat: initial-features-implementation #2117
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
❌ 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 project configuration. |
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughIntroduces dynamic dispute kit handling in NewDisputeContext and replaces the old Court step with a new modular Court flow. Adds FeatureSelection subcomponents (ShieldedVoting, JurorEligibility) that map user choices to disputeKitId and disputeKitData. Removes the previous monolithic Court.tsx and updates context types to expose disputeKitOptions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CourtPage as Court (index.tsx)
participant Context as NewDisputeContext
participant Hooks as Hooks: useCourtTree / useSupportedDisputeKits / useDisputeKitAddressesAll
participant FeatureSel as FeatureSelection
participant Shielded as ShieldedVoting
participant Eligibility as JurorEligibility
User->>CourtPage: Open Resolver > Court
CourtPage->>Hooks: useCourtTree()
Hooks-->>CourtPage: courtTree
CourtPage->>Context: read disputeData
User->>CourtPage: Select court (cascader)
CourtPage->>Context: setDisputeData({ courtId, disputeKitId: undefined })
Note over FeatureSel,Context: Render features based on disputeKitOptions
FeatureSel->>Context: read disputeKitOptions, disputeData
Context->>Hooks: useSupportedDisputeKits(), useDisputeKitAddressesAll()
Hooks-->>Context: supportedKits, availableLabels/addresses
Context-->>FeatureSel: disputeKitOptions
alt Shielded voting applicable
FeatureSel->>Shielded: mount
User->>Shielded: Choose One-step or Two-step
Shielded->>Context: setDisputeData({ disputeKitId: mappedByOptions })
end
alt Juror eligibility applicable
FeatureSel->>Eligibility: mount
User->>Eligibility: Choose Classic / Gated (ERC-20/721/1155)
Eligibility->>Eligibility: Validate token address (/id)
Eligibility->>Context: setDisputeData({ disputeKitId, disputeKitData, isTokenGateValid })
end
Note over CourtPage,Context: Dispute data drives next navigation step
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify project 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: 5
🧹 Nitpick comments (3)
web/src/pages/Resolver/Parameters/Court/FeatureSelection/index.tsx (1)
34-48: Consider memoization optimizationThe voting and eligibility feature flags are recomputed on every render. While the computation is light, these could be combined into a single
useMemofor better organization.- // if supported dispute kits have Classic and Shutter - const showVotingFeatures = useMemo(() => { - return ( - disputeKitOptions.some((dk) => dk.text === DisputeKits.Classic) && - disputeKitOptions.some((dk) => dk.text === DisputeKits.Shutter) - ); - }, [disputeKitOptions]); - - // if supported dispute kits have Classic, TokenGated, TokenGatedShutter - const showEligibilityFeatures = useMemo(() => { - return ( - disputeKitOptions.some((dk) => dk.text === DisputeKits.Classic) && - disputeKitOptions.some((dk) => dk.text === DisputeKits.GatedShutter) && - disputeKitOptions.some((dk) => dk.text === DisputeKits.Gated) - ); - }, [disputeKitOptions]); + const { showVotingFeatures, showEligibilityFeatures } = useMemo(() => { + const hasClassic = disputeKitOptions.some((dk) => dk.text === DisputeKits.Classic); + const hasShutter = disputeKitOptions.some((dk) => dk.text === DisputeKits.Shutter); + const hasGated = disputeKitOptions.some((dk) => dk.text === DisputeKits.Gated); + const hasGatedShutter = disputeKitOptions.some((dk) => dk.text === DisputeKits.GatedShutter); + + return { + showVotingFeatures: hasClassic && hasShutter, + showEligibilityFeatures: hasClassic && hasGatedShutter && hasGated, + }; + }, [disputeKitOptions]);web/src/pages/Resolver/Parameters/Court/index.tsx (1)
59-63: Redundant court ID comparisonThe check for
disputeData.courtId !== courtIdis unnecessary since React will handle the state update optimization.Simplify the handler:
const handleCourtChange = (courtId: string) => { - if (disputeData.courtId !== courtId) { - setDisputeData({ ...disputeData, courtId, disputeKitId: undefined }); - } + setDisputeData({ ...disputeData, courtId, disputeKitId: undefined }); };web/src/pages/Resolver/Parameters/Court/FeatureSelection/JurorEligibility.tsx (1)
179-179: Typo in subtitle textThere's an unnecessary period at the end of the question.
- <SubTitle>Who can be selected as a juror?.</SubTitle> + <SubTitle>Who can be selected as a juror?</SubTitle>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
web/src/context/NewDisputeContext.tsx(6 hunks)web/src/pages/Resolver/Parameters/Court.tsx(0 hunks)web/src/pages/Resolver/Parameters/Court/FeatureSelection/JurorEligibility.tsx(1 hunks)web/src/pages/Resolver/Parameters/Court/FeatureSelection/ShieldedVoting.tsx(1 hunks)web/src/pages/Resolver/Parameters/Court/FeatureSelection/index.tsx(1 hunks)web/src/pages/Resolver/Parameters/Court/index.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- web/src/pages/Resolver/Parameters/Court.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-10-28T05:55:12.728Z
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1716
File: web-devtools/src/app/(main)/dispute-template/CustomContextInputs.tsx:29-42
Timestamp: 2024-10-28T05:55:12.728Z
Learning: In the `CustomContextInputs` component located at `web-devtools/src/app/(main)/dispute-template/CustomContextInputs.tsx`, the `DisputeRequestParams` array is used to exclude certain variables from the custom input since they are already provided in a preceding component. Therefore, converting it to a type is unnecessary.
Applied to files:
web/src/pages/Resolver/Parameters/Court/FeatureSelection/index.tsxweb/src/context/NewDisputeContext.tsx
📚 Learning: 2024-10-14T13:58:25.708Z
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
Applied to files:
web/src/context/NewDisputeContext.tsx
📚 Learning: 2025-05-15T06:50:40.859Z
Learnt from: tractorss
PR: kleros/kleros-v2#1982
File: web/src/pages/Resolver/Landing/index.tsx:62-62
Timestamp: 2025-05-15T06:50:40.859Z
Learning: In the Landing component, it's safe to pass `dispute?.dispute?.arbitrated.id as 0x${string}` to `usePopulatedDisputeData` without additional null checks because the hook internally handles undefined parameters through its `isEnabled` flag and won't execute the query unless all required data is available.
Applied to files:
web/src/context/NewDisputeContext.tsx
📚 Learning: 2024-06-27T10:11:54.861Z
Learnt from: nikhilverma360
PR: kleros/kleros-v2#1632
File: web/src/components/DisputeView/DisputeInfo/DisputeInfoList.tsx:37-42
Timestamp: 2024-06-27T10:11:54.861Z
Learning: `useMemo` is used in `DisputeInfoList` to optimize the rendering of `FieldItems` based on changes in `fieldItems`, ensuring that the mapping and truncation operation are only performed when necessary.
Applied to files:
web/src/context/NewDisputeContext.tsx
🧬 Code graph analysis (5)
web/src/pages/Resolver/Parameters/Court/index.tsx (5)
web-devtools/src/styles/responsiveSize.ts (1)
responsiveSize(9-12)web/src/context/NewDisputeContext.tsx (1)
useNewDisputeContext(118-124)web/src/hooks/queries/useCourtTree.ts (2)
useCourtTree(39-47)rootCourtToItems(55-62)web/src/components/StyledSkeleton.tsx (1)
StyledSkeleton(8-10)web/src/pages/Resolver/Parameters/Court.tsx (1)
useNewDisputeContext(170-354)
web/src/pages/Resolver/Parameters/Court/FeatureSelection/index.tsx (1)
web/src/context/NewDisputeContext.tsx (1)
useNewDisputeContext(118-124)
web/src/pages/Resolver/Parameters/Court/FeatureSelection/JurorEligibility.tsx (2)
web/src/context/NewDisputeContext.tsx (2)
useNewDisputeContext(118-124)IGatedDisputeData(69-75)web/src/hooks/useTokenAddressValidation.ts (2)
useERC20ERC721Validation(75-87)useERC1155Validation(95-104)
web/src/context/NewDisputeContext.tsx (2)
web/src/hooks/queries/useSupportedDisputeKits.ts (1)
useSupportedDisputeKits(19-32)web/src/hooks/useDisputeKitAddresses.ts (1)
useDisputeKitAddressesAll(105-163)
web/src/pages/Resolver/Parameters/Court/FeatureSelection/ShieldedVoting.tsx (2)
web-devtools/src/styles/Theme.tsx (1)
theme(3-10)web/src/context/NewDisputeContext.tsx (1)
useNewDisputeContext(118-124)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- 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-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: contracts-testing
- GitHub Check: Analyze (javascript)
- GitHub Check: Mend Security Check
🔇 Additional comments (3)
web/src/pages/Resolver/Parameters/Court/FeatureSelection/index.tsx (1)
50-59: Missing dependencies in useEffect and potential race conditionThe
useEffectis missingdisputeDataandsetDisputeDatafrom its dependency array, which could lead to stale closures. Additionally, there's a potential race condition where multiple rapid changes todisputeKitOptionscould cause state inconsistencies.Apply this fix to include missing dependencies:
useEffect(() => { - // there's only one, NOTE: what happens when here only TokenGated is support? we need the value + // When there's only one dispute kit option available, auto-select it if (disputeKitOptions.length === 1) { const disputeKit = disputeKitOptions[0]; setDisputeData({ ...disputeData, disputeKitId: disputeKit.value }); setShowFeatures(false); } else { setShowFeatures(true); } - }, [disputeKitOptions]); + }, [disputeKitOptions, disputeData, setDisputeData]);⛔ Skipped due to learnings
Learnt from: tractorss PR: kleros/kleros-v2#1982 File: web/src/pages/Resolver/Parameters/NotablePersons/PersonFields.tsx:64-0 Timestamp: 2025-05-09T13:39:15.086Z Learning: In PersonFields.tsx, the useEffect hook for address validation intentionally uses an empty dependency array to run only once on component mount. This is specifically designed for the dispute duplication flow when aliasesArray is already populated with addresses that need initial validation.Learnt from: jaybuidl PR: kleros/kleros-v2#1582 File: web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx:88-90 Timestamp: 2024-10-09T10:22:41.474Z Learning: Next.js recommends using the `useEffect` hook to set `isClient` and using `suppressHydrationWarning` as a workaround for handling hydration inconsistencies, especially when dealing with data like `knownArbitrables` that may differ between server-side and client-side rendering. This approach is acceptable in TypeScript/React applications, such as in `web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx`.Learnt from: tractorss PR: kleros/kleros-v2#1982 File: web/src/pages/Resolver/Landing/index.tsx:0-0 Timestamp: 2025-05-15T06:50:45.650Z Learning: In the Kleros V2 codebase, it's acceptable to use ESLint disable comments for dependency arrays in useEffect hooks when including certain dependencies (like state that is being updated within the effect) would cause infinite loops.Learnt from: tractorss PR: kleros/kleros-v2#1982 File: web/src/pages/Resolver/Landing/index.tsx:62-62 Timestamp: 2025-05-15T06:50:40.859Z Learning: In the Landing component, it's safe to pass `dispute?.dispute?.arbitrated.id as 0x${string}` to `usePopulatedDisputeData` without additional null checks because the hook internally handles undefined parameters through its `isEnabled` flag and won't execute the query unless all required data is available.Learnt from: Harman-singh-waraich PR: kleros/kleros-v2#1703 File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61 Timestamp: 2024-10-14T13:58:25.708Z Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.Learnt from: kemuru PR: kleros/kleros-v2#1774 File: web/src/components/CasesDisplay/index.tsx:61-61 Timestamp: 2024-12-06T13:04:50.495Z Learning: In `web/src/components/CasesDisplay/index.tsx`, the variables `numberDisputes` and `numberClosedDisputes` can sometimes be `NaN`, and should default to `0` using logical OR (`||`) to prevent display issues in the `StatsAndFilters` component.web/src/pages/Resolver/Parameters/Court/index.tsx (1)
71-71: Non-null assertion could throw runtime errorUsing the non-null assertion operator (
!) onpath.split("/").pop()is risky and could throw if the path format is unexpected.Add proper null checking:
- onSelect={(path: string | number) => typeof path === "string" && handleCourtChange(path.split("/").pop()!)} + onSelect={(path: string | number) => { + if (typeof path === "string") { + const courtId = path.split("/").pop(); + if (courtId) { + handleCourtChange(courtId); + } + } + }}⛔ Skipped due to learnings
Learnt from: Harman-singh-waraich PR: kleros/kleros-v2#1729 File: web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx:125-127 Timestamp: 2024-10-29T10:14:52.512Z Learning: In `web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx`, when `isEmailUpdateable` is false, `user?.emailUpdateableAt` is always defined. Therefore, using the non-null assertion `!` with `user?.emailUpdateableAt!` is acceptable because TypeScript may not infer its definiteness.web/src/pages/Resolver/Parameters/Court/FeatureSelection/JurorEligibility.tsx (1)
134-173: Missing dependencies and complex logic flowThe
useEffecthas missing dependencies and contains complex branching logic that could be simplified.Refactor for clarity and add missing dependencies:
useEffect(() => { + if (eligibilityType === undefined) return; + if (eligibilityType === EligibilityType.Classic) { const disputeKit = disputeKitOptions.find((dk) => dk.text === DisputeKits.Classic); - - setDisputeData({ ...disputeData, disputeKitId: disputeKit?.value, disputeKitData: undefined }); + if (disputeKit) { + setDisputeData({ ...disputeData, disputeKitId: disputeKit.value, disputeKitData: undefined }); + } } else if (eligibilityType === EligibilityType.GatedERC20 || eligibilityType === EligibilityType.GatedERC1155) { const disputeKitGated = disputeKitOptions.find((dk) => dk.text === DisputeKits.Gated); const disputeKitGatedShutter = disputeKitOptions.find((dk) => dk.text === DisputeKits.GatedShutter); - const currentDisputeKit = disputeKitOptions.find((dk) => dk.value === disputeData.disputeKitId); const disputeKitData: IGatedDisputeData = { ...(disputeData.disputeKitData as IGatedDisputeData), type: "gated", isERC1155: eligibilityType === EligibilityType.GatedERC1155, + tokenGate: (disputeData.disputeKitData as IGatedDisputeData)?.tokenGate || "", + tokenId: (disputeData.disputeKitData as IGatedDisputeData)?.tokenId || "0", }; + + let selectedKitId: number | undefined; // classic is selected, so here we change it to TokenGated if (currentDisputeKit?.text === DisputeKits.Classic) { - setDisputeData({ - ...disputeData, - disputeKitId: disputeKitGated?.value, - disputeKitData, - }); + selectedKitId = disputeKitGated?.value; } // shutter is selected, so here we change it to TokenGatedShutter else if (currentDisputeKit?.text === DisputeKits.Shutter) { - setDisputeData({ - ...disputeData, - disputeKitId: disputeKitGatedShutter?.value, - disputeKitData, - }); + selectedKitId = disputeKitGatedShutter?.value; } else { + selectedKitId = disputeKitGated?.value; + } + + if (selectedKitId !== undefined) { setDisputeData({ ...disputeData, - disputeKitId: disputeKitGated?.value, + disputeKitId: selectedKitId, disputeKitData, }); } } - }, [eligibilityType, disputeKitOptions]); + }, [eligibilityType, disputeKitOptions, disputeData, setDisputeData]);⛔ Skipped due to learnings
Learnt from: tractorss PR: kleros/kleros-v2#1982 File: web/src/pages/Resolver/Parameters/NotablePersons/PersonFields.tsx:64-0 Timestamp: 2025-05-09T13:39:15.086Z Learning: In PersonFields.tsx, the useEffect hook for address validation intentionally uses an empty dependency array to run only once on component mount. This is specifically designed for the dispute duplication flow when aliasesArray is already populated with addresses that need initial validation.Learnt from: jaybuidl PR: kleros/kleros-v2#1582 File: web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx:88-90 Timestamp: 2024-10-09T10:22:41.474Z Learning: Next.js recommends using the `useEffect` hook to set `isClient` and using `suppressHydrationWarning` as a workaround for handling hydration inconsistencies, especially when dealing with data like `knownArbitrables` that may differ between server-side and client-side rendering. This approach is acceptable in TypeScript/React applications, such as in `web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx`.Learnt from: tractorss PR: kleros/kleros-v2#1982 File: web/src/pages/Resolver/Landing/index.tsx:0-0 Timestamp: 2025-05-15T06:50:45.650Z Learning: In the Kleros V2 codebase, it's acceptable to use ESLint disable comments for dependency arrays in useEffect hooks when including certain dependencies (like state that is being updated within the effect) would cause infinite loops.Learnt from: Harman-singh-waraich PR: kleros/kleros-v2#1703 File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61 Timestamp: 2024-10-14T13:58:25.708Z Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.Learnt from: tractorss PR: kleros/kleros-v2#1982 File: web/src/pages/Resolver/Landing/index.tsx:62-62 Timestamp: 2025-05-15T06:50:40.859Z Learning: In the Landing component, it's safe to pass `dispute?.dispute?.arbitrated.id as 0x${string}` to `usePopulatedDisputeData` without additional null checks because the hook internally handles undefined parameters through its `isEnabled` flag and won't execute the query unless all required data is available.Learnt from: nikhilverma360 PR: kleros/kleros-v2#1632 File: web/src/components/DisputeView/DisputeInfo/DisputeInfoList.tsx:37-42 Timestamp: 2024-06-27T10:11:54.861Z Learning: `useMemo` is used in `DisputeInfoList` to optimize the rendering of `FieldItems` based on changes in `fieldItems`, ensuring that the mapping and truncation operation are only performed when necessary.
Comment on lines
+151
to
+165
| const { data: supportedDisputeKits } = useSupportedDisputeKits(disputeData.courtId); | ||
| const { availableDisputeKits } = useDisputeKitAddressesAll(); | ||
|
|
||
| const disputeKitOptions: DisputeKitOption[] = useMemo(() => { | ||
| return ( | ||
| supportedDisputeKits?.court?.supportedDisputeKits.map((dk) => { | ||
| const text = availableDisputeKits[dk.address.toLowerCase()] ?? ""; | ||
| return { | ||
| text, | ||
| value: Number(dk.id), | ||
| gated: text === DisputeKits.Gated || text === DisputeKits.GatedShutter, | ||
| }; | ||
| }) || [] | ||
| ); | ||
| }, [supportedDisputeKits, availableDisputeKits]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle potential edge cases in dispute kit options mapping
The current implementation might produce incomplete options if availableDisputeKits doesn't contain a matching address. The fallback to empty string ("") could lead to confusing UI states.
Consider filtering out invalid entries and adding logging:
const disputeKitOptions: DisputeKitOption[] = useMemo(() => {
return (
- supportedDisputeKits?.court?.supportedDisputeKits.map((dk) => {
- const text = availableDisputeKits[dk.address.toLowerCase()] ?? "";
- return {
- text,
- value: Number(dk.id),
- gated: text === DisputeKits.Gated || text === DisputeKits.GatedShutter,
- };
- }) || []
+ supportedDisputeKits?.court?.supportedDisputeKits
+ .map((dk) => {
+ const text = availableDisputeKits[dk.address.toLowerCase()];
+ if (!text) {
+ console.warn(`No dispute kit label found for address: ${dk.address}`);
+ return null;
+ }
+ return {
+ text,
+ value: Number(dk.id),
+ gated: text === DisputeKits.Gated || text === DisputeKits.GatedShutter,
+ };
+ })
+ .filter((option): option is DisputeKitOption => option !== null) || []
);
}, [supportedDisputeKits, availableDisputeKits]);📝 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.
Suggested change
| const { data: supportedDisputeKits } = useSupportedDisputeKits(disputeData.courtId); | |
| const { availableDisputeKits } = useDisputeKitAddressesAll(); | |
| const disputeKitOptions: DisputeKitOption[] = useMemo(() => { | |
| return ( | |
| supportedDisputeKits?.court?.supportedDisputeKits.map((dk) => { | |
| const text = availableDisputeKits[dk.address.toLowerCase()] ?? ""; | |
| return { | |
| text, | |
| value: Number(dk.id), | |
| gated: text === DisputeKits.Gated || text === DisputeKits.GatedShutter, | |
| }; | |
| }) || [] | |
| ); | |
| }, [supportedDisputeKits, availableDisputeKits]); | |
| const { data: supportedDisputeKits } = useSupportedDisputeKits(disputeData.courtId); | |
| const { availableDisputeKits } = useDisputeKitAddressesAll(); | |
| const disputeKitOptions: DisputeKitOption[] = useMemo(() => { | |
| return ( | |
| supportedDisputeKits?.court?.supportedDisputeKits | |
| .map((dk) => { | |
| const text = availableDisputeKits[dk.address.toLowerCase()]; | |
| if (!text) { | |
| console.warn(`No dispute kit label found for address: ${dk.address}`); | |
| return null; | |
| } | |
| return { | |
| text, | |
| value: Number(dk.id), | |
| gated: text === DisputeKits.Gated || text === DisputeKits.GatedShutter, | |
| }; | |
| }) | |
| .filter((option): option is DisputeKitOption => option !== null) || [] | |
| ); | |
| }, [supportedDisputeKits, availableDisputeKits]); |
🤖 Prompt for AI Agents
In web/src/context/NewDisputeContext.tsx around lines 151 to 165, the mapping
over supportedDisputeKits can emit options with an empty text when
availableDisputeKits lacks the address; update the useMemo to filter out entries
without a matching availableDisputeKits entry (skip if lookup yields undefined
or empty), and add a concise debug log (or console.warn) when an
unsupported/missing address is encountered so invalid entries are visible; keep
the gated flag logic the same and ensure address matching uses toLowerCase()
consistently.
Comment on lines
+65
to
+66
| const [eligibilityType, setEligibilityType] = useState<EligibilityType>(); | ||
| const { disputeData, setDisputeData, disputeKitOptions } = useNewDisputeContext(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Initialize eligibilityType based on current state
Similar to ShieldedVoting, this component should initialize its state based on the current dispute data.
Initialize the state appropriately:
- const [eligibilityType, setEligibilityType] = useState<EligibilityType>();
+ const [eligibilityType, setEligibilityType] = useState<EligibilityType>(() => {
+ const currentKit = disputeKitOptions.find((dk) => dk.value === disputeData.disputeKitId);
+ const kitData = disputeData.disputeKitData as IGatedDisputeData;
+
+ if (currentKit?.gated && kitData?.type === "gated") {
+ return kitData.isERC1155 ? EligibilityType.GatedERC1155 : EligibilityType.GatedERC20;
+ }
+ if (currentKit?.text === DisputeKits.Classic) {
+ return EligibilityType.Classic;
+ }
+ return undefined;
+ });📝 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.
Suggested change
| const [eligibilityType, setEligibilityType] = useState<EligibilityType>(); | |
| const { disputeData, setDisputeData, disputeKitOptions } = useNewDisputeContext(); | |
| const [eligibilityType, setEligibilityType] = useState<EligibilityType>(() => { | |
| const currentKit = disputeKitOptions.find( | |
| (dk) => dk.value === disputeData.disputeKitId | |
| ); | |
| const kitData = disputeData.disputeKitData as IGatedDisputeData; | |
| if (currentKit?.gated && kitData?.type === "gated") { | |
| return kitData.isERC1155 | |
| ? EligibilityType.GatedERC1155 | |
| : EligibilityType.GatedERC20; | |
| } | |
| if (currentKit?.text === DisputeKits.Classic) { | |
| return EligibilityType.Classic; | |
| } | |
| return undefined; | |
| }); | |
| const { disputeData, setDisputeData, disputeKitOptions } = useNewDisputeContext(); |
🤖 Prompt for AI Agents
In web/src/pages/Resolver/Parameters/Court/FeatureSelection/JurorEligibility.tsx
around lines 65-66, eligibilityType is created without reading the existing
dispute data; initialize it from the current disputeData (e.g.
disputeData.parameters or the specific juror eligibility field) and provide a
sensible default. Replace the plain useState with an initializer function that
returns the value from disputeData if present, and add a small useEffect to
sync/set eligibilityType when disputeData changes so the component reflects the
existing dispute state.
Comment on lines
+102
to
+112
| useEffect(() => { | ||
| if (disputeData.disputeKitData) { | ||
| const currentData = disputeData.disputeKitData as IGatedDisputeData; | ||
| if (currentData.isTokenGateValid !== isValidToken) { | ||
| setDisputeData({ | ||
| ...disputeData, | ||
| disputeKitData: { ...currentData, isTokenGateValid: isValidToken }, | ||
| }); | ||
| } | ||
| } | ||
| }, [isValidToken, disputeData.disputeKitData, setDisputeData]); |
There was a problem hiding this comment.
Potential infinite re-render loop
The useEffect that updates validation state modifies disputeData which is also in its dependency array, potentially causing infinite re-renders.
Use a more precise comparison to prevent unnecessary updates:
useEffect(() => {
- if (disputeData.disputeKitData) {
+ if (disputeData.disputeKitData?.type === "gated") {
const currentData = disputeData.disputeKitData as IGatedDisputeData;
if (currentData.isTokenGateValid !== isValidToken) {
setDisputeData({
...disputeData,
disputeKitData: { ...currentData, isTokenGateValid: isValidToken },
});
}
}
- }, [isValidToken, disputeData.disputeKitData, setDisputeData]);
+ }, [isValidToken, disputeData, setDisputeData]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/src/pages/Resolver/Parameters/Court/FeatureSelection/JurorEligibility.tsx
around lines 102 to 112, the useEffect updates disputeData while depending on
disputeData.disputeKitData which can cause an infinite re-render; instead,
narrow dependencies and perform a guarded, conditional update: depend only on
isValidToken and disputeData.disputeKitData?.isTokenGateValid (or better yet
read current isTokenGateValid from a functional state updater), compare the
existing isTokenGateValid value to isValidToken, and call setDisputeData(prev =>
({ ...prev, disputeKitData: { ...prev.disputeKitData, isTokenGateValid:
isValidToken } })) only when they differ; remove the broad disputeData object
from the dependency array to avoid needless re-renders.
Comment on lines
+51
to
+52
| const [votingType, setVotingType] = useState<VotingType>(); | ||
| const { disputeData, setDisputeData, disputeKitOptions } = useNewDisputeContext(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Initialize votingType based on current selection
The component doesn't initialize votingType based on the current disputeKitId, which could lead to UI state being out of sync with the actual data.
Initialize the state based on current dispute kit:
- const [votingType, setVotingType] = useState<VotingType>();
+ const [votingType, setVotingType] = useState<VotingType>(() => {
+ const currentKit = disputeKitOptions.find((dk) => dk.value === disputeData.disputeKitId);
+ if (currentKit?.text === DisputeKits.Classic) return VotingType.TwoStep;
+ if (currentKit?.text === DisputeKits.Shutter || currentKit?.text === DisputeKits.GatedShutter) {
+ return VotingType.OneStep;
+ }
+ return undefined;
+ });Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+56
to
+77
| useEffect(() => { | ||
| // disable TokenGatedShutter if selected, if TokenGated Selected do nothing | ||
| if (votingType === VotingType.TwoStep && disputeData.disputeKitData?.type !== "gated") { | ||
| const disputeKit = disputeKitOptions.find((dk) => dk.text === DisputeKits.Classic); | ||
|
|
||
| setDisputeData({ ...disputeData, disputeKitId: disputeKit?.value }); | ||
| } | ||
|
|
||
| if (votingType === VotingType.OneStep) { | ||
| // user has already selected TokenGated, so selecting Shutter here, we need to select TokenGatedShutter | ||
| if (disputeData.disputeKitData?.type === "gated") { | ||
| const disputeKit = disputeKitOptions.find((dk) => dk.text === DisputeKits.GatedShutter); | ||
|
|
||
| // no need to set DisputeKitData, will already be set by JurorEligibility | ||
| setDisputeData({ ...disputeData, disputeKitId: disputeKit?.value }); | ||
| } else { | ||
| const disputeKit = disputeKitOptions.find((dk) => dk.text === DisputeKits.Shutter); | ||
|
|
||
| setDisputeData({ ...disputeData, disputeKitId: disputeKit?.value }); | ||
| } | ||
| } | ||
| }, [votingType]); |
There was a problem hiding this comment.
Missing dependencies in useEffect and potential undefined disputeKit
The useEffect is missing several dependencies and doesn't handle the case where disputeKit might be undefined after the find operation.
Fix the missing dependencies and add null checks:
useEffect(() => {
- // disable TokenGatedShutter if selected, if TokenGated Selected do nothing
+ if (!votingType) return;
+
if (votingType === VotingType.TwoStep && disputeData.disputeKitData?.type !== "gated") {
const disputeKit = disputeKitOptions.find((dk) => dk.text === DisputeKits.Classic);
-
- setDisputeData({ ...disputeData, disputeKitId: disputeKit?.value });
+ if (disputeKit) {
+ setDisputeData({ ...disputeData, disputeKitId: disputeKit.value });
+ }
}
if (votingType === VotingType.OneStep) {
// user has already selected TokenGated, so selecting Shutter here, we need to select TokenGatedShutter
if (disputeData.disputeKitData?.type === "gated") {
const disputeKit = disputeKitOptions.find((dk) => dk.text === DisputeKits.GatedShutter);
-
- // no need to set DisputeKitData, will already be set by JurorEligibility
- setDisputeData({ ...disputeData, disputeKitId: disputeKit?.value });
+ if (disputeKit) {
+ setDisputeData({ ...disputeData, disputeKitId: disputeKit.value });
+ }
} else {
const disputeKit = disputeKitOptions.find((dk) => dk.text === DisputeKits.Shutter);
-
- setDisputeData({ ...disputeData, disputeKitId: disputeKit?.value });
+ if (disputeKit) {
+ setDisputeData({ ...disputeData, disputeKitId: disputeKit.value });
+ }
}
}
- }, [votingType]);
+ }, [votingType, disputeData, setDisputeData, disputeKitOptions]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/src/pages/Resolver/Parameters/Court/FeatureSelection/ShieldedVoting.tsx
around lines 56 to 77, the useEffect is missing dependencies and can call
setDisputeData with an undefined disputeKit.value; update the dependency array
to include all referenced values (votingType, disputeData, disputeKitOptions,
setDisputeData, and DisputeKits if needed) and add null checks after each find:
only call setDisputeData when disputeKit is found (or provide a safe fallback
value) to avoid setting disputeKitId to undefined.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WIP, initial implementation
PR-Codex overview
This PR introduces a new feature selection interface for dispute kits in the Kleros arbitration system, allowing users to choose between various voting and eligibility features based on the selected dispute kit.
Detailed summary
FeatureSelectioncomponent to manage voting and eligibility features.ShieldedVotingandJurorEligibilitycomponents withinFeatureSelection.NewDisputeContextto includedisputeKitOptions.Courtcomponent to utilize the newFeatureSelection.Summary by CodeRabbit