Swaps: Asset to Pay / Asset to Receive #21140
Conversation
Jenkins BuildsClick to see older builds (114)
|
alwx
force-pushed
the
feature/20332-20333
branch
from
966bc97
to
29ecd46
Compare
alwx
requested review from
ilmotta,
ulisesmac,
smohamedjavid and
briansztamfater
and removed request for
ilmotta
alwx
changed the title
alwx
changed the title
✔️ status-mobile/prs/android-e2e/PR-21140#8 🔹 ~6 min 25 sec 🔹 de3d5b4 🔹 📦 android-e2e package |
alwx
force-pushed
the
feature/20332-20333
branch
3 times, most recently
from
f1a2576
to
e9f6227
Compare
|
Hi @alwx It'd be great if you add a section with the steps to test and also some demo screenshots or video of it working to the description. It'll also help QA to test this PR There has been a lot of files changed, so any additional note on the code will also help. 👍 Thank you! |
alwx
force-pushed
the
feature/20332-20333
branch
from
bcdc558
to
9a210d4
Compare
src/status_im/contexts/wallet/sheets/select_asset/asset_list/style.cljs
Outdated
Show resolved
Hide resolved
|
Does this PR includes integration within the swap flow or just the screens? |
|
@briansztamfater @status-im/mobile-qa the integration is done as well. This is how it works: Screen.Recording.2024-09-16.at.09.31.26.mov |
alwx
force-pushed
the
feature/20332-20333
branch
2 times, most recently
from
ccd357b
to
1866a48
Compare
alwx
force-pushed
the
feature/20332-20333
branch
from
8d65295
to
e8f1fd3
Compare
|
@VolodLytvynenko ready now. Please, check issue 8, 10 and 1 — all of them should be fixed now. |
|
hi @alwx
android_recalculation.mp4
ios.mp4Logs:Android logs IOS logs |
86% of end-end tests have passedFailed tests (1)Click to expandClass TestWalletMultipleDevice:
Passed tests (6)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestWalletMultipleDevice:
Class TestCommunityOneDeviceMerged:
|
@alwx Perhaps I didn't understand you here. Issue 10 is still reproducible for both recovered and new users after swap feature flag is enabled, and popular assets section is not shown until the user logs out and logs back in new_user.mp4logs: |
|
issue 1 is fixed |
|
@VolodLytvynenko issue 10 is fixed (I don't think it's an issue though because the loading happens on initialization — which is exactly what we needed because there won't be any feature flag in the future) issue 8 should also be fixed — I tried different approach to loading now but i'm not sure it will work in 100% of cases because there are too many things that could go wrong on that particular screen |
86% of end-end tests have passedFailed tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (6)Click to expandClass TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestWalletOneDevice:
Class TestWalletMultipleDevice:
|
Hi @alwx, thanks for the fixes! Take a look please one additional issue, now the Optimism popular assets section is limited to only 4 tokens, and I haven’t been able to see more than that, even after some relogins ISSUE 11: Optimism Assets Not Fully Fetched in "Popular Assets" SectionSteps to Reproduce:
Actual Result:Only 4 tokens are shown in the "Popular Assets" section for Optimism. 4tokens.mp4Expected Result:More popular tokens should be displayed in the "Popular Assets" section for the Optimism network, as on Desktop swap.mp4Devices:
Logs: |
@alwx Better to fix is in a follow up. WDYT? |
|
@VolodLytvynenko agree. That would make the whole process of fixing easier since it's a big PR. Regarding 4 tokens for Optimism: I've just checked and that's actually what server ( |
|
@alwx, thanks for fixing all the issues. This was a challenging feature, but I think our swap now looks even better than in some popular dapps! However, issue 8 still happens occasionally, though rarely. The important remaining issue is issue 11, which needs to be addressed in 2.31. If you think it's better to fix it in this PR, let me know, and I'll recheck it; otherwise, the PR can be merged. |
@alwx got it. Thank you for clarification |
alwx
force-pushed
the
feature/20332-20333
branch
from
231cc5b
to
633960f
Compare
fixes #20332
fixes #20333
Summary
Adds sheets for both "Asset to pay" and "Asset to receive"
Platforms
status: ready