mirror of
https://gitflic.ru/project/openide/openide.git
synced 2026-09-27 10:03:11 +07:00
[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
This commit is contained in:
committed by
intellij-monorepo-bot
parent
14113b0695
commit
fcd261bfba
+18
@@ -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) } }
|
||||
|
||||
+38
@@ -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 <index>`, except the given indexes, which are named `Banana <index>`. */
|
||||
private fun markedEntries(vararg matchIndexes: Int) =
|
||||
List(100) { index -> if (index in matchIndexes) "Banana $index" else "Item $index" }
|
||||
|
||||
private fun runComposeTest(
|
||||
listEntries: List<String> = List(500) { "Item ${it + 1}" },
|
||||
dismissOnLoseFocus: Boolean = true,
|
||||
|
||||
+20
-3
@@ -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))
|
||||
|
||||
Reference in New Issue
Block a user