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

[UI] Send: Manual Control 2 #12888

Draft
wants to merge 29 commits into
base: master
Choose a base branch
from

Conversation

ichthus1604
Copy link
Collaborator

@ichthus1604 ichthus1604 commented Apr 18, 2024

WIP.

New PR to minimize changes made in previous one.

image

  • Includes some minimal cleanup to various ViewModels in the Send flow (nothing too disruptive)
  • Implements the Manual Control screen as seen in the screenshot
  • the Max amount in the send screen is affected by this (only max amount as per selected coins, or wallet balance for regular flow)
  • the Privacy Suggestions are affected (only operate with the selected coins and not full wallet coins)
  • This PR depends on Add SubActionButton #12841. I have added a temporary additional "Manual Control" as a regular button beside "Send" in the main wallet UI. This is temporary until the SubActionButton is available.
  • Marked as draft until the above PR is merged.
  • @soosr in the meantime you might want to give an initial review for this PR.

Copy link
Collaborator

@soosr soosr left a comment

Choose a reason for hiding this comment

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

  • The amount isn't centralized and its look should match the Figma design.
    image
    image
  • Navigating back should be possible from Send.
  • After restringing the coins in the manual control screen, on the Transaction Preview when opening the Review Coins dialog, only the restricted coins should be in the list.
  • Selected 1 coin, clicked MAX, then the Privacy suggestions seem to be stuck. It was loading forever.
    • I was able to repro this even when selecting the whole Private pocket.
      image
    • It seems like the original transaction used a coin that I DID NOT select on the manual control screen. Probably that caused the forever loading issue too.

@soosr
Copy link
Collaborator

soosr commented Apr 22, 2024

Don't forget to add the select all button.
image

@soosr
Copy link
Collaborator

soosr commented Apr 24, 2024

@ichthus1604
#12841 is merged now, you can use the button.

@ichthus1604
Copy link
Collaborator Author

@soosr

The amount isn't centralized and its look should match the Figma design.

Fixed.

Navigating back should be possible from Send.

Fixed.

After restringing the coins in the manual control screen, on the Transaction Preview when opening the Review Coins dialog, only the restricted coins should be in the list.

Fixed.

#12841 is merged now, you can use the button.

Done.

@ichthus1604 ichthus1604 requested a review from soosr April 26, 2024 11:37
@soosr
Copy link
Collaborator

soosr commented Apr 26, 2024

  • It seems like the label management still uses all the wallet coins even if it was restricted.
    image

  • Position of the dialog buttons changed.

@ichthus1604
Copy link
Collaborator Author

@soosr

Label management still uses all the wallet coins even if it was restricted

Implemented this. I'm not sure it's working 100% correctly, because I don't have a wallet with many different labels to test with. Can you confirm?

@soosr
Copy link
Collaborator

soosr commented Apr 30, 2024

@soosr

Label management still uses all the wallet coins even if it was restricted

Implemented this. I'm not sure it's working 100% correctly, because I don't have a wallet with many different labels to test with. Can you confirm?

Works fine!

Copy link
Collaborator

@soosr soosr left a comment

Choose a reason for hiding this comment

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

@ichthus1604
Copy link
Collaborator Author

@soosr

The position of Back, Cancel, and Continue buttons on the dialog is wrong.
The Amount background is visible when there is no selection.
When the Amount become visible, the position of Cancel and Continue button shifts.

Fixed.

@soosr
Copy link
Collaborator

soosr commented May 7, 2024

@ichthus1604 Is there any blocker for keeping the original positions (as on master) of the cancel, continue button on the dialogs?

@ichthus1604
Copy link
Collaborator Author

@soosr

Is there any blocker for keeping the original positions (as on master) of the cancel, continue button on the dialogs?

Fixed. I had fixed vertical margins, but I didn't realize horizontal margins were also wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants