Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Fix #7765, Fix #7618: Portfolio Group By filter & Empty State #7826

Merged
merged 10 commits into from
Aug 11, 2023

Conversation

StephenHeaps
Copy link
Contributor

@StephenHeaps StephenHeaps commented Aug 3, 2023

Summary of Changes

  • Implements the Group By filter with none (default), accounts and networks grouping options.
  • Implements loading state when loading balances.
    • Loading state is only shown if hiding small balances, OR assets are grouped (groups with 0 total balance are hidden)

This pull request fixes #7765, fixes #7618

Submitter Checklist:

  • Unit Tests are updated to cover new or changed functionality
  • User-facing strings use NSLocalizableString()
  • New or updated UI has been tested across:
    • Light & dark mode
    • Different size classes (iPhone, landscape, iPad)
    • Different dynamic type sizes

Test Plan:

  1. Open Wallet Portfolio, it should not be grouped by default.
  2. Tap Filters & Display Settings button and then tap Reset
  3. Verify default Group By option is None.
  4. Change Group By to accounts and tap Save Changes.
  5. Verify Portfolio is now grouped by each account, with the assets below showing the balance only for that account.
    • Note: Groups with 0 total balance are hidden automatically if Hide Small Balances is enabled.
  6. Verify groups are sorted by fiat value.
  7. Tap Filters & Display Settings button, then de-select some accounts and tap Save Changes
  8. Verify de-selected account groups are no longer shown.
  9. Tap Filters & Display Settings button then tap Reset (re-selects all accounts again), then change Group By to Networks and tap Save Changes
  10. Verify Portfolio is now grouped by each network with only assets for that network shown in each group.
    • Note: Groups with 0 total balance are hidden automatically if Hide Small Balances is enabled.
  11. Verify groups are sorted by fiat value.
  12. Tap Filters & Display Settings button then tap Reset.
  13. De-select all accounts, or all networks and tap Save Changes.
  14. Verify empty state is shown.
    • Empty state should be shown whenever no assets are displayed. This can occur with many combinations, for example hiding small balances, grouping assets and then hiding accounts with balances or hiding networks with balances, etc.
  15. Close the wallet, or quit Brave. Re-open Portfolio and verify Group By filter is preserved.

Screenshots:

portfolio.grouping.mp4

Reviewer Checklist:

  • Issues include necessary QA labels:
    • QA/(Yes|No)
    • bug / enhancement
  • Necessary security reviews have taken place.
  • Adequate unit test coverage exists to prevent regressions.
  • Adequate test plan exists for QA to validate (if applicable).
  • Issue and pull request is assigned to a milestone (should happen at merge time).

@StephenHeaps StephenHeaps requested a review from nuo-xu August 3, 2023 19:03
@StephenHeaps StephenHeaps requested a review from a team as a code owner August 3, 2023 19:03
@StephenHeaps StephenHeaps self-assigned this Aug 3, 2023
@nuo-xu
Copy link
Contributor

nuo-xu commented Aug 4, 2023

is it possible to substitute the disclosure symbol?

simulator_screenshot_4213928C-DFCB-4C1A-AC45-6548149A6E72 Simulator Screenshot - iPhone 14 - 2023-08-04 at 09 46 17 Screenshot 2023-08-04 at 9 47 30 AM
Current Similar in Wallet Desktop

@nuo-xu
Copy link
Contributor

nuo-xu commented Aug 4, 2023

once assets are grouped, some rows have a divider but some don't (in the screenshot, there is no divider under Wrapped Ether) maybe related to the padding of each row.
Screenshot 2023-08-04 at 10 02 11 AM

@nuo-xu
Copy link
Contributor

nuo-xu commented Aug 4, 2023

want to mention here again.

Note: Groups with 0 total balance are hidden automatically.

looks like a bug. wdyt? cc @jamesmudgett @srirambv

@nuo-xu
Copy link
Contributor

nuo-xu commented Aug 4, 2023

not sure if this is what we want. but its a minor issue.
for assets with zero balance will be re-ordered (core-data doesn't gives same order), if i tap save changes with no actual changes.
https://github.com/brave/brave-ios/assets/1187676/d52fa84a-026e-4535-9799-734f0b3ffc86

@StephenHeaps StephenHeaps force-pushed the wallet/portfolio-grouping branch from 7ee627e to a94a5dd Compare August 9, 2023 23:45
@StephenHeaps
Copy link
Contributor Author

is it possible to substitute the disclosure symbol?

Add a custom WalletDisclosureGroup for this as DisclosureGroupStyle is iOS 16+. a94a5dd

once assets are grouped, some rows have a divider but some don't (in the screenshot, there is no divider under Wrapped Ether) maybe related to the padding of each row.

Padding and separator issue should be resolved now.

want to mention here again.
Note: Groups with 0 total balance are hidden automatically.
looks like a bug. wdyt?

Groups are now only hidden here now when Hide Small Balances is enabled. eac4c70

not sure if this is what we want. but its a minor issue.
for assets with zero balance will be re-ordered (core-data doesn't gives same order), if i tap save changes with no actual changes.

Updated sort logic when sorting empty balances by value to move native token(s) to top of list, otherwise will sort alphabetically when 2+ assets have 0 balance 92eadf2.

@StephenHeaps StephenHeaps requested a review from nuo-xu August 9, 2023 23:50
@StephenHeaps StephenHeaps added this to the 1.57 milestone Aug 10, 2023
@nuo-xu
Copy link
Contributor

nuo-xu commented Aug 10, 2023

all my comments have been addressed. Portfolio looks pretty awesome now! good work here!

@StephenHeaps StephenHeaps merged commit 79d46fb into development Aug 11, 2023
@StephenHeaps StephenHeaps deleted the wallet/portfolio-grouping branch August 11, 2023 15:20
arthuredelstein pushed a commit to brave/brave-core that referenced this pull request Feb 13, 2024
…y filter & Empty State (brave/brave-ios#7826)

* Support Group By Filter & Display Setting in Portfolio

* Update PortfolioStoreTests to test each filter separately, add tests for grouping by accounts and grouping by networks

* Add empty state, loading state to Portfolio Assets

* Sort order of 0 balance assets can change when tapping save filters without changing filters.

* Update `Hide Small Balances` threshold from $1 to $0.05.
Update logic so groups with no balance are only hidden when `Hide Small Balances` is enabled

* Add new `WalletDisclosureGroup` replacement for `DisclosureGroup` which uses `leo.carat.down` for the expand button and has no indent. Fix group sorting for iOS 16.4 ..< iOS 17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Portfolio Group By Filter Portfolio Filters Empty State(s)
2 participants