feat(walletui): wallet ux improvements - #1572
Conversation
WalkthroughAdds a global error-notification context and integrates it in the app shell. Introduces a reusable PopupTitle component and replaces multiple DialogTitle usages. Implements dynamic currency symbol via a new Zustand store and syncing hook. Adds a ClaimCoinsButton and improves NFT/Token empty states and refresh/invalidation. Standardizes path aliases and updates imports. Adds wallet info and formatting updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Component as Any Component
participant Hook as useErrorNotification
participant Provider as ErrorNotificationProvider
participant UI as Snackbar/Alert
User->>Component: Triggers action
Component->>Hook: useErrorNotification()
Hook->>Provider: showError/showSuccess(...)
Provider->>Provider: Set notification state + timer
Provider-->>UI: Render Snackbar with Alert
User->>UI: Close (click) or timer elapses
UI->>Provider: clearNotification()
Provider->>UI: Unmount Snackbar
sequenceDiagram
autonumber
participant App as App
participant Hook as useCurrencySync
participant API1 as useAccountsGetDefault
participant API2 as useAccountsGetBalances
participant Store as currencyStore
participant Util as formatCurrency
App->>Hook: invoke
Hook->>API1: fetch default account
API1-->>Hook: default account
Hook->>API2: fetch balances(account.address)
API2-->>Hook: balances[]
Hook->>Store: setCurrencySymbol(firstBalance.token_symbol)
Util->>Store: getState().currencySymbol
Util-->>App: formatCurrency(..., symbol)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
applications/tari_walletd/web_ui/src/services/api/hooks/useNfts.tsx (2)
23-41: Pagination bug: request limit changes to 1 and offset skips items.Second page fetch uses limit: 1 while offset increases by 100, skipping 99 items per loop and causing excessive calls.
- const limit = 100; + const limit = 100; let offset = 0; let nfts = await nftList({ account: request.account, limit: limit, offset: offset, }); let result = nfts.nfts; - while (nfts.nfts.length > 0) { + while (nfts.nfts.length === limit) { offset += limit; nfts = await nftList({ account: request.account, - limit: 1, + limit: limit, offset: offset, }); result = result.concat(nfts.nfts); }
57-64: Cache invalidation narrowed; may miss nfts_list_ keys.*Switching to strict equality for "nfts_list" can leave stale caches if other places use "nfts_list_*". Standardize on startsWith or centralize a predicate.
- return typeof key === "string" && ( - key === "nfts" || - key === "list_nfts" || - key === "nfts_list" - ); + return ( + typeof key === "string" && + (key === "nfts" || key === "list_nfts" || key.startsWith("nfts_list")) + );#!/bin/bash # Find all NFT query keys and invalidations to ensure consistency rg -nP --type=ts --type=tsx -C2 "(queryKey|invalidateQueries).*(nfts|list_nfts|nfts_list)" rg -nP --type=ts --type=tsx "nfts_list_" -napplications/tari_walletd/web_ui/src/routes/AssetVault/NFTs/components/SendNft.tsx (2)
151-154: Side‑effect during render: unconditional refetch inside component body.Calling refetchNfts() during render can cause render loops and performance issues.
-// Only refetch NFTs when account is available -if (account) { - refetchNfts().catch(console.error); -} +// Only refetch NFTs when account is available +useEffect(() => { + if (!account) return; + refetchNfts().catch(console.error); +}, [account, refetchNfts]);
207-216: Fragile setTimeout to “wait for state” before estimating fee.Using an artificial delay to sync state is race‑prone.
Prefer passing variables directly to the mutation (if supported) or compute the params locally from the latest state without relying on interim setState + delay.
applications/tari_walletd/web_ui/src/routes/AssetVault/Components/ClaimBurn.tsx (1)
145-156: Select value is an object; requests will send “[object Object]” as addressMenuItem values are objects (
account.account.address) but state stores a string;e.target.valuebecomes “[object Object]”, breakingaccountsClaimBurn. UsesubstateIdToStringconsistently and default to empty string in the Select.+import { substateIdToString } from "@utils/helpers"; @@ - <InputLabel id="key">Key</InputLabel> + <InputLabel id="key">Account</InputLabel> <Select labelId="key" name="key" - label="Key" - value={claimBurnFormState.account} + label="Account" + value={claimBurnFormState.account || ""} onChange={onClaimBurnKeyChange} style={{ flexGrow: 1, minWidth: "200px" }} disabled={claimBurnFormState.disabled} > - {accountsList?.accounts?.map((account: AccountInfo, i: number) => ( - <MenuItem key={i} value={account.account.address}> + {accountsList?.accounts?.map((account: AccountInfo, i: number) => ( + <MenuItem key={i} value={substateIdToString(account.account.address)}> <div> <i> {account.account.name} ({account.public_key}) </i> </div> </MenuItem> ))}applications/tari_walletd/web_ui/src/routes/AssetVault/Components/PublishTemplate.tsx (2)
206-213: Default account stored as name, but API expects addressYou set
formState.accountto the account name; later it’s used as a ComponentAddress, causing submission to use an invalid address.- useEffect(() => { - let account = accounts?.find((a: AccountInfo) => a.account.is_default)?.account.name || null; - if (account) { - setFormState({ ...INITIAL_VALUES, account }); - setValidity({ ...validity, account: true }); - } - }, [accounts]); + useEffect(() => { + const def = accounts?.find((a: AccountInfo) => a.account.is_default); + const addr = def ? substateIdToString(def.account.address) : null; + if (addr) { + setFormState({ ...INITIAL_VALUES, account: addr }); + setValidity({ ...validity, account: true }); + } + }, [accounts]);
223-236: Select value type mismatch leads to uncontrolled/incorrect selectionEnsure the Select value is the address string; don’t pass an AccountInfo object as a fallback.
- <Select + <Select id="select-account" name="account" disabled={disabled} displayEmpty - value={formState.account || accounts.find((a: AccountInfo) => a.account.is_default) || ""} + value={formState.account || ""} onChange={setSelectFormValue} variant="outlined" > {accounts.map((account: AccountInfo, i: number) => ( <MenuItem key={i} value={substateIdToString(account.account.address)}> {account.account.name} {account.account.is_default ? "(default)" : ""} </MenuItem> ))}
🧹 Nitpick comments (57)
applications/tari_walletd/web_ui/src/routes/Settings/Components/GeneralSettings.tsx (7)
71-75: Handle RPC errors and guard setState on unmount in NetworkSettings.Prevents a no-op state update warning and surfaces failures gracefully.
Apply this diff:
-useEffect(() => { - settingsGet().then((res) => { - setNetwork(res.network.name); - }); -}, []); +useEffect(() => { + let cancelled = false; + (async () => { + try { + const res = await settingsGet(); + if (!cancelled) setNetwork(res?.network?.name ?? "Unknown"); + } catch (err) { + if (!cancelled) setNetwork("Unknown"); + // Optionally notify via the app's error-notification system + console.error("Failed to load settings", err); + } + })(); + return () => { + cancelled = true; + }; +}, []);
55-61: Prefer MUI sx over inline style for consistency and theme integration.Simplifies spacing and avoids manual theme plumbing.
Apply this diff:
- return ( - <Box - style={{ - display: "flex", - flexDirection: "column", - gap: theme.spacing(3), - paddingTop: theme.spacing(3), - }} - > + return ( + <Box + sx={{ + display: "flex", + flexDirection: "column", + gap: 3, + pt: 3, + }} + >
25-25: Remove unused useTheme import (if adopting sx).Avoids an unused import after the change above.
Apply this diff:
-import { useTheme } from "@mui/material/styles";
32-32: Remove unused theme variable (if adopting sx).No longer needed once inline styles are replaced with sx.
Apply this diff:
- const theme = useTheme();
44-51: Avoid array index as React key.Use a stable identifier to prevent reconciliation glitches.
Apply this diff:
- <React.Fragment key={i}> + <React.Fragment key={item.label}>
39-41: Fix casing: “URL”.Minor UX polish for user-facing text.
Apply this diff:
- { - label: "Indexer Url", - content: <IndexerSettings />, - }, + { + label: "Indexer URL", + content: <IndexerSettings />, + },
69-69: Show a friendly placeholder before the RPC resolves.Prevents a blank label on first render.
Apply this diff:
- const [network, setNetwork] = useState(""); + const [network, setNetwork] = useState<string>("Loading…");applications/tari_walletd/web_ui/vite.config.ts (1)
27-43: Optional: make config ESM-safe for __dirname.If this config runs in pure ESM, prefer fileURLToPath to compute paths.
-import path from "path"; +import path from "path"; +import { fileURLToPath } from "url"; +const __dirname = path.dirname(fileURLToPath(import.meta.url));applications/tari_walletd/web_ui/src/routes/AssetVault/Components/SelectAccount.tsx (1)
63-76: Optional: extract value computation to a memo for readability.Small readability win; avoids recomputation in render.
applications/tari_walletd/web_ui/src/routes/AssetVault/Components/MyAssets.tsx (2)
53-58: Good: explicit NFT cache invalidation on refresh.Consider centralizing the NFT predicate to avoid drift across files.
122-134: Add an accessible label to the refresh control.Improve a11y with aria-label (Tooltip optional).
- <IconButton + <IconButton + aria-label="Refresh assets" title="Refresh all accounts" color="primary" disabled={refreshBalances.isPending} onClick={handleRefreshClicked} size="small" sx={{ marginLeft: theme.spacing(1), }} >applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/components/ClaimCoinsButton.tsx (1)
41-69: Harden error typing and message extraction; consider broader invalidation.Type the error as unknown/ApiError and extract safely. Also confirm if accounts list or account info queries should be invalidated too.
- onError: (error: any) => { - console.error("Error claiming coins:", error); - const errorMessage = error?.message || "Failed to claim testnet coins. Please try again."; - showError(errorMessage); - }, + onError: (error: unknown) => { + console.error("Error claiming coins:", error); + const msg = + typeof error === "object" && error && "message" in error + ? String((error as { message?: unknown }).message) + : undefined; + showError(msg || "Failed to claim testnet coins. Please try again."); + },#!/bin/bash # Ensure balance-related query keys match across hooks/components rg -nP --type=ts --type=tsx -C2 "(queryKey|invalidateQueries).*(balances|accounts_balances|accounts_get_balances)"applications/tari_walletd/web_ui/src/services/store/currencyStore.ts (1)
30-33: Consider persisting and sanitizing the currency symbol.Persist across reloads and guard against empty values.
-import { create } from "zustand"; +import { create } from "zustand"; +import { persist } from "zustand/middleware"; -const useCurrencyStore = create<CurrencyStore>((set) => ({ - currencySymbol: "XTR", - setCurrencySymbol: (symbol) => set({ currencySymbol: symbol }), -})); +const useCurrencyStore = create<CurrencyStore>()( + persist( + (set) => ({ + currencySymbol: "XTR", + setCurrencySymbol: (symbol: string) => + set({ currencySymbol: symbol?.trim() || "XTR" }), + }), + { name: "currency-symbol" } + ), +);applications/tari_walletd/web_ui/src/Components/PopupTitle.tsx (2)
31-54: Add accessible labelling and remove empty style.Expose an id and wire it to Dialog’s aria-labelledby; drop the no-op sx prop.
-interface PopupTitleProps { - title: string; - onClose?: () => void; -} +interface PopupTitleProps { + title: string; + onClose?: () => void; + id?: string; +} -function PopupTitle({ title, onClose }: PopupTitleProps) { +function PopupTitle({ title, onClose, id = "dialog-title" }: PopupTitleProps) { return ( - <DialogTitle sx={{}}> + <DialogTitle id={id}>
36-41: Avoid forced uppercase for i18n.Uppercasing can break localization and readability. Prefer theme styles or leave casing to callers.
applications/tari_walletd/web_ui/src/routes/AssetVault/NFTs/components/SendNft.tsx (4)
156-173: Non‑null assertions can leak nulls to the mutation hooks.sourceAccount and feePayerAccount can be null; asserting with ! only silences TS and may pass null at runtime.
Either:
- Construct the variables at call time (inside onTransfer/estimateFeeWithTargetAccount) when you know they’re defined, or
- Type the hooks to accept undefined and validate inside mutate.
Example adjustment (conceptual, avoid ! and defer construction):
const buildFeeEstimateVars = () => { if (!sourceAccount || !feePayerAccount) return undefined; return { /* ...compose using current state... */ }; };Also applies to: 175-193
377-390: Remove unused step logic.steps and getStepIndex are no longer used after switching to PopupTitle.
- const steps = ["Enter Details", "Confirm Transfer", "Complete"]; - - const getStepIndex = () => { - switch (currentStep) { - case "form": - return 0; - case "confirmation": - return 1; - case "result": - return 2; - default: - return 0; - } - };
393-395: Wire Dialog to header for a11y.Connect aria-labelledby to PopupTitle id.
- return ( - <Dialog open={props.open} onClose={handleClose} maxWidth="sm" fullWidth> - <PopupTitle onClose={handleClose} title="Transfer NFT" /> + return ( + <Dialog open={props.open} onClose={handleClose} maxWidth="sm" fullWidth aria-labelledby="transfer-nft-title"> + <PopupTitle id="transfer-nft-title" onClose={handleClose} title="Transfer NFT" />
397-400: Minor: prefer MUI Box over inline styles.For consistency and theming.
applications/tari_walletd/web_ui/src/routes/AssetVault/Components/AddAccount.tsx (2)
86-90: Wire Dialog to header for a11y.Provide aria-labelledby and set matching id on PopupTitle.
- return ( - <Dialog open={open} onClose={handleClose}> - <PopupTitle title="Add Account" onClose={handleClose} /> + return ( + <Dialog open={open} onClose={handleClose} aria-labelledby="add-account-title"> + <PopupTitle id="add-account-title" title="Add Account" onClose={handleClose} />
76-84: Narrow error typing.Use unknown and safe extraction to avoid any.
- const getErrorMessage = (error: any): string => { - if (!error) return ""; - const message = error.message || ""; + const getErrorMessage = (error: unknown): string => { + if (!error) return ""; + const message = + typeof error === "string" + ? error + : (error as { message?: string })?.message || "";applications/tari_walletd/web_ui/src/contexts/ErrorNotificationContext.tsx (2)
44-56: Use platform‑agnostic timeout type and ensure cleanup on unmount.Avoid NodeJS.Timeout in browser apps and clear pending timers.
-import React, { createContext, useContext, useState, useRef } from "react"; +import React, { createContext, useContext, useState, useRef, useEffect } from "react"; @@ - const timeoutRef = useRef<NodeJS.Timeout | null>(null); + const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); @@ const showNotification = (message: string, severity: AlertColor = "error", duration: number = 8000) => { if (timeoutRef.current) { clearTimeout(timeoutRef.current); } @@ timeoutRef.current = setTimeout(() => { setNotification(null); }, duration); }; + useEffect(() => { + return () => { + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); + } + }; + }, []);
65-71: API naming nit.showError taking a custom severity is confusing. Either lock severity to "error" or rename to showNotification.
applications/tari_walletd/web_ui/src/routes/WebauthnRegistration/Components/Registration.tsx (1)
181-184: Add rel="noopener noreferrer" to external link opened in a new tab.Prevents tab‑nabbing and removes the opener reference.
- <a href="https://webauthn.io/" target="_blank"> + <a href="https://webauthn.io/" target="_blank" rel="noopener noreferrer">applications/tari_walletd/web_ui/src/routes/WebauthnRegistration/Components/Login.tsx (1)
15-15: Alias import migration LGTM; keep alias usage consistent project‑wide.If some modules still use long relative paths, consider migrating them in a follow‑up for consistency.
- import { unauthenticated_client, webauthnStartAuth } from "../../../utils/json_rpc"; + import { unauthenticated_client, webauthnStartAuth } from "@utils/json_rpc";applications/tari_walletd/web_ui/src/routes/WebauthnRegistration/Webauthn.tsx (2)
26-29: Avoid navigate() during render; move redirect into useEffect.Calling navigate in render can cause warnings and unpredictable updates.
- if (authToken) { - navigate(redirect); - } + // Redirect after auth token is present + useEffect(() => { + if (authToken) { + navigate(redirect); + } + }, [authToken, redirect, navigate]);
30-38: Tighten effect deps to match referenced vars.Include alreadyRegisteredError to avoid stale logging.
- }, [alreadyRegisteredResponse, alreadyRegisteredIsError]); + }, [alreadyRegisteredResponse, alreadyRegisteredIsError, alreadyRegisteredError]);applications/tari_walletd/web_ui/src/routes/Transactions/Transactions.tsx (3)
43-43: Unify alias style for helpers.Other files import helpers via "@/utils/helpers". Standardize to one style to prevent duplicate modules.
- import { emptyRows, handleChangePage, handleChangeRowsPerPage, formatCurrency } from "@utils/helpers"; + import { emptyRows, handleChangePage, handleChangeRowsPerPage, formatCurrency } from "@/utils/helpers";
104-106: Treat 0 fees as a valid value (don’t render “--”).The truthy check hides legitimate 0 values.
- <DataTableCell> - {fee_receipt?.total_fees_paid ? formatCurrency(fee_receipt.total_fees_paid) : "--"} - </DataTableCell> + <DataTableCell> + {fee_receipt?.total_fees_paid != null + ? formatCurrency(fee_receipt.total_fees_paid) + : "--"} + </DataTableCell>
127-127: Fix TableCell colSpan: there are 4 columns.Prevents layout misalignment on empty rows.
- <TableCell colSpan={3} /> + <TableCell colSpan={4} />applications/tari_walletd/web_ui/src/routes/AssetVault/AssetVault.tsx (1)
30-39: Remove unused wallet info fetch and debug log (or wire it into UI/state).Avoids unnecessary network call and console noise.
-import { useWalletInfo } from "@api/hooks/useWalletInfo"; +// import { useWalletInfo } from "@api/hooks/useWalletInfo"; @@ - const { data: walletInfo } = useWalletInfo(); + // const { data: walletInfo } = useWalletInfo(); @@ - console.log("walletInfo", walletInfo); + // console.log("walletInfo", walletInfo);If this was intended to drive currency sync, prefer doing that once in App-level hook (useCurrencySync) rather than here.
applications/tari_walletd/web_ui/src/routes/Transactions/FeeReceipt.tsx (2)
25-25: Align helper import alias with the rest of the app.Use a single convention (“@/…“ or “@utils/…”) to avoid duplicate bundles.
- import { formatCurrency } from "@/utils/helpers"; + import { formatCurrency } from "@utils/helpers";
39-41: Guard against undefined values before formatting.Prevents “NaN”/“undefined” render when fields are missing.
- const feeItems = [ - { label: "Total Fee Payment", value: formatCurrency(data.total_fee_payment), color: "primary" as const }, - { label: "Total Fees Paid", value: formatCurrency(data.total_fees_paid), color: "success" as const }, - ]; + const feeItems = [ + { + label: "Total Fee Payment", + value: data.total_fee_payment != null ? formatCurrency(data.total_fee_payment) : "--", + color: "primary" as const, + }, + { + label: "Total Fees Paid", + value: data.total_fees_paid != null ? formatCurrency(data.total_fees_paid) : "--", + color: "success" as const, + }, + ];applications/tari_walletd/web_ui/src/routes/AssetVault/Components/ClaimFees.tsx (3)
215-216: Restore Dialog a11y: wire aria-labelledby to PopupTitle.MUI’s Dialog gets its accessible name from DialogTitle; with a custom header you must set the id and connect it.
- <Dialog open={open} onClose={handleClose}> - <PopupTitle onClose={handleClose} title="Claim Fees" /> + <Dialog open={open} onClose={handleClose} aria-labelledby="claim-fees-title"> + <PopupTitle id="claim-fees-title" onClose={handleClose} title="Claim Fees" />Confirm PopupTitle forwards id to the underlying heading element (or DialogTitle). If not, add it in PopupTitle.
163-166: Re-enable form after request completes.Setting state back to the captured “disabled” value is brittle; just set false.
- .finally(() => { - // Previous value of disabled - setDisabled(disabled); - }); + .finally(() => { + setDisabled(false); + });
244-250: Use centralized currency formatting and dynamic symbol (avoid hardcoded XTR).Aligns with the new formatCurrency helper.
- <Box> - Found fees in {Object.entries(scannedFees.fees).length} shards. Total:{" "} - {Object.values(scannedFees.fees) - .map((info) => info!.amount) - .reduce((acc, amt) => acc + amt, 0)}{" "} - XTR - </Box> + <Box> + {(() => { + const total = Object.values(scannedFees.fees) + .map((info) => info!.amount) + .reduce((acc, amt) => acc + amt, 0); + return <>Found fees in {Object.keys(scannedFees.fees).length} shards. Total: {formatCurrency(total)}</>; + })()} + </Box>Don’t forget to import:
+ import { formatCurrency } from "@utils/helpers";applications/tari_walletd/web_ui/src/routes/Transactions/TransactionDetails.tsx (2)
59-59: Normalize helper alias to match the chosen convention.Keep imports consistent with the rest of the codebase.
- import { formatCurrency } from "@/utils/helpers"; + import { formatCurrency } from "@utils/helpers";
165-175: Use formatted zero for consistency when no fee receipt.Ensures uniform currency formatting.
- {feeReceipt ? formatCurrency(feeReceipt.total_fees_paid) : "0"} + {feeReceipt ? formatCurrency(feeReceipt.total_fees_paid) : formatCurrency(0)}applications/tari_walletd/web_ui/src/services/api/hooks/useWalletInfo.ts (1)
26-31: Stabilize query behavior and export a shared query keySet explicit staleTime/refetch options and export the query key to avoid typos across invalidations.
-export const useWalletInfo = () => { - return useQuery({ - queryKey: ["wallet_info"], - queryFn: () => walletGetInfo(), - }); -}; +export const walletInfoQueryKey = ["wallet_info"] as const; + +export const useWalletInfo = () => { + return useQuery({ + queryKey: walletInfoQueryKey, + queryFn: walletGetInfo, + staleTime: 30_000, + refetchOnWindowFocus: false, + }); +};applications/tari_walletd/web_ui/src/routes/AssetVault/Components/ClaimBurn.tsx (2)
31-31: Fix MUI Select import pathImporting from "@mui/material/Select/Select" relies on internal paths and can break with MUI updates.
-import Select, { SelectChangeEvent } from "@mui/material/Select/Select"; +import Select, { SelectChangeEvent } from "@mui/material/Select";
175-181: Fee input should be numeric with basic constraintsImproves UX and reduces parse errors.
-<TextField +<TextField name="fee" label="Fee" + type="number" value={claimBurnFormState.fee} onChange={onClaimBurnFeeChange} style={{ flexGrow: 1 }} disabled={claimBurnFormState.disabled} + inputProps={{ min: 0 }} />applications/tari_walletd/web_ui/src/App.tsx (1)
37-37: Use the path alias for the auth store to avoid duplicate module instancesKeeps imports consistent with the rest of the codebase and reduces risk of duplicate singletons in bundlers.
-import useAuthStore from "./services/store/authStore"; +import useAuthStore from "@store/authStore";applications/tari_walletd/web_ui/src/services/store/hooks/useCurrencySync.ts (2)
33-33: Select only the needed action from the Zustand storePrevents re-renders on unrelated state changes.
- const { setCurrencySymbol } = useCurrencyStore(); + const setCurrencySymbol = useCurrencyStore((s) => s.setCurrencySymbol);
35-43: Small effect tidy-upsDrop unused
isLoadingfrom deps and defensively check before setting.- useEffect(() => { - if (defaultAccount && balancesData?.balances && balancesData.balances.length > 0) { + useEffect(() => { + if (defaultAccount && balancesData?.balances?.length > 0) { const mainBalance = balancesData.balances[0]; if (mainBalance.token_symbol) { setCurrencySymbol(mainBalance.token_symbol); } } - }, [defaultAccount, balancesData?.balances, isLoading, setCurrencySymbol]); + }, [defaultAccount, balancesData?.balances, setCurrencySymbol]);applications/tari_walletd/web_ui/src/routes/AssetVault/Components/PublishTemplate.tsx (2)
136-144: Treat maxFee = 0 as “publish”, not “estimate”Using
!formState.maxFeemakes 0 behave like dry‑run. Check for null/undefined instead.- const isDryRun = !formState.maxFee; + const isDryRun = formState.maxFee == null; @@ - max_fee: isDryRun ? 1_000_000 : Number(formState.maxFee) || 0, + max_fee: isDryRun ? 1_000_000 : Number(formState.maxFee),
110-121: Form state typing is too loose
validityis typed asobjectandsetFormValuewrites string values intoFormStatefields. Tighten types to prevent accidental string vs number issues formaxFee.applications/tari_walletd/web_ui/src/routes/FlowEditor/FlowEditor.tsx (3)
162-166: Type the default account lookupAvoid
anyto catch shape changes at compile time.+import type { AccountInfo } from "@tari-project/typescript-bindings"; @@ - if (dataAccountsList?.accounts && dataAccountsList.accounts.length > 0) { - const defaultAcc = dataAccountsList.accounts.find((acc: any) => acc.account.is_default); + if (dataAccountsList?.accounts && dataAccountsList.accounts.length > 0) { + const defaultAcc = dataAccountsList.accounts.find((acc: AccountInfo) => acc.account.is_default); setAccount(defaultAcc || dataAccountsList.accounts[0]); }
179-183: Type the account comparison in onAccountChangeImproves safety of address access.
- const selected = dataAccountsList?.accounts.find( - (acc: any) => substateIdToString(acc.account.address) === e.target.value, - ); + const selected = dataAccountsList?.accounts.find( + (acc: AccountInfo) => substateIdToString(acc.account.address) === e.target.value, + );
325-330: Type accounts in the Select listSmall improvement for clarity and safety.
-{dataAccountsList?.accounts?.map((acc: any) => ( +{dataAccountsList?.accounts?.map((acc: AccountInfo) => (applications/tari_walletd/web_ui/src/routes/AssetVault/NFTs/components/ClaimNftsButton.tsx (3)
28-28: Prefer path alias over deep relative importUse the project alias (e.g., "@/contexts/ErrorNotificationContext") for consistency and maintainability.
-import { useErrorNotification } from "../../../../contexts/ErrorNotificationContext"; +import { useErrorNotification } from "@/contexts/ErrorNotificationContext";
50-59: Make query invalidation more robust; drop console.log in production
- The current predicate only matches exact string keys. If your queries use array keys (common with TanStack Query) or prefixes, some caches may not invalidate, leaving stale NFT lists.
- Avoid leaving
console.login shipped UI.Proposed options (pick one):
Option A: key-scoped invalidation (preferred if your keys are arrays)
- console.log(resp); showSuccess("Successfully claimed NFTs!"); - // Invalidate NFT queries to refresh the list - queryClient.invalidateQueries({ - predicate: (query) => { - const key = query.queryKey[0]; - return typeof key === "string" && (key === "nfts" || key === "list_nfts" || key === "nfts_list"); - }, - }); + // Invalidate NFT queries to refresh the list + queryClient.invalidateQueries({ queryKey: ["nfts"] }); + queryClient.invalidateQueries({ queryKey: ["list_nfts"] }); + queryClient.invalidateQueries({ queryKey: ["nfts_list"] });Option B: keep predicate but allow prefixes
- console.log(resp); showSuccess("Successfully claimed NFTs!"); // Invalidate NFT queries to refresh the list queryClient.invalidateQueries({ predicate: (query) => { const key = query.queryKey[0]; - return typeof key === "string" && (key === "nfts" || key === "list_nfts" || key === "nfts_list"); + return typeof key === "string" && ( + key === "nfts" || + key === "list_nfts" || + key === "nfts_list" || + key.startsWith("nfts") || + key.startsWith("list_nfts") + ); }, });Please confirm which query key shapes you use across NFT hooks (strings vs arrays).
61-66: Type errors as unknown; derive a safe messageAvoid
any; handle unexpected error shapes safely.- onError: (error: any) => { - console.error("Error claiming NFTs:", error); - // Show user-friendly error message - const errorMessage = error?.message || "Failed to claim NFTs. Please ensure you have sufficient funds to pay for transaction fees."; - showError(errorMessage); - }, + onError: (error: unknown) => { + console.error("Error claiming NFTs:", error); + const message = + typeof error === "object" && error && "message" in error && typeof (error as any).message === "string" + ? (error as any).message + : "Failed to claim NFTs. Please ensure you have sufficient funds to pay for transaction fees."; + showError(message); + },applications/tari_walletd/web_ui/src/routes/AssetVault/NFTs/NFTList.tsx (1)
110-128: Good empty state; consider extracting to a small reusable componentThe gate avoids flashing during fetches and the copy is clear. If similar empty states exist (tokens, etc.), consider a small
<EmptyState title description />to keep consistency.applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/components/SendMoney.tsx (1)
372-374: Add accessible title wiring between Dialog and PopupTitleHook up
aria-labelledbyon Dialog to anidon PopupTitle to improve a11y.- return ( - <Dialog open={props.open} onClose={handleClose} maxWidth="md" fullWidth> - <PopupTitle onClose={handleClose} title="Send Tari" /> + return ( + <Dialog open={props.open} onClose={handleClose} maxWidth="md" fullWidth aria-labelledby="send-tari-title"> + <PopupTitle id="send-tari-title" onClose={handleClose} title="Send Tari" /> <DialogContent>{renderStepContent()}</DialogContent> </Dialog>If
PopupTitledoes not yet accept/forwardid, add it to its props and pass it to the heading element inside.applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/Tokens.tsx (3)
136-137: Ensure hasBalances is strictly booleanThe current expression can produce
undefined. Use length with nullish coalescing.-const hasBalances = balancesData?.balances && balancesData.balances.length > 0; +const hasBalances = (balancesData?.balances?.length ?? 0) > 0;
152-155: Avoid unnecessary casts on booleansThese values are already booleans per hook typing; the casts add noise.
-<FetchStatusCheck - isError={balancesIsError as boolean} - errorMessage={(balancesError as { message?: string })?.message || "Error fetching data"} - isLoading={(balancesIsFetching as boolean) && !balancesData} -> +<FetchStatusCheck + isError={balancesIsError} + errorMessage={(balancesError as { message?: string })?.message || "Error fetching data"} + isLoading={balancesIsFetching && !balancesData} +>
180-226: Prefer stable keys and remove function cast by tightening types
- Use a stable key (e.g.,
resource_addressor${vault_address ?? ""}-${resource_address}) instead of the index to reduce re-render churn.- The
onSendClickedcast can be avoided by typing the handler to the expected signature.- {balancesData?.balances.map( - ( - { resource_address, balance, resource_type, confidential_balance, token_symbol, vault_address, divisibility }: BalanceEntry, - i: number, - ) => ( - <BalanceRow - key={i} + {balancesData?.balances.map( + ({ + resource_address, + balance, + resource_type, + confidential_balance, + token_symbol, + vault_address, + divisibility, + }: BalanceEntry) => ( + <BalanceRow + key={`${vault_address ?? "no-vault"}-${resource_address}`} token_symbol={token_symbol || ""} resource_address={resource_address} resource_type={resource_type} balance={balance} confidential_balance={confidential_balance} vault_address={vault_address ?? undefined} // convert null to undefined divisibility={divisibility} - onSendClicked={ - handleSendResourceClicked as ( - resource_address: ResourceAddress, - resource_type: ResourceType, - ) => void - } + onSendClicked={handleSendResourceClicked} /> ), )}Also update the handler signature to match (outside this hunk):
// ensure this is typed explicitly const handleSendResourceClicked: (address: ResourceAddress, resource_type: ResourceType) => void = (address, resource_type) => { setResourceToSend({ address, resource_type }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
applications/tari_walletd/web_ui/src/App.tsx(4 hunks)applications/tari_walletd/web_ui/src/Components/PopupTitle.tsx(1 hunks)applications/tari_walletd/web_ui/src/contexts/ErrorNotificationContext.tsx(1 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/AssetVault.tsx(2 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Components/ActionMenu.tsx(1 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Components/AddAccount.tsx(2 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Components/ClaimBurn.tsx(3 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Components/ClaimFees.tsx(2 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Components/MyAssets.tsx(4 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Components/PublishTemplate.tsx(6 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Components/SelectAccount.tsx(1 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/NFTs/NFTList.tsx(2 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/NFTs/components/ClaimNftsButton.tsx(2 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/NFTs/components/SendNft.tsx(3 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/Tokens.tsx(4 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/components/ClaimCoinsButton.tsx(1 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/components/SendMoney.tsx(2 hunks)applications/tari_walletd/web_ui/src/routes/FlowEditor/FlowEditor.tsx(7 hunks)applications/tari_walletd/web_ui/src/routes/Settings/Components/GeneralSettings.tsx(1 hunks)applications/tari_walletd/web_ui/src/routes/Templates/Templates.tsx(2 hunks)applications/tari_walletd/web_ui/src/routes/Transactions/FeeReceipt.tsx(2 hunks)applications/tari_walletd/web_ui/src/routes/Transactions/TransactionDetails.tsx(3 hunks)applications/tari_walletd/web_ui/src/routes/Transactions/Transactions.tsx(2 hunks)applications/tari_walletd/web_ui/src/routes/Wallet/Components/AccessTokens.tsx(1 hunks)applications/tari_walletd/web_ui/src/routes/WebauthnRegistration/Components/Login.tsx(1 hunks)applications/tari_walletd/web_ui/src/routes/WebauthnRegistration/Components/Registration.tsx(1 hunks)applications/tari_walletd/web_ui/src/routes/WebauthnRegistration/Webauthn.tsx(1 hunks)applications/tari_walletd/web_ui/src/services/api/hooks/useNfts.tsx(1 hunks)applications/tari_walletd/web_ui/src/services/api/hooks/useWalletInfo.ts(1 hunks)applications/tari_walletd/web_ui/src/services/store/currencyStore.ts(1 hunks)applications/tari_walletd/web_ui/src/services/store/hooks/useCurrencySync.ts(1 hunks)applications/tari_walletd/web_ui/src/utils/helpers.tsx(2 hunks)applications/tari_walletd/web_ui/tsconfig.json(1 hunks)applications/tari_walletd/web_ui/vite.config.ts(1 hunks)
🔇 Additional comments (27)
applications/tari_walletd/web_ui/src/routes/Settings/Components/GeneralSettings.tsx (1)
29-29: Verify @utils alias is wired in all toolchains (tsconfig, Vite, tests, ESLint).Path aliases can break builds/tests if not consistently configured.
Run this script to confirm mappings are present:
#!/bin/bash set -euo pipefail echo "— tsconfig paths for @utils —" fd -a 'tsconfig*.json' | while read -r f; do echo ">>> $f" rg -nP '"paths"\s*:\s*{[^}]*"@utils/\*"' "$f" || echo "(@utils/* not found in $f)" done echo echo "— Vite alias for @utils —" fd -a 'vite.config.*' | while read -r f; do echo ">>> $f" rg -nP 'resolve\s*:\s*{|\balias\s*:' -nC2 "$f" || true rg -n '@utils' -nC2 "$f" || echo "(@utils not referenced in $f)" done echo echo "— Test runner (Jest/Vitest) moduleNameMapper —" fd -a 'jest*.config.*' 'vitest.config.*' 2>/dev/null | while read -r f; do echo ">>> $f" rg -nP 'moduleNameMapper|@utils' -nC2 "$f" || true done echo echo "— ESLint import/resolver —" fd -a '.eslintrc*' | while read -r f; do echo ">>> $f" rg -nP 'import/resolver|typescript|alias|@utils' -nC2 "$f" || true doneapplications/tari_walletd/web_ui/src/routes/Templates/Templates.tsx (3)
4-4: Good switch to @api alias.No issues spotted.
6-6: Good switch to @api alias for accounts.No issues spotted.
35-35: Store alias migration looks consistent.Confirm other files importing the same store also use @store.
#!/bin/bash rg -nP --type=ts --type=tsx "from\s+['\"]@store/" -Sapplications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/components/ClaimCoinsButton.tsx (1)
72-76: LGTM: clear UX and disabled state.Button behavior and pending text are appropriate.
applications/tari_walletd/web_ui/vite.config.ts (1)
38-39: Aliases update looks good; verify tsconfig parity and resolver configs.Ensure tsconfig paths, ESLint import/resolver, and Jest (if used) reflect these new targets to avoid IDE/import resolution drift.
#!/bin/bash # Confirm alias usage and look for legacy paths rg -nP --type=ts --type=tsx "from\s+['\"](@api|@store)/" -S rg -nP --type=ts --type=tsx "(from\s+['\"][.]{1,2}/src/(api|store)/|from\s+['\"]@/src/(api|store)/)" -S # Spot ESLint/Jest configs that may need updates fd -a "eslint" | xargs -I{} sh -c 'echo "--- {} ---"; rg -n "(import/resolver|alias|paths)" "{}" -n || true' fd -a "jest.config" | xargs -I{} sh -c 'echo "--- {} ---"; rg -n "(moduleNameMapper|alias|paths)" "{}" -n || true'applications/tari_walletd/web_ui/src/routes/Wallet/Components/AccessTokens.tsx (1)
49-49: Alias import change is correct; ensure cache invalidation on revoke.Assuming useAuthRevokeToken invalidates related queries; if not, add an explicit invalidate to keep the table fresh.
#!/bin/bash # Check hook implementation for cache invalidation after revoke rg -nP --type=ts --type=tsx "(useAuthRevokeToken|invalidateQueries).*token" -C2applications/tari_walletd/web_ui/tsconfig.json (1)
33-34: Paths aligned with Vite aliases.Looks consistent.
#!/bin/bash # Verify TS can resolve aliases across the codebase rg -nP --type=ts --type=tsx "from\s+['\"](@api|@store)/" -S # If using Jest or ts-node, ensure their configs have matching moduleNameMapper/paths. fd -a "jest.config.*" | xargs -I{} rg -n "(moduleNameMapper|paths).*(@api|@store)" "{}" || trueapplications/tari_walletd/web_ui/src/services/store/currencyStore.ts (1)
35-35: LGTM — simple store fits the current usage.applications/tari_walletd/web_ui/src/Components/PopupTitle.tsx (1)
57-57: LGTM — reusable header is a good consolidation.applications/tari_walletd/web_ui/src/routes/AssetVault/Components/AddAccount.tsx (1)
39-67: Create flow changes look good.Awaited mutate, store update, invalidation, and timed auto‑close are clear and user‑friendly.
applications/tari_walletd/web_ui/src/contexts/ErrorNotificationContext.tsx (1)
73-101: LGTM — lightweight, reusable notifications with sane defaults.applications/tari_walletd/web_ui/src/routes/WebauthnRegistration/Webauthn.tsx (1)
6-10: Alias imports: OK; confirm alias mapping is uniformly configured.No functional changes here; just ensure @api/* and @store/* resolve in IDE, dev, test, and build.
applications/tari_walletd/web_ui/src/routes/WebauthnRegistration/Components/Registration.tsx (1)
14-14: Alias import looks good; verify alias config across build/test.Ensure @store/* is mapped in both tsconfig and Vite, and test tooling (Vitest/Jest, ESLint) resolves it too.
Run to confirm alias config and inconsistent usages:
#!/bin/bash # Verify alias config and usage fd -a 'tsconfig*.json' -x jq -r '.compilerOptions.paths // {}' {} 2>/dev/null fd -a 'vite.config.*' -x rg -n "alias|resolve" # Find mixed alias styles for utils/helpers rg -nP 'from\s+["\']@/?utils/helpers["\']' -S # Check @store/authStore imports rg -nP 'from\s+["\']@store/authStore["\']' -Sapplications/tari_walletd/web_ui/src/utils/helpers.tsx (1)
254-274: LGTM on dynamic currency symbol usageReading the symbol from the store keeps outputs consistent with the active token.
applications/tari_walletd/web_ui/src/App.tsx (1)
199-272: Wrapping routes with ErrorNotificationProvider looks goodProvider placement is appropriate and non-invasive to routing.
applications/tari_walletd/web_ui/src/routes/AssetVault/Components/ActionMenu.tsx (1)
35-41: LGTM: simpler composition and spacingUsing Stack simplifies layout; removal/move of faucet actions aligns with PR goals.
applications/tari_walletd/web_ui/src/routes/AssetVault/NFTs/components/ClaimNftsButton.tsx (2)
31-34: Nice: pending state wired to disable and label switchingUsing
isPendingfrom the mutation to disable the button and flip the label is the right UX.
72-77: LGTM: clear affordance while claimingOutlined variant, disabled state, and "Claiming..." feedback are good UX touches.
applications/tari_walletd/web_ui/src/routes/AssetVault/NFTs/NFTList.tsx (1)
34-36: LGTM: imports for empty-state UIAdding
TypographyandBoxis appropriate for the new message block.applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/components/SendMoney.tsx (2)
42-42: LGTM: standardized header via PopupTitleUsing the shared
PopupTitlealigns dialogs visually.
49-49: LGTM: typed predicate for balance lookupAnnotating the
findcallback improves type safety.applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/Tokens.tsx (5)
29-29: LGTM: UI building blocks imported
Typography,Box,Stackare appropriate for the new layout.
39-39: LGTM: ClaimCoinsButton integrationPlacing the testnet-claim CTA on the tokens tab improves discovery.
147-149: LGTM: typed resource lookup for token_symbolThis improves safety when passing the symbol into the dialog.
156-160: LGTM: top-right ClaimCoinsButton layoutClear placement and spacing.
161-179: Empty state reads wellConcise message and consistent styling with NFTs list.
| import queryClient from "@api/queryClient"; | ||
| import { Stack, Fade, Typography } from "@mui/material"; | ||
| import CheckCircleRoundedIcon from "@mui/icons-material/CheckCircleRounded"; | ||
| import PopupTitle from "@/components/PopupTitle"; |
There was a problem hiding this comment.
Build will fail on case‑sensitive filesystems (import path case).
Adjust PopupTitle import to match folder casing.
-import PopupTitle from "@/components/PopupTitle";
+import PopupTitle from "@/Components/PopupTitle";📝 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.
| import PopupTitle from "@/components/PopupTitle"; | |
| import PopupTitle from "@/Components/PopupTitle"; |
🤖 Prompt for AI Agents
In
applications/tari_walletd/web_ui/src/routes/AssetVault/Components/AddAccount.tsx
around line 37, the import path uses incorrect casing for PopupTitle which will
break on case‑sensitive filesystems; change the import to match the actual
folder/file casing exactly (e.g., adjust "@/components/PopupTitle" to the
correct case-sensitive path used in the repo) and ensure any other references
use the same exact casing so the build succeeds on case-sensitive environments.
| account && dataAccountsList?.accounts.some((info: AccountInfo) => info.account.address === account?.address) | ||
| ? substateIdToString(account.address) | ||
| : dataAccountsList?.accounts.length | ||
| ? substateIdToString(dataAccountsList.accounts[0].account.address) | ||
| : "addAccount" |
There was a problem hiding this comment.
Address comparison uses object identity; compare by string value.
Comparing addresses by object reference can fail; use substateIdToString on both sides.
- value={
- account && dataAccountsList?.accounts.some((info: AccountInfo) => info.account.address === account?.address)
- ? substateIdToString(account.address)
- : dataAccountsList?.accounts.length
- ? substateIdToString(dataAccountsList.accounts[0].account.address)
+ value={
+ account &&
+ dataAccountsList?.accounts.some(
+ (info: AccountInfo) =>
+ substateIdToString(info.account.address) === substateIdToString(account.address),
+ )
+ ? substateIdToString(account.address)
+ : dataAccountsList?.accounts.length
+ ? substateIdToString(dataAccountsList.accounts[0].account.address)
: "addAccount"
}📝 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.
| account && dataAccountsList?.accounts.some((info: AccountInfo) => info.account.address === account?.address) | |
| ? substateIdToString(account.address) | |
| : dataAccountsList?.accounts.length | |
| ? substateIdToString(dataAccountsList.accounts[0].account.address) | |
| : "addAccount" | |
| account && | |
| dataAccountsList?.accounts.some( | |
| (info: AccountInfo) => | |
| substateIdToString(info.account.address) === substateIdToString(account.address), | |
| ) | |
| ? substateIdToString(account.address) | |
| : dataAccountsList?.accounts.length | |
| ? substateIdToString(dataAccountsList.accounts[0].account.address) | |
| : "addAccount" |
🤖 Prompt for AI Agents
In
applications/tari_walletd/web_ui/src/routes/AssetVault/Components/SelectAccount.tsx
around lines 71 to 75, the current address comparison uses object identity
(info.account.address === account?.address) which can fail; change the
comparison to compare string values by calling substateIdToString on both sides
(e.g., substateIdToString(info.account.address) ===
substateIdToString(account?.address)), and keep the existing fallback logic
unchanged so the first account is selected or "addAccount" returned as before.
| import FormStep from "../steps/FormStep"; | ||
| import ConfirmationStep from "../steps/ConfirmationStep"; | ||
| import ResultStep from "../steps/ResultStep"; | ||
| import PopupTitle from "@/components/PopupTitle"; |
There was a problem hiding this comment.
Build will fail on case‑sensitive filesystems (import path case).
The file lives under “Components/PopupTitle.tsx” but the import path uses “components”. Fix the casing.
-import PopupTitle from "@/components/PopupTitle";
+import PopupTitle from "@/Components/PopupTitle";📝 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.
| import PopupTitle from "@/components/PopupTitle"; | |
| import PopupTitle from "@/Components/PopupTitle"; |
🤖 Prompt for AI Agents
In
applications/tari_walletd/web_ui/src/routes/AssetVault/NFTs/components/SendNft.tsx
around line 43, the import uses the wrong folder casing
("components/PopupTitle") which will break on case‑sensitive filesystems; change
the import path to match the actual file casing ("Components/PopupTitle") so the
module resolves correctly.
| const divisor = BigInt(CURRENCY.DIVISOR); | ||
| const integerPart = amount / divisor; | ||
| const remainder = amount % divisor; | ||
|
|
||
| // Convert remainder to fractional string padded to CURRENCY.DECIMALS | ||
| const fractionalPart = remainder.toString().padStart(CURRENCY.DECIMALS, "0"); | ||
|
|
||
| return `${Number(integerPart).toLocaleString("en-US")}.${fractionalPart} ${CURRENCY.SYMBOL}`; | ||
| return `${Number(integerPart).toLocaleString("en-US")}.${fractionalPart} ${currencySymbol}`; | ||
| } else if (typeof amount === "number") { |
There was a problem hiding this comment.
Avoid precision loss: do not cast BigInt to Number
Casting integerPart to Number can lose precision for large values. Use BigInt’s toLocaleString.
- return `${Number(integerPart).toLocaleString("en-US")}.${fractionalPart} ${currencySymbol}`;
+ return `${integerPart.toLocaleString("en-US")}.${fractionalPart} ${currencySymbol}`;📝 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 divisor = BigInt(CURRENCY.DIVISOR); | |
| const integerPart = amount / divisor; | |
| const remainder = amount % divisor; | |
| // Convert remainder to fractional string padded to CURRENCY.DECIMALS | |
| const fractionalPart = remainder.toString().padStart(CURRENCY.DECIMALS, "0"); | |
| return `${Number(integerPart).toLocaleString("en-US")}.${fractionalPart} ${CURRENCY.SYMBOL}`; | |
| return `${Number(integerPart).toLocaleString("en-US")}.${fractionalPart} ${currencySymbol}`; | |
| } else if (typeof amount === "number") { | |
| const divisor = BigInt(CURRENCY.DIVISOR); | |
| const integerPart = amount / divisor; | |
| const remainder = amount % divisor; | |
| const fractionalPart = remainder.toString().padStart(CURRENCY.DECIMALS, "0"); | |
| return `${integerPart.toLocaleString("en-US")}.${fractionalPart} ${currencySymbol}`; | |
| } else if (typeof amount === "number") { |
🤖 Prompt for AI Agents
In applications/tari_walletd/web_ui/src/utils/helpers.tsx around lines 258 to
265, the code casts a BigInt integerPart to Number which can lose precision for
large values; replace the cast with BigInt's built-in toLocaleString (e.g.
integerPart.toLocaleString("en-US")) so the integer portion is formatted without
converting to Number, keep the fractionalPart assembly as-is, and return the
final string using the BigInt-formatted integer plus the fractional and currency
symbol.
Test Results (CI)419 tests +24 413 ✅ +18 1h 17m 36s ⏱️ + 31m 54s For more details on these failures, see this check. Results for commit 5b7499f. ± Comparison against base commit 2c938d1. |
* development: feat(walletui): wallet ux improvements (tari-project#1572)
* development: feat(template_lib): adds engine schnorr signature verification (tari-project#1574) feat(wallet)!: add bech32 address with view-only key (tari-project#1573) feat(walletui): wallet ux improvements (tari-project#1572) fix(wallet)!: private derived tag and optimised* sync protocol (tari-project#1571) feat(walletui): send flow ux improvements (tari-project#1570) doc: update openrpc.json get_connections method (tari-project#1567) chore(deps): bump actions/setup-node from 4 to 5 (tari-project#1565)
3 participants
Description
Added various ux improvements:
Motivation and Context
General UX improvements
How Has This Been Tested?
Manually
What process can a PR reviewer use to test or verify this change?
Breaking Changes
x
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores