diff options
| author | weinan li <[email protected]> | 2025-11-10 08:38:31 +0000 |
|---|---|---|
| committer | Méven Car <[email protected]> | 2025-11-10 08:38:31 +0000 |
| commit | 3858afbfa4f2f60cc33f39a471d36a1e1a3514c7 (patch) | |
| tree | 5109e8c061e5f298cc1a2f6e254ea462f7f855c3 /src | |
| parent | 4159b9c7874facc3774b59ed3aa13692bf14bb55 (diff) | |
dolphin: improve keyboard search behavior for repeated characters
The original keyboard search implementation had a limitation when dealing with files containing repeated characters like 44.txt. When typing 44, it would trigger rapid navigation to the next item starting with 4 instead of matching the full string 44.
This patch introduces a smarter search strategy:
1. First attempt full string matching for exact file names
2. If full match fails and user is typing repeating characters, fall back to rapid navigation using either:
- Last successful search string (for extended searches like 444 -> 4444)
- First character only (original rapid navigation behavior)
The changes include:
- Modified changeCurrentItem signal to include a found parameter for search result feedback
- Added m_lastSuccessfulSearch to track successful searches for better fallback behavior
- Used Qt::DirectConnection to ensure synchronous execution for immediate result feedback
This provides better user experience when searching for files with repeated characters while maintaining the rapid navigation feature for quick browsing.
BUG: 501851
Diffstat (limited to 'src')
| -rw-r--r-- | src/kitemviews/kitemlistcontroller.cpp | 6 | ||||
| -rw-r--r-- | src/kitemviews/kitemlistcontroller.h | 2 | ||||
| -rw-r--r-- | src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp | 34 | ||||
| -rw-r--r-- | src/kitemviews/private/kitemlistkeyboardsearchmanager.h | 3 | ||||
| -rw-r--r-- | src/tests/kitemlistkeyboardsearchmanagertest.cpp | 143 |
5 files changed, 139 insertions, 49 deletions
diff --git a/src/kitemviews/kitemlistcontroller.cpp b/src/kitemviews/kitemlistcontroller.cpp index fdde48abc..7971de5b7 100644 --- a/src/kitemviews/kitemlistcontroller.cpp +++ b/src/kitemviews/kitemlistcontroller.cpp @@ -58,7 +58,7 @@ KItemListController::KItemListController(KItemModelBase *model, KItemListView *v , m_keyboardAnchorIndex(-1) , m_keyboardAnchorPos(0) { - connect(m_keyboardManager, &KItemListKeyboardSearchManager::changeCurrentItem, this, &KItemListController::slotChangeCurrentItem); + connect(m_keyboardManager, &KItemListKeyboardSearchManager::changeCurrentItem, this, &KItemListController::slotChangeCurrentItem, Qt::DirectConnection); connect(m_selectionManager, &KItemListSelectionManager::currentChanged, m_keyboardManager, &KItemListKeyboardSearchManager::slotCurrentChanged); connect(m_selectionManager, &KItemListSelectionManager::selectionChanged, m_keyboardManager, &KItemListKeyboardSearchManager::slotSelectionChanged); @@ -515,8 +515,9 @@ bool KItemListController::keyPressEvent(QKeyEvent *event) return true; } -void KItemListController::slotChangeCurrentItem(const QString &text, bool searchFromNextItem) +void KItemListController::slotChangeCurrentItem(const QString &text, bool searchFromNextItem, bool *found) { + *found = false; if (!m_model || m_model->count() == 0) { return; } @@ -542,6 +543,7 @@ void KItemListController::slotChangeCurrentItem(const QString &text, bool search } m_view->scrollToItem(index, KItemListView::ViewItemPosition::Beginning); + *found = true; } } diff --git a/src/kitemviews/kitemlistcontroller.h b/src/kitemviews/kitemlistcontroller.h index 48da07206..48a518610 100644 --- a/src/kitemviews/kitemlistcontroller.h +++ b/src/kitemviews/kitemlistcontroller.h @@ -233,7 +233,7 @@ private Q_SLOTS: */ void slotRubberBandChanged(); - void slotChangeCurrentItem(const QString &text, bool searchFromNextItem); + void slotChangeCurrentItem(const QString &text, bool searchFromNextItem, bool *found); void slotAutoActivationTimeout(); diff --git a/src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp b/src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp index afc3bb071..f2f834c4c 100644 --- a/src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp +++ b/src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp @@ -35,6 +35,7 @@ void KItemListKeyboardSearchManager::addKeys(const QString &keys) { if (shouldClearSearchIfInputTimeReached()) { m_searchedString.clear(); + m_lastSuccessfulSearch.clear(); } const bool newSearch = m_searchedString.isEmpty(); @@ -48,17 +49,32 @@ void KItemListKeyboardSearchManager::addKeys(const QString &keys) if (!keys.isEmpty()) { m_searchedString.append(keys); - // Special case: - // If the same key is pressed repeatedly, the next item matching that key should be highlighted - const QChar firstKey = m_searchedString.length() > 0 ? m_searchedString.at(0) : QChar(); - const bool sameKey = m_searchedString.length() > 1 && m_searchedString.count(firstKey) == m_searchedString.length(); + const QChar firstChar = m_searchedString.at(0); + const bool allSameChars = m_searchedString.length() > 1 && + m_searchedString.count(firstChar) == m_searchedString.length(); - // Searching for a matching item should start from the next item if either - // 1. a new search is started, or - // 2. a 'repeated key' search is done. - const bool searchFromNextItem = newSearch || sameKey; + // Overall strategy: + // 1. First attempt full string matching for exact file names + // 2. If full match fails and user is typing repeating characters, + // fall back to rapid navigation using either: + // - Last successful search string (for extended searches like "444" -> "4444") + // - First character only (original rapid navigation behavior) + bool found = false; + Q_EMIT changeCurrentItem(m_searchedString, newSearch, &found); - Q_EMIT changeCurrentItem(sameKey ? firstKey : m_searchedString, searchFromNextItem); + if (found) { + m_lastSuccessfulSearch = m_searchedString; + } else if (allSameChars && !newSearch) { + QString rapidSearchString; + + if (!m_lastSuccessfulSearch.isEmpty() && m_searchedString.startsWith(m_lastSuccessfulSearch)) { + rapidSearchString = m_lastSuccessfulSearch; + } else { + rapidSearchString = QString(firstChar); + } + + Q_EMIT changeCurrentItem(rapidSearchString, true, &found); + } } m_keyboardInputTime.start(); } diff --git a/src/kitemviews/private/kitemlistkeyboardsearchmanager.h b/src/kitemviews/private/kitemlistkeyboardsearchmanager.h index 5cb8effbf..4437f97b4 100644 --- a/src/kitemviews/private/kitemlistkeyboardsearchmanager.h +++ b/src/kitemviews/private/kitemlistkeyboardsearchmanager.h @@ -66,7 +66,7 @@ Q_SIGNALS: */ // TODO: Think about getting rid of the bool parameter // (see https://doc.qt.io/archives/qq/qq13-apis.html#thebooleanparametertrap) - void changeCurrentItem(const QString &string, bool searchFromNextItem); + void changeCurrentItem(const QString &string, bool searchFromNextItem, bool *found); private: bool shouldClearSearchIfInputTimeReached(); @@ -76,6 +76,7 @@ private: QElapsedTimer m_keyboardInputTime; /** Time in milliseconds in which a key press is considered as a continuation of the previous search input. */ qint64 m_timeout; + QString m_lastSuccessfulSearch; }; #endif diff --git a/src/tests/kitemlistkeyboardsearchmanagertest.cpp b/src/tests/kitemlistkeyboardsearchmanagertest.cpp index bbdb9c397..feba6acba 100644 --- a/src/tests/kitemlistkeyboardsearchmanagertest.cpp +++ b/src/tests/kitemlistkeyboardsearchmanagertest.cpp @@ -10,6 +10,8 @@ #include <QStandardPaths> #include <QTest> +#include <source_location> + class KItemListKeyboardSearchManagerTest : public QObject { Q_OBJECT @@ -25,6 +27,68 @@ private Q_SLOTS: private: KItemListKeyboardSearchManager m_keyboardSearchManager; + + // Helper function to verify signal parameters + void verifySignal(QSignalSpy &spy, + const QString &expectedString, + bool expectedSearchFromNextItem, + const std::source_location &location = std::source_location::current()) + { + if (spy.count() != 1) { + QTest::qFail(QString("Compared values are not the same\n" + " Actual (spy.count()): %1\n" + " Expected (1) : 1") + .arg(spy.count()) + .toUtf8() + .constData(), + location.file_name(), + location.line()); + return; + } + + QList<QVariant> arguments = spy.takeFirst(); + + if (arguments.size() != 3) { + QTest::qFail(QString("Compared values are not the same\n" + " Actual (arguments.size()): %1\n" + " Expected (3) : 3") + .arg(arguments.size()) + .toUtf8() + .constData(), + location.file_name(), + location.line()); + return; + } + + QString actualText = arguments.at(0).toString(); + if (actualText != expectedString) { + QTest::qFail(QString("Compared values are not the same\n" + " Actual (arguments.at(0).toString()): \"%1\"\n" + " Expected (QString(\"%2\")) : \"%2\"") + .arg(actualText) + .arg(expectedString) + .toUtf8() + .constData(), + location.file_name(), + location.line()); + return; + } + + bool actualBool = arguments.at(1).toBool(); + if (actualBool != expectedSearchFromNextItem) { + QTest::qFail(QString("Compared values are not the same\n" + " Actual (arguments.at(1).toBool()): %1\n" + " Expected (%2) : %2") + .arg(actualBool ? "true" : "false") + .arg(expectedSearchFromNextItem ? "true" : "false") + .toUtf8() + .constData(), + location.file_name(), + location.line()); + return; + } + // Ignore the third parameter (bool* found) + } }; void KItemListKeyboardSearchManagerTest::initTestCase() @@ -44,20 +108,16 @@ void KItemListKeyboardSearchManagerTest::testBasicKeyboardSearch() QVERIFY(spy.isValid()); m_keyboardSearchManager.addKeys("f"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "f" << true); + verifySignal(spy, "f", true); m_keyboardSearchManager.addKeys("i"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "fi" << false); + verifySignal(spy, "fi", false); m_keyboardSearchManager.addKeys("l"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "fil" << false); + verifySignal(spy, "fil", false); m_keyboardSearchManager.addKeys("e"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "file" << false); + verifySignal(spy, "file", false); } void KItemListKeyboardSearchManagerTest::testAbortedKeyboardSearch() @@ -70,60 +130,74 @@ void KItemListKeyboardSearchManagerTest::testAbortedKeyboardSearch() QVERIFY(spy.isValid()); m_keyboardSearchManager.addKeys("f"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "f" << true); + verifySignal(spy, "f", true); m_keyboardSearchManager.addKeys("i"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "fi" << false); + verifySignal(spy, "fi", false); // If the delay between two key presses is larger than the chosen timeout, // a new search is started. We add a small safety margin to avoid race conditions. QTest::qWait(m_keyboardSearchManager.timeout() + 10); m_keyboardSearchManager.addKeys("l"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "l" << true); + verifySignal(spy, "l", true); m_keyboardSearchManager.addKeys("e"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "le" << false); + verifySignal(spy, "le", false); // the selection was deselected, for instance with Esc or a click outside the selection m_keyboardSearchManager.slotSelectionChanged(KItemSet(), KItemSet() << 1); m_keyboardSearchManager.addKeys("a"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "a" << true); + verifySignal(spy, "a", true); } void KItemListKeyboardSearchManagerTest::testRepeatedKeyPress() { - // If the same key is pressed repeatedly, the next matching item should be highlighted after - // each key press. To achieve, that, the manager emits the changeCurrentItem(QString,bool) - // signal, where - // 1. the string contains the repeated key only once, and - // 2. the bool searchFromNextItem is true. + // With the new implementation, pressing the same key repeatedly will: + // 1. First attempt a full match with the complete string + // 2. If full match fails, fall back to rapid navigation with the first character QSignalSpy spy(&m_keyboardSearchManager, &KItemListKeyboardSearchManager::changeCurrentItem); QVERIFY(spy.isValid()); + // First key press - new search, only one signal m_keyboardSearchManager.addKeys("p"); QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "p" << true); + verifySignal(spy, "p", true); + // Second key press - repeating character, two signals m_keyboardSearchManager.addKeys("p"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "p" << true); + QCOMPARE(spy.count(), 2); + // First signal: full string match attempt + QList<QVariant> arguments = spy.takeFirst(); + QCOMPARE(arguments.size(), 3); + QCOMPARE(arguments.at(0).toString(), QString("pp")); + QCOMPARE(arguments.at(1).toBool(), false); + // Second signal: rapid navigation fallback + arguments = spy.takeFirst(); + QCOMPARE(arguments.size(), 3); + QCOMPARE(arguments.at(0).toString(), QString("p")); + QCOMPARE(arguments.at(1).toBool(), true); + // Third key press - repeating character, two signals m_keyboardSearchManager.addKeys("p"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "p" << true); + QCOMPARE(spy.count(), 2); + // First signal: full string match attempt + arguments = spy.takeFirst(); + QCOMPARE(arguments.size(), 3); + QCOMPARE(arguments.at(0).toString(), QString("ppp")); + QCOMPARE(arguments.at(1).toBool(), false); + // Second signal: rapid navigation fallback + arguments = spy.takeFirst(); + QCOMPARE(arguments.size(), 3); + QCOMPARE(arguments.at(0).toString(), QString("p")); + QCOMPARE(arguments.at(1).toBool(), true); - // Now press another key -> the search string contains all pressed keys + // Now press another key -> only one signal for non-repeating character m_keyboardSearchManager.addKeys("q"); QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "pppq" << false); + verifySignal(spy, "pppq", false); } void KItemListKeyboardSearchManagerTest::testPressShift() @@ -138,19 +212,16 @@ void KItemListKeyboardSearchManagerTest::testPressShift() // Simulate that the user enters "a_b". m_keyboardSearchManager.addKeys("a"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "a" << true); + verifySignal(spy, "a", true); m_keyboardSearchManager.addKeys(""); QCOMPARE(spy.count(), 0); m_keyboardSearchManager.addKeys("_"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "a_" << false); + verifySignal(spy, "a_", false); m_keyboardSearchManager.addKeys("b"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList<QVariant>() << "a_b" << false); + verifySignal(spy, "a_b", false); } QTEST_GUILESS_MAIN(KItemListKeyboardSearchManagerTest) |
