-
Notifications
You must be signed in to change notification settings - Fork 399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix/wallet issues OK-34910 OK-34875 OK-34849 #6491
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning eslint@8.57.1: This version is no longer supported. Please see https://eslint.org/version-support for other options. WalkthroughThe pull request introduces enhancements to transaction display and processing components across multiple files. The changes focus on improving asset rendering, transaction data parsing, and component flexibility. Key modifications include adding support for internal assets, refining transaction component types, and updating the rendering logic to handle different asset types more effectively. Changes
Assessment against linked issues
Finishing Touches
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
packages/kit/src/views/SignatureConfirm/components/SignatureConfirmAdvanced/TxAdvancedSettings.tsx
(4 hunks)packages/kit/src/views/SignatureConfirm/components/SignatureConfirmComponents/Assets.tsx
(8 hunks)packages/kit/src/views/SignatureConfirm/components/SignatureConfirmDataViewer/DataViewer.tsx
(2 hunks)packages/kit/src/views/SignatureConfirm/components/SignatureConfirmDetails/SignatureConfirmDetails.tsx
(3 hunks)packages/shared/src/utils/txActionUtils.ts
(3 hunks)packages/shared/types/signatureConfirm.ts
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint (20.x)
🔇 Additional comments (16)
packages/kit/src/views/SignatureConfirm/components/SignatureConfirmDetails/SignatureConfirmDetails.tsx (3)
9-12
: Imported types are correctly added.The addition of
IDisplayComponentAssets
to the imports is appropriate.
66-113
: Assets merging logic is implemented correctly.The code merges token, NFT, and internal asset components based on labels when
isLocalParsed
is true. The logic is sound.
152-159
: Added rendering for internal assets component.The new case for
EParseTxComponentType.InternalAssets
correctly handles internal assets rendering.packages/shared/types/signatureConfirm.ts (4)
16-16
: IncludedInternalAssets
in component types enum.Adding
InternalAssets
toEParseTxComponentType
expands the component types appropriately.
72-77
: Updatedassets
property inIDisplayComponentAssets
.The
assets
array now accommodatesIDisplayComponentInternalAssets
along with NFT and token components.
Line range hint
79-89
: DefinedIDisplayComponentInternalAssets
interface.The new interface correctly describes internal assets components.
116-116
: ExtendedIDisplayComponent
union type.Including
IDisplayComponentInternalAssets
ensures internal assets are properly recognized.packages/kit/src/views/SignatureConfirm/components/SignatureConfirmComponents/Assets.tsx (5)
39-39
: AddedhideLabel
prop to common asset properties.This enhances component flexibility.
Line range hint
76-88
: UpdatedSignatureAssetDetailItem
to accepthideLabel
.Allowing conditional display of labels improves UI control.
Line range hint
263-269
: ImplementedAssetsInternalAssets
component.The new component properly renders internal assets.
281-313
: ModifiedAssets
function to handle multiple asset types.The function now correctly maps and renders different asset components based on their type.
319-319
: ExportedAssets.InternalAssets
correctly.This ensures the
AssetsInternalAssets
component is accessible where needed.packages/kit/src/views/SignatureConfirm/components/SignatureConfirmAdvanced/TxAdvancedSettings.tsx (2)
24-24
: LGTM: Added fee info atom importClean addition of the fee information atom for transaction processing.
284-284
: Good defensive programming with null coalescingSmart use of the null coalescing operator to prevent undefined data in the viewer.
packages/shared/src/utils/txActionUtils.ts (1)
277-278
: Type safety improvement for asset componentsGood conversion to more specific internal asset types. This improves type safety and makes the code more maintainable.
Also applies to: 301-302
packages/kit/src/views/SignatureConfirm/components/SignatureConfirmDataViewer/DataViewer.tsx (1)
12-21
: Enhanced layout with proper spacingGood addition of Stack wrapper with bottom padding. This improves readability and component spacing.
Summary by CodeRabbit
New Features
Improvements
Technical Updates