refactor(qt): simplify GUIUtill[fonts] API#7336
Conversation
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98ac82eb41
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ui->fontWeightNormalSlider->setMinimum(nMin); | ||
| ui->fontWeightNormalSlider->setMaximum(nMax); | ||
|
|
||
| ui->fontWeightNormalSlider->setMinimum(0); | ||
| ui->fontWeightNormalSlider->setMaximum(nMaximum); | ||
| ui->fontWeightBoldSlider->setMinimum(nMin); | ||
| ui->fontWeightBoldSlider->setMaximum(nMax); |
There was a problem hiding this comment.
updateWeightSlider() now sets each slider to a continuous [min,max] range from supportedWeightArgs(), but supported weights are often sparse (for example, a font may support only a subset like 3 and 6). That lets users pick intermediate values that are unsupported; updateFontWeightNormal/Bold() then calls setWeightFromArg(...) and silently ignores failure, so the UI value can diverge from the applied font and an invalid weight can be persisted. Previously the slider operated on dense indices over only supported weights, so this regression is new.
Useful? React with 👍 / 👎.
WalkthroughCentralize Qt font handling by removing the public guiutil_font.h surface, internalizing the font registry in guiutil_font.cpp, and expanding GUIUtil (guiutil.h/cpp) with platform, monospace, weight-arg, scale, active-font, setFont overloads, and setStyledHtml. Migrate AppearanceWidget, OptionsModel, bitcoin startup, and all UI files to the new API and update build/test headers and circular-dependency expectations. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/qt/appearancewidget.cpp`:
- Around line 286-299: The sliders currently use a continuous min/max derived
from supportedWeightArgs(), which allows selecting unsupported intermediate
values; change the slider logic to only expose valid supported args by using the
supported list indices as the slider range (e.g., setMinimum(0),
setMaximum(supported.size()-1)) and map slider positions to supported[i] when
reading/writing weights; update initial setting code to set the slider value to
the index of GUIUtil::defaultWeightArg(...) and modify
updateFontWeightNormal/updateFontWeightBold handlers to convert the slider index
into the corresponding supported value (or snap to nearest supported arg) before
calling GUIUtil::setWeightFromArg()/the existing setter.
- Around line 330-340: The heading font changes are only queued by
GUIUtil::setFont for lblHeading and lblSubHeading and won't apply immediately;
after calling GUIUtil::setFont(...) for those labels, call
GUIUtil::updateFonts() before showing the dialog (i.e., before dlg.exec()) so
the queued font updates are applied; ensure you keep the existing
QObject::connect and dlg.exec() calls and only insert the GUIUtil::updateFonts()
call after the setFont calls and before dlg.exec().
In `@src/qt/guiutil.cpp`:
- Around line 166-175: The file still references BitcoinGUI::DEFAULT_UIPLATFORM
in loadStyleSheet(), so remove that dependency by replacing the fallback there
with a call to the local helper defaultUIPlatform() (or mirror its
platform-detection logic) instead of BitcoinGUI::DEFAULT_UIPLATFORM; update the
loadStyleSheet() code paths that set the "-uiplatform" default to use
defaultUIPlatform() so this translation unit no longer depends on BitcoinGUI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 86c748b4-fd15-40ab-b799-57e77ef84e7a
📒 Files selected for processing (42)
src/Makefile.qt.includesrc/qt/addressbookpage.cppsrc/qt/addresstablemodel.cppsrc/qt/appearancewidget.cppsrc/qt/appearancewidget.hsrc/qt/askpassphrasedialog.cppsrc/qt/bitcoin.cppsrc/qt/bitcoingui.cppsrc/qt/bitcoingui.hsrc/qt/coincontroldialog.cppsrc/qt/descriptiondialog.cppsrc/qt/guiutil.cppsrc/qt/guiutil.hsrc/qt/guiutil_font.cppsrc/qt/guiutil_font.hsrc/qt/informationwidget.cppsrc/qt/masternodelist.cppsrc/qt/modaloverlay.cppsrc/qt/networkwidget.cppsrc/qt/openuridialog.cppsrc/qt/optionsdialog.cppsrc/qt/optionsmodel.cppsrc/qt/overviewpage.cppsrc/qt/proposalcreate.cppsrc/qt/proposalinfo.cppsrc/qt/proposallist.cppsrc/qt/proposalmodel.cppsrc/qt/proposalresume.cppsrc/qt/qrdialog.cppsrc/qt/qrimagewidget.cppsrc/qt/receivecoinsdialog.cppsrc/qt/receiverequestdialog.cppsrc/qt/rpcconsole.cppsrc/qt/sendcoinsdialog.cppsrc/qt/sendcoinsentry.cppsrc/qt/signverifymessagedialog.cppsrc/qt/splashscreen.cppsrc/qt/test/apptests.cppsrc/qt/trafficgraphwidget.cppsrc/qt/utilitydialog.cppsrc/qt/walletview.cpptest/lint/lint-circular-dependencies.py
💤 Files with no reviewable changes (12)
- src/qt/splashscreen.cpp
- src/qt/receiverequestdialog.cpp
- src/qt/addresstablemodel.cpp
- src/qt/utilitydialog.cpp
- src/qt/openuridialog.cpp
- src/Makefile.qt.include
- src/qt/qrimagewidget.cpp
- src/qt/trafficgraphwidget.cpp
- src/qt/bitcoingui.h
- src/qt/addressbookpage.cpp
- src/qt/guiutil_font.h
- test/lint/lint-circular-dependencies.py
| const auto supported = GUIUtil::supportedWeightArgs(); | ||
| const int nMin = supported.front(); | ||
| const int nMax = supported.back(); | ||
|
|
||
| ui->fontWeightNormalSlider->setMinimum(nMin); | ||
| ui->fontWeightNormalSlider->setMaximum(nMax); | ||
|
|
||
| ui->fontWeightNormalSlider->setMinimum(0); | ||
| ui->fontWeightNormalSlider->setMaximum(nMaximum); | ||
| ui->fontWeightBoldSlider->setMinimum(nMin); | ||
| ui->fontWeightBoldSlider->setMaximum(nMax); | ||
|
|
||
| ui->fontWeightBoldSlider->setMinimum(0); | ||
| ui->fontWeightBoldSlider->setMaximum(nMaximum); | ||
| if (fForce) { | ||
| updateFontWeightNormal(GUIUtil::defaultWeightArg(GUIUtil::FontWeight::Normal), true); | ||
| updateFontWeightBold(GUIUtil::defaultWeightArg(GUIUtil::FontWeight::Bold), true); | ||
| } |
There was a problem hiding this comment.
Don't expose unsupported weight args on the sliders.
supportedWeightArgs() can be sparse, but this turns it into a continuous min/max range. That lets users stop on values that GUIUtil::setWeightFromArg() rejects, so the slider can show and persist a weight that was never applied. Please map slider positions to the supported list, or snap to the nearest supported arg before calling the setter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/qt/appearancewidget.cpp` around lines 286 - 299, The sliders currently
use a continuous min/max derived from supportedWeightArgs(), which allows
selecting unsupported intermediate values; change the slider logic to only
expose valid supported args by using the supported list indices as the slider
range (e.g., setMinimum(0), setMaximum(supported.size()-1)) and map slider
positions to supported[i] when reading/writing weights; update initial setting
code to set the slider value to the index of GUIUtil::defaultWeightArg(...) and
modify updateFontWeightNormal/updateFontWeightBold handlers to convert the
slider index into the corresponding supported value (or snap to nearest
supported arg) before calling GUIUtil::setWeightFromArg()/the existing setter.
| // Adjust the headings | ||
| GUIUtil::setFont({&lblHeading}, GUIUtil::FontWeight::Bold, 16); | ||
| GUIUtil::setFont({&lblSubHeading}, GUIUtil::FontWeight::Normal, 14, true); | ||
| // Make sure the dialog closes and accepts the settings if save has been pressed | ||
| QObject::connect(&buttonBox, &QDialogButtonBox::accepted, [&]() { | ||
| QSettings().setValue("fAppearanceSetupDone", true); | ||
| appearance.accept(); | ||
| dlg.accept(); | ||
| }); | ||
| // And fire it! | ||
| dlg.exec(); |
There was a problem hiding this comment.
Apply the heading styles before showing the dialog.
GUIUtil::setFont() only queues the update. Without an GUIUtil::updateFonts() pass here, these labels keep their default styling unless some unrelated font/theme refresh happens while the dialog is open.
Suggested fix
// Adjust the headings
GUIUtil::setFont({&lblHeading}, GUIUtil::FontWeight::Bold, 16);
GUIUtil::setFont({&lblSubHeading}, GUIUtil::FontWeight::Normal, 14, true);
+ GUIUtil::updateFonts();
// Make sure the dialog closes and accepts the settings if save has been pressed
QObject::connect(&buttonBox, &QDialogButtonBox::accepted, [&]() {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/qt/appearancewidget.cpp` around lines 330 - 340, The heading font changes
are only queued by GUIUtil::setFont for lblHeading and lblSubHeading and won't
apply immediately; after calling GUIUtil::setFont(...) for those labels, call
GUIUtil::updateFonts() before showing the dialog (i.e., before dlg.exec()) so
the queued font updates are applied; ensure you keep the existing
QObject::connect and dlg.exec() calls and only insert the GUIUtil::updateFonts()
call after the setFont calls and before dlg.exec().
…IPlatform It helps to break circular dependency over qt/guiutil
knst
force-pushed
the
refactor-qt-circulars-1
branch
from
98ac82e to
95a3867
Compare
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Verified against HEAD 98ac82e and the requested diff. Both reported issues are introduced by this PR: the weight sliders now expose raw weight-arg ranges instead of dense supported-weight indices, which can leave the slider and registry out of sync for sparse supported sets, and fixedPitchFont() was changed from getFont({... , FontWeight::Normal}) to constructing a plain QFont, so monospace callers no longer inherit the configured normal weight.
🔴 1 blocking | 🟡 1 suggestion(s)
Reviewed queued commit 98ac82eb. The PR head has since advanced to 95a3867a; a follow-up queue item exists for the newer commit.
Inline comment posting failed; posting findings in the review body instead.
Blocking: Weight sliders can now select unsupported font weights and leave UI/model out of sync
src/qt/appearancewidget.cpp:245
This PR changes the sliders from dense indices over GetSupportedWeights() to raw weight args. updateWeightSlider() now sets the range to supported.front()..supported.back(), while updateFontWeightNormal() / updateFontWeightBold() pass the slider value directly to setWeightFromArg(). supportedWeightArgs() is built from the active font's supported subset, and CalcSupportedWeights() can produce sparse sets, so intermediate slider positions are possible. For those positions setWeightFromArg() returns false and leaves the registry unchanged, but the return value is ignored, so the slider can move to a value the model did not accept.
Suggestion: fixedPitchFont() no longer respects the configured normal font weight
src/qt/guiutil.cpp:241
Before this PR, fixedPitchFont() returned getFont({use_embedded_font ? ROBOTO_MONO_FONT_STR : OS_MONO_FONT_STR, FontWeight::Normal}), which resolved through the font registry and therefore used the configured normal-weight selection. This PR replaces that with a plain QFont that only sets family/style-hint, so monospace callers such as OptionsModel::getFontForChoice(), ProposalModel, and RPCConsole stop tracking the user's configured normal weight.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/qt/proposalresume.cpp (1)
258-263:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRecalculate the description height after changing the HTML.
entry.descriptionis kept at a fixed height fromrecalculateDescHeights(), but this refresh path replaces the document content without recomputing that height. When the status text wraps differently, the text edit can clip until the dialog is reopened.💡 Suggested fix
void ProposalResume::refreshConfirmations() { bool all_confirmed{true}; + bool layout_changed{false}; for (auto& entry : m_entries) { const int confs = queryConfirmations(entry.proposal.collateralHash); if (confs != entry.collateral_confs) { entry.collateral_confs = confs; GUIUtil::setStyledHtml(entry.description, formatProposalHtml(entry.proposal, confs)); entry.broadcast_btn->setEnabled(confs >= m_relay_confs); + layout_changed = true; } if (confs < m_relay_confs) { all_confirmed = false; } } + + if (layout_changed) { + recalculateDescHeights(); + } // Stop polling when all entries are confirmed if (all_confirmed && m_refresh_timer) { m_refresh_timer->stop(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/qt/proposalresume.cpp` around lines 258 - 263, After updating an entry's HTML via GUIUtil::setStyledHtml(entry.description, formatProposalHtml(...)) you must recompute the stored text heights so the QTextEdit doesn't clip; call the existing recalculateDescHeights() (or at minimum recalculate the single entry's height) after changing entry.description and before/after enabling the broadcast button. Locate the update inside the loop over m_entries and invoke recalculateDescHeights() (or the per-entry height recalculation routine) so entry.description's fixed height matches the new content.src/qt/guiutil_font.cpp (1)
398-427:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winProbe
SystemMonospacewith the real fixed-pitch family.
getFontWithWeight()mirrors the sentinel handling forOS_FONT_STR, but not forOS_MONO_FONT_STR. Right now the support/default-weight probing for the system monospace path runs against the literal"SystemMonospace"family name instead ofQFontDatabase::systemFont(QFontDatabase::FixedFont), so the registry can learn the wrong weights for that font.💡 Suggested fix
QFont getFontWithWeight(const QString& font_name, QFont::Weight weight, double point_size) { QFont font; if (font_name == MONTSERRAT_FONT_STR) { if (mapMontserrat.count(weight)) { `#ifdef` Q_OS_MACOS font.setFamily(font_name); font.setStyleName(QString::fromStdString(mapMontserrat.at(weight).first)); `#else` if (weight == QFont::Normal || weight == QFont::Bold) { font.setFamily(font_name); } else { font.setFamily(qstrprintf("%s %s", font_name.toStdString(), mapMontserrat.at(weight).first)); } `#endif` } } else if (font_name == OS_FONT_STR && g_default_font) { font.setFamily(g_default_font->family()); + } else if (font_name == OS_MONO_FONT_STR) { + font.setFamily(QFontDatabase::systemFont(QFontDatabase::FixedFont).family()); } else { font.setFamily(font_name); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/qt/guiutil_font.cpp` around lines 398 - 427, getFontWithWeight currently treats OS_MONO_FONT_STR like a literal family name, causing probes to use "SystemMonospace" instead of the real fixed-pitch family; change the function to handle OS_MONO_FONT_STR similarly to OS_FONT_STR by resolving the actual system monospace family (e.g. use QFontDatabase::systemFont(QFontDatabase::FixedFont).family() or the existing g_default_font equivalent for monospace) before setting the family and weight so probing uses the real fixed-pitch family when font_name == OS_MONO_FONT_STR and g_default_mono_font (or systemFont) is available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/qt/guiutil_font.cpp`:
- Around line 398-427: getFontWithWeight currently treats OS_MONO_FONT_STR like
a literal family name, causing probes to use "SystemMonospace" instead of the
real fixed-pitch family; change the function to handle OS_MONO_FONT_STR
similarly to OS_FONT_STR by resolving the actual system monospace family (e.g.
use QFontDatabase::systemFont(QFontDatabase::FixedFont).family() or the existing
g_default_font equivalent for monospace) before setting the family and weight so
probing uses the real fixed-pitch family when font_name == OS_MONO_FONT_STR and
g_default_mono_font (or systemFont) is available.
In `@src/qt/proposalresume.cpp`:
- Around line 258-263: After updating an entry's HTML via
GUIUtil::setStyledHtml(entry.description, formatProposalHtml(...)) you must
recompute the stored text heights so the QTextEdit doesn't clip; call the
existing recalculateDescHeights() (or at minimum recalculate the single entry's
height) after changing entry.description and before/after enabling the broadcast
button. Locate the update inside the loop over m_entries and invoke
recalculateDescHeights() (or the per-entry height recalculation routine) so
entry.description's fixed height matches the new content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 38165005-bb6a-4d3d-9671-bbe42085840b
📒 Files selected for processing (41)
src/Makefile.qt.includesrc/qt/addressbookpage.cppsrc/qt/addresstablemodel.cppsrc/qt/appearancewidget.cppsrc/qt/askpassphrasedialog.cppsrc/qt/bitcoin.cppsrc/qt/bitcoingui.cppsrc/qt/bitcoingui.hsrc/qt/coincontroldialog.cppsrc/qt/descriptiondialog.cppsrc/qt/guiutil.cppsrc/qt/guiutil.hsrc/qt/guiutil_font.cppsrc/qt/guiutil_font.hsrc/qt/informationwidget.cppsrc/qt/masternodelist.cppsrc/qt/modaloverlay.cppsrc/qt/networkwidget.cppsrc/qt/openuridialog.cppsrc/qt/optionsdialog.cppsrc/qt/optionsmodel.cppsrc/qt/overviewpage.cppsrc/qt/proposalcreate.cppsrc/qt/proposalinfo.cppsrc/qt/proposallist.cppsrc/qt/proposalmodel.cppsrc/qt/proposalresume.cppsrc/qt/qrdialog.cppsrc/qt/qrimagewidget.cppsrc/qt/receivecoinsdialog.cppsrc/qt/receiverequestdialog.cppsrc/qt/rpcconsole.cppsrc/qt/sendcoinsdialog.cppsrc/qt/sendcoinsentry.cppsrc/qt/signverifymessagedialog.cppsrc/qt/splashscreen.cppsrc/qt/test/apptests.cppsrc/qt/trafficgraphwidget.cppsrc/qt/utilitydialog.cppsrc/qt/walletview.cpptest/lint/lint-circular-dependencies.py
💤 Files with no reviewable changes (33)
- src/qt/modaloverlay.cpp
- src/qt/coincontroldialog.cpp
- src/qt/openuridialog.cpp
- src/qt/bitcoingui.h
- src/qt/utilitydialog.cpp
- src/qt/networkwidget.cpp
- src/qt/receiverequestdialog.cpp
- src/qt/trafficgraphwidget.cpp
- src/qt/qrimagewidget.cpp
- src/qt/descriptiondialog.cpp
- src/qt/proposalinfo.cpp
- src/qt/walletview.cpp
- src/qt/guiutil_font.h
- src/Makefile.qt.include
- src/qt/bitcoingui.cpp
- src/qt/splashscreen.cpp
- test/lint/lint-circular-dependencies.py
- src/qt/optionsdialog.cpp
- src/qt/informationwidget.cpp
- src/qt/overviewpage.cpp
- src/qt/sendcoinsentry.cpp
- src/qt/addressbookpage.cpp
- src/qt/addresstablemodel.cpp
- src/qt/masternodelist.cpp
- src/qt/qrdialog.cpp
- src/qt/signverifymessagedialog.cpp
- src/qt/sendcoinsdialog.cpp
- src/qt/askpassphrasedialog.cpp
- src/qt/rpcconsole.cpp
- src/qt/proposallist.cpp
- src/qt/receivecoinsdialog.cpp
- src/qt/appearancewidget.cpp
- src/qt/optionsmodel.cpp
✅ Files skipped from review due to trivial changes (2)
- src/qt/test/apptests.cpp
- src/qt/proposalmodel.cpp
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95a3867a9b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ui->fontWeightNormalSlider->setMinimum(0); | ||
| ui->fontWeightNormalSlider->setMaximum(nMaximum); | ||
| ui->fontWeightBoldSlider->setMinimum(nMin); | ||
| ui->fontWeightBoldSlider->setMaximum(nMax); |
There was a problem hiding this comment.
updateWeightSlider() now maps sliders to a continuous [min,max] argument range, but many fonts expose only a sparse subset of supported weight args. In that case users can select in-between values that setWeightFromArg(...) rejects, and updateFontWeightNormal/Bold() ignore that failure and still keep the slider at the rejected value. This leaves the UI and persisted setting out of sync with the actual applied font weight after changing fonts or moving the slider.
Useful? React with 👍 / 👎.
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Latest delta (98ac82eb..95a3867a) only updates the stylesheet default platform lookup and removes dead includes; I found no new latest-delta findings. Cumulatively, both prior Qt font findings are STILL VALID at 95a3867a: the unsupported weight-slider issue remains blocking, and fixedPitchFont() still bypasses the configured normal weight.
🔴 1 blocking | 🟡 1 suggestion(s)
Findings
🔴 Blocking: Weight sliders still allow unsupported values and can persist a font weight the active font never accepted
src/qt/appearancewidget.cpp:245-299
Prior review finding revalidated: STILL VALID. updateWeightSlider() sets each slider to the numeric range between supported.front() and supported.back(), but supportedWeightArgs() is built from FontInfo::CalcSupportedWeights(), which can return a sparse subset of the 0..8 weight args. That lets the sliders stop on values that are inside the range but not actually supported by the active font. When that happens, updateFontWeightNormal() and updateFontWeightBold() pass the raw slider value into GUIUtil::setWeightFromArg(), which rejects unsupported weights and leaves the registry unchanged, but the return value is ignored. Because the sliders are mapped directly to OptionsModel::FontWeightNormal and OptionsModel::FontWeightBold and submitted on accept, the UI can save an unsupported weight arg even though the live font state never switched to it.
🟡 Suggestion: fixedPitchFont() no longer follows the configured normal-weight selection
src/qt/guiutil.cpp:241-247
Prior review finding revalidated: STILL VALID. Before this refactor, fixedPitchFont() returned a registry-backed font via getFont(...), so the monospace helper inherited the current normal-weight selection. The current implementation constructs a fresh QFont from either "Roboto Mono" or QFontDatabase::systemFont(QFontDatabase::FixedFont) and only sets the monospace style hint, so the selected normal weight is dropped. That difference is observable at current HEAD in direct consumers such as ProposalModel::data() and RPCConsole::clear(), which use the returned QFont as-is instead of overriding its weight afterward.
2 participants
Issue being fixed or feature implemented
There's overcomplicated public header
guiutil_fonts.hwhich introduces bunch of static methods inside global variables, such as g_fonts_known, g_font_registryguiutil.h has internal classes
FontInfoandFontRegistryin public namespace.class
FontAttribis also non-needed abstraction.What was done?
This PR simplifies public API for GUIUtil [fonts].
Particularly, done:
setWeightFromArg,currentWeightArg,defaultWeightArg,supportedWeightArgsfor UI settingsFontInfo,FontRegistryand global objectsg_fonts_known,g_font_registryare removed from public API GUIUtil [font]weightFromArg,weightToArg,WeightToIdxandIdxToWeight` helpers from public API GUIUtil [font]getFonts,registerFontandsetActiveFontto justsetActiveFont()isValidWeightArgandsetWeightFromArgtobool setWeightFromArg()LoadFonthelper that is super-seeded by multiple rich dash core helpersqt/guiutil_font.his inlined to toguiutil.hUnrelated extra changes:
registerWidgettosetStyledHtmlwhich is more clear for develop.As a result of this PR multiple usages of GUIUtil[font] became much simplier, for example:
How Has This Been Tested?
Run
dash-qtand played with all sliders and combo-boxes on Appearance Tab. Seems as nothing is broken.Two circular dependencies are gone:
Breaking Changes
N/A
Checklist: