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.

Sign up for GitHub

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

Jump to bottom

Fix/cardano #12319

Merged
merged 4 commits into from
May 9, 2024
Merged

Fix/cardano #12319

merged 4 commits into from
May 9, 2024

Conversation

tomasklim
Copy link
Member

@tomasklim tomasklim commented May 8, 2024 •
edited
Loading

Description

  • there was endless loading for cardano tokens when coingecko fiat fetch failed
  • there was loading state when fiat rate had error
  • solana and cardano had different logic for missing symbol name, unified
  • remove using custom cardano componens for select. Now it is same for EVM. Solana and Cardano.
    • I guess we will have to improve it anyway in the future to show contract address somehow, maybe on hover..

Related Issue

Fix #11539

Screenshots:

Screenshot 2024-05-08 at 16 51 23
Screenshot 2024-05-08 at 16 51 12
Screenshot 2024-05-08 at 16 51 05
Screenshot 2024-05-08 at 16 49 46
Screenshot 2024-05-08 at 16 49 30
Screenshot 2024-05-08 at 16 49 20

@tomasklim tomasklim requested a review from marekrjpolak as a code owner May 8, 2024 14:56
@tomasklim tomasklim added cardano Related to Cardano (ADA) blockchain platform solana labels May 8, 2024
marekrjpolak
marekrjpolak approved these changes May 8, 2024
View reviewed changes
Copy link
Contributor

@marekrjpolak marekrjpolak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't reviewed UI part, the rest seems good.

AdamSchinzel
AdamSchinzel approved these changes May 9, 2024
View reviewed changes
Copy link
Contributor

@AdamSchinzel AdamSchinzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MiroslavProchazka
Copy link
Contributor

Please adjust the commit messages and squash it to preferably one commit. Thx!

@MiroslavProchazka
Copy link
Contributor

/rebase

@github-actions GitHub Actions
Copy link

github-actions bot commented May 9, 2024

Start rebasing: https://github.com/trezor/trezor-suite/actions/runs/9017194051

@tomasklim tomasklim merged commit fc6f517 into develop May 9, 2024
23 checks passed
@tomasklim tomasklim deleted the fix/cardano branch May 9, 2024 12:44
@tomasklim tomasklim mentioned this pull request May 10, 2024
Closed
@bosomt
Copy link
Contributor

bosomt commented May 14, 2024

QA OK

Info:

  • Suite version: desktop 24.5.2 (1ca8b80)
  • Browser: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) TrezorSuite/24.5.2 Chrome/118.0.5993.159 Electron/27.3.8 Safari/537.36
  • OS: MacIntel
  • Screen: 1512x982
  • Device: Trezor T2T1 2.7.0 regular (revision 45e8a842a31e62a6d43d7f6ccac62a45e1198ef0)
  • Transport: BridgeTransport 2.0.33

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marekrjpolak marekrjpolak marekrjpolak approved these changes

@AdamSchinzel AdamSchinzel AdamSchinzel approved these changes

Assignees
No one assigned
Labels
cardano Related to Cardano (ADA) blockchain platform solana
Projects
Suite Desktop
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Short symbol name unrecognised tokens include '...' in its name in send form
5 participants
@tomasklim @MiroslavProchazka @bosomt @marekrjpolak @AdamSchinzel