feat: governance token claim manager UI and fee breakdown table#432
Conversation
- Create governance dashboard page with unclaimed rewards, voting weight, and lockup period selector (30/90/365 days) - Add Claim All button with transaction feedback - Add lockup period multiplier calculation and display - Replace simple price breakdown with detailed fee breakdown table in FundSessionModal - Add Platform Service Fee (2%) row alongside Initial Deposit and Soroban Gas fees - Update confirm step summary to include all fee components Closes LightForgeHub#364 Closes LightForgeHub#363
📝 WalkthroughWalkthroughAdds a new client-side GovernancePage for claiming governance tokens and locking them for boosted voting power, with mock state/handlers. Updates FundSessionModal to compute and display a detailed fee breakdown (deposit, gas, platform fee, total) and revises the required-balance calculation accordingly. ChangesGovernance Token Claim Manager Page
FundSessionModal Fee Breakdown
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant GovernancePage
participant calculateVotingWeight
User->>GovernancePage: Click "Claim All"
GovernancePage->>GovernancePage: handleClaimAll (simulate delay)
GovernancePage-->>User: Show TX hash success panel
User->>GovernancePage: Enter lock amount, select lockup period
GovernancePage->>calculateVotingWeight: compute raw/boosted weight
calculateVotingWeight-->>GovernancePage: return voting weight preview
User->>GovernancePage: Click "Lock & Boost Voting Power"
GovernancePage->>GovernancePage: handleLockTokens (validate, simulate delay)
GovernancePage-->>User: Clear input, update staked balance
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/marketplace/FundSessionModal.tsx (1)
289-294: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStray literal
$before XLM amount.
${depositAmount.toFixed(2)} XLM has been escrowed...renders a literal dollar sign in front of the XLM figure, misleadingly implying a USD value on a funds-confirmation screen.🐛 Proposed fix
<p className="text-muted-foreground text-sm mb-4"> - ${depositAmount.toFixed(2)} XLM has been escrowed for your session with {expertName} + {depositAmount.toFixed(2)} XLM has been escrowed for your session with {expertName} </p>🤖 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/marketplace/FundSessionModal.tsx` around lines 289 - 294, The success message in FundSessionModal is rendering a stray literal dollar sign before the XLM amount, which makes the confirmation look like a USD value. Update the JSX in the success message block inside FundSessionModal so the displayed amount uses only depositAmount.toFixed(2) with the XLM unit and no leading currency symbol, keeping the wording consistent with the escrow confirmation text.
🧹 Nitpick comments (4)
src/app/dashboard/governance/page.tsx (2)
181-205: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
aria-pressedto lockup period toggle buttons.The selected state is conveyed only visually (gradient background + floating "Active" badge). Screen reader users have no way to determine which lockup period is currently selected.
♻️ Proposed fix
<button key={option.days} onClick={() => setLockupDays(option.days)} + aria-pressed={lockupDays === option.days} className={cn(🤖 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/app/dashboard/governance/page.tsx` around lines 181 - 205, The lockup period toggle buttons in the LOCKUP_OPTIONS map only expose selection visually, so add an aria-pressed state to each button based on whether lockupDays matches option.days. Keep the existing onClick and styling logic in place, and update the button element in the governance page so screen readers can announce which lockup period is currently active.
176-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLabels not programmatically associated with their inputs.
The
<label>elements for "Lockup Period" and "Amount to Lock" lackhtmlFor/idpairing with their respective controls, so assistive technologies can't announce the field purpose when focus moves to the input/button group.Also applies to: 210-213
🤖 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/app/dashboard/governance/page.tsx` around lines 176 - 179, The “Lockup Period” and “Amount to Lock” labels in the governance page are not associated with their controls, so fix the accessibility wiring in the relevant form sections. Update the label/input/button groups in the page component so each <label> has an htmlFor that matches a unique id on its corresponding control, and make sure the same pairing is applied to both the Lockup Period and Amount to Lock fields. Use the existing form sections in page.tsx to locate the affected controls and keep the association consistent across the related input/button group.src/components/marketplace/FundSessionModal.tsx (2)
176-176: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent decimal precision across fee rows.
depositAmountis formatted with.toFixed(2)whilegasFee,platformFee, andtotalAmountuse.toFixed(3). This inconsistency, plus rounding each row independently, can make the displayed total appear not to match the sum of the displayed rows. Consider using a single consistent precision throughout the breakdown.Also applies to: 185-185, 195-195, 201-201, 236-236, 240-240, 244-244
🤖 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/marketplace/FundSessionModal.tsx` at line 176, The fee breakdown in FundSessionModal is using inconsistent decimal precision, which can make the displayed rows and total look mismatched. Update the amount formatting in the fee row renderers and the total display (including depositAmount, gasFee, platformFee, and totalAmount) to use a single consistent precision, and keep the logic in FundSessionModal aligned so the breakdown sums visually match.
171-205: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTable rows use
flexdisplay, breaking table semantics.
<tr>elements getflex justify-between, overriding their implicitdisplay: table-row. This is unreliable across browsers and defeats the purpose of using semantic<table>/<tbody>/<tr>/<td>markup (also degrades table accessibility for screen readers). Prefer a plaindiv-based grid/flex layout, or drop the flex overrides and rely on native table cell layout with<td>alignment classes.♻️ Suggested refactor (div-based rows)
- <table className="w-full text-sm"> - <tbody className="space-y-2"> - <tr className="flex justify-between py-1.5"> - <td className="text-muted-foreground">Initial Deposit</td> - <td className="font-medium text-white"> - {depositAmount.toFixed(2)} XLM - </td> - </tr> + <div className="w-full text-sm space-y-2"> + <div className="flex justify-between py-1.5"> + <span className="text-muted-foreground">Initial Deposit</span> + <span className="font-medium text-white"> + {depositAmount.toFixed(2)} XLM + </span> + </div> ... - </tbody> - </table> + </div>🤖 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/marketplace/FundSessionModal.tsx` around lines 171 - 205, The table markup in FundSessionModal is mixing semantic <table>/<tr>/<td> elements with flex layout, which breaks native table behavior and accessibility. Update the rendering block that maps the Initial Deposit / Estimated Soroban Gas / Platform Service Fee / Total Amount to Lock rows so the row layout uses a div-based flex/grid structure, or remove the flex utilities from the <tr> elements and rely on normal table cell alignment instead. Keep the existing values and formatting logic, but preserve proper semantics or switch fully to a non-table layout.
🤖 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/app/dashboard/governance/page.tsx`:
- Line 33: The claim flow in the governance page keeps rendering the original
reward amount because `unclaimedRewards` is only read from `useState` and never
updated after `handleClaimAll` succeeds. Update the `unclaimedRewards` state in
the claim handler for the `page.tsx` dashboard so the metric card and “Claim
Rewards” card reflect zero (or the decremented amount) after a successful claim.
Use the existing `handleClaimAll` flow and the `unclaimedRewards` state variable
to locate the fix, and make sure the claim button state also follows the updated
balance.
- Around line 56-63: The handleLockTokens flow in governance/page.tsx finishes
silently and never changes the available stakedBalance, so mirror the
handleClaimAll success UX by showing a success/tx-style confirmation after the
async lock completes and then decrement stakedBalance by the locked amount
before clearing lockAmount; update the handleLockTokens logic (and the related
lock UI state it drives) so the user sees completion feedback and cannot re-lock
the same tokens immediately.
- Around line 41-44: The voting-weight preview in the governance page is using a
falsy fallback that turns an explicit "0" into the staked balance. Update the
logic around calculateVotingWeight in page.tsx to distinguish an empty
lockAmount from a valid numeric zero by checking for an empty string or NaN
explicitly before falling back to stakedBalance. Keep the existing
parseFloat-based flow, but ensure a user-entered "0" is passed through as 0
instead of being replaced.
---
Outside diff comments:
In `@src/components/marketplace/FundSessionModal.tsx`:
- Around line 289-294: The success message in FundSessionModal is rendering a
stray literal dollar sign before the XLM amount, which makes the confirmation
look like a USD value. Update the JSX in the success message block inside
FundSessionModal so the displayed amount uses only depositAmount.toFixed(2) with
the XLM unit and no leading currency symbol, keeping the wording consistent with
the escrow confirmation text.
---
Nitpick comments:
In `@src/app/dashboard/governance/page.tsx`:
- Around line 181-205: The lockup period toggle buttons in the LOCKUP_OPTIONS
map only expose selection visually, so add an aria-pressed state to each button
based on whether lockupDays matches option.days. Keep the existing onClick and
styling logic in place, and update the button element in the governance page so
screen readers can announce which lockup period is currently active.
- Around line 176-179: The “Lockup Period” and “Amount to Lock” labels in the
governance page are not associated with their controls, so fix the accessibility
wiring in the relevant form sections. Update the label/input/button groups in
the page component so each <label> has an htmlFor that matches a unique id on
its corresponding control, and make sure the same pairing is applied to both the
Lockup Period and Amount to Lock fields. Use the existing form sections in
page.tsx to locate the affected controls and keep the association consistent
across the related input/button group.
In `@src/components/marketplace/FundSessionModal.tsx`:
- Line 176: The fee breakdown in FundSessionModal is using inconsistent decimal
precision, which can make the displayed rows and total look mismatched. Update
the amount formatting in the fee row renderers and the total display (including
depositAmount, gasFee, platformFee, and totalAmount) to use a single consistent
precision, and keep the logic in FundSessionModal aligned so the breakdown sums
visually match.
- Around line 171-205: The table markup in FundSessionModal is mixing semantic
<table>/<tr>/<td> elements with flex layout, which breaks native table behavior
and accessibility. Update the rendering block that maps the Initial Deposit /
Estimated Soroban Gas / Platform Service Fee / Total Amount to Lock rows so the
row layout uses a div-based flex/grid structure, or remove the flex utilities
from the <tr> elements and rely on normal table cell alignment instead. Keep the
existing values and formatting logic, but preserve proper semantics or switch
fully to a non-table layout.
🪄 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
Run ID: 3b583c39-8ee5-4727-bafe-842d4a95bfba
📒 Files selected for processing (2)
src/app/dashboard/governance/page.tsxsrc/components/marketplace/FundSessionModal.tsx
| } | ||
|
|
||
| export default function GovernancePage() { | ||
| const [unclaimedRewards] = useState(MOCK_UNCLAIMED_REWARDS) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Claim flow shows success but never updates the displayed balance.
unclaimedRewards is a const [unclaimedRewards] = useState(...) with no setter used anywhere. After handleClaimAll completes and shows the "Successfully claimed!" banner (Line 154-162), the "Unclaimed Rewards Balance" metric card (Line 81-88) and "Claim Rewards" card (Line 140-142) still display the original 2,450 SKILL, and the Claim button remains enabled to "claim" the same amount again. Even for a simulated/mock flow, this is inconsistent and misleading — the state should be updated to zero (or decremented) on successful claim to reflect reality.
🐛 Proposed fix
- const [unclaimedRewards] = useState(MOCK_UNCLAIMED_REWARDS)
+ const [unclaimedRewards, setUnclaimedRewards] = useState(MOCK_UNCLAIMED_REWARDS) const handleClaimAll = async () => {
if (unclaimedRewards <= 0) return
setIsClaiming(true)
await new Promise((r) => setTimeout(r, 2000))
setClaimTxHash("0x" + Array.from({ length: 40 }, () =>
Math.floor(Math.random() * 16).toString(16)
).join(""))
+ setUnclaimedRewards(0)
setIsClaiming(false)
}Also applies to: 46-54, 136-163
🤖 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/app/dashboard/governance/page.tsx` at line 33, The claim flow in the
governance page keeps rendering the original reward amount because
`unclaimedRewards` is only read from `useState` and never updated after
`handleClaimAll` succeeds. Update the `unclaimedRewards` state in the claim
handler for the `page.tsx` dashboard so the metric card and “Claim Rewards” card
reflect zero (or the decremented amount) after a successful claim. Use the
existing `handleClaimAll` flow and the `unclaimedRewards` state variable to
locate the fix, and make sure the claim button state also follows the updated
balance.
| const votingWeight = calculateVotingWeight( | ||
| parseFloat(lockAmount) || stakedBalance, | ||
| lockupDays | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Falsy-zero fallback misfires on explicit "0" input.
parseFloat(lockAmount) || stakedBalance treats an explicitly entered "0" the same as an empty string, silently substituting the full staked balance in the voting-weight preview instead of showing 0. Use an explicit empty-string/NaN check instead.
♻️ Proposed fix
- const votingWeight = calculateVotingWeight(
- parseFloat(lockAmount) || stakedBalance,
- lockupDays
- )
+ const parsedLockAmount = parseFloat(lockAmount)
+ const votingWeight = calculateVotingWeight(
+ lockAmount === "" || Number.isNaN(parsedLockAmount) ? stakedBalance : parsedLockAmount,
+ lockupDays
+ )📝 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.
| const votingWeight = calculateVotingWeight( | |
| parseFloat(lockAmount) || stakedBalance, | |
| lockupDays | |
| ) | |
| const parsedLockAmount = parseFloat(lockAmount) | |
| const votingWeight = calculateVotingWeight( | |
| lockAmount === "" || Number.isNaN(parsedLockAmount) ? stakedBalance : parsedLockAmount, | |
| lockupDays | |
| ) |
🤖 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/app/dashboard/governance/page.tsx` around lines 41 - 44, The
voting-weight preview in the governance page is using a falsy fallback that
turns an explicit "0" into the staked balance. Update the logic around
calculateVotingWeight in page.tsx to distinguish an empty lockAmount from a
valid numeric zero by checking for an empty string or NaN explicitly before
falling back to stakedBalance. Keep the existing parseFloat-based flow, but
ensure a user-entered "0" is passed through as 0 instead of being replaced.
| const handleLockTokens = async () => { | ||
| const amount = parseFloat(lockAmount) | ||
| if (isNaN(amount) || amount <= 0 || amount > stakedBalance) return | ||
| setIsLocking(true) | ||
| await new Promise((r) => setTimeout(r, 2000)) | ||
| setIsLocking(false) | ||
| setLockAmount("") | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Lock action gives no success feedback and never updates staked balance.
Unlike handleClaimAll, handleLockTokens doesn't surface any confirmation to the user after "locking" completes (only clears the input), and stakedBalance is never decremented — so a user could immediately re-lock the same tokens for another voting-weight boost. Consider adding a success indicator (mirroring the claim flow's tx banner) and reducing the available balance to lock.
Also applies to: 267-286
🤖 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/app/dashboard/governance/page.tsx` around lines 56 - 63, The
handleLockTokens flow in governance/page.tsx finishes silently and never changes
the available stakedBalance, so mirror the handleClaimAll success UX by showing
a success/tx-style confirmation after the async lock completes and then
decrement stakedBalance by the locked amount before clearing lockAmount; update
the handleLockTokens logic (and the related lock UI state it drives) so the user
sees completion feedback and cannot re-lock the same tokens immediately.
2 participants
Summary
#364 Governance Token Claim Manager UI
New dashboard page at
src/app/dashboard/governance/page.tsxwith:#363 Detailed Transaction Fee Breakdown
Updated
src/components/marketplace/FundSessionModal.tsxwith:PLATFORM_FEE_PERCENTconstantCloses #364, Closes #363
Summary by CodeRabbit
New Features
Bug Fixes