Escrow page improvements#555
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR updates escrow management UI so authorization views display node identity, the create/edit flows use node-aware labels and address resolution, and the token panel uses a shared move-funds amount field with updated copy and status labels. ChangesEscrow Node Identity and UI Updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/escrow/escrow-token-panel.tsx (1)
392-393: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEmpty-state text doesn't match the specified copy.
The linked issue specifies keeping only "No authorizations" for the empty state, but the rendered text is "No authorization yet".
As per PR objectives: "Simplify the empty state by removing the explanatory sentence and keeping only 'No authorizations'."✏️ Proposed fix
- <span className={styles.noAuthTitle}>No authorization yet</span> + <span className={styles.noAuthTitle}>No authorizations</span>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/escrow/escrow-token-panel.tsx` around lines 392 - 393, Update the empty-state copy in the escrow token panel so it matches the specified wording. In the EscrowTokenPanel component, replace the current no-auth message text shown with styles.noAuthTitle from “No authorization yet” to exactly “No authorizations”, and remove any extra explanatory empty-state sentence so only that single line remains.
🧹 Nitpick comments (1)
src/components/escrow/edit-authorization-modal.tsx (1)
2-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the duplicated node-identity header into a shared component.
This header block (name fallback + Peer ID/ETH Address copy rows) is duplicated almost verbatim in
AuthorizationCard(escrow-token-panel.tsxlines 62-89). Importingescrow-token-panel.module.csshere to reuse.copyRow/.hashalso couples this modal to another component's stylesheet. A sharedNodeIdentityHeadercomponent (with its own CSS module) would remove the coupling and prevent future drift between the two copies (the codebase already shows label drift elsewhere in this PR between duplicated forms).Also applies to: 23-52
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/escrow/edit-authorization-modal.tsx` around lines 2 - 8, The node-identity header markup in this modal is duplicated with AuthorizationCard, and this file also depends on another component’s stylesheet for shared copy row/hash styles. Extract the fallback name plus Peer ID/ETH Address copy rows into a shared NodeIdentityHeader component with its own CSS module, then update edit-authorization-modal and AuthorizationCard to use it so the duplicated logic and styling stay in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/escrow/authorization-form.tsx`:
- Around line 81-89: The max concurrent jobs field in the authorization edit
form still uses the old endAdornment value, which is inconsistent with the
renamed label. Update the Input in authorization-form.tsx (the same field used
in the form component) so its endAdornment matches the create flow’s “jobs”
terminology, keeping the shared authorization UI consistent.
In `@src/components/escrow/escrow-token-panel.tsx`:
- Around line 62-63: The node name heading in EscrowTokenPanel is not using the
new truncation styling, so long names can overflow. Add the `authName` class to
the `<h4>` that renders `spender.nodeFriendlyName` in `escrow-token-panel.tsx`,
and make the same className update in the equivalent heading inside
`edit-authorization-modal.tsx` so both places use the new ellipsis CSS from
`escrow-token-panel.module.css`.
---
Outside diff comments:
In `@src/components/escrow/escrow-token-panel.tsx`:
- Around line 392-393: Update the empty-state copy in the escrow token panel so
it matches the specified wording. In the EscrowTokenPanel component, replace the
current no-auth message text shown with styles.noAuthTitle from “No
authorization yet” to exactly “No authorizations”, and remove any extra
explanatory empty-state sentence so only that single line remains.
---
Nitpick comments:
In `@src/components/escrow/edit-authorization-modal.tsx`:
- Around line 2-8: The node-identity header markup in this modal is duplicated
with AuthorizationCard, and this file also depends on another component’s
stylesheet for shared copy row/hash styles. Extract the fallback name plus Peer
ID/ETH Address copy rows into a shared NodeIdentityHeader component with its own
CSS module, then update edit-authorization-modal and AuthorizationCard to use it
so the duplicated logic and styling stay in sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b50cfc04-bb8a-47bf-9c18-8eb5aae3e961
📒 Files selected for processing (8)
src/components/button/copy-button.tsxsrc/components/escrow/authorization-form.tsxsrc/components/escrow/create-authorization-modal.tsxsrc/components/escrow/edit-authorization-modal.tsxsrc/components/escrow/escrow-token-panel.module.csssrc/components/escrow/escrow-token-panel.tsxsrc/lib/use-escrow-data.tssrc/types/nodes.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/use-escrow-data.ts (1)
45-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer axios
paramsover manual query-string building.Manually concatenating
page,size, andfiltersinto the URL withencodeURIComponent(JSON.stringify(...))duplicates work axios already does via itsparamsoption (which auto-encodes and is the pattern used elsewhere in this codebase, e.g.leaderboard.tsx'saxios.get(getApiRoute('nodes'), { params: {...} })). This is more error-prone ifgetApiRoute('nodes')ever returns a path with its own query string.♻️ Proposed refactor
- const filters = { address: { value: wallets.join(','), operator: 'terms' } }; - const res = await axios.get( - `${getApiRoute('nodes')}?page=0&size=${wallets.length}&filters=${encodeURIComponent(JSON.stringify(filters))}` - ); + const filters = { address: { value: wallets.join(','), operator: 'terms' } }; + const res = await axios.get(getApiRoute('nodes'), { + params: { page: 0, size: wallets.length, filters: JSON.stringify(filters) }, + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/use-escrow-data.ts` around lines 45 - 49, The axios call in useEscrowData is manually building the query string for page, size, and filters, which should be moved to axios params handling. Update the axios.get call that fetches getApiRoute('nodes') so it passes page, size, and the JSON filters object through the params option instead of concatenating and encoding them into the URL string. Keep the existing filters shape and use the same axios params pattern already used in leaderboard.tsx.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib/use-escrow-data.ts`:
- Around line 45-49: The axios call in useEscrowData is manually building the
query string for page, size, and filters, which should be moved to axios params
handling. Update the axios.get call that fetches getApiRoute('nodes') so it
passes page, size, and the JSON filters object through the params option instead
of concatenating and encoding them into the URL string. Keep the existing
filters shape and use the same axios params pattern already used in
leaderboard.tsx.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a43cc3b0-d6e7-4ee3-b27c-fed00517a47d
📒 Files selected for processing (4)
src/components/escrow/authorization-form.tsxsrc/components/escrow/escrow-token-panel.module.csssrc/components/escrow/escrow-token-panel.tsxsrc/lib/use-escrow-data.ts
💤 Files with no reviewable changes (1)
- src/components/escrow/escrow-token-panel.module.css
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/escrow/authorization-form.tsx
- src/components/escrow/escrow-token-panel.tsx
Fixes #553.
Merge https://github.com/oceanprotocol/incentive-backend/pull/166 first
Changes proposed in this PR:
Summary by CodeRabbit