Feat/add asset action #675
Conversation
WalkthroughThe changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AssetDetailView
participant AssetDetailAction
participant NetworkAssetsItem
participant NetworkAssets
User->>AssetDetailAction: Clicks "Buy/Sell" button
AssetDetailAction-->>AssetDetailView: Emits open:buy-action (token)
AssetDetailView-->>NetworkAssetsItem: Emits open:buy-action (token)
NetworkAssetsItem-->>NetworkAssets: Emits open:buy-action (token)
sequenceDiagram
participant Route
participant SendTransactionView
participant AssetFetcher
Route->>SendTransactionView: Navigates with tokenParamData
SendTransactionView->>AssetFetcher: fetchAssets()
AssetFetcher-->>SendTransactionView: Returns asset list
SendTransactionView->>SendTransactionView: Selects asset matching tokenParamData.contract (if present), else first asset
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
… into feat/add-asset-action
|
💼 Build Files |
![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
🧹 Nitpick comments (7)
packages/extension/src/providers/ethereum/ui/send-transaction/index.vue (1)
130-130: Unused lodash importThe
hasfunction is imported from lodash but doesn't appear to be used in this file.-import { debounce, has } from 'lodash'; +import { debounce } from 'lodash';packages/extension/src/ui/action/views/asset-detail-view/components/asset-detail-action.vue (2)
46-49: Inconsistent event naming conventionThere's a naming inconsistency between the event emitted here (
open:buyAction- camelCase) and how it's listened for in the parent component (open:buy-action- kebab-case). Vue is case-insensitive for template event listeners, but consistent naming would improve code quality.defineEmits<{ (e: 'toggle:deposit'): void; - (e: 'open:buyAction'): void; + (e: 'open:buy-action'): void; }>();And in the template:
<button class="asset-detail__action-item" - @click="$emit('open:buyAction')" + @click="$emit('open:buy-action')" >
47-47: Unused event definitionThe
toggle:depositevent is defined but never emitted in this component. Consider removing it if it's not needed or add documentation about its intended use.defineEmits<{ - (e: 'toggle:deposit'): void; (e: 'open:buyAction'): void; }>();packages/extension/src/ui/onboard/restore-wallet/backup-detected.vue (1)
192-206: Leverage theme variables for background color
Hardcodingbackground: white;may break dark mode or future theme overrides. Use the@whiteor a semantic color token to keep styling consistent.- background: white; + background: @white;packages/hw-wallets/package.json (1)
39-39: Ensure consistent version spec fortypescript-eslint
You’ve pinned"typescript-eslint": "8.30.1"exactly while the rest use caret ranges. For consistency and easier patch upgrades, consider changing it to^8.30.1.- "typescript-eslint": "8.30.1", + "typescript-eslint": "^8.30.1",packages/extension/package.json (1)
55-55: Use caret for@solana-developers/helpers
The version for@solana-developers/helpersis pinned exactly ("2.8.0"). For consistency with other dependencies, consider switching to a caret range ("^2.8.0").- "@solana-developers/helpers": "2.8.0", + "@solana-developers/helpers": "^2.8.0",packages/signers/bitcoin/package.json (1)
35-38: Ensure ESLint & Prettier plugin upgrades remain compatibleThe bumps to
@types/node,@typescript-eslint/eslint-plugin,@typescript-eslint/parser,eslint, andeslint-config-prettiermay introduce new linting rules or deprecations. Please run the lint pipeline and address any new violations.
Additionally, consider consolidating shared devDependencies at the root workspace to keep versions in sync across all signer packages.Also applies to: 40-40
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (26)
package.json(1 hunks)packages/extension-bridge/package.json(1 hunks)packages/extension/package.json(4 hunks)packages/extension/src/libs/rate-state/index.ts(2 hunks)packages/extension/src/providers/ethereum/ui/send-transaction/index.vue(4 hunks)packages/extension/src/providers/solana/ui/send-transaction/index.vue(3 hunks)packages/extension/src/ui/action/views/accounts/components/rename-account-form.vue(2 hunks)packages/extension/src/ui/action/views/asset-detail-view/components/asset-detail-action.vue(1 hunks)packages/extension/src/ui/action/views/asset-detail-view/index.vue(3 hunks)packages/extension/src/ui/action/views/network-assets/components/network-assets-item.vue(2 hunks)packages/extension/src/ui/action/views/network-assets/index.vue(1 hunks)packages/extension/src/ui/action/views/network-nfts/components/network-nfts-category.vue(1 hunks)packages/extension/src/ui/onboard/App.vue(1 hunks)packages/extension/src/ui/onboard/restore-wallet/backup-detected.vue(3 hunks)packages/hw-wallets/package.json(2 hunks)packages/keyring/package.json(1 hunks)packages/name-resolution/package.json(1 hunks)packages/request/package.json(1 hunks)packages/signers/bitcoin/package.json(2 hunks)packages/signers/ethereum/package.json(1 hunks)packages/signers/kadena/package.json(1 hunks)packages/signers/polkadot/package.json(1 hunks)packages/storage/package.json(1 hunks)packages/swap/package.json(1 hunks)packages/types/package.json(1 hunks)packages/utils/package.json(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: buildAll
🔇 Additional comments (42)
packages/extension/src/ui/action/views/network-nfts/components/network-nfts-category.vue (2)
7-7: Updated tooltip positioning approach.The
is-top-rightprop has been replaced withteleport-to-app, changing how the tooltip is positioned and rendered. Theteleport-to-appprop moves the tooltip to the app's root level in the DOM, which helps prevent z-index or overflow issues that might occur with nested positioning.
10-10: Reduced collection name truncation length.The truncation length for collection names has been reduced from 40 to 25 characters, making tooltips more concise. This change makes sense considering the limited UI space in the extension popup.
packages/extension/src/ui/action/views/network-assets/index.vue (1)
28-28: Added attribute inheritance to network-assets-item components.Adding
v-bind="$attrs"propagates any attributes (including event listeners) from the parent component to eachnetwork-assets-iteminstance. This change enables the newopen:buy-actionevents to properly flow through the component hierarchy.packages/extension/src/ui/action/views/network-assets/components/network-assets-item.vue (3)
58-58: Added event listener for buy/sell action.The
@open:buy-action="openBuySell"listener connects the asset detail view's buy/sell action to the parent component's event handling system.
164-164: Added event type declaration for buy/sell action.The
defineEmitsdeclaration now includes the newopen:buy-actionevent with the appropriate token payload type, ensuring proper type safety when emitting this event.
167-169: Implemented openBuySell event handler.The new
openBuySellmethod propagates the buy/sell action to parent components while passing the current token data. This method completes the event chain for the new asset action feature.packages/extension/src/providers/solana/ui/send-transaction/index.vue (3)
75-75: Updated NFT parameter name for consistency.Changed attribute name from
paramNFTDatatotokenParamDatafor improved naming consistency across the codebase.
206-208: Renamed variable for better semantic meaning.The variable for NFT data from route parameters was renamed from
paramNFTDatatotokenParamData, making it clearer that this parameter can apply to both NFTs and tokens.
451-462: Enhanced token selection logic with route parameter support.Added conditional logic to select the appropriate token based on route parameters. When sending a token, the component now tries to match the selected asset with the token passed in the route parameters, improving the user experience by maintaining context when navigating between views.
packages/extension/src/ui/action/views/asset-detail-view/index.vue (4)
44-55: UI Enhancement - Token actions added conditionallyThe token balance section and new asset-detail-action component are now correctly displayed only for non-custom tokens, providing a cleaner UI with appropriate actions for each token type.
84-84: Import for new component looks goodAppropriate import of the AssetDetailAction component.
145-145: New event signature addedThe TypeScript emit definition now correctly includes the new
open:buy-actionevent with token payload.
148-151: Event handler implementationThe
openBuySellmethod correctly passes the token prop to parent components and closes the popup after emitting the event.packages/extension/src/providers/ethereum/ui/send-transaction/index.vue (3)
207-209: Variable renamed for clarityRenamed from
paramNFTDatatotokenParamData, providing better context as this variable is used for both tokens and NFTs.
548-559: Enhanced token selection logicThis change improves user experience by intelligently selecting the appropriate token from assets based on the token contract address passed in route parameters, rather than always defaulting to the first token.
80-80: Consistent naming in templateThe variable has been consistently renamed in both script and template sections.
packages/extension/src/ui/action/views/asset-detail-view/components/asset-detail-action.vue (2)
1-34: Well-structured action component templateThe template provides a good layout with three clear actions (Buy/Sell, Send, Swap) that are visually consistent and properly connected to their respective actions.
59-129: Well-designed component stylingThe styling is clean and consistent, with appropriate hover states and accessibility considerations like focus outlines. The layout adapts well with flex properties.
packages/extension/src/ui/onboard/App.vue (1)
112-112: UI adjustment looks goodIncreasing the height of the onboard container from 600px to 650px should provide more vertical space to accommodate content comfortably.
package.json (1)
37-37: Dependency update looks goodUpdating @swc/core to a newer version is part of the broader development dependency updates across the project.
packages/extension/src/libs/rate-state/index.ts (2)
5-5: Good UX improvement for rating popupsIncreasing the popup interval from 14 days to 30 days provides a better user experience by making the rating prompts less frequent.
35-35: Helpful code commentThe added comment clarifies when popup time and new state are set, improving code readability.
packages/signers/ethereum/package.json (1)
35-50: Development dependency updates look goodThe updates to TypeScript, ESLint, and testing-related packages are consistent with the broader updates across the monorepo and keep development tooling current.
packages/extension/src/ui/onboard/restore-wallet/backup-detected.vue (1)
181-181: Spacing adjustment on<h3>margin looks correct
Reducing the bottom margin from 24px to 12px tightens the vertical rhythm in this view and matches the updated design spec.packages/storage/package.json (1)
30-35: DevDependencies bumped to align with monorepo standards
The updates to@types/node,@typescript-eslint,eslint,typescript, andvitestmirror changes in other packages. No breaking changes expected since these are development tools.Also applies to: 43-45
packages/keyring/package.json (1)
32-32: Upgrade of@polkadot/utiland devDependencies
Bumping@polkadot/utilto^13.4.4and synchronizing ESLint/TypeScript tooling keeps consistency. No runtime API changes in patch releases, but please re-run the keyring test suite to confirm.Also applies to: 37-40, 50-52
packages/name-resolution/package.json (1)
25-31:Details
❓ Verification inconclusive
Development dependencies updated for linting and testing
Moving to@typescript-eslintv8.x and ESLint v9.x witheslint-config-prettier@10aligns with the broader monorepo. Verify that existing lint rules and formatting still work as expected.Run:
Also applies to: 38-41
🏁 Script executed:
#!/bin/bash npm install npm run lint npm run testLength of output: 7403
Development dependencies updated for linting and testing
Moving to@typescript-eslintv8.x and ESLint v9.x witheslint-config-prettier@10aligns with the broader monorepo. Automatednpm install && npm run lint && npm run testin this sandbox failed to bootstrap workspace dependencies (unsupported “workspace:” protocol). Please manually verify that all lint rules and formatting still pass:Run:
# At the repo root yarn install yarn workspaces foreach run lint yarn workspaces foreach run testAlso applies to lines 38–41.
packages/extension/src/ui/action/views/accounts/components/rename-account-form.vue (1)
57-57: Import for BackupState addedGood addition of
BackupStateimport to support the newly added backup functionality after renaming accounts.packages/request/package.json (1)
34-38: Updated development dependenciesAll the updated development dependencies look good. This package follows the consistent pattern of dependency upgrades across the repository.
Key updates include:
- ESLint plugins and parsers from 5.x to 8.x
- TypeScript from 5.8.2 to 5.8.3
- Vitest from 3.0.8 to 3.1.1
Also applies to: 39-39, 47-49
packages/types/package.json (1)
27-33: Updated development dependenciesThe dependency updates look good and follow the consistent pattern across the repository's packages.
Key updates include:
- TypeScript ESLint plugins from 5.x to 8.x
- ESLint from 9.22.0 to 9.24.0
- Prettier config from 9.1.0 to 10.1.2
Also applies to: 40-41
packages/signers/polkadot/package.json (2)
27-28: Updated Polkadot runtime dependenciesGood update of the Polkadot utility packages from 13.4.3 to 13.4.4. This minor version bump should maintain backward compatibility.
34-39: Updated development dependenciesThe development dependency updates look good and follow the same pattern as other packages in the repository.
Key updates include:
- TypeScript ESLint plugins from 5.x to 8.x
- Node types from 22.13.10 to 22.14.1
- Vitest from 3.0.8 to 3.1.1
Also applies to: 40-40, 47-49
packages/hw-wallets/package.json (2)
25-28: DevDependencies upgraded to TS 5.8, ESLint 9 and vitest 3
You've bumped key dev tools: TypeScript (^5.8.3), ESLint (^9.24.0), @typescript-eslint (8.30.1), vitest (^3.1.1), etc. Please verify that your tsconfig, ESLint rules, Prettier settings, and CI pipelines are compatible with these versions to avoid unexpected linter or compile errors.Also applies to: 30-30, 38-40
55-57: Runtime libs bumped for Ledger, Polkadot & Trezor
You’ve upgraded @ledgerhq/hw-app-* to the latest minors and Polkadot/Trezor packages to ^15.9.2 / ^9.5.4. These libraries sometimes introduce breaking API changes—please run integration tests or dry‐run hardware wallet flows to confirm nothing regressed.Also applies to: 61-64
packages/extension/package.json (2)
26-26: Runtime dependencies bumped across analytics, Metaplex, Polkadot, Solana, & Pinia
Multiple core libraries have been upgraded (@amplitude/analytics-browser, @metaplex-foundation/, @polkadot/, @solana-developers/helpers, bignumber.js, pinia). Given potential breaking changes (especially in Polkadot v15, Metaplex v4 and Umi v1), please smoke-test all transaction flows, token-selection logic, UI components, and extension behaviors.Also applies to: 43-53, 55-55, 61-61, 76-76
100-103: Build & test toolchain upgrades—Vite, Rollup, TS, Vitest
You've updated @vitejs/plugin-vue (^5.2.3), vite (^6.3.1), rollup (^4.40.0), typescript (~5.8.3), vitest (^3.1.1), jsdom (^26.1.0), etc. Please rebuild the extension for all target browsers, run the full test suite, and validate that no new warnings or errors are introduced.Also applies to: 114-114, 118-118, 120-123, 129-129, 134-134, 136-136, 139-139
packages/swap/package.json (2)
29-29: Dependencies bumped: bignumber.js & rango-sdk-basic
You’ve upgraded bignumber.js to ^9.2.1 and rango-sdk-basic to ^0.1.65. Verify swap calculations and external API integrations still behave correctly with the new numeric precision and SDK version.Also applies to: 34-34
42-47: DevDependencies upgraded: TS, ESLint, vitest
The swap package’s tooling has been bumped—@types/node (^22.14.1), @typescript-eslint (8.30.1), eslint (^9.24.0), eslint-config-prettier (^10.1.2), typescript (^5.8.3), vitest (^3.1.1). Ensure linting, type-checking, and tests pass without new errors in this library.Also applies to: 55-57
packages/extension-bridge/package.json (1)
47-47: DevDependencies bumped for TS and linting
You’ve updated @types/node (^22.14.1), @typescript-eslint (8.30.1), eslint (^9.24.0), eslint-config-prettier (^10.1.2), type-fest (^4.40.0), typescript (^5.8.3). Though this package has no tests, please runnpm run lintand rebuild to verify no formatting or type errors are introduced.Also applies to: 49-50, 52-52, 54-54, 62-64
packages/utils/package.json (2)
27-27: Runtime bump: @polkadot/util-crypto
You upgraded@polkadot/util-cryptoto ^13.4.4. Please run any crypto-related utilities and ensure they still correctly derive keys, sign, and verify as expected.
33-36: DevDependencies bumped across TS, ESLint, Vitest
Utility package dev tools now include @types/node (^22.14.1), @typescript-eslint (8.30.1), eslint (^9.24.0), eslint-config-prettier (^10.1.2), typescript (^5.8.3), vitest (^3.1.1). Run the lint and test commands to confirm the upgrade is non-disruptive.Also applies to: 38-38, 47-49
packages/signers/bitcoin/package.json (1)
26-26:Details
❓ Verification inconclusive
Verify @noble/secp256k1 v1.7.2 for breaking changes
The patch update from v1.7.1 → v1.7.2 may contain fixes or minor behavior changes. Please review the official CHANGELOG or release notes to confirm this bump doesn’t introduce incompatibilities.
🏁 Script executed:
#!/bin/bash # Fetch and display the changelog entry for v1.7.2 curl -s https://raw.githubusercontent.com/paulmillr/noble-secp256k1/main/CHANGELOG.md \ | awk '/^## \[1\.7\.2\]/{flag=1;next}/^## \[/{flag=0}flag'Length of output: 146
🏁 Script executed:
#!/bin/bash echo "Searching for '1.7.2' in CHANGELOG.md:" curl -s https://raw.githubusercontent.com/paulmillr/noble-secp256k1/main/CHANGELOG.md \ | grep -n '1\.7\.2' echo echo "Fetching GitHub release info for v1.7.2:" curl -s https://api.github.com/repos/paulmillr/noble-secp256k1/releases/tags/v1.7.2Length of output: 532
Verify @noble/secp256k1 v1.7.2 release details
I ran automated checks against the upstream CHANGELOG and GitHub releases but found no entry or tag for v1.7.2. Please manually review the upstream commits or official release notes to ensure this bump doesn’t introduce any breaking changes before merging.
- Location: packages/signers/bitcoin/package.json (line 26)
|
|
||
| <div class="backup-detected__details"> | ||
| Enkrypt creates backups of generated user accounts, derivation paths, and | ||
| account names across all networks to make it easier to restore your | ||
| wallet. | ||
| <a | ||
| href="https://help.myetherwallet.com/en/articles/6437190-enkrypt-add-and-import-accounts-and-connect-a-hardware-wallet#h_62bd285c76" | ||
| target="_blank" | ||
| >Learn more</a | ||
| > | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add rel="noopener noreferrer" for external link security
Using target="_blank" without rel="noopener noreferrer" exposes window.opener and can be exploited for phishing or malicious redirects.
Apply this diff:
- <a
- href="https://help.myetherwallet.com/en/articles/6437190-enkrypt-add-and-import-accounts-and-connect-a-hardware-wallet#h_62bd285c76"
- target="_blank"
- >Learn more</a
- >
+ <a
+ href="https://help.myetherwallet.com/en/articles/6437190-enkrypt-add-and-import-accounts-and-connect-a-hardware-wallet#h_62bd285c76"
+ target="_blank"
+ rel="noopener noreferrer"
+ >Learn more</a
+ >📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div class="backup-detected__details"> | |
| Enkrypt creates backups of generated user accounts, derivation paths, and | |
| account names across all networks to make it easier to restore your | |
| wallet. | |
| <a | |
| href="https://help.myetherwallet.com/en/articles/6437190-enkrypt-add-and-import-accounts-and-connect-a-hardware-wallet#h_62bd285c76" | |
| target="_blank" | |
| >Learn more</a | |
| > | |
| </div> | |
| <div class="backup-detected__details"> | |
| Enkrypt creates backups of generated user accounts, derivation paths, and | |
| account names across all networks to make it easier to restore your | |
| wallet. | |
| <a | |
| href="https://help.myetherwallet.com/en/articles/6437190-enkrypt-add-and-import-accounts-and-connect-a-hardware-wallet#h_62bd285c76" | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| >Learn more</a | |
| > | |
| </div> |
| "@polkadot/util-crypto": "^13.4.4", | ||
| "@types/hdkey": "^2.1.0", | ||
| "@types/node": "^22.13.10", | ||
| "@typescript-eslint/eslint-plugin": "^5.62.0", | ||
| "@typescript-eslint/parser": "^5.62.0", | ||
| "eslint": "^9.22.0", | ||
| "@types/node": "^22.14.1", | ||
| "@typescript-eslint/eslint-plugin": "^8.30.1", | ||
| "@typescript-eslint/parser": "^8.30.1", | ||
| "eslint": "^9.24.0", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Patch bump for @polkadot/util-crypto and dev tools
Upgrading @polkadot/util-crypto to ^13.4.4 is a safe patch release. DevDependencies align with monorepo toolchain upgrades. Ensure tests around cryptographic utilities still pass after the bump.
Run:
Also applies to: 38-38, 46-48
🏁 Script executed:
#!/bin/bash
# Verify that all Kadena signer tests pass after bump
npm install
npm testLength of output: 4232
Update verification steps to use Yarn workspaces
The dependency bump itself is fine, but the proposed npm install && npm test fails in a workspace setup. Please update the verification commands as follows:
• File: packages/signers/kadena/package.json (lines 31–36)
Replace:
npm install
npm testWith:
yarn install
# To run all workspace tests:
yarn workspaces foreach --worktree -v --exclude packages/swap run test
# Or to test just the Kadena signer:
yarn workspace @enkryptcom/signer-kadena testEnsure all tests pass after bumping @polkadot/util-crypto to ^13.4.4.
| const backupState = new BackupState(); | ||
| backupState.backup(false).catch(() => { | ||
| console.error('Failed to backup'); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider awaiting backup operation completion
The backup operation is initiated after renaming but not awaited before closing the window. This could potentially lead to incomplete backups if the window is closed too quickly.
- const backupState = new BackupState();
- backupState.backup(false).catch(() => {
- console.error('Failed to backup');
- });
- closeWindow();
- emit('update:init');
+ const backupState = new BackupState();
+ isProcessing.value = true;
+ backupState.backup(false)
+ .then(() => {
+ closeWindow();
+ emit('update:init');
+ })
+ .catch((error) => {
+ console.error('Failed to backup', error);
+ })
+ .finally(() => {
+ isProcessing.value = false;
+ });📝 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 backupState = new BackupState(); | |
| backupState.backup(false).catch(() => { | |
| console.error('Failed to backup'); | |
| }); | |
| const backupState = new BackupState(); | |
| - backupState.backup(false).catch(() => { | |
| - console.error('Failed to backup'); | |
| - }); | |
| - closeWindow(); | |
| - emit('update:init'); | |
| + isProcessing.value = true; | |
| + backupState.backup(false) | |
| + .then(() => { | |
| + closeWindow(); | |
| + emit('update:init'); | |
| + }) | |
| + .catch((error) => { | |
| + console.error('Failed to backup', error); | |
| + }) | |
| + .finally(() => { | |
| + isProcessing.value = false; | |
| + }); |
Summary by CodeRabbit
New Features
Improvements
Style