Improve Collect modal UX#166
Conversation
- Rename Withdraw to Collect - Add dynamic submit button labels - Add MAX button for amount input
📝 WalkthroughWalkthroughProjectDetails.svelte adds a Changes
Sequence Diagram(s)(omitted — changes are UI-level tweaks without multi-component sequential flow) Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/ProjectDetails.svelte (1)
1101-1112: Fix duplicatedisabledattribute and malformed button tag.The button element has a duplicate
disabledattribute specification (lines 1105-1106 and 1108-1109), and the closing>tag appears twice. This will cause HTML parsing issues.🐛 Proposed fix
<Button class="action-btn" style="background-color: #FF8C00; color: black;" on:click={setupWithdrawTokens} disabled={!$connected || maxWithdrawTokenAmount <= 0} - > - disabled={!$connected || - maxWithdrawTokenAmount <= 0} - > - Collect {project.token_details.name} + > + Collect {project.token_details.name} </Button>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/ProjectDetails.svelte
🔇 Additional comments (5)
src/routes/ProjectDetails.svelte (5)
146-146: LGTM! Good addition for dynamic button labeling.The new
submit_btn_labelstate variable enables context-specific button text, making the UI more intuitive for users.
414-428: LGTM! Well-structured helper function.The
setMax()function correctly maps each action type to its corresponding maximum value, providing a clean way to auto-fill the maximum allowable amount.
430-440: LGTM! Excellent UX improvements with dynamic action labels.All setup functions now set appropriate
submit_btn_labelvalues that clearly communicate the action being performed:
- "Add" for adding tokens
- "Collect" for withdrawing/collecting tokens and funds (replacing generic "Withdraw")
- "Contribute" for buying/contributing
- "Refund" for refund actions
These changes directly address the PR objectives and make the UI more intuitive.
Also applies to: 454-464, 478-493, 507-522, 559-569, 590-604
1334-1340: LGTM! Clean MAX button implementation.The MAX button is well-positioned next to the input field and properly styled to match the application's orange theme. It effectively calls the
setMax()helper to auto-fill the maximum allowable amount.
1363-1372: LGTM! Dynamic submit button implementation is complete.The submit button correctly displays the dynamic
submit_btn_labelduring normal operation and shows "Processing..." during submission. This completes the PR's objective of replacing generic "Submit" text with context-specific action labels.
hemantch01
left a comment
hemantch01
left a comment
There was a problem hiding this comment.
You’ve accidentally put the style attribute inside the button’s content, and also duplicated the closing >
find and fix it here --> it's on src/routes/projectDetails.svelte on line 1363
|
@hemantch01 Thanks for catching this! You’re right — the style attribute was accidentally placed inside the button content and the closing > got duplicated. I’ve fixed it in src/routes/projectDetails.svelte (around line 1363). |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/routes/ProjectDetails.svelte:
- Around line 1108-1111: The Collect button in ProjectDetails.svelte renders
with a duplicate disabled attribute; remove the repeated disabled binding on the
element that renders "Collect {project.token_details.name}" so only the original
disabled={!$connected || maxWithdrawTokenAmount <= 0} remains (keep the existing
condition, delete the extra disabled declaration).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/ProjectDetails.svelte
🔇 Additional comments (7)
src/routes/ProjectDetails.svelte (7)
146-146: LGTM!The
submit_btn_labelstate variable properly enables dynamic button labeling. The default "Submit" value serves as a reasonable fallback, though it should never display since all action flows properly set this value in their setup functions.
414-428: LGTM!The
setMax()function correctly maps each action type to its corresponding maximum value. The implementation properly handles all six action types (buy, refund, temp_exchange, add_tokens, withdraw_tokens, withdraw_erg).
434-434: LGTM! Excellent action-specific labeling.All setup functions now properly assign descriptive, context-specific labels that clearly communicate the action to users. The labels ("Add", "Collect", "Contribute", "Refund") are much more intuitive than the generic "Submit" and align perfectly with the PR objectives.
Also applies to: 458-458, 487-487, 516-516, 563-563, 598-598
457-457: LGTM! Improved terminology.The label text changes from "withdraw" to "collect" improve clarity and consistency. This user-facing terminology better reflects the action's intent and aligns with the updated button labels.
Also applies to: 486-486
1334-1340: LGTM! Clean MAX button implementation.The MAX button is well-implemented with appropriate styling, positioning, and click handling. It provides a clear, accessible way for users to quickly fill in the maximum allowable amount.
1370-1370: LGTM! Dynamic button text implementation.The submit button correctly displays action-specific labels when idle and "Processing..." during submission, providing clear feedback to users about both the action and its state.
1261-1318: Verify if clickable balance displays are intended.The PR description mentions "Add a MAX button and make displayed balances clickable to quickly fill the maximum allowed amount." The MAX button is implemented, but the balance displays in the form info section (e.g., "Your Wallet Balance", "Max You Can Buy", "Refundable Amount", etc.) are not clickable.
If making these balances clickable to auto-fill the amount was intended, click handlers would need to be added to these display elements.
|
@0xf965 @Zahnentferner could you please review this PR when you get a chance? Thanks! |
|
@sagar-h007 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/routes/ProjectDetails.svelte:
- Around line 1278-1283: The span with class "clickable-amount" only has
on:click={() => setMax()} so keyboard users can’t activate it; make it focusable
and respond to Enter/Space by adding tabindex="0", role="button", and an
on:keydown handler that calls setMax() when event.key is "Enter" or " " (Space)
— update the span element (the clickable-amount span that shows
{projectFundsAmount.toFixed(4)} {baseTokenName}) to include these attributes and
the keydown handler so it is keyboard-accessible.
- Around line 1371-1374: The CSS rule for .clickable-amount is incorrectly
embedded inside the HTML markup of ProjectDetails.svelte; remove the
.clickable-amount block from its current location in the form and place that
rule into the component's existing <style> block (e.g., inside the style tag in
ProjectDetails.svelte where other component styles live) so the styling is
applied correctly; ensure you only move the CSS rule (cursor: pointer;
text-decoration: underline;) and do not alter the class name usage in the
markup.
🧹 Nitpick comments (1)
src/routes/ProjectDetails.svelte (1)
414-428: Consider using a switch statement for cleaner code.The
setMax()function works correctly but could be more maintainable with a switch statement or object lookup pattern.♻️ Alternative implementation using switch
function setMax() { - if (function_submit === buy) { - value_submit = maxContributeAmount; - } else if (function_submit === refund) { - value_submit = maxRefundAmount; - } else if (function_submit === temp_exchange) { - value_submit = maxCollectAmount; - } else if (function_submit === add_tokens) { - value_submit = maxAddTokenAmount; - } else if (function_submit === withdraw_tokens) { - value_submit = maxWithdrawTokenAmount; - } else if (function_submit === withdraw_erg) { - value_submit = maxWithdrawErgAmount; + switch (function_submit) { + case buy: + value_submit = maxContributeAmount; + break; + case refund: + value_submit = maxRefundAmount; + break; + case temp_exchange: + value_submit = maxCollectAmount; + break; + case add_tokens: + value_submit = maxAddTokenAmount; + break; + case withdraw_tokens: + value_submit = maxWithdrawTokenAmount; + break; + case withdraw_erg: + value_submit = maxWithdrawErgAmount; + break; } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/ProjectDetails.svelte
🔇 Additional comments (6)
src/routes/ProjectDetails.svelte (6)
146-146: LGTM: Dynamic button label state added.The new
submit_btn_labelvariable properly supports dynamic button text based on the current action.
174-190: LGTM: Terminology improved for clarity.The prefix changes from "Withdraw:" to "Collect:" better communicate user intent and align with the PR objectives.
430-604: LGTM: Setup functions properly configure dynamic labels.All action setup functions correctly set
submit_btn_labelwith context-appropriate text (Add, Collect, Contribute, Refund), and the label text changes improve terminology consistency.
1101-1108: LGTM: Owner action button updated with consistent terminology.The button text change to "Collect" maintains consistency with the updated terminology throughout the modal.
1334-1340: LGTM: Max button properly implemented.The Max button provides a clear, accessible way for users to quickly fill the maximum allowed amount.
1363-1370: LGTM: Submit button now uses dynamic labels.The button correctly displays context-specific text based on the current action, improving user clarity about what action will be performed.
| <span | ||
| class="clickable-amount" | ||
| on:click={() => setMax()} | ||
| > | ||
| {projectFundsAmount.toFixed(4)} {baseTokenName} | ||
| </span> |
There was a problem hiding this comment.
Add keyboard accessibility for clickable amount.
The clickable amount is missing keyboard event handlers. Users navigating with keyboard cannot activate this feature.
🔧 Suggested fix to add keyboard accessibility
<p>
<strong>Project Funds:</strong>
<span
class="clickable-amount"
- on:click={() => setMax()}
+ role="button"
+ tabindex="0"
+ on:click={() => setMax()}
+ on:keypress={(e) => { if (e.key === 'Enter' || e.key === ' ') setMax(); }}
>
{projectFundsAmount.toFixed(4)} {baseTokenName}
</span>
</p>🤖 Prompt for AI Agents
In @src/routes/ProjectDetails.svelte around lines 1278 - 1283, The span with
class "clickable-amount" only has on:click={() => setMax()} so keyboard users
can’t activate it; make it focusable and respond to Enter/Space by adding
tabindex="0", role="button", and an on:keydown handler that calls setMax() when
event.key is "Enter" or " " (Space) — update the span element (the
clickable-amount span that shows {projectFundsAmount.toFixed(4)}
{baseTokenName}) to include these attributes and the keydown handler so it is
keyboard-accessible.
| .clickable-amount { | ||
| cursor: pointer; | ||
| text-decoration: underline; | ||
| } |
There was a problem hiding this comment.
Critical: CSS placed inside HTML markup.
The .clickable-amount CSS rule (lines 1371-1374) is incorrectly placed inside the form markup instead of the <style> block. This will cause rendering issues and the styles won't be applied.
🐛 Fix: Move CSS to the style block
Remove lines 1371-1374 from their current location and add them to the <style> block (after line 1404 or anywhere within the existing styles):
<Button
on:click={function_submit}
disabled={isSubmitting || value_submit <= 0}
class="submit-btn"
style="background-color: #FF8C00; color: black;"
>
{isSubmitting ? "Processing..." : submit_btn_label}
</Button>
- .clickable-amount {
- cursor: pointer;
- text-decoration: underline;
- }Then add to the <style> block:
<style>
/* Base Layout */
.project-detail {
min-height: 100vh;
height: auto;
width: 100%;
padding: 2rem;
overflow-y: auto;
overflow-x: hidden;
position: relative;
}
+
+ .clickable-amount {
+ cursor: pointer;
+ text-decoration: underline;
+ }
.project-container {
display: grid;🤖 Prompt for AI Agents
In @src/routes/ProjectDetails.svelte around lines 1371 - 1374, The CSS rule for
.clickable-amount is incorrectly embedded inside the HTML markup of
ProjectDetails.svelte; remove the .clickable-amount block from its current
location in the form and place that rule into the component's existing <style>
block (e.g., inside the style tag in ProjectDetails.svelte where other component
styles live) so the styling is applied correctly; ensure you only move the CSS
rule (cursor: pointer; text-decoration: underline;) and do not alter the class
name usage in the markup.
|
@Zahnentferner @0xf965 Thanks for flagging this — it’s fixed now 👍 The stray disabled=false text is removed, wording is cleaned up to use Collect consistently, and I’ve added a Max / click-to-fill option for the amount. |
|
hi @0xf965 @AdityaGupta20871 , Is there anything that needs to be changed ? |
4 participants
This PR improves the Collect modal by making actions clearer and easier to use.
-Renamed “Withdraw” to “Collect” to better match what users are actually doing.
-Updated the submit button to show the current action (Collect, Contribute, Refund, etc.) instead of a generic “Submit”.
-Added a MAX button and made balances clickable to quickly fill the maximum allowed amount.
Testing:
Manually tested all action flows to confirm labels and MAX behavior work as expected.
UI-only change; no contract or protocol logic is affected.
Fixes:#165
Summary by CodeRabbit
New Features
UI / Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.