Created Solana staking banners#708
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a persistent banner system for promoting Solana staking within the extension's UI. It adds a state management class utilizing browser storage to track banner visibility, integrates new banner components into the app menu and network assets views, and provides associated SVG icon components. Banner visibility is managed asynchronously, allowing users to dismiss banners, with their choices saved for future sessions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppMenu
participant NetworkAssets
participant BannersState
participant BrowserStorage
Note over AppMenu,NetworkAssets: On component mount
AppMenu->>BannersState: showSolanaStackingBanner()
BannersState->>BrowserStorage: get(bannersInfo)
BrowserStorage-->>BannersState: bannersInfo state or default
BannersState-->>AppMenu: true/false
NetworkAssets->>BannersState: showNetworkAssetsSolanaStackingBanner()
BannersState->>BrowserStorage: get(bannersInfo)
BrowserStorage-->>BannersState: bannersInfo state or default
BannersState-->>NetworkAssets: true/false
Note over User,AppMenu: User clicks close on banner
AppMenu->>BannersState: hideSolanaStackingBanner()
BannersState->>BrowserStorage: set(bannersInfo, isHideSolanStackingBanner=true)
BrowserStorage-->>BannersState: ack
Note over User,NetworkAssets: User clicks close on banner
NetworkAssets->>BannersState: hideNetworkAssetsSolanaStackingBanner()
BannersState->>BrowserStorage: set(bannersInfo, isHideNetworkAssetSolanStackingBanner=true)
BrowserStorage-->>BannersState: ack
Suggested reviewers
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
|
|
💼 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: 13
🧹 Nitpick comments (6)
packages/extension/src/ui/action/icons/common/close-icon-white.vue (1)
16-21: DRY up repeated icon styles
The scoped CSS targetingsvg { display: inline-block; vertical-align: baseline; }is repeated across multiple icon components. Consider extracting these rules into a shared CSS module or global mixin to reduce duplication and ensure consistent styling.packages/extension/src/ui/action/icons/banners/attractive-apr-icon.vue (1)
6-11: Consolidate common icon CSS
The same inline-block and vertical-align styling is applied here. Extract to a shared stylesheet or Vue mixin to avoid repetition and keep icon components consistent.packages/extension/src/ui/action/icons/common/enkrypt-staking-logo.vue (1)
30-35: Extract shared SVG styling
The scoped CSS here duplicates thedisplayandvertical-alignrules used by other icons. Consider moving these styles to a shared CSS file or Vue mixin to adhere to DRY principles.packages/extension/src/ui/action/icons/banners/consistent-rewards-icon.vue (1)
1-14: Add accessibility attributes to the SVG icon.The SVG icon is missing accessibility attributes which could impact screen reader users.
Apply this diff to improve accessibility:
-<template> - <svg width="20" height="20" viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg"> +<template> + <svg width="20" height="20" viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg" role="img" aria-label="Consistent rewards icon">packages/extension/src/ui/action/components/app-menu/components/solana-staking-banner.vue (1)
34-46: Consider using relative positioning instead of absolute.The banner uses absolute positioning which could cause layout issues or overlap with other UI elements, especially on different screen sizes.
Consider whether this banner should use relative positioning or ensure proper z-index management to prevent UI conflicts.
packages/extension/src/ui/action/icons/common/enkrypt-staking-logo-white.vue (1)
1-36: Add accessibility attributes to the SVG logo.The SVG logo is missing accessibility attributes which could impact screen reader users.
Apply this diff to improve accessibility:
-<template> - <svg width="121" height="24" viewBox="0 0 121 24" fill="none" xmlns="http://www.w3.org/2000/svg"> +<template> + <svg width="121" height="24" viewBox="0 0 121 24" fill="none" xmlns="http://www.w3.org/2000/svg" role="img" aria-label="Enkrypt staking logo">
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/extension/src/ui/action/assets/banners/solana-stacking-banner-bg.pngis excluded by!**/*.pngpackages/extension/src/ui/action/assets/banners/solana-stacking-banner-tokens-img.pngis excluded by!**/*.pngpackages/extension/src/ui/action/assets/banners/solana-stacking-banner.pngis excluded by!**/*.png
📒 Files selected for processing (12)
packages/extension/src/libs/banners-state/index.ts(1 hunks)packages/extension/src/libs/banners-state/types.ts(1 hunks)packages/extension/src/types/provider.ts(1 hunks)packages/extension/src/ui/action/components/app-menu/components/solana-staking-banner.vue(1 hunks)packages/extension/src/ui/action/components/app-menu/index.vue(4 hunks)packages/extension/src/ui/action/icons/banners/attractive-apr-icon.vue(1 hunks)packages/extension/src/ui/action/icons/banners/consistent-rewards-icon.vue(1 hunks)packages/extension/src/ui/action/icons/common/close-icon-white.vue(1 hunks)packages/extension/src/ui/action/icons/common/enkrypt-staking-logo-white.vue(1 hunks)packages/extension/src/ui/action/icons/common/enkrypt-staking-logo.vue(1 hunks)packages/extension/src/ui/action/views/network-assets/components/network-assets-solana-staking-banner.vue(1 hunks)packages/extension/src/ui/action/views/network-assets/index.vue(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/extension/src/libs/banners-state/index.ts (1)
packages/extension/src/libs/banners-state/types.ts (1)
IState(5-8)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: buildAll
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (8)
packages/extension/src/ui/action/views/network-assets/components/network-assets-solana-staking-banner.vue (1)
23-23: Good use of event modifier.The use of
@click.stopis appropriate here to prevent the close button click from bubbling up to the parent link element.packages/extension/src/ui/action/components/app-menu/index.vue (3)
208-209: LGTM! Clean import setup for banner functionality.The imports are properly organized and the BannersState import follows the correct path structure.
161-164: Banner integration looks good.The conditional rendering logic is correct, and passing the close callback as a prop follows Vue best practices.
544-550: Banner state management implementation is correct.The reactive variable initialization, BannersState instantiation, and close handler follow proper patterns. The close handler correctly updates both the UI state and persistent storage.
packages/extension/src/ui/action/views/network-assets/index.vue (4)
91-92: LGTM! Proper imports for banner functionality.The component and state management imports are correctly structured.
17-20: Good network-specific banner logic.The banner is correctly rendered only for the SOLANA network, which makes sense in the network assets context. The conditional logic is clean and follows Vue best practices.
154-159: Verify async onMounted implementation.The async onMounted hook is implemented correctly. However, ensure that the component can handle cases where the banner state check fails.
Consider adding error handling for the banner state check:
onMounted(async () => { updateAssets(); - if (await bannersState.showNetworkAssetsSolanaStackingBanner()) { - isSolanaStackingBanner.value = true; - } + try { + if (await bannersState.showNetworkAssetsSolanaStackingBanner()) { + isSolanaStackingBanner.value = true; + } + } catch (error) { + console.error('Failed to check banner visibility:', error); + } });
188-191: Banner close handler is correctly implemented.The close handler properly updates both the UI state and persistent storage state.
...nsion/src/ui/action/views/network-assets/components/network-assets-solana-staking-banner.vue
Show resolved
Hide resolved
kvhnuke
changed the title
2 participants
Summary by CodeRabbit
New Features
Style