fix(coinmarket): send and verify modals #14230
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adderpositive
added
bug
Could you please delete this border? 🙏 (it shouldn't be white) |
jvaclavik
requested changes
Sep 10, 2024
adderpositive
force-pushed
the
fix/coinmarket-modals
branch
from
35ce994
to
22e2078
Compare
jvaclavik
approved these changes
Sep 10, 2024
adderpositive
force-pushed
the
fix/coinmarket-modals
branch
from
22e2078
to
0d92859
Compare
LukasRada
approved these changes
Sep 10, 2024
komret
requested changes
Sep 10, 2024
There was a problem hiding this comment.
I'd prefer to do this without the useEffects, if possible. They are not linked to the modals closely enough and I am afraid that it could lead to future bugs where we expect the reducer value to be cleared while it is not. Perhaps the same result could be achived with props or middlewares.
packages/suite/src/hooks/wallet/coinmarket/form/common/useCoinmarketAccount.ts
Outdated
Show resolved
Hide resolved
packages/suite/src/hooks/wallet/coinmarket/offers/useCoinmarketCommonOffers.ts
Outdated
Show resolved
Hide resolved
|
Great improvements and added unit tests 👏 But note that some unit tests are failing. |
adderpositive
force-pushed
the
fix/coinmarket-modals
branch
2 times, most recently
from
0f448ab
to
b6471e4
Compare
|
I have found one more bug. It should be ETH account, not BTC in exchange. |
should be fixed |
|
/rebase |
|
Start rebasing: https://github.com/trezor/trezor-suite/actions/runs/10811779764 |
trezor-ci
force-pushed
the
fix/coinmarket-modals
branch
from
549d977
to
8713f5a
Compare
komret
approved these changes
Sep 11, 2024
|
QA OK Info:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
1. An incorrectly selected account in the verify step
Describe the bug
Instead of the currently selected Trezor account, the account from the exchange form should be selected.
Expected behavior
You should be able to interact with an ETH account, not a Polygon account in the send modal.
How to reproduce
FromETHToUSDT and someAmountto proceed with the tradeConfirm on Trezor & sendin the tradeScreenshot
Correct behavior
Related Issue
Resolve #14218
2. An incorrect account is selected in the Send modal form
Describe the bug
Incorrect account selected in the Verify step – this can be recognized by the presence of 'Including tokens' when buying BTC.
Expected behavior
There should be selected the correct account and should not be visible
Including tokenssubheading.How to reproduce
You buyBTCBuyConfirm on Trezor(verify)Buggy
Expected behavior
Related Issue
Resolve #14231
Other info
Please also check if the Send form is not affected.