From 3858afbfa4f2f60cc33f39a471d36a1e1a3514c7 Mon Sep 17 00:00:00 2001 From: weinan li Date: Mon, 10 Nov 2025 08:38:31 +0000 Subject: 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 --- src/tests/kitemlistkeyboardsearchmanagertest.cpp | 147 +++++++++++++++++------ 1 file changed, 109 insertions(+), 38 deletions(-) (limited to 'src/tests') 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 #include +#include + 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 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() << "f" << true); + verifySignal(spy, "f", true); m_keyboardSearchManager.addKeys("i"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList() << "fi" << false); + verifySignal(spy, "fi", false); m_keyboardSearchManager.addKeys("l"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList() << "fil" << false); + verifySignal(spy, "fil", false); m_keyboardSearchManager.addKeys("e"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList() << "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() << "f" << true); + verifySignal(spy, "f", true); m_keyboardSearchManager.addKeys("i"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList() << "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() << "l" << true); + verifySignal(spy, "l", true); m_keyboardSearchManager.addKeys("e"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList() << "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() << "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() << "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() << "p" << true); - + QCOMPARE(spy.count(), 2); + // First signal: full string match attempt + QList 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() << "p" << true); - - // Now press another key -> the search string contains all pressed keys + 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 -> only one signal for non-repeating character m_keyboardSearchManager.addKeys("q"); QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList() << "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() << "a" << true); + verifySignal(spy, "a", true); m_keyboardSearchManager.addKeys(""); QCOMPARE(spy.count(), 0); m_keyboardSearchManager.addKeys("_"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList() << "a_" << false); + verifySignal(spy, "a_", false); m_keyboardSearchManager.addKeys("b"); - QCOMPARE(spy.count(), 1); - QCOMPARE(spy.takeFirst(), QList() << "a_b" << false); + verifySignal(spy, "a_b", false); } QTEST_GUILESS_MAIN(KItemListKeyboardSearchManagerTest) -- cgit v1.3