-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(qt): add global zoom shortcuts #7216
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
base: develop
Are you sure you want to change the base?
Changes from all commits
0856ab9
62ceb00
bf517df
12c0ebd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,6 +71,7 @@ | |
| #include <QVBoxLayout> | ||
| #include <QWindow> | ||
|
|
||
| #include <algorithm> | ||
| #include <functional> | ||
|
|
||
| namespace { | ||
|
|
@@ -85,6 +86,10 @@ constexpr int GOV_CYCLE_FRAME_MS{STATUSBAR_ICON_CYCLE_MS / (GOV_CYCLE_FRAME_COUN | |
|
|
||
| // Per-frame interval for the spinner animation | ||
| constexpr int SPINNER_FRAME_MS{STATUSBAR_ICON_CYCLE_MS / SPINNER_FRAMES}; | ||
|
|
||
| constexpr int FONT_SCALE_MIN{-50}; | ||
| constexpr int FONT_SCALE_MAX{100}; | ||
| constexpr int FONT_SCALE_SHORTCUT_STEP{3}; | ||
| } // anonymous namespace | ||
|
|
||
| const std::string BitcoinGUI::DEFAULT_UIPLATFORM = | ||
|
|
@@ -517,12 +522,27 @@ void BitcoinGUI::createActions() | |
| m_mask_values_action->setStatusTip(tr("Mask the values in the Overview tab")); | ||
| m_mask_values_action->setCheckable(true); | ||
|
|
||
| m_zoom_in_action = new QAction(tr("Zoom &In"), this); | ||
| m_zoom_in_action->setShortcuts({QKeySequence(QKeySequence::ZoomIn), QKeySequence(tr("Ctrl+="))}); | ||
| m_zoom_in_action->setStatusTip(tr("Increase the font size")); | ||
|
|
||
| m_zoom_out_action = new QAction(tr("Zoom &Out"), this); | ||
| m_zoom_out_action->setShortcuts({QKeySequence(QKeySequence::ZoomOut), QKeySequence(tr("Ctrl+_"))}); | ||
| m_zoom_out_action->setStatusTip(tr("Decrease the font size")); | ||
|
PastaPastaPasta marked this conversation as resolved.
|
||
|
|
||
| m_zoom_reset_action = new QAction(tr("Reset &Zoom"), this); | ||
| m_zoom_reset_action->setShortcut(QKeySequence(tr("Ctrl+0"))); | ||
| m_zoom_reset_action->setStatusTip(tr("Reset the font size to default")); | ||
|
Comment on lines
+525
to
+535
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Zoom actions remain enabled when When Consider disabling the actions when the option is overridden: 💡 Proposed fix (disable actions when overridden)In const bool font_scale_locked{optionsModel->isOptionOverridden("-font-scale")};
m_zoom_in_action->setEnabled(!font_scale_locked);
m_zoom_out_action->setEnabled(!font_scale_locked);
m_zoom_reset_action->setEnabled(!font_scale_locked);🤖 Prompt for AI Agents
Comment on lines
+525
to
+535
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💬 Nitpick: Zoom actions remain enabled when When source: ['coderabbit'] |
||
|
|
||
| connect(quitAction, &QAction::triggered, this, &BitcoinGUI::quitRequested); | ||
| connect(aboutAction, &QAction::triggered, this, &BitcoinGUI::aboutClicked); | ||
| connect(aboutQtAction, &QAction::triggered, qApp, QApplication::aboutQt); | ||
| connect(optionsAction, &QAction::triggered, this, &BitcoinGUI::optionsClicked); | ||
| connect(showHelpMessageAction, &QAction::triggered, this, &BitcoinGUI::showHelpMessageClicked); | ||
| connect(showCoinJoinHelpAction, &QAction::triggered, this, &BitcoinGUI::showCoinJoinHelpClicked); | ||
| connect(m_zoom_in_action, &QAction::triggered, this, [this] { adjustFontScale(FONT_SCALE_SHORTCUT_STEP); }); | ||
| connect(m_zoom_out_action, &QAction::triggered, this, [this] { adjustFontScale(-FONT_SCALE_SHORTCUT_STEP); }); | ||
| connect(m_zoom_reset_action, &QAction::triggered, this, [this] { adjustFontScale(0); }); | ||
|
|
||
| // Jump directly to tabs in RPC-console | ||
| connect(openInfoAction, &QAction::triggered, this, &BitcoinGUI::showInfo); | ||
|
|
@@ -677,6 +697,11 @@ void BitcoinGUI::createMenuBar() | |
| } | ||
| settings->addAction(optionsAction); | ||
|
|
||
| QMenu* view = appMenuBar->addMenu(tr("&View")); | ||
| view->addAction(m_zoom_in_action); | ||
| view->addAction(m_zoom_out_action); | ||
| view->addAction(m_zoom_reset_action); | ||
|
|
||
| QMenu* window_menu = appMenuBar->addMenu(tr("&Window")); | ||
|
|
||
| QAction* minimize_action = window_menu->addAction(tr("&Minimize")); | ||
|
|
@@ -735,6 +760,31 @@ void BitcoinGUI::createMenuBar() | |
| help->addAction(aboutQtAction); | ||
| } | ||
|
|
||
| void BitcoinGUI::adjustFontScale(int delta) | ||
| { | ||
| if (clientModel && clientModel->getOptionsModel() && | ||
| clientModel->getOptionsModel()->isOptionOverridden("-font-scale")) { | ||
| return; | ||
| } | ||
|
Comment on lines
+765
to
+768
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: Menu zoom items remain enabled when -font-scale CLI override is active When source: ['codex', 'claude']
Comment on lines
+763
to
+768
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: Zoom actions stay enabled when
source: ['claude'] 🤖 Fix this with AI agents |
||
|
|
||
| const int current_scale{GUIUtil::g_font_registry.GetFontScale()}; | ||
| const int new_scale{delta == 0 | ||
| ? GUIUtil::FontRegistry::DEFAULT_FONT_SCALE | ||
| : std::clamp(current_scale + delta, FONT_SCALE_MIN, FONT_SCALE_MAX)}; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be resolved
PastaPastaPasta marked this conversation as resolved.
|
||
|
|
||
| if (new_scale == current_scale) { | ||
| return; | ||
| } | ||
|
|
||
| GUIUtil::g_font_registry.SetFontScale(new_scale); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The shortcut handler updates Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be resolved |
||
| GUIUtil::updateFonts(); | ||
| updateWidth(); | ||
|
|
||
| if (clientModel && clientModel->getOptionsModel()) { | ||
| clientModel->getOptionsModel()->setOption(OptionsModel::FontScale, new_scale); | ||
|
Comment on lines
+779
to
+784
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: Shortcut-driven font-scale changes do not update the Appearance slider The new shortcut path writes the persisted value with source: ['claude'] 🤖 Fix this with AI agents |
||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| void BitcoinGUI::createToolBars() | ||
| { | ||
| #ifdef ENABLE_WALLET | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,10 +183,10 @@ | |
| </size> | ||
| </property> | ||
| <property name="minimum"> | ||
| <number>-30</number> | ||
| <number>-50</number> | ||
| </property> | ||
| <property name="maximum"> | ||
| <number>30</number> | ||
| <number>100</number> | ||
|
Comment on lines
185
to
+189
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect every place that defines, clamps, persists, or applies font scale.
rg -n -C3 'SetFontScale\(|GetFontScale\(|DEFAULT_FONT_SCALE|OptionID::FontScale|FontScale|fontScaleSlider|FONT_SCALE_MIN|FONT_SCALE_MAX|-30|-100' srcRepository: dashpay/dash Length of output: 50369 🏁 Script executed: # Search for minimum font size checks and scale_steps modifications
rg -n 'MIN_FONT_SIZE|min.*font.*size|m_scale_steps|SetFontScale|GetScaledFontSize' src/qt/guiutil_font.h src/qt/guiutil_font.cpp --max-count=50Repository: dashpay/dash Length of output: 928 🏁 Script executed: # Check if there's any validation or clamping of font_scale when it's set
rg -n -A 5 'void SetFontScale|SetFontScale\(' src/qt/ --max-count=50Repository: dashpay/dash Length of output: 2458 🏁 Script executed: # Check the UI file to see if the slider has any other constraints
rg -n -B 5 -A 10 'fontScaleSlider' src/qt/forms/appearancewidget.ui | head -80Repository: dashpay/dash Length of output: 650 🏁 Script executed: # Check if there's any minimum font size validation or constraints
rg -n 'pointSizeF|setPointSize|MIN|MINIMUM|clamp.*size' src/qt/guiutil_font.cpp | head -50Repository: dashpay/dash Length of output: 628 🏁 Script executed: # Check git history or find if there was a previous version with -30..30
rg -n '\-30|\-100' src/qt/forms/appearancewidget.ui src/qt/guiutil_font.h src/qt/bitcoingui.cppRepository: dashpay/dash Length of output: 185 🏁 Script executed: # Check the DEFAULT_FONT_SCALE and see if there's documentation
rg -n 'DEFAULT_FONT_SCALE' src/qt/ -A 2 -B 2Repository: dashpay/dash Length of output: 3384 🏁 Script executed: # Check if there's any clamping in the font rendering or if Qt has built-in protection
rg -n 'setPointSizeF' src/qt/guiutil_font.cpp -B 3 -A 3 | head -60Repository: dashpay/dash Length of output: 1280 🏁 Script executed: # Check if there's any minimum font size handling or if Qt's setPointSizeF has built-in protection
rg -n 'pointSize.*0|font.*size.*0|MIN.*POINT|pointSizeF.*<' src/qt/guiutil_font.cppRepository: dashpay/dash Length of output: 97 🏁 Script executed: # Check the context around line 582 in guiutil_font.cpp
sed -n '575,590p' src/qt/guiutil_font.cppRepository: dashpay/dash Length of output: 610 🏁 Script executed: # Let's find the full context of where GetScaledFontSize is used and verify the calculation
sed -n '620,635p' src/qt/guiutil_font.cppRepository: dashpay/dash Length of output: 710 🏁 Script executed: # Verify by calculating what font size would result at the extremes
# Let's check if there are any tests for font scaling
rg -n 'font.*scale|FontScale.*-100|-30' src/test/ --max-count=20Repository: dashpay/dash Length of output: 764 🏁 Script executed: # Let's verify the exact calculation by checking if there's a minimum size applied anywhere
rg -n 'std::max\|clamp\|min.*size' src/qt/guiutil_font.h src/qt/guiutil_font.cpp | head -30Repository: dashpay/dash Length of output: 38 🏁 Script executed: # Let's verify the complete picture - check all the places where SetFontScale is called to see if there's any clamping
rg -n -B 3 'SetFontScale\(nScale\)|SetFontScale\(new_scale\)' src/qt/Repository: dashpay/dash Length of output: 805 🏁 Script executed: # Check if AppearanceWidget's updateFontScale clamps or validates the input
sed -n '239,243p' src/qt/appearancewidget.cppRepository: dashpay/dash Length of output: 189 AppearanceWidget::updateFontScale() must validate the slider range before passing to SetFontScale(). The expanded slider range of With Add a clamp in void AppearanceWidget::updateFontScale(int nScale)
{
constexpr int FONT_SCALE_MIN{-100};
constexpr int FONT_SCALE_MAX{100};
nScale = std::clamp(nScale, FONT_SCALE_MIN, FONT_SCALE_MAX);
GUIUtil::g_font_registry.SetFontScale(nScale);
GUIUtil::updateFonts();
}🤖 Prompt for AI Agents |
||
| </property> | ||
| <property name="singleStep"> | ||
| <number>10</number> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -558,6 +558,17 @@ RPCConsole::RPCConsole(interfaces::Node& node, QWidget* parent, Qt::WindowFlags | |
|
|
||
| setButtonIcons(); | ||
|
|
||
| // Register console font-size shortcuts once (not in setButtonIcons which | ||
| // re-runs on theme changes and would create duplicate QShortcut objects). | ||
| //: Main shortcut to increase the RPC console font size. | ||
| GUIUtil::AddButtonShortcut(ui->fontBiggerButton, tr("Ctrl++"), Qt::WidgetWithChildrenShortcut); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Using Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Binding the RPC console zoom shortcuts with Useful? React with 👍 / 👎. |
||
| //: Secondary shortcut to increase the RPC console font size. | ||
| GUIUtil::AddButtonShortcut(ui->fontBiggerButton, tr("Ctrl+="), Qt::WidgetWithChildrenShortcut); | ||
|
Comment on lines
+564
to
+566
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Binding these Useful? React with 👍 / 👎. |
||
| //: Main shortcut to decrease the RPC console font size. | ||
| GUIUtil::AddButtonShortcut(ui->fontSmallerButton, tr("Ctrl+-"), Qt::WidgetWithChildrenShortcut); | ||
| //: Secondary shortcut to decrease the RPC console font size. | ||
| GUIUtil::AddButtonShortcut(ui->fontSmallerButton, tr("Ctrl+_"), Qt::WidgetWithChildrenShortcut); | ||
|
Comment on lines
+564
to
+570
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: Overlapping shortcuts between global zoom and RPC console font-size buttons The new window-level zoom actions use source: ['claude']
Comment on lines
+564
to
+570
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion: RPC console font shortcuts are scoped to the buttons instead of the console
source: ['codex'] 🤖 Fix this with AI agents |
||
|
|
||
| // Install event filter for up and down arrow | ||
| ui->lineEdit->installEventFilter(this); | ||
| ui->lineEdit->setMaxLength(16 * 1024 * 1024); | ||
|
|
@@ -1031,8 +1042,8 @@ void RPCConsole::clear(bool keep_prompt) | |
| " without fully understanding the ramifications of a command.%8") | ||
| .arg(PACKAGE_NAME, | ||
| "<b>" + ui->clearButton->shortcut().toString(QKeySequence::NativeText) + "</b>", | ||
| "<b>" + ui->fontBiggerButton->shortcut().toString(QKeySequence::NativeText) + "</b>", | ||
| "<b>" + ui->fontSmallerButton->shortcut().toString(QKeySequence::NativeText) + "</b>", | ||
| "<b>" + QKeySequence(tr("Ctrl++")).toString(QKeySequence::NativeText) + "</b>", | ||
| "<b>" + QKeySequence(tr("Ctrl+-")).toString(QKeySequence::NativeText) + "</b>", | ||
|
Comment on lines
1044
to
+1046
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💬 Nitpick: Shortcut display text in clear() help message is hardcoded separately The source: ['claude'] |
||
| "<b>help</b>", | ||
| "<b>help-console</b>", | ||
| "<span class=\"secwarning\">", | ||
|
|
@@ -1347,16 +1358,7 @@ void RPCConsole::setButtonIcons() | |
| GUIUtil::setIcon(ui->clearButton, "remove", GUIUtil::ThemedColor::RED, consoleButtonsSize); | ||
|
|
||
| GUIUtil::setIcon(ui->fontBiggerButton, "fontbigger", GUIUtil::ThemedColor::BLUE, consoleButtonsSize); | ||
| //: Main shortcut to increase the RPC console font size. | ||
| ui->fontBiggerButton->setShortcut(tr("Ctrl++")); | ||
| //: Secondary shortcut to increase the RPC console font size. | ||
| GUIUtil::AddButtonShortcut(ui->fontBiggerButton, tr("Ctrl+=")); | ||
|
|
||
| GUIUtil::setIcon(ui->fontSmallerButton, "fontsmaller", GUIUtil::ThemedColor::BLUE, consoleButtonsSize); | ||
| //: Main shortcut to decrease the RPC console font size. | ||
| ui->fontSmallerButton->setShortcut(tr("Ctrl+-")); | ||
| //: Secondary shortcut to decrease the RPC console font size. | ||
| GUIUtil::AddButtonShortcut(ui->fontSmallerButton, tr("Ctrl+_")); | ||
| } | ||
|
|
||
| void RPCConsole::reloadThemedWidgets() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These new window-level shortcuts reuse
Ctrl++/Ctrl+=andCtrl+-/Ctrl+_, which are already bound inRPCConsole::setButtonIcons()for console font resizing. In-disablewalletmode (or no-wallet builds),BitcoinGUIembedsRPCConsoleas the central widget, so both shortcut sets live in the same window context and become ambiguous; users can get inconsistent behavior (global UI scale changes instead of, or competing with, console-only font resizing). This regression is introduced by adding the duplicate key sequences here without scoping them away from the console context.Useful? React with 👍 / 👎.