Skip to content

fix: resolve font weights dynamically for live slider updates#7120

Merged
PastaPastaPasta merged 1 commit intodashpay:developfrom
UdjinM6:fix-bold-slider-live-update
Feb 5, 2026
Merged

fix: resolve font weights dynamically for live slider updates#7120
PastaPastaPasta merged 1 commit intodashpay:developfrom
UdjinM6:fix-bold-slider-live-update

Conversation

@UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Jan 30, 2026

Issue being fixed or feature implemented

Widgets registered via GUIUtil::setFont() stored the actual QFont::Weight value at registration time. When font weight sliders were adjusted, the stored weight remained unchanged, preventing live updates.

Discovered while reviewing #7112

What was done?

This refactors FontAttrib to use the existing FontWeight enum (Bold/Normal) instead of storing QFont::Weight values. Both setFont() and getFont() now accept FontWeight, which is resolved to the current registry value during updateFonts() and getFont() calls.

How Has This Been Tested?

Run dash-qt, go to Preferences -> Appearance tab and move Font Weight Bold back and forth.
develop: no visual changes on Overview tab
this branch: text on Overview tab adjusts accordingly

Breaking Changes

n/a

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

Widgets registered via GUIUtil::setFont() stored the actual QFont::Weight
value at registration time. When font weight sliders were adjusted, the
stored weight remained unchanged, preventing live updates.

This refactors FontAttrib to use the existing FontWeight enum (Bold/Normal)
instead of storing QFont::Weight values. Both setFont() and getFont() now
accept FontWeight, which is resolved to the current registry value during
updateFonts() and getFont() calls.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@UdjinM6 UdjinM6 added this to the 23.1 milestone Jan 30, 2026
@github-actions
Copy link

@coderabbitai
Copy link

coderabbitai bot commented Jan 30, 2026

Walkthrough

This pull request refactors the font weight handling system in the Qt GUI. Previously, font weights were retrieved dynamically using registry accessor methods (GUIUtil::g_font_registry.GetWeightBold() and GUIUtil::g_font_registry.GetWeightNormal()). The changes introduce a static FontWeight enum and update all call sites across approximately 23 GUI files to use the enum values directly. The core implementation in guiutil_font.cpp and guiutil_font.h is updated to support the new enum-based weight system, including a new internal helper function getFontWithWeight and changes to how the FontAttrib struct stores and resolves font weights.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.89% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: refactoring font weight handling to resolve values dynamically rather than storing them statically, enabling live slider updates.
Description check ✅ Passed The description clearly explains the issue, the solution, and testing approach. It details that FontAttrib now uses FontWeight enum instead of QFont::Weight, enabling dynamic resolution during getFont() calls.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

🧪 Unit Test Generation v2 is now available!

We have significantly improved our unit test generation capabilities.

To enable: Add this to your .coderabbit.yaml configuration:

reviews:
  finishing_touches:
    unit_tests:
      enabled: true

Try it out by using the @coderabbitai generate unit tests command on your code files or under ✨ Finishing Touches on the walkthrough!

Have feedback? Share your thoughts on our Discord thread!


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/qt/guiutil_font.cpp`:
- Around line 189-219: The code in getFontWithWeight (references:
MONTSERRAT_FONT_STR, mapMontserrat, g_default_font) has formatting drift; re-run
the project's clang-format (or clang-format-diff.py) on src/qt/guiutil_font.cpp
(at the getFontWithWeight function) to apply the repository's style rules, then
stage the updated file so CI sees no formatting diffs.

Copy link
Collaborator

@kwvg kwvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When testing this PR, it's noted that the observation about bolding made earlier (comment) seems to only apply to Montserrat but not SystemDefault, otherwise can confirm that bolding live updates work as expected.

Description develop (1ea35a4) This PR (f0ca408)
Using Montserrat
Using SystemDefault

Copy link
Collaborator

@kwvg kwvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK f0ca408

@PastaPastaPasta PastaPastaPasta merged commit e65174b into dashpay:develop Feb 5, 2026
43 of 48 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