luke-jr
commented at 6:36 am on December 13, 2021:
member
Margin of QRCode should be constant, not change based on complexity of QRCode
Text region should be sized based on height of actual text, not a constant guess
Width of image should not be adjusted based on vertical text size/margins
Adjusted constant QRCode and margin sizes to visually match what we had previously
If text won’t fit, grow up to 25% wider, or split across multiple lines
(Note: Out of scope for this PR, after #497, I think we should allow customising the font used here and default to the embedded monospace for better privacy)
- Margin of QRCode should be constant, not change based on complexity of QRCode
- Text region should be sized based on height of actual text, not a constant guess
- Width of image should not be adjusted based on vertical text size/margins
- Adjusted constant QRCode and margin sizes to visually match what we had previously
f9d9a1d1cf
Bugfix: GUI/QRImageWidget: If text won't fit, split across multiple lines
This ensures even with a font we can't scale down enough, the text doesn't get cut off
91d24c84ba
GUI/QRImageWidget: Allow image to grow up to 25% wider to accomidate single-line text
Accomidates addresses that are just barely too long for the QRCode width
4c5143d56c
promag
commented at 2:27 pm on December 13, 2021:
contributor
Concept ACK, however, I’ll try a different approach for the multi-line case.
promag
commented at 0:10 am on December 14, 2021:
contributor
@luke-jr feel free to try the commit 0ac4f7535a90bacb0e5ffe8f110505aa44075ad6 (branch) which avoids manual text wrap by using Qt::TextWrapAnywhere flag.
Another thing I noticed is that the default minimum font size in calculateIdealFontSize is too small.
luke-jr
commented at 2:11 am on December 14, 2021:
member
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
No conflicts as of last run.
jarolrod
commented at 6:14 am on June 28, 2022:
member
Concept ACK, stylistic changes look fine to me.
master
PR (rebased)
hebasto
commented at 2:25 pm on July 19, 2022:
member
Tested 4c5143d56c98d4f9f961059692f155bbe2248c44.
This PR effectively renders font size smaller that makes it harder to read.
master branch:
this PR:
DrahtBot
commented at 1:21 am on May 6, 2023:
contributor
There hasn’t been much activity lately. What is the status here?
Finding reviewers may take time. However, if the patch is no longer relevant, please close this pull request. If the author lost interest or time to work on this, please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.
hebasto
commented at 2:53 pm on May 16, 2023:
member
This PR effectively renders font size smaller that makes it harder to read.
remains unaddressed.
DrahtBot added the label
CI failed
on Aug 18, 2023
DrahtBot removed the label
CI failed
on Aug 23, 2023
DrahtBot added the label
CI failed
on Aug 31, 2023
DrahtBot removed the label
CI failed
on Sep 4, 2023
pablomartin4btc
commented at 7:54 pm on September 17, 2023:
contributor
Concept ACK, I agree with the improvements on achieving a more consistent adjustment of the QR code image size. However, it appears that there is still an issue with the font size, which makes the address in the picture difficult to read. Is there a way to improve the legibility of the address text?
pablomartin4btc
commented at 7:52 pm on September 18, 2023:
contributor
DrahtBot added the label
CI failed
on Sep 22, 2023
DrahtBot removed the label
CI failed
on Sep 24, 2023
DrahtBot added the label
CI failed
on Oct 18, 2023
DrahtBot
commented at 2:34 pm on February 5, 2024:
contributor
🤔 There hasn’t been much activity lately and the CI seems to be failing.
If no one reviewed the current pull request by commit hash, a rebase can be considered. While the CI failure may be a false positive, the CI hasn’t been running for some time, so there may be a real issue hiding as well. A rebase triggers the latest CI and makes sure that no silent merge conflicts have snuck in.
hebasto
commented at 11:26 am on February 12, 2024:
member
This is a metadata mirror of the GitHub repository
bitcoin-core/gui.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2024-12-03 20:20 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me