From 33a6d74a4370e3091ab85518a8338c78f30c572e Mon Sep 17 00:00:00 2001 From: Robert Colton Date: Tue, 1 Sep 2020 12:56:19 -0400 Subject: [PATCH] Extended Selection & Deletion of Path Points (#179) * Overload `RowRemovalOperation::RemoveRows` to accept a `QModelIndexList` of all selected rows. * Set [QAbstractItemView::ExtendedSelection](https://doc.qt.io/qt-5/qabstractitemview.html#SelectionMode-enum) on Path editor points list view so we can select to delete many points at once. This does not change the fact that there is a current index inside the selection which has the focus if an edit is requested. * Set [QAbstractItemView::SelectRows](https://doc.qt.io/qt-5/qabstractitemview.html#SelectionBehavior-enum) on Path editors points list view so we are actually selecting the entire point, which is similar to GM/LGM and feels less awkward. There is still a current index inside the selected row which has the focus if an edit is requested in a specific column item (aka the field). * Use the new `RowRemovalOperation` overload to bulk delete all the selected points in the path editor. I used [QItemSelectionModel::selectedRows](https://doc.qt.io/qt-5/qitemselectionmodel.html#selectedRows) so that we only delete points where every column (aka the entire row) is selected. This should always be the case since I've also changed the selection behavior to select rows instead of items. * Remove selection after setting the current index, which already does a correct selection. As I've stated, the current index resides in the selection and belongs to the selection model. * Add clarifying comments to the selection update slot about interaction with the above changes. * The selected indexes passed to selection change slot may be empty and yet there is still a selection and so enabling of the delete button now needs to check the global selection model of the points table to determine if anything is selected. It should also check if there are selected rows and not just selected items. * The selected point in the preview area is now the last, most recently selected point in the global, cumulative list selection. * Deleting a point now keeps the same row selected, unless the last point is being deleted. This is how GM and the main Qt examples work with as well. Bounds checking is done against the row count so when the last point is deleted we do go to the previous row. --- Editors/PathEditor.cpp | 32 +++++++++++++++++--------------- Editors/PathEditor.ui | 4 ++-- Models/RepeatedModel.h | 4 ++++ 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/Editors/PathEditor.cpp b/Editors/PathEditor.cpp index d5cb8463f..7977a2f64 100644 --- a/Editors/PathEditor.cpp +++ b/Editors/PathEditor.cpp @@ -195,8 +195,6 @@ void PathEditor::MousePressed(Qt::MouseButton button) { if (pt == _ui->roomView->mousePos) { QModelIndex newSelectIndex = _pointsModel->index(i, Path::Point::kXFieldNumber); _ui->pointsTableView->setCurrentIndex(newSelectIndex); - _ui->pointsTableView->selectionModel()->select(newSelectIndex, - QItemSelectionModel::QItemSelectionModel::ClearAndSelect); return; } } @@ -212,12 +210,17 @@ void PathEditor::MouseReleased(Qt::MouseButton button) { } } -void PathEditor::UpdateSelection(const QItemSelection& selected, const QItemSelection& /*deselected*/) { - int selectIndex = -1; - if (!selected.indexes().empty()) selectIndex = selected.indexes()[0].row(); - _ui->roomView->selectedPointIndex = selectIndex; +void PathEditor::UpdateSelection(const QItemSelection& /*selected*/, const QItemSelection& /*deselected*/) { + auto selectedPoints = _ui->pointsTableView->selectionModel()->selectedRows(); + bool hasSelectedPoint = !selectedPoints.empty(); + // delete button should be enabled if the cumulative selection is not empty + // and contains at least one selected row with every column selected + _ui->deletePointButton->setEnabled(hasSelectedPoint); + // keep most recently selected point selected in the preview + if (hasSelectedPoint) + _ui->roomView->selectedPointIndex = selectedPoints.last().row(); + // update preview on point selection and deselection _ui->pathPreviewBackground->update(); - _ui->deletePointButton->setDisabled((selectIndex == -1)); } void PathEditor::on_addPointButton_pressed() { @@ -232,17 +235,16 @@ void PathEditor::on_insertPointButton_pressed() { void PathEditor::on_deletePointButton_pressed() { int deleteIndex = _ui->pointsTableView->selectionModel()->currentIndex().row(); - { - RepeatedMessageModel::RowRemovalOperation remover(_pointsModel); - remover.RemoveRow(deleteIndex); - } + // this operation is temporary and will self destruct immediately removing the rows + RepeatedMessageModel::RowRemovalOperation(_pointsModel) + .RemoveRows(_ui->pointsTableView->selectionModel()->selectedRows()); if (_pointsModel->rowCount() > 0) { - QModelIndex newSelectIndex = - _pointsModel->index((deleteIndex == 0) ? 0 : deleteIndex - 1, Path::Point::kXFieldNumber); + auto rowCount = _ui->pointsTableView->model()->rowCount(); + QModelIndex newSelectIndex = _pointsModel->index( + (deleteIndex >= rowCount) ? rowCount - 1 : deleteIndex, + Path::Point::kXFieldNumber); _ui->pointsTableView->setCurrentIndex(newSelectIndex); - _ui->pointsTableView->selectionModel()->select(newSelectIndex, - QItemSelectionModel::QItemSelectionModel::ClearAndSelect); } else { _ui->deletePointButton->setDisabled(true); _ui->pointsTableView->selectionModel()->clearSelection(); diff --git a/Editors/PathEditor.ui b/Editors/PathEditor.ui index 0ae089e00..a0052bf17 100644 --- a/Editors/PathEditor.ui +++ b/Editors/PathEditor.ui @@ -513,10 +513,10 @@ - QAbstractItemView::SingleSelection + QAbstractItemView::ExtendedSelection - QAbstractItemView::SelectItems + QAbstractItemView::SelectRows true diff --git a/Models/RepeatedModel.h b/Models/RepeatedModel.h index 9ac943c91..60e57cfa0 100644 --- a/Models/RepeatedModel.h +++ b/Models/RepeatedModel.h @@ -144,6 +144,10 @@ class RepeatedModel : public ProtoModel { void RemoveRows(int row, int count) { for (int i = row; i < row + count; ++i) _rows.insert(i); } + void RemoveRows(const QModelIndexList& indexes) { + foreach (auto index, indexes) + RemoveRow(index.row()); + } ~RowRemovalOperation() { if (_rows.empty()) return;