-
Notifications
You must be signed in to change notification settings - Fork 177
fix: add loading spinner and disable create process button during tra… #212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: add loading spinner and disable create process button during tra… #212
Conversation
📝 WalkthroughWalkthroughRefactors the create-process UI and deployment flow: removes unused imports, adds client-side proposal validation and formatting, introduces Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateProcessPage
participant deployVotingProcess
participant Blockchain
Note over CreateProcessPage: Form validation & formatting
User->>CreateProcessPage: Submit form (name, desc, proposals)
CreateProcessPage->>CreateProcessPage: isFormValid() & formatProposals()
CreateProcessPage->>CreateProcessPage: set pending = true, clear transactionResult
CreateProcessPage->>deployVotingProcess: deploy(name, desc, proposalsUtf8, startDate, endDate)
alt deploy succeeds
deployVotingProcess->>Blockchain: send transaction
Blockchain-->>deployVotingProcess: txReceipt
deployVotingProcess-->>CreateProcessPage: txReceipt
CreateProcessPage->>CreateProcessPage: set transactionResult, set open = true
CreateProcessPage-->>User: show modal with collapsible tx details
else deploy fails
deployVotingProcess-->>CreateProcessPage: throw error
CreateProcessPage->>CreateProcessPage: log & alert error
end
CreateProcessPage->>CreateProcessPage: set pending = false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (3)
30-30: Remove commented-out dead code.This leftover comment serves no documentation purpose and should be deleted to keep the codebase clean.
🔎 Proposed fix
- // const refValue = useRef(false); -
44-50: Extract magic numbers into named constants.The hardcoded values
1000000lack context. Consider extracting them into well-named constants to clarify their purpose (e.g., gas limits, vote thresholds, or other contract parameters).🔎 Proposed fix
+const DEFAULT_GAS_LIMIT = 1000000; // Adjust name based on actual purpose +const DEFAULT_PARAMETER = 1000000; // Adjust name based on actual purpose + const CreateProcess = () => {Then update the call:
const result = await deployVotingProcess( name, description, proposalArray, - 1000000, - 1000000 + DEFAULT_GAS_LIMIT, + DEFAULT_PARAMETER );
69-76: Consider using idiomatic array methods.The function can be simplified with
mapand filtering empty entries to align with the validation fix.🔎 Proposed fix
const formatProposals = (input) => { - let proposals = input.split(','); - let array = [] - for(let i=0; i < proposals.length; i ++){ - array.push(ethers.utils.toUtf8Bytes(proposals[i].trim())); - } - return array; + return input + .split(',') + .map(p => p.trim()) + .filter(p => p.length > 0) + .map(p => ethers.utils.toUtf8Bytes(p)); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js
🔇 Additional comments (2)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (2)
7-9: LGTM!Imports are streamlined appropriately for the component's needs. The
Spinnerimport supports the new loading state feature.
114-131: LGTM! Loading state implementation achieves PR objectives.The button correctly disables during pending operations and shows a spinner with updated text, providing clear visual feedback to users during transaction processing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (1)
30-30: Remove commented-out code.Dead code from the previous implementation should be deleted rather than commented out. Version control preserves the history if needed.
🔎 Proposed fix
- // const refValue = useRef(false); -
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js
🔇 Additional comments (3)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (3)
8-9: LGTM! Clean import simplification.The reduced imports reflect the refactored state management approach, removing unused hooks like
useRefanduseEffect.
32-61: Excellent refactoring of the async flow!The changes implement robust error handling with proper state management:
- Early validation prevents unnecessary API calls
- Try/catch/finally ensures
pendingstate is always reset- Clear user feedback on both success and failure
121-139: Perfect implementation of loading state UX!The button correctly implements the PR objectives:
- Displays a spinner with "Creating..." text during processing
- Disables the button to prevent duplicate submissions
- Provides clear visual feedback to users
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (1)
77-84: Fix inconsistency: formatProposals should filter empty strings.
isFormValidfilters out empty proposals after trimming, butformatProposalsdoes not. This means input like"a,,b,c"would pass validation (3 valid proposals) but submit 4 items including an empty byte array.🔎 Proposed fix to align formatProposals with isFormValid
const formatProposals = (input) => { - let proposals = input.split(','); - let array = [] - for(let i=0; i < proposals.length; i ++){ - array.push(ethers.utils.toUtf8Bytes(proposals[i].trim())); - } - return array; + return input + .split(',') + .map(p => p.trim()) + .filter(p => p.length > 0) + .map(p => ethers.utils.toUtf8Bytes(p)); }
♻️ Duplicate comments (1)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (1)
64-74: Critical: Remove the extra closing brace on line 73.Line 73 contains a stray
};that causes a syntax error. This was already flagged in the previous review and must be removed before merging.🔎 Proposed fix
return validProposals.length >= 2; }; -}; -
🧹 Nitpick comments (1)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (1)
30-30: Remove commented-out code.The commented
refValueref is no longer used and should be deleted rather than left as a comment.🔎 Proposed fix
- // const refValue = useRef(false); -
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js
🔇 Additional comments (3)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (3)
8-9: LGTM! Clean import reduction.The import cleanup appropriately removes unused dependencies (useEffect, useRef, test contract functions) while retaining only what's needed for the new pending-state flow.
40-60: Solid async flow with proper error handling.The try/catch/finally structure ensures
pendingis always reset, and early validation prevents invalid submissions. The success and error paths are handled appropriately.
122-140: Excellent UX improvement with loading state.The button implementation perfectly addresses the PR objectives:
- Displays a spinner during transaction processing
- Disables the button to prevent duplicate submissions
- Updates the label to indicate processing state ("Creating...")
This provides clear visual feedback to users during the asynchronous blockchain transaction.
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js
🔇 Additional comments (5)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (5)
1-10: LGTM: Import cleanup is appropriate.The removal of unused imports (
useEffect,useRef, test contract functions) and the retention of necessary dependencies align well with the refactored implementation.
18-22: LGTM: State management correctly supports loading feedback.The addition of
pending,open, andtransactionResultstate variables properly implements the loading/disabled button behavior described in the PR objectives.
29-44: Validation logic is correct, but formatProposals is unused.The
isFormValidfunction correctly ensures at least two non-empty proposals. However,formatProposalsis defined but never utilized—see the critical issue flagged below regarding missing proposals in the deployment call.
119-136: Excellent UI improvements—PR objectives achieved!The loading spinner, disabled button state, and updated button text ("Creating...") provide clear visual feedback during transaction execution. This prevents duplicate submissions and aligns with expected UX for async blockchain operations, exactly as described in the PR objectives.
140-181: LGTM: Modal provides clear transaction feedback.The success modal with collapsible transaction details (hash and nonce) provides appropriate feedback to users. The conditional rendering based on
transactionResultprevents errors if the state is null.
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js
Outdated
Show resolved
Hide resolved
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (2)
55-56: Date computation is correct, but consider making duration configurable.The timestamps are now computed correctly (current time and 1 day later), addressing the previous hardcoded values issue. However, a 1-day voting window might be short for some use cases. Consider adding a duration input field to let users specify the voting period.
70-75: Consider enhancing error presentation.The error handling correctly logs the issue and resets the pending state. However, using
window.alertfor errors could be improved with a more polished UI (e.g., an error modal or toast notification) for better UX consistency with the success modal.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js
🔇 Additional comments (7)
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (7)
1-8: LGTM!Imports are clean and align with the component's functionality. The addition of
ethersfor proposal encoding andSpinnerfor loading feedback directly supports the PR objectives.
16-19: LGTM!State management correctly supports the loading/disabled button feature and transaction result display.
26-32: LGTM!Validation logic correctly ensures at least two non-empty proposals with proper trimming and filtering.
34-40: LGTM!Proposal formatting correctly converts strings to UTF-8 bytes using the ethers library, consistent with the validation logic.
116-133: LGTM! PR objectives successfully implemented.The button correctly:
- Shows a loading spinner during transaction processing
- Disables to prevent duplicate submissions
- Provides clear visual feedback with "Creating..." text
This directly addresses the PR's goal of improving UX during asynchronous blockchain transactions.
154-170: LGTM!Modal correctly displays transaction details with proper null-checking. The transaction hash and nonce display assumes the result structure from
deployVotingProcessincludes these properties (verification requested in earlier comment).
45-48: LGTM!Form validation correctly prevents submission with fewer than 2 proposals, providing clear feedback to users.
| const createProcess = async (e) => { | ||
| e.preventDefault(); | ||
| console.log("name: ", name); | ||
| //format proposals | ||
| let proposalArray = formatProposals(proposals); | ||
| //check form inputs | ||
| if(!isFormValid()){ | ||
| window.alert("Form is not valid"); | ||
|
|
||
| if (!isFormValid()) { | ||
| window.alert("Form is not valid. Enter at least 2 proposals."); | ||
| return; | ||
| } | ||
| setPending(true); | ||
| refValue.current = true; | ||
| //deploy new process contract | ||
| const result = await deployVotingProcess(name, description, proposalArray,1000000,1000000); | ||
| setTransactionResult(result); | ||
|
|
||
| refValue.current = false; | ||
| setPending(false); | ||
|
|
||
| setShow(true); | ||
| } | ||
|
|
||
| const isFormValid = () => { | ||
| if(proposals < 2) | ||
| return false; | ||
| return true; | ||
| } | ||
| try { | ||
| setPending(true); | ||
|
|
||
| const formatProposals = (input) => { | ||
| let proposals = input.split(','); | ||
| let array = [] | ||
| for(let i=0; i < proposals.length; i ++){ | ||
| array.push(ethers.utils.toUtf8Bytes(proposals[i].trim())); | ||
| } | ||
| return array; | ||
| } | ||
| const _formattedProposals = formatProposals(proposals); | ||
|
|
||
| const startDate = Math.floor(Date.now() / 1000); // current time | ||
| const endDate = startDate + 24 * 60 * 60; // 1 day later | ||
|
|
||
| // Deploy with proposals | ||
| const result = await deployVotingProcess( | ||
| name, | ||
| description, | ||
| startDate, | ||
| endDate, | ||
| _formattedProposals // <-- send proposals to contract | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| // setPending(!pending); | ||
| }, [pending]) | ||
| setTransactionResult(result); | ||
| setShow(true); | ||
|
|
||
| return ( | ||
| <div > | ||
| } catch (err) { | ||
| console.error(err); | ||
| window.alert("Transaction failed: " + (err.message || err)); | ||
| } finally { | ||
| setPending(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the contracts.js file
echo "=== Finding contracts.js file ==="
find . -name "contracts.js" -type f
echo -e "\n=== Content of contracts.js ==="
# Get the contracts file path
CONTRACT_FILE=$(find . -name "contracts.js" -type f | head -1)
if [ -n "$CONTRACT_FILE" ]; then
wc -l "$CONTRACT_FILE"
cat -n "$CONTRACT_FILE"
fiRepository: AOSSIE-Org/Agora-Blockchain
Length of output: 5494
Update deployVotingProcess to accept and use proposals parameter.
The function signature on line 12 of contracts.js accepts only 4 parameters (name, description, startDate, endDate), but CreateProcessPage.js calls it with 5 arguments, passing _formattedProposals as the 5th parameter. This parameter is currently ignored. The function must be updated to accept proposals and pass them to the contract's createVotingProcess method.
🤖 Prompt for AI Agents
In anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js
around lines 42 to 76, CreateProcessPage passes a fifth parameter
(_formattedProposals) to deployVotingProcess but the function in contracts.js
only accepts four params and ignores proposals; update the deployVotingProcess
signature to accept a proposals parameter, include that parameter in the call to
the smart-contract method (e.g., pass proposals to contract.createVotingProcess
as the proposals argument), ensure proposals are validated/serialized in the
same format expected by the contract, propagate errors as before, and update any
other callers/tests to pass the proposals argument if necessary.
What does this PR do?
This PR improves user experience when creating a new voting process by adding
visual feedback during transaction execution.
#208
Changes
Why this change is needed
Previously, there was no visual feedback while the transaction was processing,
which could confuse users and give the impression that the button was unresponsive
or broken.
This change aligns the behavior with expected UX for async blockchain transactions.
Screenshots (if applicable)
(Optional but nice if you add one later)
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
UI Improvements
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.