From fcd261bfbab9ac8c14fe829003ca84f22fb5b6f7 Mon Sep 17 00:00:00 2001 From: Daniel Bertoldi Date: Tue, 7 Jul 2026 11:59:48 -0300 Subject: [PATCH] [JEWEL-1355] Select first match when the selected item is filtered out SpeedSearchableLazyColumn could select the second match instead of the first when the item that was selected before typing got filtered out of the results. It surfaced as a flaky, CI-only failure in SpeedSearchAreaFilteringTest. The scroll effect that decides the selection and the list re-filtering run on independent reactive graphs with no ordering between them. When the re-filter cleared the selection before the scroll effect ran, the "closest visible match to the current selection" step produced nothing (there was no selection left to anchor to) and control fell through to the branch meant for off-screen matches, which selects the first match after the last visible item instead of the first match. Fall back to the first visible match when there is no selection to anchor to, so the first result is always selected regardless of which reaction wins the race. This matches the IntelliJ Platform behavior, which also resolves to the top row when the previously selected item leaves the filtered set. Document the full selection algorithm on the scroll effect: a forward scan from the top of the viewport that wraps to the top of the list, mirroring Swing's SpeedSearchBase.findElement rather than ListWithFilter's always-select-row-0. Add a regression test that reproduces the losing interleaving deterministically by clearing the selection before typing, and three tests pinning the viewport rule with matches inside, only below, and only above the viewport. closes https://github.com/JetBrains/intellij-community/pull/3564 (cherry picked from commit 8683754e04bfb94f6eab8394c3fea5d1c53a95d0) (cherry picked from commit 522e653658ba06e64a432835d51df338f034ec9d) IJ-MR-215743 GitOrigin-RevId: dfb04284a5537052ba05a6205dd57bb5edb48134 --- .../search/SpeedSearchAreaFilteringTest.kt | 18 +++++++++ .../search/SpeedSearchableLazyColumnTest.kt | 38 +++++++++++++++++++ .../search/SpeedSearchableLazyColumn.kt | 23 +++++++++-- 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchAreaFilteringTest.kt b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchAreaFilteringTest.kt index 438d077253d0..538f41fddf7f 100644 --- a/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchAreaFilteringTest.kt +++ b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchAreaFilteringTest.kt @@ -37,6 +37,7 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.setMain +import org.jetbrains.jewel.foundation.lazy.SelectableLazyListState import org.jetbrains.jewel.foundation.lazy.rememberSelectableLazyListState import org.jetbrains.jewel.foundation.search.EmptySpeedSearchMatcher import org.jetbrains.jewel.foundation.search.filter @@ -74,6 +75,8 @@ class SpeedSearchAreaFilteringTest { private val testDispatcher = UnconfinedTestDispatcher() + private var capturedListState: SelectableLazyListState? = null + private val ComposeContentTestRule.onLazyColumn get() = onNodeWithTag("LazyColumn") @@ -210,6 +213,20 @@ class SpeedSearchAreaFilteringTest { onLazyColumnItem("Angular").assertIsDisplayed().assertIsSelected() } + @Test + fun `when the previously selected item is filtered out, the first visible match is selected`() = + runFilteringComposeTest { + capturedListState!!.selectedKeys = emptySet() + waitForIdle() + + // Type "Angular" - two matches, both visible, first one at index 0 of the filtered list. + onLazyColumn.performKeyPress("Angular", rule = this) + + // The first match must be selected, never the second one. + onLazyColumnItem("Angular").assertIsDisplayed().assertIsSelected() + onLazyColumnItem("AngularJS").assertIsDisplayed() + } + @Test fun `partial match filtering should work correctly`() = runFilteringComposeTest { // Type "Nest" - should match NestJS @@ -442,6 +459,7 @@ class SpeedSearchAreaFilteringTest { val speedSearchState = rememberSpeedSearchState() capturedSpeedSearchState = speedSearchState val listState = rememberSelectableLazyListState() + capturedListState = listState // Filter the list based on the current matcher - same pattern as showcase val filteredItems by remember { derivedStateOf { listEntries.filter(speedSearchState.currentMatcher) } } diff --git a/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableLazyColumnTest.kt b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableLazyColumnTest.kt index 4ecc179ee617..d72b98e344ad 100644 --- a/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableLazyColumnTest.kt +++ b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableLazyColumnTest.kt @@ -281,6 +281,44 @@ class SpeedSearchableLazyColumnTest { onLazyColumnItem("Item 7").assertIsSelected() } + @Test + fun `with no matching selection, select the topmost visible match`() = + runComposeTest(listEntries = markedEntries(20, 45, 70)) { + onLazyColumn.performScrollToIndex(43) + + onLazyColumn.performKeyPress("Banana", rule = this) + + // 45 is inside the viewport; it wins over the global first match (20) and the one below (70) + onLazyColumnItem("Banana 45").assertIsDisplayed().assertIsSelected() + } + + @Test + fun `with no visible match, select the first match below the viewport rather than the global first`() = + runComposeTest(listEntries = markedEntries(20, 70)) { + onLazyColumn.performScrollToIndex(43) + + onLazyColumn.performKeyPress("Banana", rule = this) + + // Neither match is visible; the forward scan reaches 70 (below) before wrapping to 20 (above) + onLazyColumnItem("Banana 70").assertIsDisplayed().assertIsSelected() + } + + @Test + fun `with matches only above the viewport, wrap to the first match from the top`() = + runComposeTest(listEntries = markedEntries(5, 20)) { + onLazyColumn.performScrollToIndex(43) + + onLazyColumn.performKeyPress("Banana", rule = this) + + // The wrap selects the first match from the top of the list (5), not the nearest one above (20), + // matching SpeedSearchBase.findElement's wrap-around behavior + onLazyColumnItem("Banana 5").assertIsDisplayed().assertIsSelected() + } + + /** 100 entries named `Item `, except the given indexes, which are named `Banana `. */ + private fun markedEntries(vararg matchIndexes: Int) = + List(100) { index -> if (index in matchIndexes) "Banana $index" else "Item $index" } + private fun runComposeTest( listEntries: List = List(500) { "Item ${it + 1}" }, dismissOnLoseFocus: Boolean = true, diff --git a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableLazyColumn.kt b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableLazyColumn.kt index 52513e47d329..4ff2d25f0cb2 100644 --- a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableLazyColumn.kt +++ b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableLazyColumn.kt @@ -281,6 +281,22 @@ internal class SpeedSearchableLazyColumnScopeImpl( } } +/** + * Keeps the list selection in sync with the speed-search matches. + * + * Whenever [SpeedSearchState.matchingIndexes] changes while the search is active, the selection is decided as follows: + * 1. If a currently selected item still matches, keep it selected (scrolling to it if it is offscreen). + * 2. Otherwise, if a selection exists, select the visible match closest to it. + * 3. Otherwise, select the topmost visible match. + * 4. If no match is visible, select the first match after the viewport. + * 5. Failing that, select the first match before the viewport, i.e., the first match from the top of the list. + * + * Steps 3–5 amount to a single rule: a forward scan starting at the top of the viewport that wraps around to the top of + * the list. This deliberately preserves the user's rough position instead of always jumping to the first match, and + * mirrors Swing's `SpeedSearchBase.findElement` (forward from the current position, wrap to the top — including + * wrapping to the *first* match from the top rather than the nearest one above) rather than `ListWithFilter`'s + * always-select-row-0 behavior. + */ @Composable internal fun SpeedSearchableLazyColumnScrollEffect( selectableLazyListState: SelectableLazyListState, @@ -345,8 +361,9 @@ internal fun SpeedSearchableLazyColumnScrollEffect( return@combine } - // If any of the visible items match the filter, just select the one closest to any of the selected - // items + // If any of the visible items match the filter, select the one closest to any of the currently + // selected items. When there is no selection left to anchor to (e.g. the previously selected item + // was filtered out), fall back to the first visible match so the first result is always selected. val indexOfVisibleMatches = visibleItemIndexes.filter { indexesMatchingSearchText.binarySearch(it) >= 0 } @@ -358,7 +375,7 @@ internal fun SpeedSearchableLazyColumnScrollEffect( ?.let { visibleMatchIndex to it } } .minByOrNull { (it.first - it.second).absoluteValue } - ?.first + ?.first ?: indexOfVisibleMatches.firstOrNull() if (bestVisibleMatch != null) { selectableLazyListState.selectedKeys = setOfNotNull(keyValues.getOrNull(bestVisibleMatch))