[Completer] Minimum size popup, fall back to compact design if not possible. (#7111)
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 cover (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Desktop / Prune caches (push) Waiting to run
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions

* [Completer] Minimum size popup, fall back to compact design if not possible.

Took 15 minutes

Took 6 seconds


Took 5 minutes

* [Completer] Fix popup going offscreen by clamping in screen coordinates

The position clamping in applyPopupWidth() was comparing popup->pos()
(parent coordinates) against screen->availableGeometry() (screen
coordinates).  When the parent widget is not at the screen origin the
clamping computed a position in the wrong coordinate space, placing the
popup offscreen.  Convert to global coordinates first, clamp, then map
back to parent coordinates.

* [Completer] Clamp popup from geometry() in screen coordinates

setMinimumWidth() already widens the popup synchronously, so the old
width() < minWidth guard never ran and the popup grew past the right
screen edge. The popup is also a top-level Qt::Popup window, so move()
takes global coordinates and the mapFromGlobal round-trip moved it to
the top-left of the screen. Clamp x from geometry() after
setMinimumWidth() and move(QPoint(clampedX, popup->y())) directly.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
BruebachL 2026-10-15 18:10:07 +02:00 • committed by GitHub
parent 96b0888a73
commit 213ce691dd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 89 additions and 14 deletions

View file

@ -46,6 +46,22 @@ template <class T> class QSharedPointer;
namespace namespace
{ {
// Below this row width the delegate switches to a compact layout so the card
// name stays readable when the popup cannot be as wide as PopupRowWidth.
constexpr int CompactRowWidth = 380;
constexpr int CompactSymbolRadius = 6;
constexpr int CompactSymbolSpacing = 1;
constexpr int CompactRightPad = 8;
constexpr int ManaZoneWidth = 110;
constexpr int CompactManaZoneWidth = 70;
constexpr int RightPad = 14;
constexpr int TextLeftPad = 12;
constexpr int TextRightGap = 8;
// Below this text width the type-line band is dropped so the name can use the
// whole row height instead of being elided to a single readable character.
constexpr int MinNameWidth = 120;
struct ManaColor struct ManaColor
{ {
QColor fill; QColor fill;
@ -125,7 +141,7 @@ QSize CardCompleterDelegate::sizeHint(const QStyleOptionViewItem &option, const
} }
// Fixed wide rows so the popup has room for name, type line, set and mana // Fixed wide rows so the popup has room for name, type line, set and mana
return {480, CardRowHeight}; return {PopupRowWidth, CardRowHeight};
} }
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
@ -176,7 +192,12 @@ void CardCompleterDelegate::drawManaSymbol(QPainter *p, QPoint centre, const QSt
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
int CardCompleterDelegate::drawManaCost(QPainter *p, const QRect &row, const QString &manaCost, int radius) const int CardCompleterDelegate::drawManaCost(QPainter *p,
const QRect &row,
const QString &manaCost,
int radius,
int spacing,
int rightPad) const
{ {
if (manaCost.isEmpty()) { if (manaCost.isEmpty()) {
return row.right(); return row.right();
@ -205,11 +226,9 @@ int CardCompleterDelegate::drawManaCost(QPainter *p, const QRect &row, const QSt
totalW += PartGap; totalW += PartGap;
} }
totalW += parts.at(i).size() * diam + qMax(0, parts.at(i).size() - 1) * SymbolSpacing; totalW += parts.at(i).size() * diam + qMax(0, parts.at(i).size() - 1) * spacing;
} }
const int rightPad = 14;
int x = row.right() - rightPad - totalW + radius; int x = row.right() - rightPad - totalW + radius;
const int cy = row.center().y(); const int cy = row.center().y();
@ -219,11 +238,11 @@ int CardCompleterDelegate::drawManaCost(QPainter *p, const QRect &row, const QSt
for (const QString &sym : symbols) { for (const QString &sym : symbols) {
drawManaSymbol(p, {x, cy}, sym, radius); drawManaSymbol(p, {x, cy}, sym, radius);
x += diam + SymbolSpacing; x += diam + spacing;
} }
if (i < parts.size() - 1) { if (i < parts.size() - 1) {
x += PartGap - SymbolSpacing; x += PartGap - spacing;
} }
} }
@ -290,6 +309,14 @@ void CardCompleterDelegate::paint(QPainter *painter, const QStyleOptionViewItem
const QString typeLine = card->getCardType(); const QString typeLine = card->getCardType();
const QString setCode = setCodeForCard(card); const QString setCode = setCodeForCard(card);
// When the row is too narrow for the full layout, use smaller mana pips and
// a slimmer mana zone so the card name keeps as much room as possible.
const bool compact = rect.width() < CompactRowWidth;
const int symbolRadius = compact ? CompactSymbolRadius : SymbolRadius;
const int symbolSpacing = compact ? CompactSymbolSpacing : SymbolSpacing;
const int rightPad = compact ? CompactRightPad : RightPad;
const int manaZoneWidth = compact ? CompactManaZoneWidth : ManaZoneWidth;
const QColor accent = accentForColors(card->getColors()); const QColor accent = accentForColors(card->getColors());
const QColor base = pal.color(QPalette::Base); const QColor base = pal.color(QPalette::Base);
@ -338,7 +365,7 @@ void CardCompleterDelegate::paint(QPainter *painter, const QStyleOptionViewItem
// Right mana zone // Right mana zone
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
const QRectF manaZone(cardRect.right() - 110, cardRect.top(), 110, cardRect.height()); const QRectF manaZone(cardRect.right() - manaZoneWidth, cardRect.top(), manaZoneWidth, cardRect.height());
QLinearGradient manaGrad(manaZone.topLeft(), manaZone.bottomLeft()); QLinearGradient manaGrad(manaZone.topLeft(), manaZone.bottomLeft());
@ -380,22 +407,27 @@ void CardCompleterDelegate::paint(QPainter *painter, const QStyleOptionViewItem
// Mana cost // Mana cost
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
const int costLeft = drawManaCost(painter, cardRect.toRect(), manaCost, SymbolRadius); const int costLeft = drawManaCost(painter, cardRect.toRect(), manaCost, symbolRadius, symbolSpacing, rightPad);
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
// Card name + type line + set code // Card name + type line + set code
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
const int textLeft = cardRect.left() + AccentBarWidth + 12; const int textLeft = cardRect.left() + AccentBarWidth + TextLeftPad;
const int textRight = costLeft - 8; const int textRight = costLeft - TextRightGap;
const int textWidth = qMax(0, textRight - textLeft); const int textWidth = qMax(0, textRight - textLeft);
// If even the compact layout leaves too little room for a readable name,
// drop the type-line band and let the name use the full row height.
const bool showInfoBand = textWidth >= MinNameWidth;
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
// Card name (top band) // Card name (top band)
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
{ {
const QRect nameRect(textLeft, rect.top() + 2, textWidth, 20); const QRect nameRect = showInfoBand ? QRect(textLeft, rect.top() + 2, textWidth, 20)
: QRect(textLeft, rect.top() + 2, textWidth, rect.height() - 4);
QFont f = option.font; QFont f = option.font;
f.setPixelSize(13); f.setPixelSize(13);
@ -416,7 +448,7 @@ void CardCompleterDelegate::paint(QPainter *painter, const QStyleOptionViewItem
// Type line + set code (bottom band) // Type line + set code (bottom band)
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
{ if (showInfoBand) {
const QRect infoRect(textLeft, rect.top() + 22, textWidth, rect.height() - 24); const QRect infoRect(textLeft, rect.top() + 22, textWidth, rect.height() - 24);
QFont f = option.font; QFont f = option.font;

View file

@ -36,6 +36,9 @@ public:
void paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const override; void paint(QPainter *painter, const QStyleOptionViewItem &option, const QModelIndex &index) const override;
QSize sizeHint(const QStyleOptionViewItem &option, const QModelIndex &index) const override; QSize sizeHint(const QStyleOptionViewItem &option, const QModelIndex &index) const override;
// Content width of one popup row; the completer popup is sized to fit it.
static constexpr int PopupRowWidth = 480;
private: private:
// Set short codes, resolved once per card name and cached // Set short codes, resolved once per card name and cached
mutable QCache<QString, QString> setCodeCache; mutable QCache<QString, QString> setCodeCache;
@ -48,7 +51,8 @@ private:
// Draw all mana pips for a cost string like "2RG" or "{2}{R}{G}"; split and // Draw all mana pips for a cost string like "2RG" or "{2}{R}{G}"; split and
// adventure costs ("1W // W") are drawn as separate groups. Returns the left-most x used // adventure costs ("1W // W") are drawn as separate groups. Returns the left-most x used
int drawManaCost(QPainter *p, const QRect &row, const QString &manaCost, int radius) const; int
drawManaCost(QPainter *p, const QRect &row, const QString &manaCost, int radius, int spacing, int rightPad) const;
// Resolve the preferred printing's set short code for a card // Resolve the preferred printing's set short code for a card
QString setCodeForCard(const QSharedPointer<CardInfo> &card) const; QString setCodeForCard(const QSharedPointer<CardInfo> &card) const;

View file

@ -20,6 +20,7 @@
#include <QPropertyAnimation> #include <QPropertyAnimation>
#include <QRect> #include <QRect>
#include <QScreen> #include <QScreen>
#include <QScrollBar>
#include <QSharedPointer> #include <QSharedPointer>
#include <QSize> #include <QSize>
#include <QString> #include <QString>
@ -103,6 +104,7 @@ bool CardCompleterStyler::eventFilter(QObject *obj, QEvent *ev)
case QEvent::Show: case QEvent::Show:
case QEvent::Move: case QEvent::Move:
case QEvent::Resize: case QEvent::Resize:
applyPopupWidth();
updateOrientation(); updateOrientation();
reposition(); reposition();
break; break;
@ -159,6 +161,42 @@ bool CardCompleterStyler::handlePopupKeyPress(QKeyEvent *event)
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
void CardCompleterStyler::applyPopupWidth()
{
auto *popup = completer->popup();
// QCompleter sizes its popup to the line edit, so a narrow search field
// would crush the rows. Ask for the width the rows are designed for (plus
// the frame and scrollbar) and let QWidget's geometry clamping keep the
// popup that wide, clamped to the screen it is shown on.
int contentWidth = CardCompleterDelegate::PopupRowWidth + popup->frameWidth() * 2;
if (auto *vbar = popup->verticalScrollBar(); vbar != nullptr) {
contentWidth += vbar->sizeHint().width();
}
const QScreen *screen = popup->screen();
const QRect available = screen ? screen->availableGeometry() : QRect();
const int minWidth = available.isEmpty() ? contentWidth : qMin(contentWidth, available.width());
popup->setMinimumWidth(minWidth);
// setMinimumWidth() already widened the popup if it was narrower, but
// QCompleter positioned it for the line-edit width, so the widened popup
// can now extend past the right screen edge. The popup is a top-level
// Qt::Popup window, so geometry()/move() are already in screen
// coordinates: clamp the x from geometry() and move directly.
if (!available.isEmpty()) {
const QRect geom = popup->geometry();
const int clampedX =
qBound(available.left(), geom.x(), qMax(available.left(), available.right() - geom.width() + 1));
if (clampedX != geom.x()) {
popup->move(QPoint(clampedX, popup->y()));
}
}
}
// ---------------------------------------------------------------------------
void CardCompleterStyler::updateOrientation() void CardCompleterStyler::updateOrientation()
{ {
above = isPopupAboveWidget(); above = isPopupAboveWidget();

View file

@ -52,6 +52,7 @@ private:
void stopPreviewFade(); void stopPreviewFade();
void reposition(); void reposition();
void applyPopupWidth();
void updateOrientation(); void updateOrientation();
void ensureClosestSelected(); void ensureClosestSelected();