Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redesign Account History #1737

Closed
wants to merge 69 commits into from
Closed

Redesign Account History #1737

wants to merge 69 commits into from

Conversation

CassioMG
Copy link
Contributor

Closes #1542

This PR redesigns the Account History screen on both the main History tab and the individual asset History.

Screenshot 2024-12-12 at 17 04 37 Screenshot 2024-12-12 at 17 05 19 Screenshot 2024-12-12 at 17 05 34 Screenshot 2024-12-12 at 17 09 07 Screenshot 2024-12-12 at 17 08 55 Screenshot 2024-12-12 at 17 08 36

Navigating on main History tab

Screen.Recording.2024-12-12.at.17.29.40.mov

Navigating on each asset History

Screen.Recording.2024-12-12.at.17.09.28.mov

aristidesstaffieri and others added 30 commits November 15, 2024 10:32
* makes swap amount persist across destination asset changes

* updates swap test state to init with balances
* Allow creating a new wallet from initial screen

* Added translations
* maintain password when switching between accounts

* Added translations

* add test and update e2e token
* enable e2e tests in CI and add cleanup tasks

* fix asset code change
* Added translations

* Add 'Disconnect all' button

* Added translations

* Display favicon
* Skip password when creating new address

* Better copy for Enter Password screen

* Added translations

* Allow user to input password again in case of error + remove spinner while redirecting
* adds use-get-history hook, and replaces data fetching for history using it

* removes stray log

* move loading render branch out of main render to avoid ternary

* tweak hasEmptyHistory check to account for an empty operations list

* renames use get history hook file to match repo conventions

* renames var to follow code convention
* redesign the Create New Wallet screens and update tests

* rm console.logs

* fix checks in e2e login helpers

* updating recovery snapshot. this screen will be redesigned in next PR

* fix incorrect mnemonic phrase test
* Redesign Unfunded account notification

* Add "Learn more" link on notification body
* redesign recover flow and add tests

* Added translations

* rm unnecessary pw check in cleanup and increase threshold of 24 word snapshot

* fix threshold adjustment

* make Toggle component tab-able

* use box-shadow to align with Select

* updating cleanup tasks in e2e tests
* redesign account view + account details QR code

* apply same styling to network selector dropdown

* fix checks for both dropdowns

* Charles PR comments
* add github actions for deploying @stellar/freighter-api

* use yarn to config tag and commit strings
* Redesign footer on UnlockAccount

* Redesign input and button on UnlockAccount

* Redesign text on UnlockAccount

* Add identicon UI on UnlockAccount

* Add service to set/load last used account

* Workaround lint error

* UI tweak

* Extract reusable EnterPassword component

* Use EnterPassword component on AddAccount screen

* Use EnterPassword component on VerifyAccount screen

* Added translations

* Redesign NavBar for UnlockAccount screen

* Make 1.5rem padding consistent in all screen contents (Figma)

* Use pxToRem()

* Hide Identicon on EnterPassword component in case address is not provided

* Avoid using same name of the duck function

* Update lastUsedAccount on Account screen instead of Sign out action

* Change default description for EnterPassword

* Added translations

* Tweak text size and alignment

* Set lastUsedAccount every time user subscribes to an account
Bumps the all-actions group with 7 updates in the / directory:

