all new UI kits added successfully - #618
Open
Sadeequ wants to merge 2 commits into
Open
Conversation
|
@Sadeequ Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix Report
I investigated and fixed four issues in the codebase. Here is the full report of what I did:
Issue 1: WalletConnectButton onOpenModal is never passed from TopBar
I investigated this issue thoroughly. After tracing through the code in
src/components/TopBar.tsxandsrc/components/WalletConnectButton.tsx, I found thatonOpenModalIS correctly passed from TopBar at line 73-75:And the WalletConnectButton component correctly handles this prop at lines 44-50 — when the user clicks the address pill, it calls
onOpenModal()which opens the AccountSidebar. I verified this behavior is correct and working as intended, so no code change was needed for this issue.Issue 2: SorokitProvider error state clobbered by parallel fetches and not cleared on reconnect
I fixed two problems in
src/context/SorokitProvider.tsx:Problem A — Parallel fetches clobbering errors: I found that when
getAccountandgetBalanceswere called in parallel viaPromise.all, the original code used anelse ifchain that only reported one error:This meant that if both requests failed, only the account error would surface — the balances error was silently discarded. I changed this to handle all three cases (both failed, only account failed, only balances failed):
I applied this same fix in two places: the
useEffectthat loads account data on address change (line 162) and therefreshAccountcallback (line 334).Problem B — Errors not cleared on reconnect: I found that
connectWalletonly clearedwalletErrorbefore attempting a connection. If a previous session had left anaccountErrorornetworkErrorin state, those would persist across a reconnect. I added two lines to clear all error states:Issue 3: switchNetwork adapter ignores the network argument and always returns testnet config
I fixed this in
src/lib/mock-client.ts. The root cause was thatactiveNetworkwas declared asconst, so it was set once at client creation and never updated. WhenswitchNetworkwas called, it returned the correct network info to the caller, but the internalactiveNetworkvariable still pointed to the original network. This meant subsequent calls togetNetworkwould return the stale network.I made two changes:
Changed
const activeNetworktolet activeNetworkso it can be reassigned.Added updates to
activeNetworkinside theswitchNetworkfunction:activeNetwork = param.namewhen the param is aNetworkInfoobjectactiveNetwork = paramwhen the param is aNetworkNamestringThis ensures the mock client correctly tracks the active network after a switch.
Issue 4: ClaimableBalanceCard silently discards claimBalance errors and never removes claimed rows
I fixed two problems in
src/components/ClaimableBalanceCard.tsx:Problem A — Silently discarding thrown errors: I found that the
doClaimfunction only handled errors returned in the{ error }response object. IfclaimBalancethrew an exception (e.g., a network failure or runtime error), it would propagate as an unhandled rejection and never be shown to the user. I added acatchblock:Problem B — Claimed rows not removed: I found that after a successful claim, the row would show a "Claimed" badge but remain in the list. I added a callback pattern to remove the row from the parent state:
onClaimSuccess?: (balanceId: string) => voidtoBalanceRowPropsonClaimSuccess?.(cb.id)after a successful claim indoClaimhandleClaimSuccessfunction in the parentClaimableBalanceCardthat filters out the claimed balance:handleClaimSuccessas theonClaimSuccessprop to eachBalanceRowVerification
I ran
npm run typecheckand confirmed all changes pass type checking. I also rannpm run lintand verified that all lint errors are pre-existing issues unrelated to my changes.Related Issue