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

Enable customisation of QR Code font #820

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

luke-jr
Copy link
Member

@luke-jr luke-jr commented May 11, 2024

May need the fixes in #506 to work cleanly in all cases, but should still be a net improvement on its own

@DrahtBot
Copy link
Contributor

DrahtBot commented May 11, 2024

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
Concept ACK hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@luke-jr luke-jr changed the title GUI: Enable customisation of QR Code font Enable customisation of QR Code font May 11, 2024
@hebasto
Copy link
Member

hebasto commented Jul 15, 2024

Concept ACK on adding a new customization option for the QR code font.

Lightly tested b14c9d0. It seems to work as expected.

However, I think that QR image widget must consider the font size for adjusting its own size. Otherwise, the image can be unreadable and confusing for the user:
image

The second commit modifies the code introduced in the first one. Therefore, please squash them.

@DrahtBot
Copy link
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.

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

Successfully merging this pull request may close these issues.

3 participants