[VDS] Address review: dead code, chip reparenting, filter signal and sort fast-path

This commit is contained in:
Lukas Brübach 2026-09-18 23:11:17 +02:00
parent 1dc45a4765
commit c1c94c2624
6 changed files with 29 additions and 15 deletions

View file

@ -48,6 +48,7 @@ QSize DeckPreviewTagDisplayWidget::sizeHint() const
void DeckPreviewTagDisplayWidget::mousePressEvent(QMouseEvent *event)
{
const TagState previousState = state;
switch (event->button()) {
case Qt::LeftButton:
setState(state != TagState::Selected ? TagState::Selected : TagState::NotSelected);
@ -62,7 +63,12 @@ void DeckPreviewTagDisplayWidget::mousePressEvent(QMouseEvent *event)
break;
}
emit tagClicked();
// Only announce a change when the state was actually toggled, so a click that falls
// through the switch (e.g. a button the widget does not react to) does not drive a
// full tag-filter update and layout pass for nothing.
if (state != previousState) {
emit tagClicked();
}
QWidget::mousePressEvent(event);
}

View file

@ -48,7 +48,10 @@ public:
signals:
/**
* @brief Emitted when the tag is clicked.
* @brief Emitted when a click toggles the chip's selection/exclusion state.
*
* Not emitted for clicks that leave the state unchanged. Connected handlers use
* this as the trigger to update filters built from selectedTags()/excludedTags().
*/
void tagClicked();

View file

@ -39,8 +39,3 @@ VisualDeckStorageSearchWidget::VisualDeckStorageSearchWidget(QWidget *parent) :
connect(searchDebounceTimer, &QTimer::timeout, this, [this] { emit searchTextChanged(searchBar->text()); });
}
void VisualDeckStorageSearchWidget::setPlaceholderText(const QString &text)
{
searchBar->setPlaceholderText(text);
}

View file

@ -19,8 +19,6 @@ class VisualDeckStorageSearchWidget : public QWidget
public:
explicit VisualDeckStorageSearchWidget(QWidget *parent);
void setPlaceholderText(const QString &text);
signals:
/**
* Emitted once the debounce timer fires after the user stopped typing.

View file

@ -38,15 +38,17 @@ void VisualDeckStorageTagFilterWidget::refreshTags()
// Existing chips survive if their tag is still part of the deck set, or if the chip
// is currently selected/excluded. Everything else is dropped. Dropped chips must NOT
// be re-added to the layout afterwards: they are scheduled for a deferred delete, and
// the flow layout would keep a dangling reference to them once the deletion runs on
// the next event-loop cycle.
// be re-added to the layout afterwards: they are reparented to nullptr and scheduled
// for a deferred delete (QWidget::setParent(nullptr) also hides them), and the flow
// layout would keep a dangling reference to them once the deletion runs on the next
// event-loop cycle.
QList<DeckPreviewTagDisplayWidget *> chips;
for (DeckPreviewTagDisplayWidget *tagWidget : findChildren<DeckPreviewTagDisplayWidget *>()) {
if (tagWidget->getState() != TagState::NotSelected || allTags.contains(tagWidget->getTagName())) {
chips.append(tagWidget);
} else {
flowWidget->removeWidget(tagWidget);
tagWidget->setParent(nullptr);
tagWidget->deleteLater();
}
}
@ -69,16 +71,22 @@ void VisualDeckStorageTagFilterWidget::refreshTags()
}
}
// Clear and re-add the chips in sorted order.
// Sort, but skip the full remove/re-add when the order already matches the layout.
// FlowWidget inherits QLayout::removeWidget's linear scan, so rebuilding an unchanged
// order would be quadratic plus a full relayout on every chip click and load batch.
std::sort(chips.begin(), chips.end(), [](DeckPreviewTagDisplayWidget *a, DeckPreviewTagDisplayWidget *b) {
return a->getTagName().toLower() < b->getTagName().toLower();
});
if (chips == currentChipOrder) {
return;
}
for (DeckPreviewTagDisplayWidget *tagWidget : chips) {
flowWidget->removeWidget(tagWidget);
}
for (DeckPreviewTagDisplayWidget *tagWidget : chips) {
flowWidget->addWidget(tagWidget);
}
currentChipOrder = chips;
}
QStringList VisualDeckStorageTagFilterWidget::selectedTags() const

View file

@ -11,8 +11,8 @@
#include <QWidget>
#include <functional>
class DeckPreviewTagDisplayWidget;
class FlowWidget;
class VisualDeckStorageWidget;
class VisualDeckStorageTagFilterWidget : public QWidget
{
@ -20,6 +20,7 @@ class VisualDeckStorageTagFilterWidget : public QWidget
FlowWidget *flowWidget;
std::function<QSet<QString>()> allTagsProvider;
QList<DeckPreviewTagDisplayWidget *> currentChipOrder;
public:
explicit VisualDeckStorageTagFilterWidget(QWidget *parent = nullptr);
@ -42,7 +43,10 @@ public:
signals:
/**
* Emitted whenever a chip's selection/exclusion state changes.
* Emitted when a chip's selection or exclusion state changes.
*
* The chip only emits when its state actually changed, so this fires once per
* effective toggle rather than on every click."
*/
void filterChanged();