From 46a27ca937309d97153268cd11da8b9d75d3f1f4 Mon Sep 17 00:00:00 2001 From: Frank Reininghaus Date: Mon, 7 Oct 2013 09:08:55 +0200 Subject: Include "Space" in the keyboard search string Before this commit, we only added pressed keys to the search string if they have no other meaning. This means that files containing a Space in their name could not be searched because Ctrl+Space toggles the selection state of the current item, and Space alone selects the current item. After this commit, Space is added to the search string if (a) the key press did not have any other effect, i.e., if Ctrl was not pressed, and the current item is selected already, and (b) a keyboard search has been started already (to prevent unexpected effects when pressing Space accidentally - I think that it's rather uncommon to have files whose names start with a Space - and to make the unit test simpler). I modified the unit test of KItemListController, which did not test keyboard search yet. This uncovered a small problem in KItemListController::slotChangeCurrentItem() when NoSelection mode is used. It's not really relevant for anything that is executed inside Dolphin, but I still fixed it to make the unit test happy. BUG: 324479 FIXED-IN: 4.11.3 REVIEW: 113071 --- src/kitemviews/kitemlistcontroller.cpp | 42 +++++++++++++--------- .../private/kitemlistkeyboardsearchmanager.cpp | 6 ++++ src/tests/kitemlistcontrollertest.cpp | 11 ++++-- 3 files changed, 41 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/kitemviews/kitemlistcontroller.cpp b/src/kitemviews/kitemlistcontroller.cpp index 4629b29f1..9335ab816 100644 --- a/src/kitemviews/kitemlistcontroller.cpp +++ b/src/kitemviews/kitemlistcontroller.cpp @@ -375,19 +375,6 @@ bool KItemListController::keyPressEvent(QKeyEvent* event) break; } - case Qt::Key_Space: - if (m_selectionBehavior == MultiSelection) { - if (controlPressed) { - m_selectionManager->endAnchoredSelection(); - m_selectionManager->setSelected(index, 1, KItemListSelectionManager::Toggle); - m_selectionManager->beginAnchoredSelection(index); - } else { - const int current = m_selectionManager->currentItem(); - m_selectionManager->setSelected(current); - } - } - break; - case Qt::Key_Menu: { // Emit the signal itemContextMenuRequested() in case if at least one // item is selected. Otherwise the signal viewContextMenuRequested() will be emitted. @@ -418,6 +405,25 @@ bool KItemListController::keyPressEvent(QKeyEvent* event) m_keyboardManager->cancelSearch(); break; + case Qt::Key_Space: + if (m_selectionBehavior == MultiSelection) { + if (controlPressed) { + // Toggle the selection state of the current item. + m_selectionManager->endAnchoredSelection(); + m_selectionManager->setSelected(index, 1, KItemListSelectionManager::Toggle); + m_selectionManager->beginAnchoredSelection(index); + break; + } else { + // Select the current item if it is not selected yet. + const int current = m_selectionManager->currentItem(); + if (!m_selectionManager->isSelected(current)) { + m_selectionManager->setSelected(current); + break; + } + } + } + // Fall through to the default case and add the Space to the current search string. + default: m_keyboardManager->addKeys(event->text()); // Make sure unconsumed events get propagated up the chain. #302329 @@ -474,9 +480,13 @@ void KItemListController::slotChangeCurrentItem(const QString& text, bool search } if (index >= 0) { m_selectionManager->setCurrentItem(index); - m_selectionManager->clearSelection(); - m_selectionManager->setSelected(index, 1); - m_selectionManager->beginAnchoredSelection(index); + + if (m_selectionBehavior != NoSelection) { + m_selectionManager->clearSelection(); + m_selectionManager->setSelected(index, 1); + m_selectionManager->beginAnchoredSelection(index); + } + m_view->scrollToItem(index); } } diff --git a/src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp b/src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp index 38154864b..3bd1b01f3 100644 --- a/src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp +++ b/src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp @@ -46,6 +46,12 @@ void KItemListKeyboardSearchManager::addKeys(const QString& keys) const bool newSearch = m_searchedString.isEmpty(); + // Do not start a new search if the user pressed Space. Only add + // it to the search string if a search is in progress already. + if (newSearch && keys == QLatin1String(" ")) { + return; + } + if (!keys.isEmpty()) { m_searchedString.append(keys); diff --git a/src/tests/kitemlistcontrollertest.cpp b/src/tests/kitemlistcontrollertest.cpp index d8f838873..60e93e539 100644 --- a/src/tests/kitemlistcontrollertest.cpp +++ b/src/tests/kitemlistcontrollertest.cpp @@ -98,7 +98,7 @@ void KItemListControllerTest::initTestCase() << "b1" << "c1" << "c2" << "c3" << "c4" << "c5" << "d1" << "d2" << "d3" << "d4" - << "e1" << "e2" << "e3" << "e4" << "e5" << "e6" << "e7"; + << "e" << "e 2" << "e 3" << "e 4" << "e 5" << "e 6" << "e 7"; m_testDir->createFiles(files); m_model->loadDirectory(m_testDir->url()); @@ -282,7 +282,14 @@ void KItemListControllerTest::testKeyboardNavigation_data() << qMakePair(KeyPress(Qt::Key_Home), ViewState(0, QSet() << 0)) << qMakePair(KeyPress(Qt::Key_Space, Qt::ControlModifier), ViewState(0, QSet())) << qMakePair(KeyPress(Qt::Key_Enter), ViewState(0, QSet(), true)) - << qMakePair(KeyPress(Qt::Key_Space, Qt::ControlModifier), ViewState(0, QSet() << 0)); + << qMakePair(KeyPress(Qt::Key_Space, Qt::ControlModifier), ViewState(0, QSet() << 0)) + << qMakePair(KeyPress(Qt::Key_Space, Qt::ControlModifier), ViewState(0, QSet())) + << qMakePair(KeyPress(Qt::Key_Space), ViewState(0, QSet() << 0)) + << qMakePair(KeyPress(Qt::Key_E), ViewState(13, QSet() << 13)) + << qMakePair(KeyPress(Qt::Key_Space), ViewState(14, QSet() << 14)) + << qMakePair(KeyPress(Qt::Key_3), ViewState(15, QSet() << 15)) + << qMakePair(KeyPress(Qt::Key_Home), ViewState(0, QSet() << 0)) + << qMakePair(KeyPress(Qt::Key_Escape), ViewState(0, QSet())); // Next, we test combinations of key presses which only work for a // particular number of columns and either enabled or disabled grouping. -- cgit v1.3 From 5bfb5031a593fbd7e0a60bd8ca869671c712db9d Mon Sep 17 00:00:00 2001 From: Frank Reininghaus Date: Mon, 7 Oct 2013 09:17:48 +0200 Subject: Reload the view if a previously unmounted device is mounted again The problem was that DolphinViewContainer::setUrl(newUrl) was ignored if newUrl is equal to the URL which is shown in the view already. The new approach is to reload the view in that method if it is empty, to make sure that we do not miss that a previously unmounted device has been re-mounted. Thanks to Grigoriadis Grigoris for analyzing the root cause of this issue! BUG: 161385 FIXED-IN: 4.11.3 --- src/dolphinviewcontainer.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src') diff --git a/src/dolphinviewcontainer.cpp b/src/dolphinviewcontainer.cpp index e7c571294..c8fc757ba 100644 --- a/src/dolphinviewcontainer.cpp +++ b/src/dolphinviewcontainer.cpp @@ -372,6 +372,10 @@ void DolphinViewContainer::setUrl(const KUrl& newUrl) { if (newUrl != m_urlNavigator->locationUrl()) { m_urlNavigator->setLocationUrl(newUrl); + } else if (m_view->itemsCount() == 0) { + // Maybe a previously unmounted device has been mounted again. + // Let's reload the view to be safe (see https://bugs.kde.org/show_bug.cgi?id=161385). + m_view->reload(); } #ifdef KActivities_FOUND -- cgit v1.3