Feat/22838 asset picker input in trading (swap & buy) #23314
+2,282
−1,472
Conversation
This file contains hidden or 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
cermakjiri
force-pushed
the
feat/22838-swap-to-asset-picker
branch
from
7ab878e to
2e79284
Compare
cermakjiri
force-pushed
the
feat/22838-top-assets
branch
from
d9b6ecb to
79e681a
Compare
Merged
cermakjiri
force-pushed
the
feat/22838-top-assets
branch
4 times, most recently
from
31a477b to
e0b6ee6
Compare
cermakjiri
force-pushed
the
feat/22838-swap-to-asset-picker
branch
7 times, most recently
from
4cb762f to
0faa786
Compare
cermakjiri
force-pushed
the
feat/22838-top-assets
branch
from
e0b6ee6 to
fef5c22
Compare
cermakjiri
force-pushed
the
feat/22838-swap-to-asset-picker
branch
2 times, most recently
from
cf07501 to
e8f1417
Compare
cermakjiri
force-pushed
the
feat/22838-top-assets
branch
from
fef5c22 to
a586d57
Compare
cermakjiri
force-pushed
the
feat/22838-swap-to-asset-picker
branch
from
e8f1417 to
540db0f
Compare
cermakjiri
force-pushed
the
feat/22838-top-assets
branch
from
a586d57 to
11b799f
Compare
cermakjiri
force-pushed
the
feat/22838-swap-to-asset-picker
branch
from
540db0f to
32d3d5f
Compare
cermakjiri
force-pushed
the
feat/22838-top-assets
branch
from
11b799f to
d4e9aa0
Compare
cermakjiri
force-pushed
the
feat/22838-swap-to-asset-picker
branch
from
32d3d5f to
6ef4f5f
Compare
cermakjiri
force-pushed
the
feat/22838-swap-to-asset-picker
branch
from
6ef4f5f to
1aaf0d9
Compare
cermakjiri
force-pushed
the
feat/22838-swap-to-asset-picker
branch
from
f18e159 to
43d85eb
Compare
cermakjiri
force-pushed
the
feat/22838-swap-to-asset-picker
branch
from
43d85eb to
1b19b9d
Compare
cermakjiri
force-pushed
the
feat/22838-swap-to-asset-picker
branch
2 times, most recently
from
45b78a9 to
73d8573
Compare
Contributor
Author
|
Yes, I agree the PR got much bigger than I initially intended. To make it little bit easier I've divided it to special commits so it's possible to review one group of changes at the time. 🙏 |
cermakjiri
requested review from
a team and
izmy
and removed request for
a team
cermakjiri
force-pushed
the
feat/22838-swap-to-asset-picker
branch
from
7374854 to
c595ea2
Compare
- Add a simple hook called `useCoinsAndPlatforms` that memoizes the trading info from Redux and returns callback with stable reference for getting the data. - The `useTradeingInfo` hook was incorrectly memoize. The `useTradeingUtils` uses the correctly memoized `useCoinsAndPlatforms` hook.
If clicking on disabled input, the click propagates to the form and submits it (some form values are then pushed to URL search params).
- Display `NetworkIcon` even if AssetLogo image failed to load. - Prevent opening search asset dropdown on focus (fixes bug when you clicked on an dropdown option and the dropdown got closed and opened agained). Plus add testing IDs. - TopAssets: add style for hover state and add test ID.
…or global send and receive Don't use common component since global send and receive is attached to Redux.
…picker in trading Plus fix memo. issue in useModal.
- Create a new hooks for handling trading asset options.
- It's now currectly memoized, greatly simplified and provides set of network symbols found in the trading info data.
- Also, the types has been greatly improved. There's now `asset.isNativeToken` boolean which easily narrows types.
- However, this change required to migrate all old types related to this hook including those ones in suite-native workspace.
- Instead of `toCryptoOption` method, there're now `createAssetOption` and `createDefaultAssetOption` method. Also the `useTradingAssets` hook now returns `createAssetOptionFromCryptoId` method.
…ap and quotes update in buy
cermakjiri
force-pushed
the
feat/22838-swap-to-asset-picker
branch
from
c595ea2 to
e8b5b55
Compare
Contributor
|
✅ Previously successful run of [Test] PR Suite Web e2e tests workflow has been found. |
cermakjiri
force-pushed
the
feat/22838-swap-to-asset-picker
branch
from
84d9a73 to
39edc99
Compare
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.
Please, let's focus on important feedback, no NITs this time, else it will be merge never. 🙏
Design
Figma
Major changes
Minor changes
Description
toinput with asset picker in buy and swap (packages/suite/src/views/wallet/trading/common/TradingForm/TradingFormInput/TradingFormInputAssetPicker)useTradingInfowith its utils and types (toCryptoOption,TradingCryptoSelectItemProps) and adding new ones (e.g.useTradingAssets,useTradingUtils,TradingAssetOption, etc.). This even related to suite-native.useTradingInfohook (it was wrongly memoized and was causing unnecessary re-renders). Instead, add several specialized hooks:useCoinsAndPlatforms: Returns a fn for gettingcoinsandplatformsinstead of memoized object. There's ref. preventing the callback being re-validated all the time.useTradingAssets: methods for creating asset list items and getting default trading asset option. Now there're sorted as they come from API (i.e. from highest trading volume).useTradingUtils: trading utils with the context ofcoinsandplatformsuseTradingInfo:Some tokens weren't actually rendered even though they hadOnly theexchange: trueflag set (e.g. seach forzksync)zksyncwas missing.FormProvider, invalid types / unnecessary type casting, etc. in trading. That's another bunch of changes and these we hard to expect beforehand.Notes for QA
Please test the
tofield in buy and swap.Related Issue
Resolve #22838
Resolve #22835
Screenshots:
Swap
Zaznam.obrazovky.2025-12-01.v.16.44.41.mov
Buy
Zaznam.obrazovky.2025-12-01.v.16.46.38.mov
🔍🖥️ Suite web test results: View in Currents
🔍🖥️ Suite desktop test results: View in Currents
🔍🖥️ Suite native android test results: View in Currents