| Package | From | To |
| --- | --- | --- |
| [actions/checkout](https://github.com/actions/checkout) | `2` | `4` |
| [actions/setup-node](https://github.com/actions/setup-node) | `1` | `4` |
| [jossef/action-set-json-field](https://github.com/jossef/action-set-json-field) | `1` | `2` |
| [restackio/update-json-file-action](https://github.com/restackio/update-json-file-action) | `1e34d747ca044df80b37bc4ef5a9aa41be9dac8f` | `f8ef1561cb15ba86a6367b547216375bc60e7f91` |
| [rtCamp/action-slack-notify](https://github.com/rtcamp/action-slack-notify) | `2.3.0` | `2.3.2` |
| [frabert/replace-string-action](https://github.com/frabert/replace-string-action) | `1.2` | `2.5` |
| [ruby/setup-ruby](https://github.com/ruby/setup-ruby) | `1.138.0` | `1.203.0` |



Updates `actions/checkout` from 2 to 4
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2...v4)

Updates `actions/setup-node` from 1 to 4
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v1...v4)

Updates `jossef/action-set-json-field` from 1 to 2
- [Release notes](https://github.com/jossef/action-set-json-field/releases)
- [Commits](jossef/action-set-json-field@v1...2a0f7d9)

Updates `restackio/update-json-file-action` from 1e34d747ca044df80b37bc4ef5a9aa41be9dac8f to f8ef1561cb15ba86a6367b547216375bc60e7f91
- [Release notes](https://github.com/restackio/update-json-file-action/releases)
- [Commits](restackio/update-json-file-action@1e34d74...f8ef156)

Updates `rtCamp/action-slack-notify` from 2.3.0 to 2.3.2
- [Release notes](https://github.com/rtcamp/action-slack-notify/releases)
- [Commits](rtCamp/action-slack-notify@4e5fb42...c337377)

Updates `frabert/replace-string-action` from 1.2 to 2.5
- [Release notes](https://github.com/frabert/replace-string-action/releases)
- [Commits](frabert/replace-string-action@v1.2...b6828c5)

Updates `ruby/setup-ruby` from 1.138.0 to 1.203.0
- [Release notes](https://github.com/ruby/setup-ruby/releases)
- [Changelog](https://github.com/ruby/setup-ruby/blob/master/release.rb)
- [Commits](ruby/setup-ruby@v1.138.0...v1.203.0)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: all-actions
- dependency-name: actions/setup-node
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: all-actions
- dependency-name: jossef/action-set-json-field
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: all-actions
- dependency-name: restackio/update-json-file-action
  dependency-type: direct:production
  dependency-group: all-actions
- dependency-name: rtCamp/action-slack-notify
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: all-actions
- dependency-name: frabert/replace-string-action
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: all-actions
- dependency-name: ruby/setup-ruby
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: all-actions
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@CassioMG CassioMG linked an issue Dec 12, 2024 that may be closed by this pull request
@JakeUrban
Copy link
Contributor

Overall this looks great. Just two questions / suggestions:

  • In the balances view, do we need the ticker after the amount, since the ticker is on the left as well?
  • On the history view (both the global view and per-token), does it make sense to have the "N/A" icon, as opposed to just leaving that area blank?

@piyalbasu
Copy link
Contributor

Here are some screenshots of what Soronban invocations look like with the new design. I think we may need to tweak some logic here. For ex, a sent Soroban transfer should use the same airplane icon. And I got different renders for 2 mint transactions when I'd expect them to be the same. (A mint is when the owner of a token creates new tokens and then sends them to a user)

This is for account GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF on Testnet

Screenshot 2024-12-12 at 5 42 55 PM Screenshot 2024-12-12 at 5 42 41 PM Screenshot 2024-12-12 at 5 42 12 PM

@CassioMG
Copy link
Contributor Author

  • In the balances view, do we need the ticker after the amount, since the ticker is on the left as well?

@JakeUrban do you mean removing the token code on the right here? That's actually an interesting idea so we'd have more room for the amount number
Screenshot 2024-12-13 at 10 14 42

  • On the history view (both the global view and per-token), does it make sense to have the "N/A" icon, as opposed to just leaving that area blank?

@JakeUrban I'm not against it. But curious to hear @sdfcharles thoughts on it.

Thanks for reviewing!

@CassioMG
Copy link
Contributor Author

Here are some screenshots of what Soronban invocations look like with the new design. I think we may need to tweak some logic here. For ex, a sent Soroban transfer should use the same airplane icon. And I got different renders for 2 mint transactions when I'd expect them to be the same. (A mint is when the owner of a token creates new tokens and then sends them to a user)

This is for account GBTYAFHGNZSTE4VBWZYAGB3SRGJEPTI5I4Y22KZ4JTVAN56LESB6JZOF on Testnet

@piyalbasu thanks a lot for the help testing those. I haven't changed our existing Soroban logic on this PR since it looks a bit complex so I basically added the Icon.User01 component behind the existing arrow icons to keep the pattern. I'll try to repro those same Soroban transactions you submitted with your test user and tweak the logic a bit to accommodate it.

@aristidesstaffieri
Copy link
Contributor

Screenshot 2024-12-13 at 8 45 42 AM

I think these rows have a potential to overflow their columns. Since we intend to show token transfers as sends, I think we should include some overflow/truncation of the columns. A custom token can define their decimals to be higher than what we see here which would cause the balances string to be longer.

@sdfcharles
Copy link

sdfcharles commented Dec 13, 2024

  • In the balances view, do we need the ticker after the amount, since the ticker is on the left as well?

@JakeUrban do you mean removing the token code on the right here? That's actually an interesting idea so we'd have more room for the amount number Screenshot 2024-12-13 at 10 14 42

  • On the history view (both the global view and per-token), does it make sense to have the "N/A" icon, as opposed to just leaving that area blank?

@JakeUrban I'm not against it. But curious to hear @sdfcharles thoughts on it.

Thanks for reviewing!

hey @CassioMG - i'm good to make both of these changes that @JakeUrban suggested

  • drop ticker
  • drop N/A badge

@sdfcharles
Copy link

@CassioMG everything lgtm, only one change:

  • update these buttons to tertiary stylling (dark bg)

CleanShot 2024-12-13 at 13 05 55

@CassioMG
Copy link
Contributor Author

Screenshot 2024-12-13 at 8 45 42 AM

I think these rows have a potential to overflow their columns. Since we intend to show token transfers as sends, I think we should include some overflow/truncation of the columns. A custom token can define their decimals to be higher than what we see here which would cause the balances string to be longer.

@aristidesstaffieri that's a good point, thanks for raising it.
So below (image on the left) is how the SDS Badge component behaves for long texts. The Badge component does not seem very editable (e.g. it does not take addlClassName prop as other SDS components do =/) so that apparently we can't configure it to truncate with ellipsis. With that in mind I'm wondering if a a quick fix like reducing the Badge max width a little bit could be enough for this issue (image on the right) so that it doesn't get too close to the other label. Wdyt?
Screenshot 2024-12-13 at 18 31 16

@aristidesstaffieri
Copy link
Contributor

Screenshot 2024-12-13 at 8 45 42 AM
I think these rows have a potential to overflow their columns. Since we intend to show token transfers as sends, I think we should include some overflow/truncation of the columns. A custom token can define their decimals to be higher than what we see here which would cause the balances string to be longer.

@aristidesstaffieri that's a good point, thanks for raising it. So below (image on the left) is how the SDS Badge component behaves for long texts. The Badge component does not seem very editable (e.g. it does not take addlClassName prop as other SDS components do =/) so that apparently we can't configure it to truncate with ellipsis. With that in mind I'm wondering if a a quick fix like reducing the Badge max width a little bit could be enough for this issue (image on the right) so that it doesn't get too close to the other label. Wdyt? Screenshot 2024-12-13 at 18 31 16

Oh nice I thought the pill would overflow as well so that's not as bad. Yeah it's too bad we can't tweak the classname in order to make the pill truncate the text. I think this works, this should be a less common edge case. Thanks!

Base automatically changed from release/5.26.0 to master December 16, 2024 20:25
@CassioMG CassioMG removed a link to an issue Jan 14, 2025
@CassioMG
Copy link
Contributor Author

Closing this in favor of #1785 since this one got too out of sync from master and rebasing this existing branch was too messy. I'll address the remainder PR comments on the new PR.

@CassioMG CassioMG closed this Jan 14, 2025
@CassioMG CassioMG deleted the cg-redesign-history branch January 16, 2025 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redesign Account History
5 participants