Skip to content

Commit

Permalink
Rewrite multiple selection, speeding it up
Browse files Browse the repository at this point in the history 
Also, fix DefaultSelectableLazyColumnKeyActions check for multiple selection
  • Loading branch information
@Walingar
Walingar committed Nov 2, 2023
1 parent 4228bf2 commit 8d0e1bc
Show file tree
Hide file tree
Showing 3 changed files with 175 additions and 52 deletions.
85 changes: 38 additions & 47 deletions foundation/src/main/kotlin/org/jetbrains/jewel/foundation/lazy/SelectableColumnOnKeyEvent.kt
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.jetbrains.jewel.foundation.lazy

import org.jetbrains.jewel.foundation.lazy.SelectableLazyListKey.Selectable
import java.util.Collections.addAll
import kotlin.math.max
import kotlin.math.min

Expand All @@ -26,25 +27,18 @@ interface SelectableColumnOnKeyEvent {
* Extend Selection to First Node inherited from Move Caret to Text Start with Selection
*/
fun onExtendSelectionToFirst(keys: List<SelectableLazyListKey>, state: SelectableLazyListState) {
state.lastActiveItemIndex
?.let {
val iterator = keys.listIterator(it)
val list = buildList {
while (iterator.hasPrevious()) {
val previous = iterator.previous()
if (previous is Selectable) {
add(previous.key)
state.lastActiveItemIndex = (iterator.previousIndex() + 1).coerceAtMost(keys.size)
}
}
}
if (list.isNotEmpty()) {
state.selectedKeys =
state.selectedKeys
.toMutableList()
.also { selectionList -> selectionList.addAll(list) }
}
val initialIndex = state.lastActiveItemIndex ?: return
val newSelection = ArrayList<Any>(max(initialIndex, state.selectedKeys.size)).apply {
addAll(state.selectedKeys)
}
for (index in initialIndex - 1 downTo 0) {
val key = keys[index]
if (key is Selectable) {
newSelection.add(key.key)
state.lastActiveItemIndex = index
}
}
state.selectedKeys = newSelection
}

/**
Expand All @@ -65,16 +59,18 @@ interface SelectableColumnOnKeyEvent {
* Extend Selection to Last Node inherited from Move Caret to Text End with Selection
*/
fun onExtendSelectionToLastItem(keys: List<SelectableLazyListKey>, state: SelectableLazyListState) {
state.lastActiveItemIndex?.let {
val list = mutableListOf<Any>(state.selectedKeys)
keys.subList(it, keys.lastIndex).forEachIndexed { index, selectableLazyListKey ->
if (selectableLazyListKey is Selectable) {
list.add(selectableLazyListKey.key)
}
val initialIndex = state.lastActiveItemIndex ?: return
val newSelection = ArrayList<Any>(max(keys.size - initialIndex, state.selectedKeys.size)).apply {
addAll(state.selectedKeys)
}
for (index in initialIndex + 1..keys.lastIndex) {
val key = keys[index]
if (key is Selectable) {
newSelection.add(key.key)
state.lastActiveItemIndex = index
}
state.selectedKeys = list
}
state.selectedKeys = newSelection
}

/**
Expand All @@ -96,18 +92,15 @@ interface SelectableColumnOnKeyEvent {
* Extend Selection with Previous Node inherited from Up with Selection
*/
fun onExtendSelectionWithPreviousItem(keys: List<SelectableLazyListKey>, state: SelectableLazyListState) {
state.lastActiveItemIndex?.let { lastActiveIndex ->
if (lastActiveIndex == 0) return@let
keys
.withIndex()
.toList()
.dropLastWhile { it.index >= lastActiveIndex }
.reversed()
.firstOrNull { it.value is Selectable }
?.let { (index, selectableKey) ->
state.selectedKeys = state.selectedKeys + selectableKey.key
state.lastActiveItemIndex = index
}
// todo we need deselect if we are changing direction
val initialIndex = state.lastActiveItemIndex ?: return
for (index in initialIndex - 1 downTo 0) {
val key = keys[index]
if (key is Selectable) {
state.selectedKeys += key.key
state.lastActiveItemIndex = index
return
}
}
}

Expand All @@ -131,16 +124,14 @@ interface SelectableColumnOnKeyEvent {
*/
fun onExtendSelectionWithNextItem(keys: List<SelectableLazyListKey>, state: SelectableLazyListState) {
// todo we need deselect if we are changing direction
state.lastActiveItemIndex?.let { lastActiveIndex ->
if (lastActiveIndex == keys.lastIndex) return@let
keys
.withIndex()
.dropWhile { it.index <= lastActiveIndex }
.firstOrNull { it.value is Selectable }
?.let { (index, selectableKey) ->
state.selectedKeys = state.selectedKeys + selectableKey.key
state.lastActiveItemIndex = index
}
val initialIndex = state.lastActiveItemIndex ?: return
for (index in initialIndex + 1..keys.lastIndex) {
val key = keys[index]
if (key is Selectable) {
state.selectedKeys += key.key
state.lastActiveItemIndex = index
return
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion foundation/src/main/kotlin/org/jetbrains/jewel/foundation/lazy/tree/KeyActions.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ open class DefaultSelectableLazyColumnKeyActions(
isSelectLastItem -> onSelectLastItem(keys, state)
isEdit -> onEdit()
}
if (selectionMode == SelectionMode.Single) {
if (selectionMode == SelectionMode.Multiple) {
when {
isExtendSelectionToFirstItem -> onExtendSelectionToFirst(keys, state)
isExtendSelectionToLastItem -> onExtendSelectionToLastItem(keys, state)
Expand Down
140 changes: 136 additions & 4 deletions foundation/src/test/kotlin/org/jetbrains/jewel/foundation/lazy/SelectableLazyColumnTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.performKeyInput
import androidx.compose.ui.test.pressKey
import androidx.compose.ui.test.withKeyDown
import androidx.compose.ui.unit.dp
import kotlinx.coroutines.runBlocking
import org.junit.Assert.assertEquals
Expand Down Expand Up @@ -92,8 +94,7 @@ internal class SelectableLazyColumnTest {
// press arrow up and check that selected key is changed
repeat(20) { step ->
composeRule.onNodeWithTag("list").performKeyInput {
keyDown(Key.DirectionUp)
keyUp(Key.DirectionUp)
pressKey(Key.DirectionUp)
}

// check that previous element is selected
Expand All @@ -110,8 +111,7 @@ internal class SelectableLazyColumnTest {
// press arrow down and check that selected key is changed
repeat(40) { step ->
composeRule.onNodeWithTag("list").performKeyInput {
keyDown(Key.DirectionDown)
keyUp(Key.DirectionDown)
pressKey(Key.DirectionDown)
}

// check that next element is selected
Expand All @@ -124,4 +124,136 @@ internal class SelectableLazyColumnTest {
assertTrue(state.selectedKeys.size == 1)
assertEquals(items.last(), state.selectedKeys.single())
}

@OptIn(ExperimentalTestApi::class)
@Test
fun `multiple items selection`() = runBlocking {
val items = (0..10).toList()
val state = SelectableLazyListState(LazyListState())
composeRule.setContent {
Box(modifier = Modifier.requiredHeight(300.dp)) {
SelectableLazyColumn(state = state, modifier = Modifier.testTag("list")) {
items(
items.size,
key = {
items[it]
},
) {
val itemText = "Item ${items[it]}"
BasicText(itemText, modifier = Modifier.testTag(itemText))
}
}
}
}
composeRule.awaitIdle()
// select item 5 by click
composeRule.onNodeWithTag("Item 5").assertExists()
composeRule.onNodeWithTag("Item 5").performClick()

// check that 5th element is selected
assertEquals(1, state.selectedKeys.size)
assertEquals(items[5], state.selectedKeys.single())

// press arrow up with pressed Shift and check that selected keys are changed
repeat(20) { step ->
composeRule.onNodeWithTag("list").performKeyInput {
withKeyDown(Key.ShiftLeft) {
pressKey(Key.DirectionUp)
}
}

// check that previous element is added to selection
// when started from 5th element
val expectedFirstSelectedIndex = (5 - step - 1).takeIf { it >= 0 } ?: 0
val elements = items.subList(expectedFirstSelectedIndex, 6)
assertEquals(elements.size, state.selectedKeys.size)
assertEquals(elements.toSet(), state.selectedKeys.toSet())
}

// select first item by click
composeRule.onNodeWithTag("Item 0").assertExists()
composeRule.onNodeWithTag("Item 0").performClick()

// check that first element is selected
assertEquals(1, state.selectedKeys.size)
assertEquals(items[0], state.selectedKeys.single())

// press arrow down with pressed Shift and check that selected keys are changed
repeat(20) { step ->
composeRule.onNodeWithTag("list").performKeyInput {
withKeyDown(Key.ShiftLeft) {
pressKey(Key.DirectionDown)
}
}

// check that next element is added to selection
val expectedFirstSelectedIndex = (step + 1).takeIf { it in items.indices } ?: items.lastIndex
val elements = items.subList(0, expectedFirstSelectedIndex + 1)
assertEquals(elements.size, state.selectedKeys.size)
assertEquals(elements.toSet(), state.selectedKeys.toSet())
}

// all elements should be selected in the end
assertEquals(items.size, state.selectedKeys.size)
assertEquals(items.toSet(), state.selectedKeys.toSet())
}

@OptIn(ExperimentalTestApi::class)
@Test
fun `select to first and last`() = runBlocking {
val items = (0..50).toList()
val state = SelectableLazyListState(LazyListState())
composeRule.setContent {
Box(modifier = Modifier.requiredHeight(300.dp)) {
SelectableLazyColumn(state = state, modifier = Modifier.testTag("list")) {
items(
items.size,
key = {
items[it]
},
) {
val itemText = "Item ${items[it]}"
BasicText(itemText, modifier = Modifier.testTag(itemText))
}
}
}
}
composeRule.awaitIdle()
// select item 5 by click
composeRule.onNodeWithTag("Item 5").assertExists()
composeRule.onNodeWithTag("Item 5").performClick()

// check that 5th element is selected
assertEquals(1, state.selectedKeys.size)
assertEquals(items[5], state.selectedKeys.single())

// perform home with shift, so all items until 5th should be selected
composeRule.onNodeWithTag("list").performKeyInput {
withKeyDown(Key.ShiftLeft) {
pressKey(Key.MoveHome)
}
}
val expectedElementsAfterPageUp = items.subList(0, 6)
assertEquals(expectedElementsAfterPageUp.size, state.selectedKeys.size)
assertEquals(expectedElementsAfterPageUp.toSet(), state.selectedKeys.toSet())


// select item 5 by click
composeRule.onNodeWithTag("Item 5").assertExists()
composeRule.onNodeWithTag("Item 5").performClick()

// check that 5th element is selected
assertEquals(1, state.selectedKeys.size)
assertEquals(items[5], state.selectedKeys.single())

// perform end with shift, so all items after 5th should be selected
composeRule.onNodeWithTag("list").performKeyInput {
withKeyDown(Key.ShiftLeft) {
pressKey(Key.MoveEnd)
}
}
val expectedElementsAfterPageDown = items.subList(5, items.lastIndex + 1)
assertEquals(expectedElementsAfterPageDown.size, state.selectedKeys.size)
assertEquals(expectedElementsAfterPageDown.toSet(), state.selectedKeys.toSet())
}
}

0 comments on commit 8d0e1bc

Please sign in to comment.