summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-13 16:44:16 +0200
committerDanilo M. <danix@danix.xyz>2026-08-13 16:44:16 +0200
commit1ab12862fd47bf3816f7fad7b8a96eceb8f84272 (patch)
treea170aaca6adde67b931b3899ffaadb6e788c12c8 /src
parent1dd39414fd6139f2934554fc90b1c5721603d14b (diff)
downloadqtmaildir-1ab12862fd47bf3816f7fad7b8a96eceb8f84272.tar.gz
qtmaildir-1ab12862fd47bf3816f7fad7b8a96eceb8f84272.zip
fix(rules): a long rule no longer squeezes the rule list away
Item 80. A rule with eight From conditions left the list showing about one and a half rows. The list was added with stretch 1 and the form below it with none, which looks decisive and is not: a stretch factor only distributes space above each widget's minimum, and the form's minimum grew with every condition row, so each row came straight out of the list. The builder asked for 120px with one row and 414px with eight. A QSplitter now divides the list from the editor, so the balance is the user's and is saved beside the column widths, and the condition rows sit in a QScrollArea capped at 190px so the editor cannot grow without bound however the splitter is set. The scroll area is what text mode hides; hiding the builder inside it would leave an empty frame. Three measures were tried in the test before one told the bug and the fix apart, and two passed against broken code: the dialog's minimumSizeHint does not track form rows and read 580 either way, and a qMin against the scroll area's own hint read small whether or not the cap was set, since an uncapped maximumHeight is QWIDGETSIZE_MAX. What survives mutation is the editor pane's minimum inside the splitter, plus the cap read directly, and both are asserted. A row's size hint is invalid until the event loop runs, so the test calls processEvents after selecting a rule or it measures the same height twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'src')
-rw-r--r--src/tagrulesdialog.cpp76
-rw-r--r--src/tagrulesdialog.h30
2 files changed, 100 insertions, 6 deletions
diff --git a/src/tagrulesdialog.cpp b/src/tagrulesdialog.cpp
index 54600c3..285043a 100644
--- a/src/tagrulesdialog.cpp
+++ b/src/tagrulesdialog.cpp
@@ -33,8 +33,10 @@
#include <QPlainTextEdit>
#include <QPushButton>
#include <QRadioButton>
+#include <QScrollArea>
#include <QSettings>
#include <QSpinBox>
+#include <QSplitter>
#include <QTreeWidget>
#include <QVBoxLayout>
@@ -106,7 +108,26 @@ TagRulesDialog::TagRulesDialog(QWidget *parent)
tr("Matches") });
m_list->setRootIsDecorated(false);
m_list->setUniformRowHeights(true);
- layout->addWidget(m_list, 1);
+
+ // The list and the editor go in a splitter, and the editor's own widget
+ // holds the form. A plain QVBoxLayout gave the list stretch 1 and still
+ // let a rule with eight condition rows squeeze it to about one visible
+ // row: a stretch factor only shares out space ABOVE each widget's
+ // minimum, and the form's grew with every row. Measured before the fix,
+ // the editor asked for 120px with one row and 414px with eight.
+ auto *editor = new QWidget(this);
+ auto *editorLayout = new QVBoxLayout(editor);
+ editorLayout->setContentsMargins(0, 0, 0, 0);
+
+ m_splitter = new QSplitter(Qt::Vertical, this);
+ m_splitter->addWidget(m_list);
+ m_splitter->addWidget(editor);
+ // Neither pane collapses to nothing by dragging the handle past the end,
+ // which would hide the thing the user was trying to make room for.
+ m_splitter->setChildrenCollapsible(false);
+ m_splitter->setStretchFactor(0, 1);
+ m_splitter->setStretchFactor(1, 0);
+ layout->addWidget(m_splitter, 1);
auto *form = new QFormLayout;
m_id = new QLineEdit(this);
@@ -155,7 +176,19 @@ TagRulesDialog::TagRulesDialog(QWidget *parent)
m_addExclusion = new QPushButton(tr("Add e&xclusion"), m_builder);
builderLayout->addWidget(m_addExclusion, 0, Qt::AlignLeft);
- form->addRow(tr("Match"), m_builder);
+ // The rows scroll rather than growing without bound. The splitter alone
+ // fixes the squeeze, but only until the user drags the handle down; this
+ // caps what the editor can ever demand, so a rule with thirty senders
+ // stays as workable as one with two.
+ m_builderScroll = new QScrollArea(this);
+ m_builderScroll->setWidget(m_builder);
+ m_builderScroll->setWidgetResizable(true);
+ m_builderScroll->setFrameShape(QFrame::NoFrame);
+ m_builderScroll->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
+ // Roughly four rows. Enough that the common rule needs no scrolling at
+ // all, small enough that the list keeps most of the window.
+ m_builderScroll->setMaximumHeight(190);
+ form->addRow(tr("Match"), m_builderScroll);
// The toggle sits with the QUERY line, not inside m_builder, because
// switching to text mode HIDES m_builder. A checkbox parented there
@@ -187,7 +220,7 @@ TagRulesDialog::TagRulesDialog(QWidget *parent)
});
form->addRow(tr("Note"), m_note);
- layout->addLayout(form);
+ editorLayout->addLayout(form);
auto *buttons = new QHBoxLayout;
auto *addButton = new QPushButton(tr("&New"), this);
@@ -273,6 +306,11 @@ void TagRulesDialog::restoreUiState()
if (!geometry.isEmpty())
restoreGeometry(geometry);
+ const QByteArray splitter =
+ state.value(QStringLiteral("tagrules/splitter")).toByteArray();
+ if (!splitter.isEmpty())
+ m_splitter->restoreState(splitter);
+
const QByteArray header =
state.value(QStringLiteral("tagrules/header")).toByteArray();
if (!header.isEmpty()) {
@@ -294,6 +332,8 @@ void TagRulesDialog::saveUiState()
state.setValue(QStringLiteral("tagrules/geometry"), saveGeometry());
state.setValue(QStringLiteral("tagrules/header"),
m_list->header()->saveState());
+ state.setValue(QStringLiteral("tagrules/splitter"),
+ m_splitter->saveState());
}
/// Saves on the way out, whichever way that is.
@@ -314,6 +354,30 @@ void TagRulesDialog::done(int result)
QDialog::done(result);
}
+int TagRulesDialog::heightDemandedBelowListForTest() const
+{
+ // The builder's PREFERRED height, which is what grows with each condition
+ // row and what the list ends up paying for. minimumSizeHint is the wrong
+ // measure and reads 580 either way: a QFormLayout's minimum does not
+ // track its rows, so a test on it passes against the bug.
+ // The EDITOR PANE's minimum, which is what the splitter refuses to
+ // shrink below and therefore what the rule list actually pays. Not the
+ // builder's size hint: that grows with every row by design and is capped
+ // by the scroll area rather than reduced. Not minimumSizeHint on the
+ // dialog either, which does not track form rows at all and reads the
+ // same whether the bug is present or not.
+ return m_splitter->widget(1)->minimumSizeHint().height();
+}
+
+int TagRulesDialog::conditionAreaHeightForTest() const
+{
+ // The CAP itself, not a qMin against the scroll area's own size hint: a
+ // QScrollArea reports a small hint whether or not it is capped, and an
+ // uncapped maximumHeight is QWIDGETSIZE_MAX, so qMin picked the hint and
+ // the assertion passed with the cap removed.
+ return m_builderScroll->maximumHeight();
+}
+
int TagRulesDialog::columnWidthForTest(int column) const
{
return m_list->columnWidth(column);
@@ -434,7 +498,7 @@ void TagRulesDialog::onSelectionChanged()
const QSignalBlocker blockTextMode(m_textMode);
m_textMode->setChecked(!m_loadedQuery.parsed);
}
- m_builder->setVisible(m_loadedQuery.parsed);
+ m_builderScroll->setVisible(m_loadedQuery.parsed);
m_query->setReadOnly(m_loadedQuery.parsed);
if (m_loadedQuery.parsed)
@@ -818,7 +882,7 @@ void TagRulesDialog::setTextMode(bool on)
// Show what the rows currently mean, then hand the string over.
if (m_loadedQuery.parsed)
m_query->setText(currentQueryFromRows().compile());
- m_builder->setVisible(false);
+ m_builderScroll->setVisible(false);
m_query->setReadOnly(false);
return;
}
@@ -847,7 +911,7 @@ void TagRulesDialog::setTextMode(bool on)
m_reloading = wasReloading;
m_loadedQuery = parsed;
- m_builder->setVisible(true);
+ m_builderScroll->setVisible(true);
m_query->setReadOnly(true);
// The refusal above writes into the same label the load warnings use, so
diff --git a/src/tagrulesdialog.h b/src/tagrulesdialog.h
index bdcaff9..de35a55 100644
--- a/src/tagrulesdialog.h
+++ b/src/tagrulesdialog.h
@@ -31,6 +31,8 @@ class QLineEdit;
class QPlainTextEdit;
class QPushButton;
class QRadioButton;
+class QScrollArea;
+class QSplitter;
class QSpinBox;
class QTreeWidget;
class QTreeWidgetItem;
@@ -107,6 +109,26 @@ public:
/// the width the user had dragged.
void reloadListForTest();
+ /// The height the condition-row editor asks for. This is what squeezed
+ /// the rule list: a stretch factor only shares out space ABOVE each
+ /// widget's minimum, so every row added here came out of the list.
+ /// Measured at 120px for one row and 414px for eight before the scroll
+ /// area capped it.
+ ///
+ /// Asserted on instead of the list's rendered height because the
+ /// offscreen platform does not honour a window size, so the rendered
+ /// height there is not evidence of anything (see CLAUDE.md). Note the
+ /// size HINT, not minimumSizeHint: a QFormLayout's minimum does not track
+ /// its rows and reads the same either way, which passed against the bug.
+ int heightDemandedBelowListForTest() const;
+
+ /// How tall the condition-row area may become. The scroll area caps it;
+ /// without the cap the rows grow without bound and a long rule fills the
+ /// window again, scrolling instead of squeezing. Asserted separately
+ /// because removing the cap leaves heightDemandedBelowListForTest
+ /// unchanged, so that measure alone does not cover it.
+ int conditionAreaHeightForTest() const;
+
signals:
/// Asks the owner to run countQueries() through the worker.
void countsRequested();
@@ -211,6 +233,14 @@ private:
QRadioButton *m_matchAny = nullptr;
QCheckBox *m_textMode = nullptr;
QWidget *m_builder = nullptr;
+ /// Scrolls the condition rows, so a rule with many of them cannot grow
+ /// the editor without bound. Shown and hidden in place of m_builder for
+ /// text mode: hiding the inner widget would leave an empty scroll area.
+ QScrollArea *m_builderScroll = nullptr;
+ /// Divides the rule list from the editor. The list had stretch 1 and was
+ /// still squeezed, because a stretch factor only shares out space above
+ /// each widget's minimum and the form's grew with every condition row.
+ QSplitter *m_splitter = nullptr;
QVBoxLayout *m_rowsLayout = nullptr;
QVBoxLayout *m_exclusionsLayout = nullptr;
QLabel *m_exclusionsHeader = nullptr;