Add some Asset hubs reserved amount #1394
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe updates enhance the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (4 hunks)
- packages/extension-polkagate/src/hooks/useReservedDetails.ts (8 hunks)
- packages/extension/public/locales/en/translation.json (1 hunks)
- packages/extension/public/locales/fr/translation.json (1 hunks)
- packages/extension/public/locales/hi/translation.json (1 hunks)
- packages/extension/public/locales/ru/translation.json (1 hunks)
- packages/extension/public/locales/zh/translation.json (1 hunks)
Files skipped from review due to trivial changes (3)
- packages/extension/public/locales/en/translation.json
- packages/extension/public/locales/fr/translation.json
- packages/extension/public/locales/hi/translation.json
Additional context used
Biome
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx
[error] 73-90: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 79-89: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 86-88: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 136-138: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 134-134: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. (lint/suspicious/noGlobalIsNan)
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
Additional comments not posted (9)
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (3)
9-9: Addition of React hooks is appropriate for the new functionality.The addition of
useMemo,useState, anduseParamsaligns with the new functionality related to handling dynamic UI states and parameters.
15-15: Use ofuseParamsfrom React Router is correctly implemented.This hook is essential for retrieving route parameters, which is necessary for the new feature of handling asset-specific displays.
125-159: Ensure proper conditional rendering based ongenesisHashandnotOnNativeAsset.This logic effectively prevents the component from rendering irrelevant data based on the asset context, which is crucial for user experience and data integrity.
[APROVED]Tools
Biome
[error] 136-138: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 134-134: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. (lint/suspicious/noGlobalIsNan)
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.packages/extension-polkagate/src/hooks/useReservedDetails.ts (3)
14-14: Addition of constants for asset hubs and proxy chains.This addition supports the new functionality for handling different asset types and proxy chains, which is crucial for the expanded capabilities of the hook.
17-17: Utility functions for human-readable and machine-readable conversions are correctly implemented.These utility functions are essential for handling the display and calculation of asset values, improving the flexibility and readability of the code.
Line range hint
69-374: Ensure robust error handling in asynchronous operations.The extensive use of
.catchblocks for handling API call errors is well implemented, ensuring that the UI can gracefully handle failures.packages/extension/public/locales/zh/translation.json (2)
1289-1289: Added new translation key "No reasons found!"This change aligns with the PR objectives to add new language translations. The translation is correctly formatted and matches the provided English text.
Line range hint
1-1288: General Review of the FileThe existing translations in the file appear to be consistent and correctly formatted. No syntax errors or missing values were detected.
packages/extension/public/locales/ru/translation.json (1)
1280-1280: Translation for "No reasons found!" is correct and appropriate.The translation "Причины не найдены!" accurately conveys the meaning of the original English phrase and is grammatically correct.
...ages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx
Outdated
Show resolved
Hide resolved
...ages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx
Outdated
Show resolved
Hide resolved
|
@AMIRKHANEF |
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (4 hunks)
- packages/extension-polkagate/src/popup/account/ReservedReasons.tsx (5 hunks)
- packages/extension-polkagate/src/popup/account/index.tsx (2 hunks)
Additional context used
Biome
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx
[error] 134-136: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
packages/extension-polkagate/src/popup/account/index.tsx
[error] 60-62: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
...ages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx
Outdated
Show resolved
Hide resolved
...ages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx
Outdated
Show resolved
Hide resolved
Eliminate // @ts-nocheck Apply code rabit suggestions
AMIRKHANEF
force-pushed
the
AssetHubsReservedAmount
branch
from
4c3d72a
to
230133e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (3)
packages/extension-polkagate/src/components/Assets.tsx (3)
Line range hint
23-23: Fix the type ofsetAssetIdprop.The
setAssetIdprop should be typed asReact.Dispatch<React.SetStateAction<number | undefined>>instead ofReact.Dispatch<React.SetStateAction<number | undefined>>.- setAssetId: React.Dispatch<React.SetStateAction<number | undefined>> + setAssetId: React.Dispatch<React.SetStateAction<number | undefined>>;
Line range hint
28-28: Handle missing address gracefully.The
addressprop is used directly without checking if it's defined. Add a check to handle cases whereaddressis null or undefined.- const tokens = useTokens(address as string); - const chain = useChain(address); - const assetHubOptions = useAssetHubAssets(address as string); - const multiChainAssetsOptions = useAccountAssetsOptions(address as string); + const tokens = useTokens(address ?? ''); + const chain = useChain(address ?? ''); + const assetHubOptions = useAssetHubAssets(address ?? ''); + const multiChainAssetsOptions = useAccountAssetsOptions(address ?? '');
Line range hint
37-37: InitializeisLoadingstate properly.The
isLoadingstate is declared but not initialized. Initialize it tofalse.- const [isLoading, setLoading] = useState<boolean>(); + const [isLoading, setLoading] = useState<boolean>(false);
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- packages/extension-polkagate/src/components/Assets.tsx (1 hunks)
- packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (4 hunks)
- packages/extension-polkagate/src/popup/account/LabelBalancePrice.tsx (1 hunks)
- packages/extension-polkagate/src/popup/account/ReservedReasons.tsx (5 hunks)
- packages/extension-polkagate/src/popup/account/index.tsx (2 hunks)
Files not summarized due to errors (4)
- packages/extension-polkagate/src/popup/account/LabelBalancePrice.tsx: Error: Server error. Please try again later.
- packages/extension-polkagate/src/components/Assets.tsx: Error: Server error. Please try again later.
- packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx: Error: Server error. Please try again later.
- packages/extension-polkagate/src/popup/account/ReservedReasons.tsx: Error: Server error. Please try again later.
Additional context used
Learnings (1)
packages/extension-polkagate/src/popup/account/index.tsx (1)
Learnt from: AMIRKHANEF PR: PolkaGate/extension#1394 File: packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx:0-0 Timestamp: 2024-07-01T09:27:24.482Z Learning: When checking for NaN values, always use `Number.isNaN` instead of `isNaN` to avoid unexpected type coercions.
Biome
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx
[error] 134-136: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
packages/extension-polkagate/src/popup/account/index.tsx
[error] 60-62: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
Additional comments not posted (6)
packages/extension-polkagate/src/components/Assets.tsx (2)
1-4: Remove unnecessary ESLint disable comment.The
react/jsx-max-props-per-linerule is disabled but not needed in this context.- /* eslint-disable react/jsx-max-props-per-line */
Line range hint
5-6: Remove unnecessary type import.The
SxPropsandThemetypes are imported but not used in the file.- import { Grid, type SxProps, type Theme } from '@mui/material; + import { Grid } from '@mui/material;packages/extension-polkagate/src/popup/account/LabelBalancePrice.tsx (1)
3-4: Remove unnecessary ESLint disable comment.The
header/headerrule is disabled but not needed in this context.- /* eslint-disable header/header */packages/extension-polkagate/src/popup/account/ReservedReasons.tsx (1)
14-14: Remove unnecessaryuseCallbackimport.The
useCallbackhook is imported but not used in the file.- import React, { useCallback, useMemo } from 'react'; + import React, { useMemo } from 'react';packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (2)
9-10: Remove unnecessaryuseCallbackimport.The
useCallbackhook is imported but not used in the file.- import React, { useCallback, useEffect, useMemo, useState } from 'react'; + import React, { useEffect, useMemo, useState } from 'react';
13-13: Remove unnecessaryBNimport.The
BNclass is imported but not used in the file.- import { BN } from '@polkadot/util';
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
packages/extension-polkagate/src/popup/account/LabelBalancePrice.tsx (1)
Line range hint
14-14: Consider adding TypeScript type annotations for better type safety.The component uses TypeScript, but not all functions and variables are fully typed. Adding explicit types can improve maintainability and reduce the risk of bugs.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- packages/extension-polkagate/src/components/Assets.tsx (1 hunks)
- packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (4 hunks)
- packages/extension-polkagate/src/popup/account/LabelBalancePrice.tsx (1 hunks)
- packages/extension-polkagate/src/popup/account/ReservedReasons.tsx (5 hunks)
- packages/extension-polkagate/src/popup/account/index.tsx (2 hunks)
Files not summarized due to errors (3)
- packages/extension-polkagate/src/popup/account/LabelBalancePrice.tsx: Error: Server error. Please try again later.
- packages/extension-polkagate/src/popup/account/index.tsx: Error: Server error. Please try again later.
- packages/extension-polkagate/src/popup/account/ReservedReasons.tsx: Error: Server error. Please try again later.
Files skipped from review due to trivial changes (1)
- packages/extension-polkagate/src/components/Assets.tsx
Additional context used
Learnings (1)
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (1)
Learnt from: AMIRKHANEF PR: PolkaGate/extension#1394 File: packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx:0-0 Timestamp: 2024-07-01T09:27:24.482Z Learning: When checking for NaN values, always use `Number.isNaN` instead of `isNaN` to avoid unexpected type coercions.
Additional comments not posted (6)
packages/extension-polkagate/src/popup/account/LabelBalancePrice.tsx (1)
Line range hint
19-19: Ensure consistent use of theme colors.It appears that the theme is used inconsistently. For example,
theme.palette.secondary.lightis used without checking if it exists. This could lead to runtime errors if the theme structure changes.#!/bin/bash # Description: Check for consistent use of theme across the project. # Test: Search for theme usage. Expect: Consistent theme structure usage. rg --type typescript 'theme.palette'packages/extension-polkagate/src/popup/account/ReservedReasons.tsx (2)
Line range hint
105-143: Ensure proper handling of null and undefined states.The rendering logic based on
reasonsToShowshould explicitly handleundefinedandnullto avoid rendering issues or errors in edge cases.
57-77: Revisit the conditional logic inuseMemo.The logic inside
useMemohas been a point of contention. The conditionsdetails.length === 0anddetails.every((deposit) => deposit === null)are indeed different and should not be combined as they check for different scenarios. It's important to maintain clear and separate checks for these conditions to avoid logic errors.#!/bin/bash # Description: Verify the logic of useMemo in other similar components for consistency. # Test: Search for similar useMemo usage. Expect: Consistent handling of similar conditions. rg --type typescript 'useMemo'packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (2)
129-133: Good use ofNumber.isNaNfor NaN checking.This is a good practice and aligns with previous learnings to avoid type coercion issues.
69-89: Simplify conditional logic inuseMemo.The nested conditional logic can be streamlined for better readability and performance, especially since similar suggestions have been made in the past.
- if (details.length === 0) { - return undefined - } - const noReason = details.every((deposit) => deposit === null); - if (noReason) { - return null; - } - const filteredReservedDetails = Object.fromEntries( - Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero()) - ); - return Object.values(filteredReservedDetails).length > 0 ? filteredReservedDetails : undefined; + if (details.length === 0 || details.every((deposit) => deposit === null)) return null; + const filteredReservedDetails = Object.fromEntries( + Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero()) + ); + return Object.values(filteredReservedDetails).length > 0 ? filteredReservedDetails : null;Likely invalid or redundant comment.
packages/extension-polkagate/src/popup/account/index.tsx (1)
55-60: Simplify the conditional logic.The previous comment about omitting redundant
elseclauses is still relevant. Simplifying this logic can improve readability and performance.- if (assetIdNumber > 0) { - return true; - } else { - return false; - } + return assetIdNumber > 0;Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (5)
packages/extension-polkagate/src/components/Assets.tsx (1)
Line range hint
9-9: Ensure proper handling of undefined addresses.The
useTokens,useChain, anduseAssetHubAssetshooks are called withaddresscast to a string without checking ifaddressis defined. This could lead to runtime errors ifaddressisnullorundefined.- const tokens = useTokens(address as string); - const chain = useChain(address); - const assetHubOptions = useAssetHubAssets(address as string); + const tokens = useTokens(address ?? ''); + const chain = useChain(address ?? ''); + const assetHubOptions = useAssetHubAssets(address ?? '');packages/extension-polkagate/src/popup/account/LabelBalancePrice.tsx (2)
Line range hint
41-41: Ensure proper handling of undefined addresses.The
useApianduseTokenPricehooks are called withaddresscast to a string without checking ifaddressis defined. This could lead to runtime errors ifaddressisundefined.- const api = useApi(address); - const { price } = useTokenPrice(address as string, balances?.assetId); + const api = useApi(address ?? ''); + const { price } = useTokenPrice(address ?? '', balances?.assetId);
Line range hint
107-107: Avoid using index as key in lists.Using the index as a key in lists can lead to issues with component re-rendering and state management. Consider using a unique identifier instead.
- {Object.entries(reasonsToShow)?.map(([key, value], index) => ( - <Grid container item key={index}> + {Object.entries(reasonsToShow)?.map(([key, value]) => ( + <Grid container item key={key}>packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (2)
Line range hint
57-77: Optimize the conditional logic inuseMemo.The nested conditional logic can be simplified to improve readability and maintainability.
- if (details.length === 0) { - return undefined; - } - const noReason = details.every((deposit) => deposit === null); - if (noReason) { - return null; - } - const filteredReservedDetails = Object.fromEntries( - Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero()) - ); - return Object.values(filteredReservedDetails).length > 0 ? filteredReservedDetails : undefined; + if (details.length === 0) { + return undefined; + } + if (details.every((deposit) => deposit === null)) { + return null; + } + const filteredReservedDetails = Object.fromEntries( + Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero()) + ); + return Object.values(filteredReservedDetails).length > 0 ? filteredReservedDetails : undefined;
Line range hint
107-107: Avoid using index as key in lists.Using the index as a key in lists can lead to issues with component re-rendering and state management. Consider using a unique identifier instead.
- {Object.entries(reasonsToShow)?.map(([key, value], index) => ( - <Grid container item key={index}> + {Object.entries(reasonsToShow)?.map(([key, value]) => ( + <Grid container item key={key}>
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- packages/extension-polkagate/src/components/Assets.tsx (1 hunks)
- packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (4 hunks)
- packages/extension-polkagate/src/popup/account/LabelBalancePrice.tsx (1 hunks)
- packages/extension-polkagate/src/popup/account/ReservedReasons.tsx (5 hunks)
- packages/extension-polkagate/src/popup/account/index.tsx (2 hunks)
Files not summarized due to errors (5)
- packages/extension-polkagate/src/popup/account/index.tsx: Error: Server error. Please try again later.
- packages/extension-polkagate/src/popup/account/LabelBalancePrice.tsx: Error: Server error. Please try again later.
- packages/extension-polkagate/src/components/Assets.tsx: Error: Server error. Please try again later.
- packages/extension-polkagate/src/popup/account/ReservedReasons.tsx: Error: Server error. Please try again later.
- packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx: Error: Server error. Please try again later.
Additional context used
Learnings (1)
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (1)
Learnt from: AMIRKHANEF PR: PolkaGate/extension#1394 File: packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx:0-0 Timestamp: 2024-07-01T09:27:24.482Z Learning: When checking for NaN values, always use `Number.isNaN` instead of `isNaN` to avoid unexpected type coercions.
Additional comments not posted (7)
packages/extension-polkagate/src/components/Assets.tsx (2)
Line range hint
20-20: Consider adding a loading state for chain switch.When the chain changes, the
setAssetIdis called, but there is no indication to the user that the assets are being reloaded. Consider adding a loading state to improve user experience.useEffect(() => { setAssetId(undefined); // this will set the asset to the native asset on chain switch + setLoading(true); }, [chain]);
Line range hint
24-24: Verify the logic for setting loading state.The
setLoadingfunction is called withtrueif bothassetHubOptionsandmultiChainAssetsOptionsareundefined, andfalseotherwise. Ensure this logic correctly reflects the loading state.useEffect(() => { if (assetHubOptions === undefined && multiChainAssetsOptions === undefined) { setAssetId(undefined); return setLoading(true); } setLoading(false); }, [assetHubOptions, multiChainAssetsOptions, setAssetId]);packages/extension-polkagate/src/popup/account/LabelBalancePrice.tsx (1)
3-3: Remove unnecessary ESLint disable directive.The
header/headerESLint directive is not necessary and can be removed to maintain cleaner code./* eslint-disable header/header */packages/extension-polkagate/src/popup/account/ReservedReasons.tsx (1)
57-77: Optimize the conditional logic inuseMemo.The nested conditional logic can be simplified to improve readability and maintainability.
- if (details.length === 0) { - return undefined; - } - const noReason = details.every((deposit) => deposit === null); - if (noReason) { - return null; - } - const filteredReservedDetails = Object.fromEntries( - Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero()) - ); - return Object.values(filteredReservedDetails).length > 0 ? filteredReservedDetails : undefined; + if (details.length === 0) { + return undefined; + } + if (details.every((deposit) => deposit === null)) { + return null; + } + const filteredReservedDetails = Object.fromEntries( + Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero()) + ); + return Object.values(filteredReservedDetails).length > 0 ? filteredReservedDetails : undefined;Likely invalid or redundant comment.
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (1)
129-133: Omit the redundantelseclause innotOnNativeAsset.The
elseclause is redundant because the previous branch returns early.- if (Number.isNaN(assetIdNumber) || assetIdNumber > 0) { - return true; - } else { - return false; - } + return Number.isNaN(assetIdNumber) || assetIdNumber > 0;Likely invalid or redundant comment.
packages/extension-polkagate/src/popup/account/index.tsx (2)
55-59: Omit the redundantelseclause innotOnNativeAsset.The
elseclause is redundant because the previous branch returns early.- if (assetIdNumber > 0) { - return true; - } else { - return false; - } + return assetIdNumber > 0;
60-60: LGTM!The memoization for
showReservedChevronis well-implemented and improves performance by avoiding unnecessary recalculations.
...ages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (4 hunks)
- packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx (1 hunks)
Additional context used
Learnings (2)
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (1)
Learnt from: AMIRKHANEF PR: PolkaGate/extension#1394 File: packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx:0-0 Timestamp: 2024-07-01T09:27:24.482Z Learning: When checking for NaN values, always use `Number.isNaN` instead of `isNaN` to avoid unexpected type coercions.packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx (1)
Learnt from: AMIRKHANEF PR: PolkaGate/extension#1394 File: packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx:0-0 Timestamp: 2024-07-01T09:27:24.482Z Learning: When checking for NaN values, always use `Number.isNaN` instead of `isNaN` to avoid unexpected type coercions.
GitHub Check: build
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx
[failure] 15-15:
'useParams' is declared but its value is never read.
Additional comments not posted (6)
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (5)
23-23: LGTM!The addition of the
assetIdparameter in the Props interface is appropriate.
Line range hint
99-117: LGTM!The conditional rendering logic for
reasonsToShowis correct and handles all possible states.
124-149: LGTM!The updates to the
ReservedDisplayBalancecomponent, including theassetIdparameter and related logic, are correct and well-integrated.
129-129: LGTM!The
notOnNativeAssetuseMemo hook logic is correct and efficient.
70-90: Optimize conditional logic inuseMemo.The nested conditional logic can be simplified to improve readability and maintainability.
- if (details.length === 0) { - return undefined; - } - const noReason = details.every((deposit) => deposit === null); - if (noReason) { - return null; - } - const filteredReservedDetails = Object.fromEntries( - Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero()) - ); - return Object.values(filteredReservedDetails).length > 0 ? filteredReservedDetails : undefined; + if (details.length === 0 || details.every((deposit) => deposit === null)) return null; + const filteredReservedDetails = Object.fromEntries( + Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero()) + ); + return Object.values(filteredReservedDetails).length > 0 ? filteredReservedDetails : undefined;Likely invalid or redundant comment.
packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx (1)
258-258: LGTM!The updates to the
ReservedDisplayBalancecomponent usage, including theassetIdparameter, are correct and well-integrated.
...ages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (4 hunks)
- packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx (1 hunks)
Additional context used
Learnings (2)
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (1)
Learnt from: AMIRKHANEF PR: PolkaGate/extension#1394 File: packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx:0-0 Timestamp: 2024-07-01T09:27:24.482Z Learning: When checking for NaN values, always use `Number.isNaN` instead of `isNaN` to avoid unexpected type coercions.packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx (1)
Learnt from: AMIRKHANEF PR: PolkaGate/extension#1394 File: packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx:0-0 Timestamp: 2024-07-01T09:27:24.482Z Learning: When checking for NaN values, always use `Number.isNaN` instead of `isNaN` to avoid unexpected type coercions.
GitHub Check: build
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx
[failure] 15-15:
'useParams' is declared but its value is never read.
Additional comments not posted (3)
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (3)
Line range hint
42-61: LGTM!The
WaitForReservedfunction component is implemented correctly and uses props effectively to render skeletons.
Line range hint
124-153: LGTM!The
ReservedDisplayBalancefunction component is implemented correctly and uses hooks and conditional rendering effectively.
70-90: Optimize the conditional logic inuseMemo.The nested conditional logic can be simplified to improve readability and maintainability.
- if (details.length === 0) { - return undefined; - } - const noReason = details.every((deposit) => deposit === null); - if (noReason) { - return null; - } - const filteredReservedDetails = Object.fromEntries( - Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero()) - ); - return Object.values(filteredReservedDetails).length > 0 ? filteredReservedDetails : undefined; + if (details.length === 0 || details.every((deposit) => deposit === null)) return null; + const filteredReservedDetails = Object.fromEntries( + Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero()) + ); + return Object.values(filteredReservedDetails).length > 0 ? filteredReservedDetails : undefined;Likely invalid or redundant comment.
...ages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx
Outdated
Show resolved
Hide resolved
|
address #1360 |
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (5 hunks)
Additional context used
Learnings (1)
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (1)
Learnt from: AMIRKHANEF PR: PolkaGate/extension#1394 File: packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx:0-0 Timestamp: 2024-07-01T09:27:24.482Z Learning: When checking for NaN values, always use `Number.isNaN` instead of `isNaN` to avoid unexpected type coercions.
Additional comments not posted (7)
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (7)
9-9: LGTM! The import ofuseMemois appropriate.The
useMemohook is necessary for memoizing values that depend on dependencies.
13-13: LGTM! The import ofisOnRelayChainis appropriate.The
isOnRelayChainfunction is necessary for checking if the current chain is a relay chain.
22-22: LGTM! The addition of theassetIdproperty to thePropsinterface is appropriate.This change is necessary for handling asset IDs in the component.
112-116: LGTM! The addition of the translation key"No reasons found!"is appropriate.This change adds a new translation key for localization purposes.
128-129: LGTM! The use ofuseMemofornotOnNativeAssetis appropriate.This change memoizes the calculation of
notOnNativeAssetbased on theassetId.
134-134: LGTM! The addition ofaddressandgenesisHashas dependencies for theuseEffecthook is appropriate.This change ensures that the effect runs whenever
addressorgenesisHashchanges.
140-148: LGTM! The conditional rendering logic forReservedDisplayBalanceis appropriate.This change ensures that the component renders correctly based on
genesisHashandnotOnNativeAsset.
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (5 hunks)
Additional context used
Learnings (1)
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (1)
Learnt from: AMIRKHANEF PR: PolkaGate/extension#1394 File: packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx:0-0 Timestamp: 2024-07-01T09:27:24.482Z Learning: When checking for NaN values, always use `Number.isNaN` instead of `isNaN` to avoid unexpected type coercions.
Additional comments not posted (1)
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (1)
41-41: LGTM!The
WaitForReservedfunction component is well-implemented and does not need any changes.
...ages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
Outside diff range and nitpick comments (6)
packages/extension-polkagate/src/hooks/useReservedDetails.ts (6)
Line range hint
121-162: Improve error handling for referenda fetch.Consider adding more specific error messages to identify the source of the error.
- console.error(error); + console.error('Error fetching referenda:', error);
Line range hint
167-188: Improve error handling for bounty fetch.Consider adding more specific error messages to identify the source of the error.
- console.error(error); + console.error('Error fetching bounty:', error);
Line range hint
193-214: Improve error handling for index fetch.Consider adding more specific error messages to identify the source of the error.
- console.error(error); + console.error('Error fetching index:', error);
Line range hint
219-240: Improve error handling for multisig fetch.Consider adding more specific error messages to identify the source of the error.
- console.error(error); + console.error('Error fetching multisig:', error);
Line range hint
245-272: Improve error handling for preimage fetch.Consider adding more specific error messages to identify the source of the error.
- console.error(error); + console.error('Error fetching preimage:', error);
Line range hint
277-306: Improve error handling for society fetch.Consider adding more specific error messages to identify the source of the error.
- console.error(error); + console.error('Error fetching society:', error);
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/extension-polkagate/src/hooks/useReservedDetails.ts (8 hunks)
Additional comments not posted (2)
packages/extension-polkagate/src/hooks/useReservedDetails.ts (2)
14-14: Unused Import: Consider removingPROXY_CHAINSif not used.The import statement includes
PROXY_CHAINS, but it is not used in the provided code.- import { ASSET_HUBS, PROXY_CHAINS } from '../util/constants'; + import { ASSET_HUBS } from '../util/constants';
17-17: Unused Import: Consider removingamountToHumanif not used.The import statement includes
amountToHuman, but it is not used in the provided code.- import { amountToHuman, amountToMachine } from '../util/utils'; + import { amountToMachine } from '../util/utils';
| const setValue = useCallback((item: string, value: BN | null | undefined) => { | ||
| setReserved((prev) => { | ||
| const newState = { ...prev }; | ||
|
|
||
| newState[item] = value | ||
| ? value.isZero() ? null : toBalance(value) | ||
| : value; | ||
|
|
||
| return newState; | ||
| }); | ||
| }, [toBalance]); |
There was a problem hiding this comment.
Refactor the setValue function to improve clarity and efficiency.
The function can be simplified to enhance readability and reduce redundancy.
- setReserved((prev) => {
- const newState = { ...prev };
- newState[item] = value ? (value.isZero() ? null : toBalance(value)) : value;
- return newState;
- });
+ setReserved((prev) => ({ ...prev, [item]: value ? (value.isZero() ? null : toBalance(value)) : value }));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 setValue = useCallback((item: string, value: BN | null | undefined) => { | |
| setReserved((prev) => { | |
| const newState = { ...prev }; | |
| newState[item] = value | |
| ? value.isZero() ? null : toBalance(value) | |
| : value; | |
| return newState; | |
| }); | |
| }, [toBalance]); | |
| const setValue = useCallback((item: string, value: BN | null | undefined) => { | |
| setReserved((prev) => ({ ...prev, [item]: value ? (value.isZero() ? null : toBalance(value)) : value })); | |
| }, [toBalance]); |
| setValue('identity', undefined); | ||
|
|
||
| api.query.proxy.proxies(formatted).then((p) => { | ||
| const mayBeDeposit = p?.[1] as BN; | ||
|
|
||
| if (!mayBeDeposit?.isZero()) { | ||
| setReserved((prev) => { | ||
| prev.proxy = toBalance(mayBeDeposit); | ||
|
|
||
| return prev; | ||
| }); | ||
| } | ||
| }).catch(console.error); | ||
| setValue('proxy', mayBeDeposit); | ||
| }).catch((error) => { | ||
| console.error(error); | ||
| setValue('proxy', null); | ||
| }); | ||
| } else { | ||
| setValue('proxy', null); |
There was a problem hiding this comment.
Improve error handling for proxy fetch.
Consider adding more specific error messages to identify the source of the error.
- console.error(error);
+ console.error('Error fetching proxy:', error);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.
| setValue('identity', undefined); | |
| api.query.proxy.proxies(formatted).then((p) => { | |
| const mayBeDeposit = p?.[1] as BN; | |
| if (!mayBeDeposit?.isZero()) { | |
| setReserved((prev) => { | |
| prev.proxy = toBalance(mayBeDeposit); | |
| return prev; | |
| }); | |
| } | |
| }).catch(console.error); | |
| setValue('proxy', mayBeDeposit); | |
| }).catch((error) => { | |
| console.error(error); | |
| setValue('proxy', null); | |
| }); | |
| } else { | |
| setValue('proxy', null); | |
| setValue('identity', undefined); | |
| api.query.proxy.proxies(formatted).then((p) => { | |
| const mayBeDeposit = p?.[1] as BN; | |
| setValue('proxy', mayBeDeposit); | |
| }).catch((error) => { | |
| console.error('Error fetching proxy:', error); | |
| setValue('proxy', null); | |
| }); | |
| } else { | |
| setValue('proxy', null); |
| if (api.query?.recovery) { | ||
| setValue('recovery', undefined); | ||
|
|
||
| api.query.recovery.recoverable(formatted).then((r) => { | ||
| const recoveryInfo = r.isSome ? r.unwrap() as unknown as PalletRecoveryRecoveryConfig : null; | ||
|
|
||
| recoveryInfo?.deposit && setReserved((prev) => { | ||
| prev.recovery = toBalance((recoveryInfo.deposit as unknown as BN).add(activeLost?.deposit as unknown as BN || BN_ZERO)); | ||
| const recoveryDeposit = (recoveryInfo?.deposit as unknown as BN || BN_ZERO).add(activeLost?.deposit as unknown as BN || BN_ZERO); | ||
|
|
||
| return prev; | ||
| setValue('recovery', recoveryDeposit); | ||
| }).catch((error) => { | ||
| console.error(error); | ||
| setValue('recovery', null); | ||
| }); | ||
| }).catch(console.error); | ||
| } else { | ||
| setValue('recovery', null); | ||
| } |
There was a problem hiding this comment.
Improve error handling for recovery fetch.
Consider adding more specific error messages to identify the source of the error.
- console.error(error);
+ console.error('Error fetching recovery:', error);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.
| if (api.query?.recovery) { | |
| setValue('recovery', undefined); | |
| api.query.recovery.recoverable(formatted).then((r) => { | |
| const recoveryInfo = r.isSome ? r.unwrap() as unknown as PalletRecoveryRecoveryConfig : null; | |
| recoveryInfo?.deposit && setReserved((prev) => { | |
| prev.recovery = toBalance((recoveryInfo.deposit as unknown as BN).add(activeLost?.deposit as unknown as BN || BN_ZERO)); | |
| const recoveryDeposit = (recoveryInfo?.deposit as unknown as BN || BN_ZERO).add(activeLost?.deposit as unknown as BN || BN_ZERO); | |
| return prev; | |
| setValue('recovery', recoveryDeposit); | |
| }).catch((error) => { | |
| console.error(error); | |
| setValue('recovery', null); | |
| }); | |
| }).catch(console.error); | |
| } else { | |
| setValue('recovery', null); | |
| } | |
| if (api.query?.recovery) { | |
| setValue('recovery', undefined); | |
| api.query.recovery.recoverable(formatted).then((r) => { | |
| const recoveryInfo = r.isSome ? r.unwrap() as unknown as PalletRecoveryRecoveryConfig : null; | |
| const recoveryDeposit = (recoveryInfo?.deposit as unknown as BN || BN_ZERO).add(activeLost?.deposit as unknown as BN || BN_ZERO); | |
| setValue('recovery', recoveryDeposit); | |
| }).catch((error) => { | |
| console.error('Error fetching recovery:', error); | |
| setValue('recovery', null); | |
| }); | |
| } else { | |
| setValue('recovery', null); | |
| } |
| if (api.query?.identity?.identityOf) { | ||
| setValue('identity', undefined); | ||
|
|
||
| const subs = await api.query.identity.subsOf(formatted); | ||
| api.query.identity.identityOf(formatted).then(async (id) => { | ||
| const basicDeposit = api.consts.identity.basicDeposit as unknown as BN; | ||
| const subAccountDeposit = api.consts.identity.subAccountDeposit as unknown as BN; | ||
|
|
||
| const subAccountsDeposit = (subs ? subs[0] : BN_ZERO) as unknown as BN; | ||
| const subs = await api.query.identity.subsOf(formatted); | ||
|
|
||
| const sum = (basicDeposit.add(subAccountsDeposit)) as unknown as BN; | ||
| const subAccountsDeposit = (subs ? subs[0] : BN_ZERO) as unknown as BN; | ||
|
|
||
| !id.isEmpty && setReserved((prev) => { | ||
| prev.identity = toBalance(sum); | ||
| const sum = (basicDeposit.add(subAccountsDeposit)) as unknown as BN; | ||
|
|
||
| return prev; | ||
| setValue('identity', sum); | ||
| }).catch((error) => { | ||
| console.error(error); | ||
| setValue('identity', null); | ||
| }); | ||
| }).catch(console.error); | ||
| } else { | ||
| setValue('identity', null); | ||
| } |
There was a problem hiding this comment.
Improve error handling for identity fetch.
Consider adding more specific error messages to identify the source of the error.
- console.error(error);
+ console.error('Error fetching identity:', error);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.
| if (api.query?.identity?.identityOf) { | |
| setValue('identity', undefined); | |
| const subs = await api.query.identity.subsOf(formatted); | |
| api.query.identity.identityOf(formatted).then(async (id) => { | |
| const basicDeposit = api.consts.identity.basicDeposit as unknown as BN; | |
| const subAccountDeposit = api.consts.identity.subAccountDeposit as unknown as BN; | |
| const subAccountsDeposit = (subs ? subs[0] : BN_ZERO) as unknown as BN; | |
| const subs = await api.query.identity.subsOf(formatted); | |
| const sum = (basicDeposit.add(subAccountsDeposit)) as unknown as BN; | |
| const subAccountsDeposit = (subs ? subs[0] : BN_ZERO) as unknown as BN; | |
| !id.isEmpty && setReserved((prev) => { | |
| prev.identity = toBalance(sum); | |
| const sum = (basicDeposit.add(subAccountsDeposit)) as unknown as BN; | |
| return prev; | |
| setValue('identity', sum); | |
| }).catch((error) => { | |
| console.error(error); | |
| setValue('identity', null); | |
| }); | |
| }).catch(console.error); | |
| } else { | |
| setValue('identity', null); | |
| } | |
| if (api.query?.identity?.identityOf) { | |
| setValue('identity', undefined); | |
| api.query.identity.identityOf(formatted).then(async (id) => { | |
| const basicDeposit = api.consts.identity.basicDeposit as unknown as BN; | |
| const subAccountDeposit = api.consts.identity.subAccountDeposit as unknown as BN; | |
| const subs = await api.query.identity.subsOf(formatted); | |
| const subAccountsDeposit = (subs ? subs[0] : BN_ZERO) as unknown as BN; | |
| const sum = (basicDeposit.add(subAccountsDeposit)) as unknown as BN; | |
| setValue('identity', sum); | |
| }).catch((error) => { | |
| console.error('Error fetching identity:', error); | |
| setValue('identity', null); | |
| }); | |
| } else { | |
| setValue('identity', null); | |
| } |
| if (api.query?.uniques) { | ||
| setValue('uniques_NFT', null); | ||
|
|
||
| api.query.uniques.class.entries().then((classes) => { | ||
| const myClasses = classes.filter(([uniquesId, uniquesInfo]) => String(uniquesInfo.toPrimitive().owner) === formatted).map(([uniquesId, uniquesInfo]) => uniquesInfo.toPrimitive()); | ||
|
|
||
| const totalClassesDeposit = myClasses.reduce((acc, myClass) => { | ||
| const depositAsBn = amountToMachine(String(myClass.totalDeposit / (10 ** decimal)), decimal); | ||
|
|
||
| return acc.add(depositAsBn); | ||
| }, BN_ZERO); | ||
|
|
||
| setValue('uniques_NFT', totalClassesDeposit); | ||
| }).catch((error) => { | ||
| console.error(error); | ||
| setValue('uniques_NFT', null); | ||
| }); | ||
| } else { | ||
| setValue('uniques_NFT', null); |
There was a problem hiding this comment.
Fix error handling and improve performance for unique NFT fetch.
The code for fetching unique NFTs has redundant error handling and can be optimized for performance.
- setValue('uniques_NFT', null);
+ setValue('uniques_NFT', undefined);
- api.query.uniques.class.entries().then((classes) => {
+ try {
+ const classEntries = await api.query.uniques.class.entries();
+ const myClasses = classEntries.filter(([uniquesId, uniquesInfo]) => String(uniquesInfo.toPrimitive().owner) === formatted).map(([uniquesId, uniquesInfo]) => uniquesInfo.toPrimitive());
+ const totalClassesDeposit = myClasses.reduce((acc, myClass) => acc.add(amountToMachine(String(myClass.totalDeposit / (10 ** decimal)), decimal)), BN_ZERO);
+ setValue('uniques_NFT', totalClassesDeposit);
+ } catch (error) {
+ console.error('Error fetching unique NFTs:', error);
+ setValue('uniques_NFT', null);
+ }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.
| if (api.query?.uniques) { | |
| setValue('uniques_NFT', null); | |
| api.query.uniques.class.entries().then((classes) => { | |
| const myClasses = classes.filter(([uniquesId, uniquesInfo]) => String(uniquesInfo.toPrimitive().owner) === formatted).map(([uniquesId, uniquesInfo]) => uniquesInfo.toPrimitive()); | |
| const totalClassesDeposit = myClasses.reduce((acc, myClass) => { | |
| const depositAsBn = amountToMachine(String(myClass.totalDeposit / (10 ** decimal)), decimal); | |
| return acc.add(depositAsBn); | |
| }, BN_ZERO); | |
| setValue('uniques_NFT', totalClassesDeposit); | |
| }).catch((error) => { | |
| console.error(error); | |
| setValue('uniques_NFT', null); | |
| }); | |
| } else { | |
| setValue('uniques_NFT', null); | |
| if (api.query?.uniques) { | |
| setValue('uniques_NFT', undefined); | |
| try { | |
| const classEntries = await api.query.uniques.class.entries(); | |
| const myClasses = classEntries.filter(([uniquesId, uniquesInfo]) => String(uniquesInfo.toPrimitive().owner) === formatted).map(([uniquesId, uniquesInfo]) => uniquesInfo.toPrimitive()); | |
| const totalClassesDeposit = myClasses.reduce((acc, myClass) => acc.add(amountToMachine(String(myClass.totalDeposit / (10 ** decimal)), decimal)), BN_ZERO); | |
| setValue('uniques_NFT', totalClassesDeposit); | |
| } catch (error) { | |
| console.error('Error fetching unique NFTs:', error); | |
| setValue('uniques_NFT', null); | |
| } | |
| } else { | |
| setValue('uniques_NFT', null); |
| if (api.query?.nfts) { | ||
| setValue('NFT', null); | ||
|
|
||
| api.query.nfts.item.entries().then(async (nft) => { | ||
|
|
||
| const myNFTs = nft.filter(([ntfId, nftInfo]) => String(nftInfo.toPrimitive().deposit.account) === formatted).map(([ntfId, nftInfo]) => [ntfId.toHuman(), nftInfo.toPrimitive()]); | ||
| const nftDepositRequests = myNFTs.map(([nftId, nftInfo]) => api.query.nfts.itemMetadataOf(...nftId)); | ||
| const nftDepositAmount = (await Promise.all(nftDepositRequests)).map((deposit) => deposit.toPrimitive().deposit.amount); | ||
|
|
||
| const totalNftDeposits = nftDepositAmount.reduce((acc, deposit) => { | ||
| const depositAsBn = amountToMachine(String(deposit / (10 ** decimal)), decimal); | ||
|
|
||
| return acc.add(depositAsBn); | ||
| }, BN_ZERO); | ||
|
|
||
| const myNFTDeposits = myNFTs.map(([ntfId, nftInfo]) => nftInfo.deposit.amount as number); | ||
| const totalNFTDeposit = myNFTDeposits.reduce((acc, deposit) => { | ||
| const depositAsBn = amountToMachine(String(deposit / (10 ** decimal)), decimal); | ||
|
|
||
| return acc.add(depositAsBn); | ||
| }, BN_ZERO); | ||
|
|
||
| const finalNFTDeposit = totalNFTDeposit.add(totalNftDeposits); | ||
|
|
||
| setValue('NFT', finalNFTDeposit); | ||
| }).catch((error) => { | ||
| console.error(error); | ||
| setValue('NFT', null); | ||
| }); | ||
| } else { | ||
| setValue('NFT', null); | ||
| } |
There was a problem hiding this comment.
Fix error handling and improve performance for NFT fetch.
The code for fetching NFTs has redundant error handling and can be optimized for performance.
- setValue('NFT', null);
+ setValue('NFT', undefined);
- api.query.nfts.item.entries().then(async (nft) => {
+ try {
+ const nftEntries = await api.query.nfts.item.entries();
+ const myNFTs = nftEntries.filter(([ntfId, nftInfo]) => String(nftInfo.toPrimitive().deposit.account) === formatted).map(([ntfId, nftInfo]) => [ntfId.toHuman(), nftInfo.toPrimitive()]);
+ const nftDepositRequests = myNFTs.map(([nftId, nftInfo]) => api.query.nfts.itemMetadataOf(...nftId));
+ const nftDepositAmount = (await Promise.all(nftDepositRequests)).map((deposit) => deposit.toPrimitive().deposit.amount);
+ const totalNftDeposits = nftDepositAmount.reduce((acc, deposit) => acc.add(amountToMachine(String(deposit / (10 ** decimal)), decimal)), BN_ZERO);
+ const myNFTDeposits = myNFTs.map(([ntfId, nftInfo]) => nftInfo.deposit.amount as number);
+ const totalNFTDeposit = myNFTDeposits.reduce((acc, deposit) => acc.add(amountToMachine(String(deposit / (10 ** decimal)), decimal)), BN_ZERO);
+ const finalNFTDeposit = totalNFTDeposit.add(totalNftDeposits);
+ setValue('NFT', finalNFTDeposit);
+ } catch (error) {
+ console.error('Error fetching NFTs:', error);
+ setValue('NFT', null);
+ }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.
| if (api.query?.nfts) { | |
| setValue('NFT', null); | |
| api.query.nfts.item.entries().then(async (nft) => { | |
| const myNFTs = nft.filter(([ntfId, nftInfo]) => String(nftInfo.toPrimitive().deposit.account) === formatted).map(([ntfId, nftInfo]) => [ntfId.toHuman(), nftInfo.toPrimitive()]); | |
| const nftDepositRequests = myNFTs.map(([nftId, nftInfo]) => api.query.nfts.itemMetadataOf(...nftId)); | |
| const nftDepositAmount = (await Promise.all(nftDepositRequests)).map((deposit) => deposit.toPrimitive().deposit.amount); | |
| const totalNftDeposits = nftDepositAmount.reduce((acc, deposit) => { | |
| const depositAsBn = amountToMachine(String(deposit / (10 ** decimal)), decimal); | |
| return acc.add(depositAsBn); | |
| }, BN_ZERO); | |
| const myNFTDeposits = myNFTs.map(([ntfId, nftInfo]) => nftInfo.deposit.amount as number); | |
| const totalNFTDeposit = myNFTDeposits.reduce((acc, deposit) => { | |
| const depositAsBn = amountToMachine(String(deposit / (10 ** decimal)), decimal); | |
| return acc.add(depositAsBn); | |
| }, BN_ZERO); | |
| const finalNFTDeposit = totalNFTDeposit.add(totalNftDeposits); | |
| setValue('NFT', finalNFTDeposit); | |
| }).catch((error) => { | |
| console.error(error); | |
| setValue('NFT', null); | |
| }); | |
| } else { | |
| setValue('NFT', null); | |
| } | |
| if (api.query?.nfts) { | |
| setValue('NFT', undefined); | |
| try { | |
| const nftEntries = await api.query.nfts.item.entries(); | |
| const myNFTs = nftEntries.filter(([ntfId, nftInfo]) => String(nftInfo.toPrimitive().deposit.account) === formatted).map(([ntfId, nftInfo]) => [ntfId.toHuman(), nftInfo.toPrimitive()]); | |
| const nftDepositRequests = myNFTs.map(([nftId, nftInfo]) => api.query.nfts.itemMetadataOf(...nftId)); | |
| const nftDepositAmount = (await Promise.all(nftDepositRequests)).map((deposit) => deposit.toPrimitive().deposit.amount); | |
| const totalNftDeposits = nftDepositAmount.reduce((acc, deposit) => acc.add(amountToMachine(String(deposit / (10 ** decimal)), decimal)), BN_ZERO); | |
| const myNFTDeposits = myNFTs.map(([ntfId, nftInfo]) => nftInfo.deposit.amount as number); | |
| const totalNFTDeposit = myNFTDeposits.reduce((acc, deposit) => acc.add(amountToMachine(String(deposit / (10 ** decimal)), decimal)), BN_ZERO); | |
| const finalNFTDeposit = totalNFTDeposit.add(totalNftDeposits); | |
| setValue('NFT', finalNFTDeposit); | |
| } catch (error) { | |
| console.error('Error fetching NFTs:', error); | |
| setValue('NFT', null); | |
| } | |
| } else { | |
| setValue('NFT', null); | |
| } |
| api.query.assets.asset.entries().then(async (assets) => { | ||
| const myAssets = assets.filter((asset) => asset[1].toHuman()['owner'] === formatted); | ||
| const myAssetsId = myAssets.map(([assetId, assetInfo]) => String(assetId.toHuman()[0]).replaceAll(',', '')); | ||
| const assetDeposit = api.consts.assets.assetDeposit as BN; | ||
|
|
||
| const myAssetsMetadata = await Promise.all(myAssetsId.map((assetId) => api.query.assets.metadata(assetId))); | ||
|
|
||
| const totalAssetDeposit = myAssetsMetadata.reduce((acc, metadata) => { | ||
| const metaDeposit = metadata.deposit as BN; | ||
|
|
||
| return acc.add(metaDeposit); | ||
| }, BN_ZERO); | ||
|
|
||
| const finalDeposit = totalAssetDeposit.add(assetDeposit.muln(myAssetsId.length)); | ||
|
|
||
| setValue('assets', finalDeposit); | ||
| }).catch((error) => { | ||
| console.error(error); | ||
| setValue('assets', null); | ||
| }); | ||
| } else { |
There was a problem hiding this comment.
Optimize asset handling logic in the assets section.
The logic for handling asset details can be streamlined to reduce complexity and improve performance.
- const myAssetsMetadata = await Promise.all(myAssetsId.map((assetId) => api.query.assets.metadata(assetId)));
- const totalAssetDeposit = myAssetsMetadata.reduce((acc, metadata) => {
- const metaDeposit = metadata.deposit as BN;
- return acc.add(metaDeposit);
- }, BN_ZERO);
- const finalDeposit = totalAssetDeposit.add(assetDeposit.muln(myAssetsId.length));
+ const totalAssetDeposit = await myAssetsId.reduce(async (accPromise, assetId) => {
+ const acc = await accPromise;
+ const metadata = await api.query.assets.metadata(assetId);
+ return acc.add(metadata.deposit as BN);
+ }, Promise.resolve(BN_ZERO));
+ const finalDeposit = totalAssetDeposit.add(assetDeposit.muln(myAssetsId.length));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.
| api.query.assets.asset.entries().then(async (assets) => { | |
| const myAssets = assets.filter((asset) => asset[1].toHuman()['owner'] === formatted); | |
| const myAssetsId = myAssets.map(([assetId, assetInfo]) => String(assetId.toHuman()[0]).replaceAll(',', '')); | |
| const assetDeposit = api.consts.assets.assetDeposit as BN; | |
| const myAssetsMetadata = await Promise.all(myAssetsId.map((assetId) => api.query.assets.metadata(assetId))); | |
| const totalAssetDeposit = myAssetsMetadata.reduce((acc, metadata) => { | |
| const metaDeposit = metadata.deposit as BN; | |
| return acc.add(metaDeposit); | |
| }, BN_ZERO); | |
| const finalDeposit = totalAssetDeposit.add(assetDeposit.muln(myAssetsId.length)); | |
| setValue('assets', finalDeposit); | |
| }).catch((error) => { | |
| console.error(error); | |
| setValue('assets', null); | |
| }); | |
| } else { | |
| api.query.assets.asset.entries().then(async (assets) => { | |
| const myAssets = assets.filter((asset) => asset[1].toHuman()['owner'] === formatted); | |
| const myAssetsId = myAssets.map(([assetId, assetInfo]) => String(assetId.toHuman()[0]).replaceAll(',', '')); | |
| const assetDeposit = api.consts.assets.assetDeposit as BN; | |
| const totalAssetDeposit = await myAssetsId.reduce(async (accPromise, assetId) => { | |
| const acc = await accPromise; | |
| const metadata = await api.query.assets.metadata(assetId); | |
| return acc.add(metadata.deposit as BN); | |
| }, Promise.resolve(BN_ZERO)); | |
| const finalDeposit = totalAssetDeposit.add(assetDeposit.muln(myAssetsId.length)); | |
| setValue('assets', finalDeposit); | |
| }).catch((error) => { | |
| console.error(error); | |
| setValue('assets', null); | |
| }); | |
| } else { |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (5 hunks)
- packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx
Additional context used
Learnings (1)
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (1)
Learnt from: AMIRKHANEF PR: PolkaGate/extension#1394 File: packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx:0-0 Timestamp: 2024-07-01T09:27:24.482Z Learning: When checking for NaN values, always use `Number.isNaN` instead of `isNaN` to avoid unexpected type coercions.
Additional comments not posted (4)
packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsx (4)
13-13: Ensure the imported functionisOnRelayChainis necessary.Verify that
isOnRelayChainis used in the code to avoid unnecessary imports.Verification successful
The imported function
isOnRelayChainis necessary and is used in the code.
- The function is used in the
onClickhandler within the file.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of `isOnRelayChain` in the file. # Test: Search for `isOnRelayChain` usage. Expect: At least one usage. rg --type js 'isOnRelayChain' packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsxLength of output: 314
129-129: Simplify theuseMemologic fornotOnNativeAsset.The
useMemologic fornotOnNativeAssetcan be simplified by removing the redundanttypeofcheck.- const notOnNativeAsset = useMemo(() => (assetId !== undefined && assetId > 0) || assetToken?.toLowerCase() !== token?.toLowerCase() , [assetId, assetToken, token]); + const notOnNativeAsset = useMemo(() => assetId !== undefined && assetId > 0, [assetId, assetToken, token]);Likely invalid or redundant comment.
9-9: Ensure all imports are necessary.Verify that
useMemoanduseCallbackare indeed used in the code to avoid unnecessary imports.Verification successful
Imports are necessary and used in the code.
Both
useMemoanduseCallbackare utilized inReservedDisplayBalance.tsx, validating their presence in the import statement.
useMemo: Lines showing usage.useCallback: Lines showing usage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of `useMemo` and `useCallback` in the file. # Test: Search for `useMemo` and `useCallback` usage. Expect: At least one usage for each. rg --type js 'useMemo|useCallback' packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsxLength of output: 470
22-25: Ensure the new propertiesassetIdandassetTokenare used appropriately.Verify that
assetIdandassetTokenare used correctly in the code to ensure they serve their intended purpose.Verification successful
The properties
assetIdandassetTokenare used appropriately in theReservedDisplayBalancecomponent.
assetIdandassetTokenare included in the function signature ofReservedDisplayBalance.- They are used in a
useMemohook to determine the value ofnotOnNativeAsset.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of `assetId` and `assetToken` in the file. # Test: Search for `assetId` and `assetToken` usage. Expect: At least one usage for each. rg --type js 'assetId|assetToken' packages/extension-polkagate/src/fullscreen/accountDetails/components/ReservedDisplayBalance.tsxLength of output: 539
| export default function ReservedDisplayBalance({ address, amount, assetToken, assetId, disabled, price }: Props): React.ReactElement { | ||
| const { t } = useTranslation(); | ||
| const reservedDetails = useReservedDetails(address); | ||
| const { decimal, genesisHash, token } = useInfo(address); | ||
|
|
||
| const notOnNativeAsset = useMemo(() => (assetId !== undefined && assetId > 0) || assetToken?.toLowerCase() !== token?.toLowerCase() , [assetId, assetToken, token]); | ||
|
|
||
| const [showReservedDetails, setShowReservedDetails] = useState<boolean>(false); | ||
|
|
||
| useEffect(() => { | ||
| setShowReservedDetails(false); // to reset collapsed area on chain change | ||
| }, [genesisHash]); | ||
| }, [address, genesisHash, assetId]); |
There was a problem hiding this comment.
Ensure proper handling of undefined addresses.
The useReservedDetails hook is called with address cast to a string without checking if address is defined. This could lead to runtime errors if address is undefined.
- const reservedDetails = useReservedDetails(address);
+ const reservedDetails = useReservedDetails(address ?? '');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.
| export default function ReservedDisplayBalance({ address, amount, assetToken, assetId, disabled, price }: Props): React.ReactElement { | |
| const { t } = useTranslation(); | |
| const reservedDetails = useReservedDetails(address); | |
| const { decimal, genesisHash, token } = useInfo(address); | |
| const notOnNativeAsset = useMemo(() => (assetId !== undefined && assetId > 0) || assetToken?.toLowerCase() !== token?.toLowerCase() , [assetId, assetToken, token]); | |
| const [showReservedDetails, setShowReservedDetails] = useState<boolean>(false); | |
| useEffect(() => { | |
| setShowReservedDetails(false); // to reset collapsed area on chain change | |
| }, [genesisHash]); | |
| }, [address, genesisHash, assetId]); | |
| export default function ReservedDisplayBalance({ address, amount, assetToken, assetId, disabled, price }: Props): React.ReactElement { | |
| const { t } = useTranslation(); | |
| const reservedDetails = useReservedDetails(address ?? ''); | |
| const { decimal, genesisHash, token } = useInfo(address); | |
| const notOnNativeAsset = useMemo(() => (assetId !== undefined && assetId > 0) || assetToken?.toLowerCase() !== token?.toLowerCase() , [assetId, assetToken, token]); | |
| const [showReservedDetails, setShowReservedDetails] = useState<boolean>(false); | |
| useEffect(() => { | |
| setShowReservedDetails(false); // to reset collapsed area on chain change | |
| }, [address, genesisHash, assetId]); |
| const reasonsToShow = useMemo(() => { | ||
| const details = Object.values(reservedDetails); | ||
|
|
||
| if (details.length === 0) { | ||
| return undefined | ||
| } | ||
|
|
||
| const noReason = details.every((deposit) => deposit === null); | ||
|
|
||
| if (noReason) { | ||
| return null; | ||
| } | ||
|
|
||
| const filteredReservedDetails = Object.fromEntries( | ||
| Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero()) | ||
| ); | ||
|
|
||
| return Object.values(filteredReservedDetails).length > 0 | ||
| ? filteredReservedDetails | ||
| : undefined | ||
| }, [reservedDetails]); |
There was a problem hiding this comment.
Optimize the conditional logic in useMemo.
The nested conditional logic can be simplified to improve readability and maintainability.
- if (details.length === 0) {
- return undefined;
- }
- const noReason = details.every((deposit) => deposit === null);
- if (noReason) {
- return null;
- }
- const filteredReservedDetails = Object.fromEntries(
- Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero())
- );
- return Object.values(filteredReservedDetails).length > 0 ? filteredReservedDetails : undefined;
+ if (details.length === 0) return undefined;
+ if (details.every((deposit) => deposit === null)) return null;
+ const filteredReservedDetails = Object.fromEntries(
+ Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero())
+ );
+ return Object.values(filteredReservedDetails).length > 0 ? filteredReservedDetails : undefined;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 reasonsToShow = useMemo(() => { | |
| const details = Object.values(reservedDetails); | |
| if (details.length === 0) { | |
| return undefined | |
| } | |
| const noReason = details.every((deposit) => deposit === null); | |
| if (noReason) { | |
| return null; | |
| } | |
| const filteredReservedDetails = Object.fromEntries( | |
| Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero()) | |
| ); | |
| return Object.values(filteredReservedDetails).length > 0 | |
| ? filteredReservedDetails | |
| : undefined | |
| }, [reservedDetails]); | |
| const reasonsToShow = useMemo(() => { | |
| const details = Object.values(reservedDetails); | |
| if (details.length === 0) return undefined; | |
| if (details.every((deposit) => deposit === null)) return null; | |
| const filteredReservedDetails = Object.fromEntries( | |
| Object.entries(reservedDetails).filter(([_key, value]) => value && !value.isZero()) | |
| ); | |
| return Object.values(filteredReservedDetails).length > 0 | |
| ? filteredReservedDetails | |
| : undefined; | |
| }, [reservedDetails]); |
|
I was on Westend, and then switch to People chain: 
|
AMIRKHANEF
force-pushed
the
AssetHubsReservedAmount
branch
from
579ba40
to
f734a4a
Compare
…EF/polkagate-extension into AssetHubsReservedAmount
Works Done
Summary by CodeRabbit
New Features
ReservedDetailscomponent to filter and display reasons dynamically.Improvements
useReservedDetailshook to manage state and handle API queries for multiple entities.AccountDetailsto include new asset-related properties for more detailed information display.