fix(web): juror-number-input-crash #2201
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-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
WalkthroughThe changes tighten validation for the jurors step in the Resolver form by requiring both arbitrationCost and numberOfJurors to be present before enabling the Next button, and harden the numberOfJurors input handler to validate numeric input and use a numeric default. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
❌ Deploy Preview for kleros-v2-testnet-devtools failed. Why did it fail? →
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/src/pages/Resolver/Parameters/Jurors.tsx (1)
69-76: Input hardening prevents crash, with minor refinement opportunities.The numeric validation and fallback to
0successfully prevent the crash. Two observations:
isUndefined(value)is redundant—parseIntreturns onlynumberorNaN, neverundefined.- With this logic,
numberOfJurorsis always0or a valid number. When users clear the field, it displays"0"rather than remaining empty (since line 78'sNaNcheck no longer triggers).If you prefer an empty field when cleared, consider this alternative:
const handleJurorsWrite = (event: React.ChangeEvent<HTMLInputElement>) => { - const value = parseInt(event.target.value.replace(/\D/g, ""), 10); - if (isUndefined(value) || isNaN(value)) { - setDisputeData({ ...disputeData, numberOfJurors: 0 }); - } else { - setDisputeData({ ...disputeData, numberOfJurors: value }); - } + const cleaned = event.target.value.replace(/\D/g, ""); + const value = cleaned === "" ? NaN : parseInt(cleaned, 10); + setDisputeData({ ...disputeData, numberOfJurors: value }); };This preserves
NaNfor empty input, allowing line 78 to display an empty field.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/pages/Resolver/NavigationButtons/NextButton.tsx(1 hunks)web/src/pages/Resolver/Parameters/Jurors.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1739
File: web/src/pages/Home/TopJurors/JurorCard/Coherency.tsx:22-26
Timestamp: 2024-11-07T10:48:16.774Z
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.
📚 Learning: 2024-11-07T10:48:16.774Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1739
File: web/src/pages/Home/TopJurors/JurorCard/Coherency.tsx:22-26
Timestamp: 2024-11-07T10:48:16.774Z
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.
Applied to files:
web/src/pages/Resolver/NavigationButtons/NextButton.tsxweb/src/pages/Resolver/Parameters/Jurors.tsx
📚 Learning: 2024-10-29T10:13:04.524Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1729
File: web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx:69-69
Timestamp: 2024-10-29T10:13:04.524Z
Learning: In `web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx`, when the button is disabled, the associated logic won't be reached, and certain code paths may exist for TypeScript purposes.
Applied to files:
web/src/pages/Resolver/NavigationButtons/NextButton.tsx
📚 Learning: 2025-05-15T06:50:40.859Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 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/pages/Resolver/NavigationButtons/NextButton.tsxweb/src/pages/Resolver/Parameters/Jurors.tsx
📚 Learning: 2024-10-14T13:58:25.708Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 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/pages/Resolver/NavigationButtons/NextButton.tsxweb/src/pages/Resolver/Parameters/Jurors.tsx
📚 Learning: 2024-12-06T13:04:50.495Z
Learnt from: kemuru
Repo: kleros/kleros-v2 PR: 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.
Applied to files:
web/src/pages/Resolver/Parameters/Jurors.tsx
📚 Learning: 2024-10-28T05:55:12.728Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 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/Jurors.tsx
📚 Learning: 2025-09-09T13:33:46.896Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 2117
File: web/src/components/DisputeFeatures/Features/GatedErc1155.tsx:51-66
Timestamp: 2025-09-09T13:33:46.896Z
Learning: The `setDisputeData` function in `NewDisputeContext` at `web/src/context/NewDisputeContext.tsx` has signature `(disputeData: IDisputeData) => void` and only accepts direct state values, not functional updates like standard React state setters. It cannot be used with the pattern `setDisputeData((prev) => ...)`.
Applied to files:
web/src/pages/Resolver/Parameters/Jurors.tsx
📚 Learning: 2024-10-21T10:32:16.970Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: kleros-sdk/src/utils/getDispute.ts:38-40
Timestamp: 2024-10-21T10:32:16.970Z
Learning: The variables 'arbitrableChainID' and 'externalDisputeID' are required by the context to have uppercase 'ID', so they should remain unchanged even if the corresponding source properties use 'Id'.
Applied to files:
web/src/pages/Resolver/Parameters/Jurors.tsx
📚 Learning: 2024-10-09T10:22:41.474Z
Learnt from: jaybuidl
Repo: kleros/kleros-v2 PR: 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`.
Applied to files:
web/src/pages/Resolver/Parameters/Jurors.tsx
🧬 Code graph analysis (1)
web/src/pages/Resolver/Parameters/Jurors.tsx (1)
web/src/utils/prepareArbitratorExtradata.ts (1)
prepareArbitratorExtradata(48-68)
⏰ 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-devtools
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: hardhat-tests
- GitHub Check: Analyze (javascript)
- GitHub Check: SonarCloud
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
🔇 Additional comments (2)
web/src/pages/Resolver/Parameters/Jurors.tsx (1)
61-61: LGTM: Type-correct argument for arbitration cost calculation.The change from string
"0"to numericdisputeData?.numberOfJurors ?? 0correctly matches thenumbertype expected byprepareArbitratorExtradata'snoOfVotesparameter.web/src/pages/Resolver/NavigationButtons/NextButton.tsx (1)
46-46: LGTM: Proper validation for jurors step.Requiring both
arbitrationCostandnumberOfJurorsto be truthy correctly prevents users from proceeding with invalid input (e.g.,0jurors). This validation coordinates well with the input hardening inJurors.tsx.
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.
PR-Codex overview
This PR focuses on improving validation and state management for the
NextButtonandJurorscomponents in the resolver pages. It enhances condition checks and handles potential undefined or invalid values fornumberOfJurors.Detailed summary
NextButton.tsxto check botharbitrationCostandnumberOfJurors.argsinJurors.tsxto ensurenumberOfJurorsdefaults to0instead of a string.handleJurorsWritefunction to setnumberOfJurorsto0if the input value is undefined or NaN.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.