brave / brave-ios

Brave iOS Browser
https://brave.com
Mozilla Public License 2.0
1.7k stars 440 forks source link

Fix #8600: Accounts Tab v2 #8659

Closed StephenHeaps closed 10 months ago

StephenHeaps commented 10 months ago

Summary of Changes

This pull request fixes #8600

Submitter Checklist:

Test Plan:

  1. Verify accounts tab still showing all accounts, with total fiat values and the tokens with most balance displayed
  2. Tapping the account card or tapping ... -> View Details should open account assets / transactions view
  3. Tapping ... -> Edit should open modal for the account to edit the account's name
  4. Tapping ... -> Export should open private key flow for the account
  5. Verify Backup Wallet still opens/works as expected (now accessed via ... menu in navigation bar)
  6. Verify Add Account still works (now access via ... menu in navigation bar while on Accounts tab)

Screenshots:

Accounts - light Accounts - dark

Reviewer Checklist:

nuo-xu commented 10 months ago

we probably want to add the ability to configure the space value and how they lie on each other(.zIndex) for MultipleCircleIconView also the design for MultipleCircleIconView when icons are more than maxIcons is actually the +x instead of ... as in multiple network icons.

Screenshot 2024-01-18 at 11 55 24 AM

vs

Screenshot 2024-01-18 at 11 55 39 AM

and we probably want to give a background color for AssetIconView since some icon image has transparent background for example below shows the Raydium icon has transparent background

Screenshot 2024-01-18 at 11 59 29 AM
StephenHeaps commented 10 months ago

we probably want to add the ability to configure the space value and how they lie on each other(.zIndex) for MultipleCircleIconView also the design for MultipleCircleIconView when icons are more than maxIcons is actually the +x instead of ... as in multiple network icons.

Discussed offline; these tokens are sorted by balance so we chose to keep z-index order so largest balance remains on top. Investigated the +x, but no perfect solution with larger numbers and the + would likely overlapped without larger spacing increase between icons.

and we probably want to give a background color for AssetIconView since some icon image has transparent background for example below shows the Raydium icon has transparent background

Good idea. Updated so we have a circular background instead of just a circular stroke 👍. https://github.com/brave/brave-ios/pull/8659/commits/0b43005cccc5b064e88293e1bd07b9be3b44a117

github-actions[bot] commented 10 months ago

[puLL-Merge] - brave/brave-ios@8659

Description

This pull request introduces improvements to the Brave iOS Wallet by restructuring the user interface for the accounts view and by decoupling the UI layer from the storage/ state management. The motivation behind these changes is to improve code maintainability, simplify state management, and enhance the user experience by adopting modern SwiftUI patterns.

Changes ### Changes **AccountsView.swift** - Import statements have been organized. - Redundant states `selectedAccount`, `isPresentingBackup`, and `isPresentingAddAccount` are removed. - Introduction of new states: `selectedAccountActivity`, `selectedAccountForEdit`, and `selectedAccountForExport`. - Replaced `List` view with a modern `ScrollView` and `LazyVStack` to display account cards. - Introduced `AccountCardView` components for individual accounts. - Account list is now being managed by an `@ObservedObject var store: AccountsStore`. - Removed `listRowBackground` colors and `InsetGroupedListStyle` for account list styling. - Introduced `.background` modifiers for different `NavigationLink` and `.sheet` bindings for actions like editing account details or exporting private keys. - Removed previous navigation and modal presentation styles in favor of new SwiftUI sheet presentations. - Added `handle` function to manage account card actions. **AccountDetailsView.swift** - Removed the navigation title to inherit the Accounts View's title. **AssetIconView.swift** - Refactored `AssetIconView` to add a new view called `AssetIcon` which can be reused. **CryptoTabsView.swift** - Introduced `CryptoTab` enum to simplify tab management. - Changed tab selection management to the new enum. - Added `isShowingBackup` and `isShowingAddAccount` state hooks for sheet presentations to manage backup and add account actions. **MainMenuView.swift** - Introduced `isShowingBackup` and `isShowingAddAccount` states to open new wallet actions such as backup and add accounts. **MultipleAssetIconsView.swift** - New file that introduces a view for displaying multiple asset icons together. **AccountsStore.swift** - New file introducing `AccountsStore`, which serves as a state store for managing account information, balances, and prices. **CryptoStore.swift** - Added `accountsStore` property for managing account-related data separately. **PendingTransactionView.swift & SaferSignTransactionContainerView.swift** - Modified gas fee edit button title to a more generic "edit" title. **BraveWalletExtensions.swift** - Added extension to `BraveWallet.AccountInfo` for generating a description string for different account types. **MultipleCircleIconView.swift** - Fixed icon and background rendering within the circle shape. **NetworkIcon.swift & NetworkIconView.swift** - Introduced `NetworkIconView` to wrap and size `NetworkIcon` properly. **Preview MockStores.swift** - Added `previewStore` for `AccountsStore`. **WalletStrings.swift** - Added localization strings for the new UI elements.

Security Hotspots

  1. AccountsView.swift: handle function

    Changes include user-triggered actions that might lead to data exposure if not handled correctly (e.g., account export). It is important to ensure that actions such as exporting private keys are securely verified and that any sensitive data is protected.

  2. AccountsStore.swift: updateBalancesAndPrices function

    The async operation fetches balances and prices, which could be a potential vector for race conditions or data inconsistency if not correctly synchronized.

  3. MainMenuView.swift: Button actions for backup and add accounts

    Backup operations should always require user authentication, and no sensitive data should be displayed without sufficient protection.

  4. CryptoStore.swift: accountsStore state management

    Ensure that any shared data between stores does not lead to unauthorized access or state mutations across different views or components.