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

Coin control fixes #6247

Merged
merged 2 commits into from
Sep 13, 2022
Merged

Coin control fixes #6247

merged 2 commits into from
Sep 13, 2022

Conversation

komret
Copy link
Contributor

@komret komret commented Sep 12, 2022

Coin control bugfixes

Description

  • cropped the SVG in the button
  • rounded satoshi amount to prevent displaying incorrectly computed decimals
  • changed the property hiding the message form visibility to display to prevent it from breaking the layout when too long

Related Issue

#2770

Screenshots (if appropriate):

SVG icon before and after:
Screenshot 2022-09-12 at 14 13 44
Screenshot 2022-09-12 at 14 12 33

Rounding before and after:
Screenshot 2022-09-12 at 19 14 02
Screenshot 2022-09-12 at 19 14 38

QA: This should fix incorrect computations in the "Missing to your input" message. It happened with some specific numbers in the previous version, e.g. 0.5988788. Please check the rounding is correct.

@komret komret added send Send page release Will be included in the upcoming release. Needs to be backported to the release branch. bitcoin Bitcoin related labels Sep 12, 2022
@komret
Copy link
Contributor Author

komret commented Sep 12, 2022

I removed the other commit, bitcoin-like networks have been tested now, let's support them from the beginning. It would be nice to cherry-pick the icon fix into the release, though.

@komret komret changed the title Fix/coin control button Coin control fixes Sep 12, 2022
@komret
Copy link
Contributor Author

komret commented Sep 12, 2022

I found one more bug related to computation and fixed it in 8ed9197.

@dahaca
Copy link
Contributor

dahaca commented Sep 12, 2022

Since you are making the changes, could you change the margin between the header and controls here to 20px? 🙏

image

dahaca
dahaca previously approved these changes Sep 13, 2022
@matejkriz matejkriz merged commit f203d09 into develop Sep 13, 2022
@matejkriz matejkriz deleted the fix/coin-control-button branch September 13, 2022 12:33
@STew790
Copy link
Contributor

STew790 commented Sep 15, 2022

QA OK
Icon changed correctly and computation of missing input seems correct as well.

Info:

  • Suite version: desktop 22.9.1 (a2cec27)
  • Browser: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) TrezorSuite/22.9.1 Chrome/100.0.4896.160 Electron/18.3.5 Safari/537.36
  • OS: MacIntel
  • Screen: 1440x900
  • Device: model 1 1.11.2 Universal (revision 0d87b55ba4fed7eecc72bf2a94ee473830b095e9)
  • Transport: bridge 2.0.31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bitcoin Bitcoin related coin control release Will be included in the upcoming release. Needs to be backported to the release branch. Roadmap: Product send Send page
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants