SeedSigner / seedsigner

Use an air-gapped Raspberry Pi Zero to sign for Bitcoin transactions! (and do other cool stuff)
MIT License
720 stars 168 forks source link

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

Closed newtonick closed 3 months ago

newtonick commented 3 months ago

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 (https://github.com/SeedSigner/seedsigner/pull/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:

Checklist

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

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

Suggested changes here:

https://github.com/SeedSigner/seedsigner/commit/2089f482ca4dfdff8d8b35232814c9966cb91539

PSBTAddressDetailsView PSBTMathView