Fix/copy #598
Fix/copy #598
Conversation
WalkthroughThis pull request introduces a comprehensive update to the Enkrypt browser extension, focusing on enhancing network management, tracking capabilities, and user interface improvements. The changes span multiple components and libraries, introducing new state management classes, updating metrics tracking, and refining the UI for network interactions. The version has been incremented from 2.0.2 to 2.0.3, indicating a minor release with significant feature additions and refinements. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI
participant NetworksState
participant MetricsTracker
participant RecentlySentAddresses
User->>UI: Interact with networks
UI->>NetworksState: Update network state
NetworksState-->>MetricsTracker: Track network changes
UI->>RecentlySentAddresses: Add recently sent address
RecentlySentAddresses-->>NetworksState: Update used features
MetricsTracker->>UI: Log interaction metrics
Possibly related PRs
Suggested reviewers
Finishing Touches
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
Documentation and Community
|
|
💼 Build Files |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 17
🔭 Outside diff range comments (1)
packages/extension/src/ui/action/views/add-network/components/add-network-search.vue (1)
Line range hint
24-41: Remove unused state and handlers.The
openListref andonClickOutsidehandler are no longer used since the list toggle functionality has been removed.-const openList = ref(false); const tooltip = ref(null); defineProps({ value: { type: String, default: () => { return ''; }, }, }); defineEmits<{ (e: 'action:customNetwork'): void; }>(); -onClickOutside(tooltip, () => { - if (openList.value) openList.value = false; -});
♻️ Duplicate comments (2)
packages/extension/src/providers/solana/ui/send-transaction/index.vue (1)
724-728: 🛠️ Refactor suggestionAdd error handling for recently sent addresses storage
Similar to other providers, add error handling to prevent storage failures from affecting the transaction flow.
packages/extension/src/providers/ethereum/ui/send-transaction/index.vue (1)
729-733: 🛠️ Refactor suggestionAdd error handling for recently sent addresses storage
Similar to other providers, add error handling to prevent storage failures from affecting the transaction flow.
🧹 Nitpick comments (55)
packages/extension/src/ui/action/icons/tabs/assets.vue (1)
12-14: LGTM! Consider adding ARIA labels.The SVG path changes look good and follow best practices. The semi-transparent black fill (0.9 opacity) will work well for contrast adaptability.
Consider adding an
aria-labelto improve accessibility:<svg width="28" height="28" viewBox="0 0 28 28" fill="none" xmlns="http://www.w3.org/2000/svg" + aria-label="Assets" + role="img" >packages/extension/src/ui/action/views/add-network/components/add-network-search.vue (2)
10-10: Great addition of aria-label for accessibility!The aria-label improves screen reader support. Consider also adding
role="button"since this is a clickable element styled as a button.<a @click="$emit('action:customNetwork')" - aria-label="Add Custom Network" + aria-label="Add Custom Network" + role="button" >
Line range hint
1-41: Consider extracting network-related logic to a composable.The component has been simplified, which is good. However, to improve maintainability and reusability, consider extracting network-related logic (like the customNetwork action) into a dedicated composable function (e.g.,
useNetworkManagement). This would make it easier to share network management functionality across components.packages/extension/src/ui/action/icons/actions/sort/sort-active-icon.vue (2)
2-16: Enhance SVG accessibility and maintainability.Consider the following improvements:
- Add ARIA attributes for better accessibility
- Move the color value to CSS
- Make dimensions configurable via props
<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg" + role="img" + aria-label="Sort active indicator" :class="{ active: isActive }" > <path fill-rule="evenodd" clip-rule="evenodd" d="M20.1876 4.00007C20.7399 4.41429 20.8518 5.19779 20.4376 5.75007L9.93757 19.7501C9.70151 20.0648 9.33102 20.2501 8.93757 20.2501C8.54413 20.2501 8.17364 20.0648 7.93757 19.7501L2.68757 12.7501C2.27336 12.1978 2.38529 11.4143 2.93757 11.0001C3.48986 10.5859 4.27336 10.6978 4.68757 11.2501L8.93757 16.9167L18.4376 4.25007C18.8518 3.69779 19.6353 3.58586 20.1876 4.00007Z" - fill="#684CFF" + class="icon-path" /> </svg>
27-37: Enhance styles implementation.Consider the following improvements to the styles:
- Add the icon color in CSS
- Be more specific about transition properties
- Consider adding hover state styles
<style scoped> svg { display: inline-block; vertical-align: baseline; opacity: 0; - transition: opacity 0.3s ease-in; + transition-property: opacity; + transition-duration: 0.3s; + transition-timing-function: ease-in; } + +.icon-path { + fill: #684CFF; +} + .active { opacity: 1; } + +svg:hover .icon-path { + fill: #5940d9; +} </style>packages/extension/src/ui/action/icons/common/plus-icon.vue (2)
2-8: Add accessibility attributes to SVG.Consider enhancing accessibility by adding ARIA attributes and a title element for screen readers.
<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg" + role="img" + aria-label="Add" > + <title>Add</title>
9-15: Use CSS variables for better theme compatibility.Consider replacing the hardcoded fill color and opacity with CSS variables to support different themes.
<path fill-rule="evenodd" clip-rule="evenodd" d="M12 22C17.5228 22 22 17.5228 22 12C22 6.47715 17.5228 2 12 2C6.47715 2 2 6.47715 2 12C2 17.5228 6.47715 22 12 22ZM10.9189 7.96C10.9189 7.62397 10.9189 7.45595 10.9843 7.32761C11.0419 7.21471 11.1337 7.12292 11.2466 7.0654C11.3749 7 11.5429 7 11.8789 7H11.9589C12.295 7 12.463 7 12.5913 7.0654C12.7042 7.12292 12.796 7.21471 12.8535 7.32761C12.9189 7.45595 12.9189 7.62397 12.9189 7.96V11H15.9589C16.295 11 16.463 11 16.5913 11.0654C16.7042 11.1229 16.796 11.2147 16.8535 11.3276C16.9189 11.456 16.9189 11.624 16.9189 11.96V12.04C16.9189 12.376 16.9189 12.544 16.8535 12.6724C16.796 12.7853 16.7042 12.8771 16.5913 12.9346C16.463 13 16.295 13 15.9589 13H12.9189V16.04C12.9189 16.376 12.9189 16.544 12.8535 16.6724C12.796 16.7853 12.7042 16.8771 12.5913 16.9346C12.463 17 12.295 17 11.9589 17H11.8789C11.5429 17 11.3749 17 11.2466 16.9346C11.1337 16.8771 11.0419 16.7853 10.9843 16.6724C10.9189 16.544 10.9189 16.376 10.9189 16.04V13H7.87895C7.54291 13 7.3749 13 7.24655 12.9346C7.13365 12.8771 7.04187 12.7853 6.98434 12.6724C6.91895 12.544 6.91895 12.376 6.91895 12.04V11.96C6.91895 11.624 6.91895 11.456 6.98434 11.3276C7.04187 11.2147 7.13365 11.1229 7.24655 11.0654C7.3749 11 7.54291 11 7.87895 11H10.9189V7.96Z" - fill="black" - fill-opacity="0.9" + fill="var(--icon-color, black)" + fill-opacity="var(--icon-opacity, 0.9)"packages/extension/src/ui/action/styles/hover-transitions.less (2)
1-6: Consider standardizing variable naming conventions.While the variables are well-documented, their naming could be more consistent:
@opacity-hasBG-colorvs@opacity-hasBG-transition- Consider using either camelCase or kebab-case consistently
-@opacity-hasBG-color: 0.8; -@opacity-hasBG-transition: opacity 300ms ease-in-out; -@opacity-noBG-transition: background 300ms ease-in-out; +@hover-opacity: 0.8; +@hover-transition-with-bg: opacity 300ms ease-in-out; +@hover-transition-no-bg: background 300ms ease-in-out;
1-18: Consider adding usage documentation.Since these are utility classes that other developers will reuse, consider adding a comment block explaining:
- When to use each class
- Example usage scenarios
- Any dependencies (like
@black004)+/** + * Hover transition utilities + * + * .hover-transition-bg: Use for elements with background colors that should fade on hover + * .hover-transition-no-bg: Use for elements without background that should get a light background on hover + * + * Example: + * <button class="hover-transition-bg">Click me</button> + */packages/extension/src/ui/action/icons/common/drag-icon.vue (1)
11-11: Consider optimizing the SVG path.The path data looks correct but could potentially be optimized for size. Consider running it through an SVG optimizer like SVGO to reduce the path complexity while maintaining the same visual output.
packages/extension/src/ui/action/components/app-menu/components/app-menu-item.vue (9)
3-3: Ensure theidattribute value is valid and unique.Using
network.nameas theidattribute may lead to issues ifnetwork.namecontains characters that are not valid in HTMLidvalues or if duplicate names exist. Consider sanitizingnetwork.nameor generating a uniqueid.Apply this diff to ensure the
idis valid:- :id="network.name" + :id="`network-${network.name}`"
7-10: HandleisStickyTopbeingundefinedin class bindings.The class bindings for
sticky-topandsticky-bottomdo not account forisStickyTopbeingundefined. This might lead to unintended class assignments. Ensure that theundefinedstate is handled appropriately.Update the class bindings to handle all possible values of
isStickyTop::class="[ { active: isActive }, - { 'sticky-top': isStickyTop }, - { 'sticky-bottom': isStickyTop === false }, + isStickyTop === true ? 'sticky-top' : isStickyTop === false ? 'sticky-bottom' : '', ]"
Line range hint
69-72: Mark thenetworkprop as required to prevent potential errors.The
networkprop is critical for this component but is not marked as required. If it is not provided, it may lead to runtime errors when accessing its properties. Consider marking it as required.Apply this diff to mark the
networkprop as required:network: { type: Object as PropType<NodeType>, + required: true, default: () => { return {}; }, },
183-191: Simplify nested ternary operations fornetworkType.The nested ternary operator in
setPinnedcan be hard to read and maintain. Refactor to improve readability.Apply this diff to refactor
networkTypeassignment:- const networkType = props.network.isTestNetwork - ? NetworkType.Testnet - : props.network.isCustomNetwork - ? NetworkType.Custom - : NetworkType.Regular; + let networkType = NetworkType.Regular; + if (props.network.isTestNetwork) { + networkType = NetworkType.Testnet; + } else if (props.network.isCustomNetwork) { + networkType = NetworkType.Custom; + }
255-276: Use theme variables instead of hard-coded colors.The styles contain hard-coded color values, which might lead to inconsistencies across the application. Replace them with theme variables from
theme.lessfor consistency.Apply this diff to use theme variables:
.app-menu-network-tag { - background: rgba(0, 122, 255, 1); + background: @primaryColor; color: #fff !important; - border: 1px solid @white; + border: 1px solid @whiteColor; }
297-298: Combine transition properties for efficiency.The transitions for
topandbottomcan be combined into a single declaration.Apply this diff to combine transitions:
- transition: top 1s linear; - transition: bottom 1s linear; + transition: top 1s linear, bottom 1s linear;
371-373: Remove redundant-webkit-stickyposition property.The
-webkit-stickyprefix is no longer necessary as all modern browsers supportposition: sticky. Removing it can clean up the code.Apply this diff to remove the redundant prefix:
- position: -webkit-sticky; position: sticky;
322-324: Use theme variables for SVG path fill colors.Hard-coded color values in SVG styling can cause inconsistency. Replace them with theme variables.
Apply this diff to use theme variables:
svg path { - fill: #5f6368; + fill: @iconDefaultColor; fill-opacity: 0.4; }
389-391: Avoid using!importantin styles when possible.The
!importantdeclaration can make CSS harder to maintain. Try to increase CSS specificity instead.Apply this diff to remove
!importantand adjust specificity:svg path { - fill: #684cff !important; + fill: #684cff; }packages/extension/src/ui/action/App.vue (2)
338-338: Typo in variable nameenabledTestnetworksThe variable
enabledTestnetworksappears to have a typo. To maintain consistent camelCase naming conventions, it should be renamed toenabledTestNetworkswith a capital 'N'.Apply this diff to correct the variable name:
- enabledTestnetworks.value = await networksState.getEnabledTestNetworks(); + enabledTestNetworks.value = await networksState.getEnabledTestNetworks();Ensure that all occurrences of
enabledTestnetworksin the code are updated toenabledTestNetworks.
443-446: Avoid directly manipulating DOM stylesDirectly modifying the DOM element's style can hinder maintainability and might lead to unexpected side effects. Instead, consider using Vue's reactive system to bind styles or classes.
Refactor the code to bind the style dynamically in the template. For example:
<div ref="appMenuRef" :style="appMenuStyle" class="app__menu">In your script:
const appMenuStyle = computed(() => ({ background: `radial-gradient(137.35% 97% at 100% 50%, rgba(250, 250, 250, 0.94) 0%, rgba(250, 250, 250, 0.96) 28.91%, rgba(250, 250, 250, 0.98) 100%), linear-gradient(180deg, ${newGradient.value} 80%, #684CFF 100%)`, }));This approach leverages Vue's reactivity, making the code cleaner and more maintainable.
packages/extension/src/libs/updates-state/types.ts (1)
1-9: LGTM! Consider using a more specific type for timestamp.The type definitions are clear and well-structured. Consider using a more specific type for
currentReleaseTimestampsuch asUnixTimestamp = numberto better document the expected format.+type UnixTimestamp = number; export interface IState { lastVersionViewed: string; currentRelease: string; - currentReleaseTimestamp: number; + currentReleaseTimestamp: UnixTimestamp; }packages/extension/src/types/globals.ts (1)
10-10: Consider using a more specific type instead ofany.Using
anyfor theunisatproperty reduces type safety. If possible, define an interface that represents the expected shape of the unisat object.interface UniSatProvider { // Add expected properties and methods requestAccounts?: () => Promise<string[]>; // ... other properties } // Then use: unisat?: UniSatProvider;packages/extension/src/ui/action/types/network-sort.ts (2)
1-4: Remove unnecessary semicolon after enum declaration.The semicolon after the enum declaration is not needed.
export enum NetworkSortOption { Name = 'name', Tvl = 'tvl', -}; +}
1-13: Consider adding more sort options for better network management.The current sort options are good starting points. Consider adding more options that might be useful for network management:
- Activity (recent transactions)
- Gas prices
- Network status
export enum NetworkSortOption { Name = 'name', Tvl = 'tvl', + Activity = 'activity', + GasPrice = 'gasPrice', + Status = 'status', }packages/extension/src/libs/networks-state/types.ts (1)
12-16: LGTM! Consider adding JSDoc comments.The new interface properties are well-structured. Consider adding JSDoc comments to document the purpose of these new state properties, especially explaining when and how
newUsedFeaturesdiffers from existing feature tracking.export interface IState { networks: NetworkStorageElement[]; newNetworksVersion: string; + /** List of enabled test networks */ enabledTestNetworks: NetworkStorageElement[]; + /** Tracks feature usage across different categories */ newUsedFeatures: { networks: string[]; swap: string[]; }; }packages/extension/src/ui/action/components/app-menu/store.ts (1)
9-11: Add return type annotation to setOptions method.For better maintainability and type safety:
-const setOptions = (options: CustomEvmNetworkOptions) => { +const setOptions = (options: CustomEvmNetworkOptions): void => { evmOptions.value = options; };packages/extension/src/ui/action/icons/actions/sort/sort-direction-icon.vue (2)
2-15: Add ARIA attributes for accessibility.The SVG icon should include ARIA attributes for better accessibility.
<svg width="14" height="14" viewBox="0 0 14 14" fill="none" xmlns="http://www.w3.org/2000/svg" + role="img" + aria-label="Sort direction indicator" :class="{ 'flip-vertical': isAsc }" >
26-30: Consider using transform-specific properties for better performance.Using
transformfor animation can trigger layout recalculations. Consider using transform-specific properties.svg { display: inline-block; vertical-align: baseline; - transition: transform 0.5s ease-in-out; + transition: transform 0.5s cubic-bezier(0.4, 0, 0.2, 1); + will-change: transform; }packages/extension/src/ui/action/icons/actions/sort/sort-btn-icon.vue (1)
2-24: Enhance SVG accessibility and optimize markup.The SVG icon needs accessibility attributes and could benefit from some optimizations.
<svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg" + role="img" + aria-label="Sort button" > <path fill-rule="evenodd" clip-rule="evenodd" d="M4 6.40002C4 6.12388 4.22386 5.90002 4.5 5.90002H11.5C11.7761 5.90002 12 6.12388 12 6.40002C12 6.67617 11.7761 6.90002 11.5 6.90002H4.5C4.22386 6.90002 4 6.67617 4 6.40002ZM5 8.50002C5 8.22388 5.22386 8.00002 5.5 8.00002H10.5C10.7761 8.00002 11 8.22388 11 8.50002C11 8.77617 10.7761 9.00002 10.5 9.00002H5.5C5.22386 9.00002 5 8.77617 5 8.50002ZM6.5 10.1C6.22386 10.1 6 10.3239 6 10.6C6 10.8762 6.22386 11.1 6.5 11.1H9.5C9.77614 11.1 10 10.8762 10 10.6C10 10.3239 9.77614 10.1 9.5 10.1H6.5Z" - fill="black" - fill-opacity="0.6" + fill="currentColor" + opacity="0.6" /> <circle cx="8" cy="8" r="5.9" - stroke="black" - stroke-opacity="0.6" + stroke="currentColor" + opacity="0.6" stroke-width="1.2" /> </svg>packages/extension/src/providers/bitcoin/methods/btc_getNetwork.ts (1)
30-31: Consider enhancing error handling for network detection.While the Litecoin support is correctly implemented, consider adding a default error message for unsupported networks instead of returning an empty string. This would provide better feedback to users.
- res(null, ''); + res(getCustomError(`Unsupported network: ${this.network.name}`));packages/extension/src/ui/action/utils/filters.ts (1)
21-25: Enhance truncate function with input validation.The function could benefit from additional input validation and optimization:
- Validate that length is positive
- Handle edge cases where length is 0 or greater than string length
- Consider adding an optional custom ellipsis parameter
-export const truncate = (value: string, length: number): string => { +export const truncate = ( + value: string, + length: number, + ellipsis = '...' +): string => { if (!value) return ''; + if (length <= 0) return value; value = value.toString(); - return value.length > length ? value.substring(0, length) + '...' : value; + return value.length > length + ? value.substring(0, Math.max(0, length)) + ellipsis + : value; }packages/extension/src/libs/recently-sent-addresses/index.ts (1)
7-14: Consider adding storage initialization check.The constructor should verify if the storage is properly initialized.
constructor() { + if (!InternalStorageNamespace.recentlySentAddresses) { + throw new Error('Storage namespace not properly configured'); + } this.#storage = new BrowserStorage( InternalStorageNamespace.recentlySentAddresses, ); }packages/extension/src/ui/action/icons/common/custom-network-icon.vue (1)
17-24: Consider using a more unique clipPath ID.The current clipPath ID
clip0_6753_3063could potentially conflict with other SVGs. Consider using a more unique identifier, perhaps incorporating the component name.- <clipPath id="clip0_6753_3063"> + <clipPath id="custom-network-icon-clip">packages/extension/src/libs/metrics/types.ts (1)
65-73: Fix inconsistent string quotes in UpdatesOpenLocation.The string quotes in
UpdatesOpenLocationare inconsistent.- logo = "logo", + logo = 'logo',packages/extension/src/providers/bitcoin/methods/btc_switchNetwork.ts (1)
51-54: Fix typo in error message.There's a typo in the error message.
- `btc_switchNetwork: porvided network ${payload.params![0] + `btc_switchNetwork: provided network ${payload.params![0]packages/extension/src/ui/action/utils/browser.ts (1)
64-78: Consider extracting URL constants and adding retry logic.The update fetching implementation could be improved in a few ways:
- Extract the base URL as a constant for better maintainability
- Add retry logic for network failures
- Add timeout to the fetch call
+const UPDATES_BASE_URL = 'https://raw.githubusercontent.com/enkryptcom/dynamic-data/main/configs/release-versions'; +const FETCH_TIMEOUT = 5000; export const getLatestEnkryptUpdates = (): Promise<Updates | null> => { const browser = detectBrowser(); - const url = 'https://raw.githubusercontent.com/enkryptcom/dynamic-data/main/configs/release-versions' - const fetchUrl = browser === BROWSER_NAMES.safari ? `${url}-safari.json` : `${url}.json` + const fetchUrl = browser === BROWSER_NAMES.safari ? + `${UPDATES_BASE_URL}-safari.json` : + `${UPDATES_BASE_URL}.json`; + + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), FETCH_TIMEOUT); + return fetch( - fetchUrl + fetchUrl, + { + signal: controller.signal, + headers: { 'Cache-Control': 'no-cache' } + } ) .then(res => res.json()) .catch((error) => { console.error('Failed to fetch updates:', error); return null; - } + }) + .finally(() => { + clearTimeout(timeoutId); + }); };packages/extension/src/libs/updates-state/index.ts (1)
29-59: Consider using non-null assertion for state properties.The nullish coalescing operator is good, but TypeScript's type system should guarantee these properties exist after
getState().async getLastVersionViewed(): Promise<IState['lastVersionViewed']> { const state: IState = await this.getState(); - return state?.lastVersionViewed ?? ''; + return state.lastVersionViewed; }packages/extension/src/ui/action/views/add-network/index.vue (1)
48-49: Consider adding error handling for tracking calls.The tracking calls should handle potential failures gracefully to prevent affecting the main functionality.
const toCustomNetwork = () => { isNetworkList.value = false; - trackNetwork(NetworkChangeEvents.NetworkAddCustomClicked, {}); + try { + trackNetwork(NetworkChangeEvents.NetworkAddCustomClicked, {}); + } catch (error) { + console.error('Failed to track network event:', error); + } }; const toNetworkList = () => { - trackNetwork(NetworkChangeEvents.NetworkCustomBackButton, {}); + try { + trackNetwork(NetworkChangeEvents.NetworkCustomBackButton, {}); + } catch (error) { + console.error('Failed to track network event:', error); + } isNetworkList.value = true; };Also applies to: 52-54
packages/extension/src/libs/metrics/index.ts (1)
96-103: Add error handling to trackUpdatesEvents.Consider adding validation for required fields and error handling. Also, remove the unnecessary empty line after the function body.
const trackUpdatesEvents = (event: UpdatesEventType, options: { network: NetworkNames; location?: UpdatesOpenLocation; duration?: number; }): void => { + if (!event || !options.network) { + console.error('Missing required parameters in trackUpdatesEvents'); + return; + } metrics.track('updatesClick', { event, ...options }); - }packages/extension/src/ui/action/components/app-menu/components/app-menu-tab.vue (1)
46-51: Improve type safety for NetworksCategory.Consider using a union type of literal values instead of String for better type safety.
defineProps({ activeCategory: { - type: String as PropType<NetworksCategory>, + type: Object as PropType<`${NetworksCategory}`>, required: true, }, });packages/extension/src/types/provider.ts (1)
133-133: Remove unnecessary constructor.The constructor only calls the parent constructor with the same parameters and doesn't add any additional initialization.
- constructor(node: string, options?: unknown) { }🧰 Tools
🪛 Biome (1.9.4)
[error] 133-133: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
packages/extension/src/libs/networks-state/index.ts (1)
97-107: Consider adding validation for feature type.The feature parameter should be validated against a defined enum or constant to prevent typos and ensure type safety.
+enum FeatureType { + Networks = 'networks', + Swap = 'swap' +} -async setUsedFeature(feature: 'networks' | 'swap', networkName: string) { +async setUsedFeature(feature: FeatureType, networkName: string) {packages/extension/src/ui/action/components/app-menu/components/app-menu-sort.vue (2)
84-99: Add error handling for tracking failures.The function should handle potential failures in the tracking call to ensure robustness.
Apply this diff to add error handling:
const setActiveSort = (_sort: NetworkSortOption) => { const newSortBy = { ...props.sortBy }; if (_sort === props.sortBy.name) { newSortBy.direction = newSortBy.direction === NetworkSortDirection.Asc ? NetworkSortDirection.Desc : NetworkSortDirection.Asc; } else { // Next Release Version: update this function newSortBy.name = _sort; } - trackNetwork(NetworkChangeEvents.NetworkSortOptionChanged, { - sortOption: newSortBy.name, - }); + try { + trackNetwork(NetworkChangeEvents.NetworkSortOptionChanged, { + sortOption: newSortBy.name, + }); + } catch (error) { + console.error('Failed to track network sort change:', error); + } emit('update:sort', newSortBy); };
108-117: Improve default case handling in sortString computed property.The default case concatenates raw enum values, which might not be user-friendly. Consider adding proper error handling and user-friendly fallbacks.
Apply this diff to improve the default case:
const sortString = computed(() => { switch (props.sortBy.name) { case NetworkSortOption.Name: return 'Name'; case NetworkSortOption.Tvl: return 'Total value Locked'; default: - return `${props.sortBy.name} ${props.sortBy.direction}`; + console.warn(`Unknown sort option: ${props.sortBy.name}`); + return 'Sort by'; } });packages/extension/src/providers/common/ui/send-transaction/send-contacts-list.vue (2)
Line range hint
12-50: Enhance accessibility for the recently sent addresses section.Consider adding ARIA labels and roles to improve accessibility for screen readers.
Apply this diff to enhance accessibility:
<div v-if="recentlySentAddresses && !isMyAddress" class="send-contacts-list__block" + role="region" + aria-label="Recently sent addresses" > <div class="send-contacts-list__buttons"> <base-button title="Send to my address" :click="sendToMyAddress" /> <a class="send-contacts-list__buttons-paste" @click="pasteFromClipboard" + role="button" + aria-label="Paste from clipboard" > <paste-icon /> Paste </a> </div> <template v-if="recentlySentAddresses.length"> - <h3>Recent</h3> + <h3 id="recent-addresses-heading">Recent</h3> <div class="send-contacts-list__list" + role="list" + aria-labelledby="recent-addresses-heading" >
128-156: Consider increasing the timeout duration for slow networks.The current timeout of 500ms might be too short for slow networks, potentially causing unnecessary fallbacks to an empty list.
Apply this diff to improve the timeout handling:
+const FETCH_TIMEOUT_MS = 2000; // Increased from 500ms to 2000ms for better reliability const recentlySentAddressesState = new RecentlySentAddressesState(); const recentlySentAddresses = ref<null | string[]>(null); onMounted(async function () { let timedOut = false; const timeout = setTimeout(function () { console.error('Timed out getting recently sent addresses'); recentlySentAddresses.value = []; timedOut = true; - }, 500); + }, FETCH_TIMEOUT_MS);packages/extension/src/ui/action/icons/actions/pin.vue (1)
101-105: Consider using CSS custom properties for better maintainability.The SVG styling could be more maintainable by using CSS custom properties for colors and alignment values.
+:root { + --pin-icon-color-active: #684CFF; + --pin-icon-color-inactive: #5F6368; +} svg { display: inline-block; vertical-align: baseline; + fill: var(--pin-icon-color-inactive); }packages/extension/src/ui/action/views/add-network/views/add-network-list.vue (6)
134-138: Refactor duplicated filtering logic into a reusable functionThe computed properties
displayTestNetworksanddisplayCustomNetworksuse identical filtering logic based onsearchInput. To adhere to the DRY (Don't Repeat Yourself) principle, consider abstracting this logic into a shared utility function.Here's how you can refactor the code:
const filterNetworksBySearch = <T extends { name_long: string }>(networks: T[]) => { return networks.filter(network => network.name_long.toLowerCase().startsWith(searchInput.value.toLowerCase()), ); }; const displayTestNetworks = computed<NodeType[]>(() => { return filterNetworksBySearch(allTestNets.value); }); const displayCustomNetworks = computed<CustomEvmNetwork[]>(() => { return filterNetworksBySearch(allCustomNetworks.value); });Also applies to: 140-144
146-148: Handle absence of custom networks based on search inputThe condition
hasCustomNetworksdoesn't consider the currentsearchInput, which might result in the "My custom networks" header displaying even when no networks match the search criteria. To improve user experience, adjust the condition to reflect the current search results.Update the
hasCustomNetworkscomputed property:const hasCustomNetworks = computed(() => { - return allCustomNetworks.value.length > 0; + return displayCustomNetworks.value.length > 0; });This change ensures that the "My custom networks" section only appears when there are matching networks based on the search input.
Also applies to: 35-35
28-30: Simplify conditional rendering logic for header displayThe condition used to display the "My custom networks" header can be simplified for better readability without changing its functionality.
Apply this diff to simplify the condition:
<h3 - v-if=" - searchInput === '' || - (searchInput !== '' && displayCustomNetworks.length > 0) - " + v-if="displayCustomNetworks.length > 0" class="add-network__list-header" >Since
displayCustomNetworksalready reflects thesearchInput, checking its length suffices to determine if the header should be shown.
135-137: Enhance search functionality to match anywhere in the network nameCurrently, the search matches only the beginning of the network names due to
startsWith, which may not meet user expectations when searching for networks. Consider usingincludesto allow matching any part of the network name.Update the filtering logic:
// For displayTestNetworks return allTestNets.value.filter(a => - a.name_long.toLowerCase().startsWith(searchInput.value.toLowerCase()), + a.name_long.toLowerCase().includes(searchInput.value.toLowerCase()), ); // For displayCustomNetworks return allCustomNetworks.value.filter(a => - a.name_long.toLowerCase().startsWith(searchInput.value.toLowerCase()), + a.name_long.toLowerCase().includes(searchInput.value.toLowerCase()), );This change enhances the search functionality, making it more flexible and user-friendly.
Also applies to: 141-143
193-195: Avoid redundant status updates for test networksIn
onTogglePin, when activating a network, you're setting the testnet status even if the network is already active. This might be unnecessary and could be optimized.Consider adding a check to see if the testnet status needs to be updated:
if (isActive && allTestNets.value.find(net => net.name === networkName)) { - await networksState.setTestnetStatus(networkName, isActive); + if (!enabledTestNetworks.value.includes(networkName)) { + await networksState.setTestnetStatus(networkName, isActive); + } }This ensures that you only update the testnet status if it's not already enabled.
154-162: Optimize network data retrieval and sortingThe
setNetworkListsfunction retrieves and sorts network data but could be optimized for better performance and readability.Refactor the function as follows:
const setNetworkLists = async () => { pinnedNetworks.value = await networksState.getPinnedNetworkNames(); allCustomNetworks.value = await customNetworksState.getAllCustomEVMNetworks(); const testNetworks = await getAllNetworks(); - const allNetworksTestNets = testNetworks - .filter(({ isTestNetwork }) => isTestNetwork) - .sort((a, b) => a.name_long.localeCompare(b.name_long)); - enabledTestNetworks.value = await networksState.getEnabledTestNetworks(); - allTestNets.value = allNetworksTestNets; + allTestNets.value = testNetworks + .filter(({ isTestNetwork }) => isTestNetwork) + .sort((a, b) => a.name_long.localeCompare(b.name_long)); + enabledTestNetworks.value = await networksState.getEnabledTestNetworks(); };This streamlines the data assignments and improves code clarity.
packages/extension/src/ui/action/views/add-network/views/add-custom-network.vue (1)
268-281: Consider adding error handling for state updates.The network state updates look good, but consider wrapping the state updates in a try-catch block to handle potential storage errors gracefully.
- await networksState.setNetworkStatus(customNetworkOptions.name, true); - trackNetwork(NetworkChangeEvents.NetworkCustomNetworkAdded, { - customRpcUrl: customNetworkOptions.node, - // ... other tracking properties - }); - emit('update:pinNetwork', customNetworkOptions.name, true); + try { + await networksState.setNetworkStatus(customNetworkOptions.name, true); + trackNetwork(NetworkChangeEvents.NetworkCustomNetworkAdded, { + customRpcUrl: customNetworkOptions.node, + // ... other tracking properties + }); + emit('update:pinNetwork', customNetworkOptions.name, true); + } catch (error) { + console.error('Failed to update network status:', error); + // Consider showing an error notification to the user + }
🛑 Comments failed to post (17)
packages/extension/src/ui/action/icons/common/drag-icon.vue (1)
4-8: 💡 Codebase verification
Add attribution for Material Design icon
The SVG attributes confirm this is a Material Design icon:
- The viewBox coordinates "0 -960 960 960" are specific to Material Design's coordinate system
- The fill-opacity="0.38" matches Material Design's standard disabled state
- The 24x24 dimensions follow Material Design's standard icon size
Please add a comment with the source URL for proper attribution and license compliance.
🔗 Analysis chain
Verify the Material Design icon source.
The SVG attributes suggest this icon was copied from Material Design Icons (viewBox and opacity values are typical MD patterns). Please add a comment with the source URL for future reference and license compliance.
<template> + <!-- Source: [Material Design Icons URL] --> <svg🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Check the complete drag-icon.vue file cat packages/extension/src/ui/action/icons/common/drag-icon.vue # Search for any documentation about icon sources fd -e md -x grep -l -i "material" {} \; # Look for similar Material Design viewBox patterns in other SVG files fd -e vue -e svg | xargs grep -l "viewBox=\"0 -960 960 960\""Length of output: 746
Script:
#!/bin/bash # List all icon files in the same directory ls packages/extension/src/ui/action/icons/common/ # Check viewBox and size patterns in other icon files fd -e vue -p "icons" | xargs grep -l "width=\"\|height=\"\|viewBox=\"" # Show a sample of other icon files for pattern comparison fd -e vue -p "icons/common" -x head -n 15 {}Length of output: 52603
packages/extension/src/ui/action/components/app-menu/index.vue (1)
16-16:
⚠️ Potential issueCorrect binding of the
draggableattributeThe
draggableattribute on the<draggable>component should be correctly bound using Vue's binding syntax. Currently, it's set as a static string, which may not behave as expected.Apply this diff to fix the issue:
- draggable=":not(.do-not-drag)" + :draggable="':not(.do-not-drag)'"This change ensures that the
draggableattribute is dynamically bound to the string':not(.do-not-drag)', allowing Vue to interpret it correctly.📝 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.:draggable="':not(.do-not-drag)'"packages/extension/src/ui/action/types/updates.ts (1)
3-10: 🛠️ Refactor suggestion
Improve type safety for dates and maintain consistent naming convention.
- The
release_datefield should use a more specific type to ensure valid dates.- Property names should follow camelCase convention for consistency with TypeScript standards.
export interface Version { version: string; description?: string; - chains_added?: NetworkNames[]; - swap_added?: NetworkNames[]; - release_date: string; - release_link: string; + chainsAdded?: NetworkNames[]; + swapAdded?: NetworkNames[]; + releaseDate: Date | string; + releaseLink: string; }📝 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 interface Version { version: string; description?: string; chainsAdded?: NetworkNames[]; swapAdded?: NetworkNames[]; releaseDate: Date | string; releaseLink: string; }packages/extension/src/ui/action/components/app-menu/store.ts (1)
6-7: 🛠️ Refactor suggestion
Improve type safety of evmOptions state.
Initialize
evmOptionswith proper typing to maintain type safety:-const evmOptions = ref({}); +const evmOptions = ref<CustomEvmNetworkOptions>({} as CustomEvmNetworkOptions);📝 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 evmOptions = ref<CustomEvmNetworkOptions>({} as CustomEvmNetworkOptions); const isEdit = ref(false);packages/extension/src/libs/recently-sent-addresses/index.ts (2)
32-37: 🛠️ Refactor suggestion
Add error handling for storage retrieval.
The
getRecentlySentAddressesmethod should handle storage errors gracefully.async getRecentlySentAddresses(networkName: NetworkNames): Promise<string[]> { const key = networkName - const out: IState | undefined = await this.#storage.get(key) - if (!out) return [] - return out.addresses + try { + const out: IState | undefined = await this.#storage.get(key) + if (!out) return [] + return out.addresses + } catch (error) { + throw new Error(`Failed to retrieve recently sent addresses: ${error.message}`); + } }📝 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.async getRecentlySentAddresses(networkName: NetworkNames): Promise<string[]> { const key = networkName try { const out: IState | undefined = await this.#storage.get(key) if (!out) return [] return out.addresses } catch (error) { throw new Error(`Failed to retrieve recently sent addresses: ${error.message}`); } }
16-30: 🛠️ Refactor suggestion
Add address validation and error handling.
The
addRecentlySentAddressmethod should validate the address format and handle storage errors gracefully.async addRecentlySentAddress( network: Pick<BaseNetwork, 'name' | 'displayAddress'>, address: string, ): Promise<void> { + if (!address || typeof address !== 'string') { + throw new Error('Invalid address format'); + } const key = network.name - const state: IState | undefined = await this.#storage.get(key) - const newState: IState = { - ...state, - addresses: Array.from(new Set([ - network.displayAddress(address), - ...(state?.addresses ?? []), - ])).slice(0, 5), - } - await this.#storage.set(key, newState) + try { + const state: IState | undefined = await this.#storage.get(key) + const newState: IState = { + ...state, + addresses: Array.from(new Set([ + network.displayAddress(address), + ...(state?.addresses ?? []), + ])).slice(0, 5), + } + await this.#storage.set(key, newState) + } catch (error) { + throw new Error(`Failed to update recently sent addresses: ${error.message}`); + } }📝 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.async addRecentlySentAddress( network: Pick<BaseNetwork, 'name' | 'displayAddress'>, address: string, ): Promise<void> { if (!address || typeof address !== 'string') { throw new Error('Invalid address format'); } const key = network.name try { const state: IState | undefined = await this.#storage.get(key) const newState: IState = { ...state, addresses: Array.from(new Set([ network.displayAddress(address), ...(state?.addresses ?? []), ])).slice(0, 5), } await this.#storage.set(key, newState) } catch (error) { throw new Error(`Failed to update recently sent addresses: ${error.message}`); } }packages/extension/src/libs/updates-state/index.ts (1)
16-27: 🛠️ Refactor suggestion
Add error handling for storage operations.
The
getStatemethod should handle potential storage errors that might occur duringthis.storage.get.async getState(): Promise<IState> { - const state = this.storage.get(StorageKeys.updatesInfo); + try { + const state = await this.storage.get(StorageKeys.updatesInfo); + if (!state) { + const newState: IState = { + lastVersionViewed: '', + currentRelease: '', + currentReleaseTimestamp: 0, + } + return newState; + } + return state; + } catch (error) { + console.error('Failed to get updates state:', error); + throw 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.async getState(): Promise<IState> { try { const state = await this.storage.get(StorageKeys.updatesInfo); if (!state) { const newState: IState = { lastVersionViewed: '', currentRelease: '', currentReleaseTimestamp: 0, } return newState; } return state; } catch (error) { console.error('Failed to get updates state:', error); throw error; } }packages/extension/src/ui/action/components/app-menu/components/app-menu-tab.vue (1)
3-37: 🛠️ Refactor suggestion
Enhance accessibility with keyboard navigation.
The tabs implementation should support keyboard navigation. Consider using
<button>elements and adding keyboard handlers.<div class="network-tab__container"> - <div + <button role="tab" + tabindex="0" :aria-selected="activeCategory === NetworksCategory.All" :class="[ 'network-tab__container-tab hover-transition-no-bg', activeCategory === NetworksCategory.All ? 'active' : '', ]" @click="setActiveCategory(NetworksCategory.All)" + @keydown.enter="setActiveCategory(NetworksCategory.All)" + @keydown.space="setActiveCategory(NetworksCategory.All)" > All - </div> + </button> <!-- Apply similar changes to other tabs -->📝 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="network-tab__container"> <button role="tab" tabindex="0" :aria-selected="activeCategory === NetworksCategory.All" :class="[ 'network-tab__container-tab hover-transition-no-bg', activeCategory === NetworksCategory.All ? 'active' : '', ]" @click="setActiveCategory(NetworksCategory.All)" @keydown.enter="setActiveCategory(NetworksCategory.All)" @keydown.space="setActiveCategory(NetworksCategory.All)" > All </button> <button role="tab" tabindex="0" :aria-selected="activeCategory === NetworksCategory.Pinned" :class="[ 'network-tab__container-tab hover-transition-no-bg', activeCategory === NetworksCategory.Pinned ? 'active' : '', ]" @click="setActiveCategory(NetworksCategory.Pinned)" @keydown.enter="setActiveCategory(NetworksCategory.Pinned)" @keydown.space="setActiveCategory(NetworksCategory.Pinned)" > Pinned </button> <button role="tab" tabindex="0" :aria-selected="activeCategory === NetworksCategory.New" :class="[ 'network-tab__container-tab hover-transition-no-bg', activeCategory === NetworksCategory.New ? 'active' : '', ]" @click="setActiveCategory(NetworksCategory.New)" @keydown.enter="setActiveCategory(NetworksCategory.New)" @keydown.space="setActiveCategory(NetworksCategory.New)" > New </button> </div>packages/extension/src/libs/networks-state/index.ts (1)
141-151: 🛠️ Refactor suggestion
Add error handling for invalid state.
The method should handle cases where the state is invalid or corrupted.
async getEnabledTestNetworks(): Promise<string[]> { await this.insertNetworksWithNewFeatures(); const state: IState | undefined = await this.getState(); + if (!state) { + await this.setInitialActiveNetworks(); + return []; + } if (state && state.enabledTestNetworks) { const validNetworks = state.enabledTestNetworks; return validNetworks.map(({ name }) => name); } else { this.setState(Object.assign({}, state, { enabledTestNetworks: [] })); return []; } }Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 144-144: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/extension/src/ui/action/icons/actions/pin.vue (1)
11-11:
⚠️ Potential issueFix duplicate mask IDs in SVGs.
Multiple SVG elements are using the same mask ID
path-1-inside-1_6475_5454. This can cause rendering issues if the component is used multiple times on the same page.Apply this diff to make the IDs unique:
- <mask id="path-1-inside-1_6475_5454" fill="white"> + <mask id="path-1-inside-1_6475_5454_unpinned_active" fill="white">- <mask id="path-1-inside-1_6475_5454" fill="white"> + <mask id="path-1-inside-1_6475_5454_unpinned_inactive" fill="white">Also applies to: 32-32
packages/extension/src/providers/kadena/ui/send-transaction/index.vue (1)
470-477: 🛠️ Refactor suggestion
Add error handling for recently sent addresses storage.
The
addRecentlySentAddresscall should handle potential storage failures gracefully to prevent blocking the transaction flow.const sendAction = async () => { - await recentlySentAddresses.addRecentlySentAddress( - props.network, - addressTo.value, - ); + try { + await recentlySentAddresses.addRecentlySentAddress( + props.network, + addressTo.value, + ); + } catch (error) { + console.error('Failed to save recent address:', error); + // Continue with transaction as this is non-critical + }📝 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 recentlySentAddresses = new RecentlySentAddressesState(); const sendAction = async () => { try { await recentlySentAddresses.addRecentlySentAddress( props.network, addressTo.value, ); } catch (error) { console.error('Failed to save recent address:', error); // Continue with transaction as this is non-critical }packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue (1)
432-439: 🛠️ Refactor suggestion
Add error handling for recently sent addresses storage.
The
addRecentlySentAddresscall should handle potential storage failures gracefully to prevent blocking the transaction flow.const sendAction = async () => { - await recentlySentAddresses.addRecentlySentAddress( - props.network, - addressTo.value, - ); + try { + await recentlySentAddresses.addRecentlySentAddress( + props.network, + addressTo.value, + ); + } catch (error) { + console.error('Failed to save recent address:', error); + // Continue with transaction as this is non-critical + }📝 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 recentlySentAddresses = new RecentlySentAddressesState(); const sendAction = async () => { try { await recentlySentAddresses.addRecentlySentAddress( props.network, addressTo.value, ); } catch (error) { console.error('Failed to save recent address:', error); // Continue with transaction as this is non-critical }packages/extension/src/providers/polkadot/ui/send-transaction/index.vue (1)
504-508: 🛠️ Refactor suggestion
Add error handling for recently sent addresses storage
The call to
addRecentlySentAddressshould include error handling to prevent it from affecting the transaction flow if storage fails.const sendAction = async () => { - await recentlySentAddresses.addRecentlySentAddress( - props.network, - addressTo.value, - ); + try { + await recentlySentAddresses.addRecentlySentAddress( + props.network, + addressTo.value, + ); + } catch (error) { + console.error('Failed to save recently sent address:', error); + // Continue with transaction as this is non-critical + }Committable suggestion skipped: line range outside the PR's diff.
packages/extension/src/ui/action/views/add-network/views/add-network-list.vue (3)
44-44:
⚠️ Potential issueEnsure consistent event naming between template and script
The event names used in the template (
@update:pin-network) and emitted in the script (emit('update:pinNetwork', ...)) have inconsistent casing. This inconsistency can lead to the events not being properly handled or unexpected behavior.Apply this diff to standardize the event names to
update:pin-network:// In the template (lines 44 and 65): -@update:pin-network="onTogglePin" +@update:pinNetwork="onTogglePin" // In the script setup (lines 107~ and 196~), change the emitted event names: -emit('update:pinNetwork', networkName, isActive); +emit('update:pin-network', networkName, isActive); -defineEmits<{ - (e: 'update:pinNetwork', network: string, isPinned: boolean): void; +defineEmits<{ + (e: 'update:pin-network', network: string, isPinned: boolean): void;Alternatively, standardize all event names to use camelCase consistently throughout both the template and script to align with best practices.
Also applies to: 65-65, 107-108, 196-196
209-211:
⚠️ Potential issueAwait asynchronous function to ensure proper execution flow
In
onTestnetToggle, you're calling the asynchronous functiononTogglePinwithout usingawait. This may lead to race conditions or unexpected behavior due to the promise not being handled correctly.Apply this diff to add the missing
await:if (!isActive && getIsPinned(networkName)) { - onTogglePin(networkName, false); + await onTogglePin(networkName, false); }Committable suggestion skipped: line range outside the PR's diff.
216-218:
⚠️ Potential issueEnsure consistent parameters when emitting 'update:pinNetwork' event
In
onNetworkDeleted, you're emitting'update:pinNetwork'withchainIdinstead ofnetworkName. Since other parts of the code usenetworkNameas the identifier, this could lead to inconsistencies or errors.Apply this diff to use
networkNameconsistently:await customNetworksState.deleteEVMNetwork(chainId); await setNetworkLists(); -emit('update:pinNetwork', chainId, false); +const deletedNetwork = allCustomNetworks.value.find(net => net.chainID === chainId); +if (deletedNetwork) { + emit('update:pinNetwork', deletedNetwork.name, false); +}This ensures that the correct network name is emitted with the event, maintaining consistency across the codebase.
Committable suggestion skipped: line range outside the PR's diff.
packages/extension/src/ui/action/views/add-network/views/add-custom-network.vue (1)
270-279: 💡 Codebase verification
Reduce custom network tracking to essential fields only
The custom network tracking implementation collects significantly more data compared to other network tracking events in the codebase, including potentially sensitive information like RPC URLs and block explorer URLs. Consider:
- Aligning with other network tracking implementations by only collecting:
- Network name (basic)
- Chain ID
- Provider type
- Removing collection of:
- RPC URLs (may contain credentials)
- Block explorer URLs
- Detailed names that might contain internal information
🔗 Analysis chain
Review data collection for PII concerns.
The network tracking implementation collects detailed information about custom networks. Consider:
- Whether collecting the full RPC URL is necessary (it might contain sensitive information)
- If the network name could contain personally identifiable information
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for other instances of network tracking to ensure consistent PII handling ast-grep --pattern 'trackNetwork($_, $_)'Length of output: 1448
Script:
#!/bin/bash # Check for documentation about analytics/tracking fd -e md -e txt | xargs rg -i "analytics|tracking|privacy|data.?collection" -A 2 -B 2 # Check the NetworkChangeEvents enum to understand the event categorization ast-grep --pattern 'enum NetworkChangeEvents { $$$ }'Length of output: 1864
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the release notes:
Release Notes for Enkrypt Extension v2.0.3
New Features
Improvements
Bug Fixes
Performance
This release focuses on enhancing user experience, network management, and overall application stability.