Row provided by list/table currentChanged signal is incorrect after removeRow on model

boohbah

Hi, I'm writing a program that uses List & TableViews as a means of navigation.

When a specific row is selected in the list, other widgets should receive that new row index and reset its current state to reflect the data that the new row index corresponds to (the row index directly corresponds to an element in a container, and a custom model is written as a wrapper around that).

So far I've got that to work if you click a specific row and that was simple, however I'm having issues with row removal.

When I select a row and click my button to call removeRow on it, the currentChanged signal of the list/table view's selection model is emitted, which is good & expected, however, the row index it provides in current is 1 ahead of what is actually currently selected, causing the related widgets to now show data for the wrong row.

Here is an example program that exhibits the issue:

#include <QApplication>
#include <QMainWindow>
#include <QListView>
#include <QAbstractListModel>
#include <QHBoxLayout>
#include <QVBoxLayout>
#include <QPushButton>
#include <QScreen>
#include <QLabel>

class DummyListModel : public QAbstractListModel {
public:
	explicit DummyListModel(QObject* parent = nullptr) :
		QAbstractListModel(parent),
		numDone(0)
	{
		// Just append some dummy data
		for (uint i = 0; i < 8; i++) {
			list.append(++numDone);
		}
	}

	int rowCount(const QModelIndex& parent = {}) const override {
		return list.size();
	}

	QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override {
		if (!index.isValid())
			return {};

		int row = index.row();
		if (role == Qt::DisplayRole) {
			return tr("test%1 (row %2, index %3)").arg(list[row]).arg(row + 1).arg(row);
		}

		return {};
	}

	bool insertRows(int row, int count, const QModelIndex& parent = {}) override {
		Q_UNUSED(parent);
		beginInsertRows({}, row, row + count - 1);
		list.insert(list.begin() + row, count, ++numDone);
		endInsertRows();
		return true;
	}

	bool removeRows(int row, int count, const QModelIndex& parent = {}) override {
		Q_UNUSED(parent);
		auto begin = list.begin() + row;
		beginRemoveRows({}, row, row + count - 1);
		list.erase(begin, begin + count);
		endRemoveRows();
		return true;
	}

private:
	uint numDone;
	QList<uint> list;
};

class MainWindow : public QMainWindow {
public:
	explicit MainWindow(QWidget* parent = nullptr) :
		QMainWindow(parent)
	{
		resize(250, 300);
		move(QApplication::primaryScreen()->availableGeometry().center() - frameGeometry().center());

		list = new QListView();
		btnAddItem = new QPushButton(QIcon::fromTheme("list-add"), "");
		btnDelItem = new QPushButton(QIcon::fromTheme("list-remove"), "");
		lblCur = new QLabel();

		list->setModel(new DummyListModel());
		list->setEditTriggers(QAbstractItemView::NoEditTriggers);

		connect(list->selectionModel(), &QItemSelectionModel::currentChanged, this,
		        [&](const QModelIndex& current, const QModelIndex& previous) {
			Q_UNUSED(previous);
			QString text = "Current index: " % QString::number(current.row());
			qDebug() << text;
			lblCur->setText(text);
		});

		connect(btnAddItem, &QPushButton::clicked, this, [&]() {
			int row, col;
			auto* model = list->model();
			QModelIndex cur = list->currentIndex();

			// In this case, I'd prefer inserting after the cursor
			if (cur.isValid()) {
				row = cur.row() + 1;
				col = cur.column();
			} else {
				row = col = 0;
			}

			if (model->insertRow(row)) {
				// Need to change current index after this
				list->setCurrentIndex(model->index(row, col));
			}
		});

		connect(btnDelItem, &QPushButton::clicked, this, [&]() {
			QModelIndex cur = list->currentIndex();
			auto* model = list->model();

			// Oops, after this the current row index the signal gets is off by 1
			if (cur.isValid() && model->removeRow(cur.row())) {
				return true;
			}

			return false;
		});	

		QHBoxLayout* btnLayout = new QHBoxLayout();
		btnLayout->addWidget(btnAddItem);
		btnLayout->addWidget(btnDelItem);
		btnLayout->addWidget(lblCur);
		btnLayout->addStretch(1);

		QVBoxLayout* mainLayout = new QVBoxLayout();
		mainLayout->addWidget(list);
		mainLayout->addLayout(btnLayout);

		QWidget* mainLayoutWidget = new QWidget();
		mainLayoutWidget->setLayout(mainLayout);
		setCentralWidget(mainLayoutWidget);
	}

private:
	QListView* list;
	QPushButton* btnAddItem;
	QPushButton* btnDelItem;
	QLabel* lblCur;
};

int main(int argc, char** argv) {
	QApplication app(argc, argv);

	MainWindow mainWindow;
	mainWindow.show();
	
	return app.exec();
}

Say you select row index 0 and click the remove button, the current reported index in the label at the bottom now incorrectly says index 1, despite the current selection still being at index 0. However, if you select row index 7 (the very last row) and remove it, it gives back index 6 after removal, which is correct and expected.

What shall I do? I am using Qt 6.7.2 for the record.

Thanks in advance.

artwaw

Have you tried QWidgetMapper? This is the use case.

boohbah

Hmm, I have not, but from how I understand this, I'm not sure that fits my use case. The content of a cell (which this seems to be for) is of no use, it is the index of it that is. However it does have its own currentIndexChanged signal, and perhaps that might not have the same inaccuracy problem as the selection model signals have. I'll give it a try when I can and report back.

boohbah

Okay, I've read over this again and unfortunately it doesn't seem a WidgetMapper is of any use in this case. Its currentIndexChanged signal is only emitted when you use its toFirst/toNext/etc methods, whereas I need to get the correct current row that's selected in the table. The example I provided does do this, however the issue is that after removing a row, the row provided by the list's selectionChanged model's currentChanged signal is off by one, thus providing the wrong index that I can use to access the container from.

ChrisW67

The current index changed signal is emitted during the removeRows() call. The current row is moved to the next row and its current index signalled. The target row is removed which has the effect of changing all the indexes of subsequent rows. I expect that because the same piece of data is still the new current item the currentChanged() is not emitted a second time. (Analogous to the selectionChanged behaviour).

Your label is effectively storing part of a QModelIndex. QModelIndexes are transient, hence the note in the docs, that you should not store them.
You may be able to do something with a QPersistentModelIndex or schedule the label update at the end of the remove button slot: a zero-length timer and slot that should fire after the index has changed..

boohbah

Aha, armed with what you told me, I changed my currentChanged slot lambda to be like the following:

connect(list->selectionModel(), &QItemSelectionModel::currentChanged, this, [&](const QModelIndex& current, const QModelIndex& previous) {
	Q_UNUSED(previous);
	QPersistentModelIndex idx(current);

	QTimer::singleShot(0, this, [this, idx]() {
		QString text = "Current index: " % QString::number(idx.row());
		qDebug() << text;
		lblCur->setText(text);
	});
});

And now this does seem to work. The whole QTimer thing seems a little hacky but as you've put it, the signal is emitted during the removeRows call, so there doesn't seem to be any other "proper" way.

Thank you!

GrecKo

Use a queued QMetaMethod::invoke instead of a Qtimer