From d84ba23c81fc386817198bd188bdd4d61cbd2048 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Wed, 28 Nov 2018 16:13:56 -0500 Subject: [PATCH] Correct refactor issues with entry selection and search (#2518) * Align entryview selection change signals with groupview * Eliminate redundent and confusing signals/slots * Correct group selection canceling search --- src/gui/DatabaseWidget.cpp | 46 +++++++++++++++---------------------- src/gui/DatabaseWidget.h | 7 ++---- src/gui/entry/EntryView.cpp | 13 +++++++---- src/gui/entry/EntryView.h | 3 ++- src/gui/group/GroupView.cpp | 19 +++------------ src/gui/group/GroupView.h | 5 +--- tests/gui/TestGui.cpp | 14 +++++++++-- 7 files changed, 48 insertions(+), 59 deletions(-) diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index 35beb5368..389d6fc4c 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -157,16 +157,14 @@ DatabaseWidget::DatabaseWidget(QSharedPointer db, QWidget* parent) connect(m_mainSplitter, SIGNAL(splitterMoved(int,int)), SIGNAL(mainSplitterSizesChanged())); connect(m_previewSplitter, SIGNAL(splitterMoved(int,int)), SIGNAL(previewSplitterSizesChanged())); - connect(this, SIGNAL(pressedEntry(Entry*)), m_previewView, SLOT(setEntry(Entry*))); - connect(this, SIGNAL(pressedGroup(Group*)), m_previewView, SLOT(setGroup(Group*))); connect(this, SIGNAL(currentModeChanged(DatabaseWidget::Mode)), m_previewView, SLOT(setDatabaseMode(DatabaseWidget::Mode))); connect(m_previewView, SIGNAL(errorOccurred(QString)), this, SLOT(showErrorMessage(QString))); connect(m_entryView, SIGNAL(viewStateChanged()), SIGNAL(entryViewStateChanged())); - connect(m_groupView, SIGNAL(groupChanged(Group*)), this, SLOT(onGroupChanged(Group*))); - connect(m_groupView, SIGNAL(groupChanged(Group*)), SIGNAL(groupChanged())); + connect(m_groupView, SIGNAL(groupSelectionChanged(Group*)), SLOT(onGroupChanged(Group*))); + connect(m_groupView, SIGNAL(groupSelectionChanged(Group*)), SIGNAL(groupChanged())); connect(m_entryView, SIGNAL(entryActivated(Entry*,EntryModel::ModelColumn)), SLOT(entryActivationSignalReceived(Entry*,EntryModel::ModelColumn))); - connect(m_entryView, SIGNAL(entrySelectionChanged()), SIGNAL(entrySelectionChanged())); + connect(m_entryView, SIGNAL(entrySelectionChanged(Entry*)), SLOT(onEntryChanged(Entry*))); connect(m_editEntryWidget, SIGNAL(editFinished(bool)), SLOT(switchToMainView(bool))); connect(m_editEntryWidget, SIGNAL(historyEntryActivated(Entry*)), SLOT(switchToHistoryView(Entry*))); connect(m_historyEditEntryWidget, SIGNAL(editFinished(bool)), SLOT(switchBackToEntryEdit())); @@ -180,10 +178,6 @@ DatabaseWidget::DatabaseWidget(QSharedPointer db, QWidget* parent) connect(&m_fileWatchUnblockTimer, SIGNAL(timeout()), this, SLOT(unblockAutoReload())); connect(this, SIGNAL(currentChanged(int)), this, SLOT(emitCurrentModeChanged())); - connect(m_groupView, SIGNAL(groupPressed(Group*)), SLOT(emitPressedGroup(Group*))); - connect(m_groupView, SIGNAL(groupChanged(Group*)), SLOT(emitPressedGroup(Group*))); - connect(m_editEntryWidget, SIGNAL(editFinished(bool)), SLOT(emitEntrySelectionChanged())); - connectDatabaseSignals(); m_fileWatchTimer.setSingleShot(true); @@ -700,6 +694,12 @@ void DatabaseWidget::switchToMainView(bool previousDialogAccepted) } setCurrentWidget(m_mainWidget); + + if (sender() == m_entryView) { + onEntryChanged(m_entryView->currentEntry()); + } else if (sender() == m_groupView) { + onGroupChanged(m_groupView->currentGroup()); + } } void DatabaseWidget::switchToHistoryView(Entry* entry) @@ -879,7 +879,6 @@ void DatabaseWidget::entryActivationSignalReceived(Entry* entry, EntryModel::Mod // Call this first to clear out of search mode, otherwise // the desired entry is not properly selected endSearch(); - emit clearSearch(); m_groupView->setCurrentGroup(entry->group()); m_entryView->setCurrentEntry(entry); break; @@ -1030,14 +1029,15 @@ void DatabaseWidget::setSearchLimitGroup(bool state) void DatabaseWidget::onGroupChanged(Group* group) { // Intercept group changes if in search mode - if (isSearchActive()) { + if (isSearchActive() && m_searchLimitGroup) { search(m_lastSearchText); } else if (isSearchActive()) { - // Otherwise cancel search - emit clearSearch(); + endSearch(); } else { m_entryView->displayGroup(group); } + + m_previewView->setGroup(group); } QString DatabaseWidget::getCurrentSearch() @@ -1060,6 +1060,9 @@ void DatabaseWidget::endSearch() m_searchingLabel->setText(tr("Searching...")); m_lastSearchText.clear(); + + // Tell the search widget to clear + emit clearSearch(); } void DatabaseWidget::emitGroupContextMenuRequested(const QPoint& pos) @@ -1072,26 +1075,15 @@ void DatabaseWidget::emitEntryContextMenuRequested(const QPoint& pos) emit entryContextMenuRequested(m_entryView->viewport()->mapToGlobal(pos)); } -void DatabaseWidget::emitEntrySelectionChanged() +void DatabaseWidget::onEntryChanged(Entry* entry) { - Entry* currentEntry = m_entryView->currentEntry(); - if (currentEntry) { - m_previewView->setEntry(currentEntry); + if (entry) { + m_previewView->setEntry(entry); } emit entrySelectionChanged(); } -void DatabaseWidget::emitPressedGroup(Group* currentGroup) -{ - if (!currentGroup) { - // if no group is pressed, leave in details the last group - return; - } - - emit pressedGroup(currentGroup); -} - bool DatabaseWidget::canDeleteCurrentGroup() const { bool isRootGroup = m_db->rootGroup() == m_groupView->currentGroup(); diff --git a/src/gui/DatabaseWidget.h b/src/gui/DatabaseWidget.h index 078ae9d23..1c190558c 100644 --- a/src/gui/DatabaseWidget.h +++ b/src/gui/DatabaseWidget.h @@ -132,8 +132,6 @@ signals: void databaseMerged(QSharedPointer mergedDb); void groupContextMenuRequested(const QPoint& globalPos); void entryContextMenuRequested(const QPoint& globalPos); - void pressedEntry(Entry* selectedEntry); - void pressedGroup(Group* selectedGroup); void listModeAboutToActivate(); void listModeActivated(); void searchModeAboutToActivate(); @@ -168,7 +166,6 @@ public slots: void openUrlForEntry(Entry* entry); void createGroup(); void deleteGroup(); - void onGroupChanged(Group* group); void switchToMainView(bool previousDialogAccepted = false); void switchToEntryEdit(); void switchToGroupEdit(); @@ -210,8 +207,8 @@ private slots: void switchToGroupEdit(Group* entry, bool create); void emitGroupContextMenuRequested(const QPoint& pos); void emitEntryContextMenuRequested(const QPoint& pos); - void emitPressedGroup(Group* currentGroup); - void emitEntrySelectionChanged(); + void onEntryChanged(Entry* entry); + void onGroupChanged(Group* group); void connectDatabaseSignals(); void loadDatabase(bool accepted); void unlockDatabase(bool accepted); diff --git a/src/gui/entry/EntryView.cpp b/src/gui/entry/EntryView.cpp index 64eca5ee3..fa237163e 100644 --- a/src/gui/entry/EntryView.cpp +++ b/src/gui/entry/EntryView.cpp @@ -50,7 +50,7 @@ EntryView::EntryView(QWidget* parent) setDefaultDropAction(Qt::MoveAction); connect(this, SIGNAL(doubleClicked(QModelIndex)), SLOT(emitEntryActivated(QModelIndex))); - connect(selectionModel(), SIGNAL(selectionChanged(QItemSelection,QItemSelection)), SIGNAL(entrySelectionChanged())); + connect(selectionModel(), SIGNAL(selectionChanged(QItemSelection,QItemSelection)), SLOT(emitEntrySelectionChanged())); connect(m_model, SIGNAL(usernamesHiddenChanged()), SIGNAL(viewStateChanged())); connect(m_model, SIGNAL(passwordsHiddenChanged()), SIGNAL(viewStateChanged())); @@ -144,13 +144,13 @@ void EntryView::keyPressEvent(QKeyEvent* event) void EntryView::focusInEvent(QFocusEvent* event) { - emit entrySelectionChanged(); + emit entrySelectionChanged(currentEntry()); QTreeView::focusInEvent(event); } void EntryView::focusOutEvent(QFocusEvent* event) { - emit entrySelectionChanged(); + emit entrySelectionChanged(nullptr); QTreeView::focusOutEvent(event); } @@ -181,7 +181,7 @@ void EntryView::setFirstEntryActive() QModelIndex index = m_sortModel->mapToSource(m_sortModel->index(0, 0)); setCurrentEntry(m_model->entryFromIndex(index)); } else { - emit entrySelectionChanged(); + emit entrySelectionChanged(currentEntry()); } } @@ -196,6 +196,11 @@ void EntryView::emitEntryActivated(const QModelIndex& index) emit entryActivated(entry, static_cast(m_sortModel->mapToSource(index).column())); } +void EntryView::emitEntrySelectionChanged() +{ + emit entrySelectionChanged(currentEntry()); +} + void EntryView::setModel(QAbstractItemModel* model) { Q_UNUSED(model); diff --git a/src/gui/entry/EntryView.h b/src/gui/entry/EntryView.h index 766699599..09dfd8dde 100644 --- a/src/gui/entry/EntryView.h +++ b/src/gui/entry/EntryView.h @@ -52,7 +52,7 @@ public: signals: void entryActivated(Entry* entry, EntryModel::ModelColumn column); - void entrySelectionChanged(); + void entrySelectionChanged(Entry* entry); void viewStateChanged(); public slots: @@ -66,6 +66,7 @@ protected: private slots: void emitEntryActivated(const QModelIndex& index); + void emitEntrySelectionChanged(); void showHeaderMenu(const QPoint& position); void toggleColumnVisibility(QAction* action); void fitColumnsToWindow(); diff --git a/src/gui/group/GroupView.cpp b/src/gui/group/GroupView.cpp index e98a7385e..5d7cba96a 100644 --- a/src/gui/group/GroupView.cpp +++ b/src/gui/group/GroupView.cpp @@ -34,15 +34,12 @@ GroupView::GroupView(Database* db, QWidget* parent) setHeaderHidden(true); setUniformRowHeights(true); - connect(this, SIGNAL(expanded(QModelIndex)), this, SLOT(expandedChanged(QModelIndex))); - connect(this, SIGNAL(collapsed(QModelIndex)), this, SLOT(expandedChanged(QModelIndex))); + connect(this, SIGNAL(expanded(QModelIndex)), SLOT(expandedChanged(QModelIndex))); + connect(this, SIGNAL(collapsed(QModelIndex)), SLOT(expandedChanged(QModelIndex))); connect(m_model, SIGNAL(rowsInserted(QModelIndex,int,int)), SLOT(syncExpandedState(QModelIndex,int,int))); connect(m_model, SIGNAL(modelReset()), SLOT(modelReset())); - connect(selectionModel(), SIGNAL(currentChanged(QModelIndex,QModelIndex)), SLOT(emitGroupChanged())); - connect(this, SIGNAL(clicked(QModelIndex)), SLOT(emitGroupPressed(QModelIndex))); - modelReset(); setDragEnabled(true); @@ -110,11 +107,6 @@ void GroupView::expandGroup(Group* group, bool expand) setExpanded(index, expand); } -void GroupView::emitGroupChanged(const QModelIndex& index) -{ - emit groupChanged(m_model->groupFromIndex(index)); -} - void GroupView::setModel(QAbstractItemModel* model) { Q_UNUSED(model); @@ -123,12 +115,7 @@ void GroupView::setModel(QAbstractItemModel* model) void GroupView::emitGroupChanged() { - emit groupChanged(currentGroup()); -} - -void GroupView::emitGroupPressed(const QModelIndex& index) -{ - emit groupPressed(m_model->groupFromIndex(index)); + emit groupSelectionChanged(currentGroup()); } void GroupView::syncExpandedState(const QModelIndex& parent, int start, int end) diff --git a/src/gui/group/GroupView.h b/src/gui/group/GroupView.h index 0c27b1a12..141706b07 100644 --- a/src/gui/group/GroupView.h +++ b/src/gui/group/GroupView.h @@ -37,14 +37,11 @@ public: void expandGroup(Group* group, bool expand = true); signals: - void groupChanged(Group* group); - void groupPressed(Group* group); + void groupSelectionChanged(Group* group); private slots: void expandedChanged(const QModelIndex& index); - void emitGroupChanged(const QModelIndex& index); void emitGroupChanged(); - void emitGroupPressed(const QModelIndex& index); void syncExpandedState(const QModelIndex& parent, int start, int end); void modelReset(); diff --git a/tests/gui/TestGui.cpp b/tests/gui/TestGui.cpp index 1964b0e0f..48c308243 100644 --- a/tests/gui/TestGui.cpp +++ b/tests/gui/TestGui.cpp @@ -875,18 +875,28 @@ void TestGui::testSearch() QTRY_COMPARE(entryView->model()->rowCount(), 2); // Test group search + searchWidget->setLimitGroup(false); GroupView* groupView = m_dbWidget->findChild("groupView"); QCOMPARE(groupView->currentGroup(), m_db->rootGroup()); QModelIndex rootGroupIndex = groupView->model()->index(0, 0); clickIndex(groupView->model()->index(0, 0, rootGroupIndex), groupView, Qt::LeftButton); QCOMPARE(groupView->currentGroup()->name(), QString("General")); - - searchWidget->setLimitGroup(false); + // Selecting a group should cancel search + QTRY_COMPARE(entryView->model()->rowCount(), 0); + // Restore search + QTest::keyClick(m_mainWindow.data(), Qt::Key_F, Qt::ControlModifier); + QTest::keyClicks(searchTextEdit, "someTHING"); QTRY_COMPARE(entryView->model()->rowCount(), 2); + // Enable group limiting searchWidget->setLimitGroup(true); QTRY_COMPARE(entryView->model()->rowCount(), 0); + // Selecting another group should NOT cancel search + clickIndex(rootGroupIndex, groupView, Qt::LeftButton); + QCOMPARE(groupView->currentGroup(), m_db->rootGroup()); + QTRY_COMPARE(entryView->model()->rowCount(), 2); // reset + searchWidget->setLimitGroup(false); clickIndex(rootGroupIndex, groupView, Qt::LeftButton); QCOMPARE(groupView->currentGroup(), m_db->rootGroup());