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

[Bugfix] UI BTC Address cutoff in PSBTAddressDetailsScreen screen #579

Merged
merged 4 commits into from
Jul 20, 2024

Conversation

newtonick
Copy link
Collaborator

Description

"Will Send" (PSBTAddressDetailsScreen) screen in PSBT flow of receive address has the bottom address text cutoff (bug not in 0.7.0). I believe this issue was introduced with the conversion of Pillow functions getbbox from getsize (#485). This is the best solution I can find for this issue. Happy to accept other recommendation from @kdmukai 😀.

PSBTAddressDetailsView Screen/View

0.7.0 Current Dev Branch PR Applied
PSBTAddressDetailsView 0 7 0 PSBTAddressDetailsView Current Dev PSBTAddressDetailsView Fix

PSBTAddressDetailsView Screen/View

0.7.0 Current Dev Branch PR Applied
PSBTMathView 0 7 0 PSBTMathView Current Dev PSBTMathView Fix

The addition of getmetrics is to get the height from the descent and add the height calculated of the font.

image

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Documentation
  • Other

Checklist

  • I’ve run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

  • No, I’m a fool
  • Yes
  • N/A

I have tested this PR on the following platforms/os:

Note: Keep your changes limited in scope; if you uncover other issues or improvements along the way, ideally submit those as a separate PR. The more complicated the PR the harder to review, test, and merge.

@kdmukai
Copy link
Contributor

kdmukai commented Jul 19, 2024

I will try to get hands-on with this soon, but note to others / to self:

First guess is that it would be better to just give getbbox("Q") an anchor. We typically use "ls" (left, baseline) in order to ignore pixels below baseline but capture the full height of taller characters. This lets us create consistent line spacing for multi-line text (consistent baseline spacing).

The only "gotchas" are making sure that the line spacing has enough room to allow for the pixels below the baseline to not intrude on the line below them and to make sure pixels below the baseline are included in the Component's height reporting (which may be a separate / the real bug here) so they aren't chopped off.

@jdlcdl
Copy link

jdlcdl commented Jul 19, 2024

As of a187190

I tried both before (with current dev as it is today) and with this pr and confirm the prior clipping at bottom of PSBT Will Send screen has been corrected by this pr. As well, a quick review of screenshots generated on this pr don't show anything alarming (that wasn't already known before, ie: the word "change" clipped on right side of psbt math for larger sats values.)

@kdmukai
Copy link
Contributor

kdmukai commented Jul 19, 2024

Suggested changes here:

2089f48

PSBTAddressDetailsView
PSBTMathView

@newtonick newtonick merged commit 100a974 into SeedSigner:dev Jul 20, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants