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

EPIC: Simplified Send Transaction #16696

Open
30 of 38 tasks
Khushboo-dev-cpp opened this issue Nov 4, 2024 · 5 comments
Open
30 of 38 tasks

EPIC: Simplified Send Transaction #16696

Khushboo-dev-cpp opened this issue Nov 4, 2024 · 5 comments

Comments

@Khushboo-dev-cpp
Copy link
Contributor

Khushboo-dev-cpp commented Nov 4, 2024

Description

As a first step in stabilising and having a bug free send, the team has decided to go ahead with single network send as a version 1.

This epic covers all the changes that need to be done in send as finalised by the team.

Designs: https://www.figma.com/design/FkFClTCYKf83RJWoifWgoX/Wallet-v2?node-id=25182-169274&t=Ech2sOd4zCGhGFzC-4
User Stories: https://www.notion.so/Making-transactions-Send-Bridge-Simplified-1128f96fb65c80d78d6ed0788776e98f?pvs=4

SubComponents Rework:

Main Tasks:

Miscellaneous TBD:

@micieslak
Copy link
Member

I left some comments in subtasks.

Moreover I noticed one big thing necessary to do along with the planned simplifications. It's consequence of design changes regarding scrolling:

Image

It means that top level layout of SendModal needs to be rethought and organised from scratch with that scrolling behavior in mind. Everything above the list should be put into ListView header probably.

@Khushboo-dev-cpp
Copy link
Contributor Author

I left some comments in subtasks.

Moreover I noticed one big thing necessary to do along with the planned simplifications. It's consequence of design changes regarding scrolling:

Image

It means that top level layout of SendModal needs to be rethought and organised from scratch with that scrolling behavior in mind. Everything above the list should be put into ListView header probably.

@micieslak this behaviour only applies when screen size < 720 px.

But your suggestion for moving everything above the list to listview header makes sense. We could play with sticky property based on screen height.

@anastasiyaig anastasiyaig modified the milestones: 2.32.0 Beta, 2.33.0 Beta Nov 25, 2024
@anastasiyaig
Copy link
Contributor

@alaibe @Khushboo-dev-cpp I moved that to 2.33 milestone as we discussed

@virginiabalducci
Copy link

virginiabalducci commented Jan 29, 2025

Hi @Khushboo-dev-cpp

I've started testing and I'll post issues or questions to validate whether they are issues. I'll edit this comment as I go

  • ### Issue 1

There is a padding or margin issue in the 'Assets' tab of the 'Select token' drop-down

Image

[Khushboo]: Should be fixed in latest by https://github.com/status-im/status-desktop/pull/17133

QA: verified as fixed on latest master ✅

  • ### Issue 2

The Network drop-down should be disabled when the user has selected a collectible and has opened the send modal

send.option.from.the.collectibles.details.-.network.should.be.disabled.mov
Given the user is in the All Accounts view
When the user chooses and clicks on a collectible (ERC1155 or ERC721)
When the user clicks the Send option from the footer in Collectibles detail
Then the Send modal is launched with the Account that hodls the collectible
Then the Send modal has the chain on which the collectible resides selected (dropdown disabled)
Then the Send modal has the selected collectible preselected
Then the Send modal has no preselected recipient
Then the Amount input is removed

[Khushboo]: I didnt implement this on purpose and have handled , that if the user switches network and collectible is not on that chainId the TokenSelector is reset. @benjthayer do you think its a good idea to not keep it disabled?

  • ### Issue 3

When the send modal is launched from a saved address, the name of the saved address is not displayed in the Send modal

saved.address.name.is.not.showing.mov

Actual result:
Image

Expected result:
Image

  • ### Issue 4

'To' element tabs 'Recent', 'Saved' and 'My accounts' show the same items.
It seems all tabs show the address list from the 'Recent' tab.
'Saved' tab should only show 1 account for this user
'My accounts' tab should only show the user's accounts

This console error triggers when clicking the tabs:

WRN 2025-01-29 20:51:09.644Z qt warning topics="qt" tid=18086490 text="TypeError: Cannot assign to read-only property \"selectedRecipientType\"" file=qrc:/app/AppLayouts/Wallet/panels/RecipientSelectorPanel.qml:159 category=default

Screen.Recording.2025-01-29.at.5.50.58.PM.mov

[Khushboo]: Ive fixed this here https://github.com/status-im/status-desktop/pull/17150/files#

QA: verified as fixed in the latest master build ✅

  • ### Issue 5 - this one may be related to Issue 4

When selecting a specific owned account that is not the default account, the tabs of the 'To' section (Recent, Saved, My accounts) are empty.

Screen.Recording.2025-01-29.at.6.02.57.PM.mov

QA: verified as fixed in the latest master build ✅

  • ### Issue 6

No error indicator shows when user changes from an account with balance to an account with no balance.

QA: This looks fixed on the latest master ✅

SD-408:

Given an asset and amount to send is already set
When the user clicks on an account
And that account has no balance for the preselected asset
Then that account becomes selected account
And Max button is highlighted in red
And input value is highlighted in red
And Error message is shown at the bottom with an option to add token

Instead of the expected result (highlighted items in red) the account shows the 'select token' button

Screen.Recording.2025-01-30.at.4.16.45.PM.mov

This error shows on the console log

ERR 2025-01-30 19:15:15.915Z qt error topics="qt" tid=18917649 text="\nfromExponent (qrc:/StatusQ/Core/Utils/AmountsArithmetic.qml:100)\nexpression for selectedAmount (qrc:/app/AppLayouts/Wallet/adaptors/SignSendAdaptor.qml:56)\n (qrc:/app/AppLayouts/Wallet/popups/simpleSend/SimpleSendModal.qml:276)\n (qrc:/StatusQ/Core/Backpressure/Backpressure.qml:108)\n (qrc:/StatusQ/Core/Backpressure/Backpressure.qml:33)" file=qrc:/StatusQ/Core/Utils/AmountsArithmetic.qml:100 category=default

Issue 7

Switching between balance in Fiat and balance in assets changes to 0,00 Fiat when switching amounts when the user does not have enough balance.

  1. Open the Send modal
  2. Select a token, for example SNT
  3. Before entering a number, switch to fiat amount
  4. Enter an amount higher than the account's balance
  5. click on the Switch icon to switch to SNT balance
  6. Notice that the fiat balance below the SNT balance is 0,00
Screen.Recording.2025-01-30.at.4.27.39.PM.mov

Issue 8

Dark mode - hovering the items of the account selector do not highlight the items.
This works fine with other elements such as Network selector or token selector.

Screen.Recording.2025-01-30.at.4.35.46.PM.mov

Issue 9

Issues with the 'Not enough ETH to pay for gas fees' error message:

  1. There is no 'Add ETH' button when the error of insufficient funds to pay for gas fees shows
  2. When this error displays, there is a button to show details, but when expanded, there are no details to show.
Screen.Recording.2025-01-31.at.12.55.40.PM.mov
  • Issue 10

Issue 10

QA: this looks as fixed on the latest master ✅

If an account does not have balance, at least we should show 3 tokens even with 0 balance, SNT, ETH and DAI.

Image

Should we also allow the user to search and select other tokens? The user may want to know how much a send transaction would cost (check fiat vs token balance, check fees, etc) even if they don't have any balance.

Image

Would be also worth checking this AC:

Given send modal is opened with preselected asset parameter (e.g. asset details, asset list context menu)
Then asset selector already has asset selected

Issue 11

  • Issue 11
    QA: this looks as fixed on the latest master ✅

If the asset selected has 0 balance, user should still see the asset pre-selected when launching the Send modal?
For example in the case of this AC, does this also apply to accounts with no balance?

Given send modal is opened with preselected asset parameters (e.g. asset details, asset list context menu)
Then, the asset selector already has the asset selected

Issue 12

  • Issue 12

When an asset is selected the amount input should be shown with input focus

User story: SD-375 - Asset selection

Given the send modal is open
When an asset is selected
Then the amount input is shown with input focus

As per the screen recording, the input focus should show to indicate user they can enter an amount.

Screen.Recording.2025-02-03.at.2.42.40.PM.mov

Issue 13

  • Issue 13

When an asset is already selected and the user opens the asset selector, the selected asset should shown as highlighted on the item list.

User story: SD-375 - Asset selection

Given send modal is open and any asset is selected
When asset selector is clicked
Then asset selection popup is showed and currently selected asset visible in the list as highlighted item

Image

Screen.Recording.2025-02-03.at.2.46.56.PM.mov

@virginiabalducci
Copy link

Issue 14

Send a collectible flow - Sign transaction modal: Rejecting or signing the transaction will lead the user to a blank screen with an invalid or incorrect collectible.
This happens with ERC-721 and ERC-1155 tokens

Screen.Recording.2025-02-04.at.9.35.41.AM.mov

Image

Screen.Recording.2025-02-04.at.9.28.30.AM.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

No branches or pull requests

5 participants