-
Notifications
You must be signed in to change notification settings - Fork 173
Hk2166 branch #182
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?
Hk2166 branch #182
Conversation
|
fixed the #179 |
WalkthroughAdds Help and Wallet features with new pages and UI components, updates header navigation, refactors ElectionMini update timing, enhances create flow with validations and submission handling, adjusts global scrolling behavior, and consolidates environment example variables by adding root .env.example and removing client/blockchain examples. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant WalletPage
participant Wagmi as Wagmi Hooks
participant Components as Wallet Components
User->>WalletPage: Navigate /wallet
WalletPage->>Wagmi: useAccount()
alt Not connected
WalletPage->>User: Show ConnectButton + SecurityTips
else Connected
WalletPage->>Components: Render WalletInfo(address)
WalletPage->>Components: Render BalanceCard(address, chainId)
WalletPage->>Components: Render NetworkSelector(chainId)
WalletPage->>Components: Render VotingHistory(address, chainId)
User->>WalletPage: Click Copy / Disconnect / Switch Network
WalletPage->>Wagmi: useDisconnect() / useSwitchChain()
Wagmi-->>WalletPage: Result (success/error)
WalletPage-->>User: Toast feedback
end
sequenceDiagram
autonumber
actor User
participant CreatePage
participant Wagmi as Wagmi/viem
participant Router as Next Router
participant Toast as Toast UI
User->>CreatePage: Click "Create Election"
CreatePage->>CreatePage: Validate wallet connected and on Sepolia
CreatePage->>CreatePage: Validate candidates and times
alt Validation fails
CreatePage-->>User: Show toast error
else Valid
CreatePage->>Toast: Show loading toast
CreatePage->>Wagmi: writeContractAsync(args)
alt Success
Wagmi-->>CreatePage: tx hash
CreatePage->>Toast: Success toast
CreatePage->>Router: Redirect after delay
else Error
Wagmi-->>CreatePage: error (rejected/expired/other)
CreatePage->>Toast: Dismiss loading, show error toast
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (2)
client/app/components/Wallet/NetworkSelector.tsx (1)
18-43: Derive the selectable networks from wagmi configurationRight now the UI hard-codes Sepolia/Fuji, so if the wagmi config omits one (e.g., the wallet connector was set up for just Sepolia) we still render a button that can only ever throw the “Failed to switch network” toast. Please build the list from
chains(and optionally filter/annotate) so we only show networks the current connector can really switch to. That keeps the UX honest and removes redundant toasts.- const supportedChains = [ - { id: sepolia.id, name: "Sepolia Testnet", icon: "🔵", ... }, - { id: avalancheFuji.id, name: "Avalanche Fuji", icon: "🔴", ... }, - ]; + const configuredChains = chains.filter((chain) => + [sepolia.id, avalancheFuji.id].includes(chain.id) + ); + + const getChainMeta = (id: number) => + ({ + [sepolia.id]: { icon: "🔵", color: "from-blue-400 to-blue-600", description: "Ethereum test network" }, + [avalancheFuji.id]: { icon: "🔴", color: "from-red-400 to-red-600", description: "Avalanche test network" }, + }[id]);Use
configuredChainswhen rendering to keep the UI aligned with the actual connector capabilities.client/app/components/Wallet/VotingHistory.tsx (1)
49-64: Consider extracting utility functions for reusability.The
getNetworkNameandtruncateHashfunctions are generic utilities that could be useful across multiple components (e.g., in BalanceCard, NetworkSelector). Consider extracting them to a shared utilities file likeclient/app/utils/web3.tsorclient/app/utils/formatters.ts.For example, create
client/app/utils/web3.ts:export const getNetworkName = (network: string): string => { const networkNames: Record<string, string> = { sepolia: "Sepolia", avalancheFuji: "Avalanche Fuji", polygonAmoy: "Polygon Amoy", }; return networkNames[network] || network; }; export const truncateHash = (hash: string, startChars = 6, endChars = 4): string => { return `${hash.slice(0, startChars)}...${hash.slice(-endChars)}`; };Then import and use these utilities across components.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
.DS_Storeis excluded by!**/.DS_Storeclient/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
.env.example(1 hunks)blockchain/.env.example(0 hunks)client/.env.example(0 hunks)client/WALLET_FEATURE.md(1 hunks)client/app/components/Cards/ElectionMini.tsx(2 hunks)client/app/components/Header/Header.tsx(2 hunks)client/app/components/Help/CategoryCard.tsx(1 hunks)client/app/components/Help/ContactSupport.tsx(1 hunks)client/app/components/Help/FAQSection.tsx(1 hunks)client/app/components/Help/HowToGuides.tsx(1 hunks)client/app/components/Help/TroubleshootingGuide.tsx(1 hunks)client/app/components/Wallet/BalanceCard.tsx(1 hunks)client/app/components/Wallet/NetworkSelector.tsx(1 hunks)client/app/components/Wallet/SecurityTips.tsx(1 hunks)client/app/components/Wallet/VotingHistory.tsx(1 hunks)client/app/components/Wallet/WalletInfo.tsx(1 hunks)client/app/create/page.tsx(4 hunks)client/app/globals.css(1 hunks)client/app/help/page.tsx(1 hunks)client/app/wallet/page.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- blockchain/.env.example
- client/.env.example
🧰 Additional context used
🧬 Code graph analysis (2)
client/app/help/page.tsx (5)
client/app/components/Help/CategoryCard.tsx (1)
CategoryCard(21-54)client/app/components/Help/FAQSection.tsx (1)
FAQSection(127-204)client/app/components/Help/HowToGuides.tsx (1)
HowToGuides(98-176)client/app/components/Help/TroubleshootingGuide.tsx (1)
TroubleshootingGuide(154-239)client/app/components/Help/ContactSupport.tsx (1)
ContactSupport(13-227)
client/app/create/page.tsx (3)
client/app/constants.ts (1)
ELECTION_FACTORY_ADDRESS(3-4)client/abi/artifacts/ElectionFactory.ts (1)
ElectionFactory(1-322)client/app/helpers/ErrorMessage.ts (1)
ErrorMessage(15-23)
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 3-3: [UnorderedKey] The RPC_URL_FUJI key should go before the RPC_URL_SEPOLIA key
(UnorderedKey)
[warning] 4-4: [UnorderedKey] The ETHERSCAN_KEY key should go before the PRIVATE_KEY key
(UnorderedKey)
🪛 markdownlint-cli2 (0.18.1)
client/WALLET_FEATURE.md
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (3)
client/app/components/Wallet/SecurityTips.tsx (1)
1-135: LGTM!The SecurityTips component is well-structured with smooth animations, clear security guidance, and a good user experience. The collapsible design pattern is effective for progressive disclosure of security information.
client/app/components/Help/CategoryCard.tsx (1)
1-54: LGTM!The CategoryCard component is well-implemented with clean TypeScript interfaces, smooth animations, and proper conditional styling. The pattern of passing icon components and rendering them dynamically is effective.
client/app/components/Help/HowToGuides.tsx (1)
1-176: LGTM!The HowToGuides component provides a comprehensive and user-friendly interface for step-by-step guides. The collapsible accordion pattern with smooth animations enhances the user experience, and the guide content is detailed and helpful.
| href={`https://etherscan.io/tx/${vote.txHash}`} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="text-xs text-indigo-600 hover:text-indigo-700 font-medium" | ||
| > | ||
| Tx: {truncateHash(vote.txHash)} | ||
| </a> |
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.
Fix the hardcoded block explorer URL.
The component hardcodes https://etherscan.io/tx/ which won't work correctly for testnet transactions. Each network requires its own explorer:
- Sepolia:
https://sepolia.etherscan.io/tx/ - Avalanche Fuji:
https://testnet.snowtrace.io/tx/ - Polygon Amoy:
https://amoy.polygonscan.com/tx/
This will result in broken links when users try to view transaction details.
Apply this diff to fix the explorer URL based on the network:
+ const getExplorerUrl = (network: string, txHash: string) => {
+ const explorers: Record<string, string> = {
+ sepolia: `https://sepolia.etherscan.io/tx/${txHash}`,
+ avalancheFuji: `https://testnet.snowtrace.io/tx/${txHash}`,
+ polygonAmoy: `https://amoy.polygonscan.com/tx/${txHash}`,
+ };
+ return explorers[network] || `https://etherscan.io/tx/${txHash}`;
+ };
+
...
<a
- href={`https://etherscan.io/tx/${vote.txHash}`}
+ href={getExplorerUrl(vote.network, vote.txHash)}
target="_blank"
rel="noopener noreferrer"
className="text-xs text-indigo-600 hover:text-indigo-700 font-medium"
>
Tx: {truncateHash(vote.txHash)}
</a>📝 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.
| href={`https://etherscan.io/tx/${vote.txHash}`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="text-xs text-indigo-600 hover:text-indigo-700 font-medium" | |
| > | |
| Tx: {truncateHash(vote.txHash)} | |
| </a> | |
| // Inside your VotingHistory component, add the helper function: | |
| const getExplorerUrl = (network: string, txHash: string) => { | |
| const explorers: Record<string, string> = { | |
| sepolia: `https://sepolia.etherscan.io/tx/${txHash}`, | |
| avalancheFuji: `https://testnet.snowtrace.io/tx/${txHash}`, | |
| polygonAmoy: `https://amoy.polygonscan.com/tx/${txHash}`, | |
| }; | |
| return explorers[network] || `https://etherscan.io/tx/${txHash}`; | |
| }; | |
| // …later in your JSX, replace the hardcoded href: | |
| <a | |
| href={getExplorerUrl(vote.network, vote.txHash)} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="text-xs text-indigo-600 hover:text-indigo-700 font-medium" | |
| > | |
| Tx: {truncateHash(vote.txHash)} | |
| </a> |
| {/* Connection Status */} | ||
| <div className="grid grid-cols-2 gap-4"> | ||
| <div className="p-4 bg-green-50 rounded-xl"> | ||
| <p className="text-xs font-medium text-green-600 mb-1">Status</p> | ||
| <p className="text-lg font-bold text-green-700">Active</p> | ||
| </div> | ||
| <div className="p-4 bg-blue-50 rounded-xl"> | ||
| <p className="text-xs font-medium text-blue-600 mb-1">Address Type</p> | ||
| <p className="text-lg font-bold text-blue-700">EOA</p> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Address Checksum Info */} | ||
| <div className="p-4 bg-gray-50 rounded-xl border border-gray-200"> | ||
| <p className="text-xs text-gray-600 mb-2 font-medium"> | ||
| Security Info | ||
| </p> | ||
| <p className="text-xs text-gray-500"> | ||
| ✓ Address is checksummed<br /> | ||
| ✓ Valid Ethereum address format<br /> | ||
| ✓ Compatible with EVM chains | ||
| </p> | ||
| </div> |
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.
Guard status badges when no wallet is connected
address is allowed to be undefined, yet we still render “Active” status and the green security checklist, which gives the impression that a wallet is connected and verified. Please gate these badges on the presence of a real address (and fall back to a neutral placeholder when it’s missing) so the UI stays truthful during the onboarding flow.
- {/* Connection Status */}
- <div className="grid grid-cols-2 gap-4">
- <div className="p-4 bg-green-50 rounded-xl">
- <p className="text-xs font-medium text-green-600 mb-1">Status</p>
- <p className="text-lg font-bold text-green-700">Active</p>
- </div>
- <div className="p-4 bg-blue-50 rounded-xl">
- <p className="text-xs font-medium text-blue-600 mb-1">Address Type</p>
- <p className="text-lg font-bold text-blue-700">EOA</p>
- </div>
- </div>
-
- {/* Address Checksum Info */}
- <div className="p-4 bg-gray-50 rounded-xl border border-gray-200">
- <p className="text-xs text-gray-600 mb-2 font-medium">
- Security Info
- </p>
- <p className="text-xs text-gray-500">
- ✓ Address is checksummed<br />
- ✓ Valid Ethereum address format<br />
- ✓ Compatible with EVM chains
- </p>
- </div>
+ {/* Connection Status */}
+ <div className="grid grid-cols-2 gap-4">
+ <div className={`p-4 rounded-xl ${address ? "bg-green-50" : "bg-gray-100"}`}>
+ <p className={`text-xs font-medium mb-1 ${address ? "text-green-600" : "text-gray-500"}`}>
+ Status
+ </p>
+ <p className={`text-lg font-bold ${address ? "text-green-700" : "text-gray-600"}`}>
+ {address ? "Active" : "Not connected"}
+ </p>
+ </div>
+ <div className={`p-4 rounded-xl ${address ? "bg-blue-50" : "bg-gray-100"}`}>
+ <p className={`text-xs font-medium mb-1 ${address ? "text-blue-600" : "text-gray-500"}`}>
+ Address Type
+ </p>
+ <p className={`text-lg font-bold ${address ? "text-blue-700" : "text-gray-600"}`}>
+ {address ? "EOA" : "—"}
+ </p>
+ </div>
+ </div>
+
+ {/* Address Checksum Info */}
+ {address && (
+ <div className="p-4 bg-gray-50 rounded-xl border border-gray-200">
+ <p className="text-xs text-gray-600 mb-2 font-medium">
+ Security Info
+ </p>
+ <p className="text-xs text-gray-500">
+ ✓ Address is checksummed<br />
+ ✓ Valid Ethereum address format<br />
+ ✓ Compatible with EVM chains
+ </p>
+ </div>
+ )}📝 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.
| {/* Connection Status */} | |
| <div className="grid grid-cols-2 gap-4"> | |
| <div className="p-4 bg-green-50 rounded-xl"> | |
| <p className="text-xs font-medium text-green-600 mb-1">Status</p> | |
| <p className="text-lg font-bold text-green-700">Active</p> | |
| </div> | |
| <div className="p-4 bg-blue-50 rounded-xl"> | |
| <p className="text-xs font-medium text-blue-600 mb-1">Address Type</p> | |
| <p className="text-lg font-bold text-blue-700">EOA</p> | |
| </div> | |
| </div> | |
| {/* Address Checksum Info */} | |
| <div className="p-4 bg-gray-50 rounded-xl border border-gray-200"> | |
| <p className="text-xs text-gray-600 mb-2 font-medium"> | |
| Security Info | |
| </p> | |
| <p className="text-xs text-gray-500"> | |
| ✓ Address is checksummed<br /> | |
| ✓ Valid Ethereum address format<br /> | |
| ✓ Compatible with EVM chains | |
| </p> | |
| </div> | |
| {/* Connection Status */} | |
| <div className="grid grid-cols-2 gap-4"> | |
| <div className={`p-4 rounded-xl ${address ? "bg-green-50" : "bg-gray-100"}`}> | |
| <p className={`text-xs font-medium mb-1 ${address ? "text-green-600" : "text-gray-500"}`}> | |
| Status | |
| </p> | |
| <p className={`text-lg font-bold ${address ? "text-green-700" : "text-gray-600"}`}> | |
| {address ? "Active" : "Not connected"} | |
| </p> | |
| </div> | |
| <div className={`p-4 rounded-xl ${address ? "bg-blue-50" : "bg-gray-100"}`}> | |
| <p className={`text-xs font-medium mb-1 ${address ? "text-blue-600" : "text-gray-500"}`}> | |
| Address Type | |
| </p> | |
| <p className={`text-lg font-bold ${address ? "text-blue-700" : "text-gray-600"}`}> | |
| {address ? "EOA" : "—"} | |
| </p> | |
| </div> | |
| </div> | |
| {/* Address Checksum Info */} | |
| {address && ( | |
| <div className="p-4 bg-gray-50 rounded-xl border border-gray-200"> | |
| <p className="text-xs text-gray-600 mb-2 font-medium"> | |
| Security Info | |
| </p> | |
| <p className="text-xs text-gray-500"> | |
| ✓ Address is checksummed<br /> | |
| ✓ Valid Ethereum address format<br /> | |
| ✓ Compatible with EVM chains | |
| </p> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In client/app/components/Wallet/WalletInfo.tsx around lines 66 to 88, the status
badges and security checklist are shown even when address may be undefined; wrap
the rendering of the “Active” status, “EOA” address type badge and the green
security checklist in a conditional that checks for a truthy address prop/state,
and when address is missing render a neutral placeholder (e.g., “Not connected”
/ “—” and a muted/info box) instead; ensure the conditional preserves layout
(use the same grid/containers) and avoid showing any checks or “Active” wording
unless address is present.
| const handleCopyAddress = () => { | ||
| if (address) { | ||
| navigator.clipboard.writeText(address); | ||
| setCopiedAddress(true); | ||
| toast.success("Address copied to clipboard!"); | ||
| setTimeout(() => setCopiedAddress(false), 2000); | ||
| } | ||
| }; |
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.
Handle clipboard write failures before showing success
navigator.clipboard.writeText returns a promise and can reject on HTTP origins, permission denial, or platform quirks. Because we don’t await it, we always show “Address copied” even when the copy silently fails, which is misleading. Wrap the call in try/catch (and guard for navigator.clipboard) so we only surface success on real copies and can toast an error otherwise.
- const handleCopyAddress = () => {
- if (address) {
- navigator.clipboard.writeText(address);
- setCopiedAddress(true);
- toast.success("Address copied to clipboard!");
- setTimeout(() => setCopiedAddress(false), 2000);
- }
- };
+ const handleCopyAddress = async () => {
+ if (!address || !navigator?.clipboard?.writeText) {
+ toast.error("Clipboard is not available in this environment.");
+ return;
+ }
+ try {
+ await navigator.clipboard.writeText(address);
+ setCopiedAddress(true);
+ toast.success("Address copied to clipboard!");
+ setTimeout(() => setCopiedAddress(false), 2000);
+ } catch (error) {
+ console.error("Clipboard copy failed:", error);
+ toast.error("Failed to copy address. Please try again.");
+ }
+ };📝 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 handleCopyAddress = () => { | |
| if (address) { | |
| navigator.clipboard.writeText(address); | |
| setCopiedAddress(true); | |
| toast.success("Address copied to clipboard!"); | |
| setTimeout(() => setCopiedAddress(false), 2000); | |
| } | |
| }; | |
| const handleCopyAddress = async () => { | |
| if (!address || !navigator?.clipboard?.writeText) { | |
| toast.error("Clipboard is not available in this environment."); | |
| return; | |
| } | |
| try { | |
| await navigator.clipboard.writeText(address); | |
| setCopiedAddress(true); | |
| toast.success("Address copied to clipboard!"); | |
| setTimeout(() => setCopiedAddress(false), 2000); | |
| } catch (error) { | |
| console.error("Clipboard copy failed:", error); | |
| toast.error("Failed to copy address. Please try again."); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In client/app/wallet/page.tsx around lines 27 to 34, handle clipboard failures
by making handleCopyAddress async, check for navigator.clipboard before calling,
await navigator.clipboard.writeText(address) inside a try/catch, only call
setCopiedAddress(true) and toast.success after the awaited writeText resolves,
and in the catch block toast an error (and avoid setting copied state); this
ensures we only show success when the copy actually succeeds and surface an
error when it fails.
| ## 🏗️ Architecture | ||
|
|
||
| ### File Structure | ||
| ``` |
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.
Add language specification to the fenced code block.
The fenced code block at line 57 is missing a language identifier. While this is a file structure representation rather than executable code, it's best practice to specify the language or use text for non-code blocks to maintain consistency and satisfy linting rules.
Apply this diff:
-```
+```text
client/app/
├── wallet/
│ └── page.tsx # Main wallet management pageBased on static analysis hints.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In client/WALLET_FEATURE.md around line 57, the fenced code block is missing a
language identifier; update the block to specify a language (use "text" for this
file-structure snippet) and replace the existing triple-backtick fence with
```text so the snippet reads as a text fenced code block and satisfies linting
and consistency rules.
Description
Include a summary of the change and which issue is fixed. List any dependencies that are required for this change.
Fixes:- Created a Help & Support page issue #179
Type of change
Please mark the options that are relevant.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style
Chores