From 58a53e70d1c7ffe9eaccbfb37fd0bce82a201c6b Mon Sep 17 00:00:00 2001
From: Andreas Traczyk <andreas.traczyk@savoirfairelinux.com>
Date: Mon, 4 Dec 2023 13:51:27 -0500
Subject: [PATCH] smartlist: prevent selection oscillation
The dataChanged signal should be queued to prevent the conversation selection callback from re-entering SelectableListProxyModel::updateSelection prior to sort/filter updates. This can cause desync of the source and filter indices.
Change-Id: I35b3b38512cf51b1dd7ca8110b143a9223740056
---
src/app/conversationlistmodel.cpp | 2 +-
src/app/selectablelistproxymodel.cpp | 42 ++++++++++++----------------
2 files changed, 19 insertions(+), 25 deletions(-)
diff --git a/src/app/conversationlistmodel.cpp b/src/app/conversationlistmodel.cpp
index 5b8135349..462703b82 100644
--- a/src/app/conversationlistmodel.cpp
+++ b/src/app/conversationlistmodel.cpp
@@ -58,7 +58,7 @@ ConversationListModel::ConversationListModel(LRCInstance* instance, QObject* par
connect(model_, &ConversationModel::dataChanged, this, [this](int position) {
const auto index = createIndex(position, 0);
Q_EMIT ConversationListModel::dataChanged(index, index);
- });
+ }, Qt::QueuedConnection);
}
int
diff --git a/src/app/selectablelistproxymodel.cpp b/src/app/selectablelistproxymodel.cpp
index 51410fdb0..0106d30e5 100644
--- a/src/app/selectablelistproxymodel.cpp
+++ b/src/app/selectablelistproxymodel.cpp
@@ -33,26 +33,15 @@ SelectableListProxyModel::bindSourceModel(QAbstractListModel* model)
if (!model) {
return;
}
- connect(sourceModel(),
- &QAbstractListModel::dataChanged,
- this,
- &SelectableListProxyModel::onModelUpdated,
- Qt::UniqueConnection);
- connect(model,
- &QAbstractListModel::rowsInserted,
- this,
- &SelectableListProxyModel::onModelUpdated,
- Qt::UniqueConnection);
- connect(model,
- &QAbstractListModel::rowsRemoved,
- this,
- &SelectableListProxyModel::onModelTrimmed,
- Qt::UniqueConnection);
- connect(sourceModel(),
- &QAbstractListModel::modelReset,
- this,
- &SelectableListProxyModel::deselect,
- Qt::UniqueConnection);
+
+ auto connectModelSignal = [this, model](auto signal, auto slot) {
+ connect(model, signal, this, slot, Qt::UniqueConnection);
+ };
+
+ connectModelSignal(&QAbstractListModel::dataChanged, &SelectableListProxyModel::onModelUpdated);
+ connectModelSignal(&QAbstractListModel::rowsInserted, &SelectableListProxyModel::onModelUpdated);
+ connectModelSignal(&QAbstractListModel::rowsRemoved, &SelectableListProxyModel::onModelTrimmed);
+ connectModelSignal(&QAbstractListModel::modelReset, &SelectableListProxyModel::deselect);
}
void
@@ -120,11 +109,14 @@ SelectableListProxyModel::updateSelection(bool rowsRemoved)
// if the source and filtered index is no longer valid
// this would indicate that a mutation has occured,
- // thus any arbritrary ux decision is okay here
+ // In this case, we need to update the selection to one
+ // row above the current one.
if (!selectedSourceIndex_.isValid()) {
- auto row = qMax(--currentFilteredRow_, 0);
+ auto row = qMax(currentFilteredRow_ - 1, 0);
selectedSourceIndex_ = mapToSource(index(row, 0));
filteredIndex = mapFromSource(selectedSourceIndex_);
+ // filteredIndex is not necessarily valid here, so we emit
+ // forcefully to ensure that the selection is updated
currentFilteredRow_ = filteredIndex.row();
Q_EMIT currentFilteredRowChanged();
Q_EMIT validSelectionChanged();
@@ -139,9 +131,11 @@ SelectableListProxyModel::updateSelection(bool rowsRemoved)
// want to force reselection of other ui components, as the
// source index is still valid, in that case, or if the
// row hasn't changed, don't notify
- if (filteredIndex.isValid() && lastFilteredRow != currentFilteredRow_) {
- Q_EMIT validSelectionChanged();
+ if (!filteredIndex.isValid() || lastFilteredRow == currentFilteredRow_) {
+ return;
}
+
+ Q_EMIT validSelectionChanged();
}
void
--
GitLab