Skip to content

[JEWEL-1244] Fix ListComboBox External SelectedIndex Changes Not Syncing To Internal State #3418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DanielSouzaBertoldi wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[JEWEL-1244] Fix ListComboBox External SelectedIndex Changes Not Syncing To Internal State #3418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
207 changes: 207 additions & 0 deletions ...orm/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/ListComboBoxUiTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import androidx.compose.ui.test.assertHeightIsEqualTo
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.assertIsFocused
import androidx.compose.ui.test.assertIsNotEnabled
import androidx.compose.ui.test.assertIsNotSelected
import androidx.compose.ui.test.assertIsSelected
import androidx.compose.ui.test.assertTextContains
import androidx.compose.ui.test.assertTextEquals
Expand Down Expand Up @@ -1177,6 +1178,212 @@ class ListComboBoxUiTest {
assertTrue(selectedItemChangeTriggered, "Item click should be detected for faster taps")
}

@Test
fun `when ListComboBox selectedIndex is reset externally while popup is closed, selection updates correctly`() {
var selectedIndex by mutableIntStateOf(4) // Start at "Laughter"
val focusRequester = FocusRequester()

composeRule.setContent {
IntUiTheme {
ListComboBox(
items = comboBoxItems,
selectedIndex = selectedIndex,
onSelectedItemChange = { index -> selectedIndex = index },
modifier = Modifier.testTag("ComboBox").width(200.dp).focusRequester(focusRequester),
itemKeys = { _, item -> item },
)
}
}

focusRequester.requestFocus()

comboBox.assertTextEquals("Laughter", includeEditableText = false)
assertEquals(4, selectedIndex)

// Reset selectedIndex externally while popup is closed
composeRule.runOnUiThread { selectedIndex = 0 }

// Verify combo box label updated
comboBox.assertTextEquals("Item 1", includeEditableText = false)

// Open popup and verify the correct item is selected
comboBox.performClick()
popupMenu.assertIsDisplayed()

comboBoxPopupList.performScrollToIndex(0)

composeRule
.onNode(hasAnyAncestor(hasTestTag("Jewel.ComboBox.Popup")) and hasText("Item 1"))
.assertExists()
.assertIsDisplayed()
.assertIsSelected()

// Previously selected item should not be selected anymore
composeRule
.onNode(hasAnyAncestor(hasTestTag("Jewel.ComboBox.Popup")) and hasText("Laughter"))
.assertExists()
.assertIsDisplayed()
.assertIsNotSelected()
}

@Test
fun `when ListComboBox selectedIndex is reset externally while popup is open, selection is not disrupted`() {
var selectedIndex by mutableIntStateOf(4) // Start at "Laughter"
val focusRequester = FocusRequester()

composeRule.setContent {
IntUiTheme {
ListComboBox(
items = comboBoxItems,
selectedIndex = selectedIndex,
onSelectedItemChange = { index -> selectedIndex = index },
modifier = Modifier.testTag("ComboBox").width(200.dp).focusRequester(focusRequester),
itemKeys = { _, item -> item },
)
}
}

focusRequester.requestFocus()

// Verify initial state
comboBox.assertTextEquals("Laughter", includeEditableText = false)
assertEquals(4, selectedIndex)

// Open popup
comboBox.performClick()
popupMenu.assertIsDisplayed()

// Verify "Laughter" is selected in the popup
composeRule
.onNode(hasAnyAncestor(hasTestTag("Jewel.ComboBox.Popup")) and hasText("Laughter"))
.assertExists()
.assertIsDisplayed()
.assertIsSelected()

// Reset selectedIndex externally while popup is open
composeRule.runOnUiThread { selectedIndex = 0 }

// Verify popup still shows "Laughter" as selected (not disrupted by external change)
composeRule
.onNode(hasAnyAncestor(hasTestTag("Jewel.ComboBox.Popup")) and hasText("Laughter"))
.assertExists()
.assertIsDisplayed()
.assertIsSelected()

// Press Enter to commit current selection (should reconcile to internal listState)
comboBox.performKeyPress(Key.Enter, rule = composeRule)
popupMenu.assertDoesNotExist()

// After closing, selectedIndex should be reconciled back to "Laughter" (index 4)
assertEquals(4, selectedIndex)
comboBox.assertTextEquals("Laughter", includeEditableText = false)
}

@Test
fun `when EditableListComboBox selectedIndex is reset externally while popup is closed, selection updates correctly`() {
var selectedIndex by mutableIntStateOf(4) // Start at "Laughter"
val focusRequester = FocusRequester()

composeRule.setContent {
IntUiTheme {
EditableListComboBox(
items = comboBoxItems,
selectedIndex = selectedIndex,
onSelectedItemChange = { index -> selectedIndex = index },
modifier = Modifier.testTag("ComboBox").width(200.dp).focusRequester(focusRequester),
itemKeys = { _, item -> item },
)
}
}

focusRequester.requestFocus()
composeRule.waitForIdle()

textField.assertTextEquals("Laughter")
assertEquals(4, selectedIndex)

// Reset selectedIndex externally while popup is closed
composeRule.runOnUiThread { selectedIndex = 0 }
composeRule.waitForIdle()

// Verify text field updated
textField.assertTextEquals("Item 1")

// Open popup and verify the correct item is selected
chevronContainer.performClick()
popupMenu.assertIsDisplayed()

comboBoxPopupList.performScrollToIndex(0)

composeRule
.onNode(hasAnyAncestor(hasTestTag("Jewel.ComboBox.Popup")) and hasText("Item 1"))
.assertExists()
.assertIsDisplayed()
.assertIsSelected()

// Previously selected item should not be selected anymore
composeRule
.onNode(hasAnyAncestor(hasTestTag("Jewel.ComboBox.Popup")) and hasText("Laughter"))
.assertExists()
.assertIsDisplayed()
.assertIsNotSelected()
}

@Test
fun `when EditableListComboBox selectedIndex is reset externally while popup is open, selection is not disrupted`() {
var selectedIndex by mutableIntStateOf(4) // Start at "Laughter"
val focusRequester = FocusRequester()

composeRule.setContent {
IntUiTheme {
EditableListComboBox(
items = comboBoxItems,
selectedIndex = selectedIndex,
onSelectedItemChange = { index -> selectedIndex = index },
modifier = Modifier.testTag("ComboBox").width(200.dp).focusRequester(focusRequester),
itemKeys = { _, item -> item },
)
}
}

focusRequester.requestFocus()
composeRule.waitForIdle()

// Verify initial state
textField.assertTextEquals("Laughter")
assertEquals(4, selectedIndex)

// Open popup
chevronContainer.performClick()
popupMenu.assertIsDisplayed()

// Verify "Laughter" is selected in the popup
composeRule
.onNode(hasAnyAncestor(hasTestTag("Jewel.ComboBox.Popup")) and hasText("Laughter"))
.assertExists()
.assertIsDisplayed()
.assertIsSelected()

// Reset selectedIndex externally while popup is open
composeRule.runOnUiThread { selectedIndex = 0 }
composeRule.waitForIdle()

// Verify popup still shows "Laughter" as selected (not disrupted by external change)
composeRule
.onNode(hasAnyAncestor(hasTestTag("Jewel.ComboBox.Popup")) and hasText("Laughter"))
.assertExists()
.assertIsDisplayed()
.assertIsSelected()

// Press Enter to commit current selection (should reconcile to internal listState)
comboBox.performKeyPress(Key.Enter, rule = composeRule)
popupMenu.assertDoesNotExist()

// After closing, selectedIndex should be reconciled back to "Laughter" (index 4)
assertEquals(4, selectedIndex)
textField.assertTextEquals("Laughter")
}

private fun editableListComboBox(): SemanticsNodeInteraction {
val focusRequester = FocusRequester()
composeRule.setContent {
Expand Down
52 changes: 30 additions & 22 deletions platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/ListComboBox.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,9 +433,22 @@ public fun EditableListComboBox(
var hoveredItemIndex by remember { mutableIntStateOf(-1) }
val scope = rememberCoroutineScope()

LaunchedEffect(itemKeys) {
// Select the first item in the list when creating
listState.selectedKeys = setOf(itemKeys(selectedIndex, items.getOrNull(selectedIndex).orEmpty()))
val popupManager = remember {
PopupManager(
onPopupVisibleChange = {
hoveredItemIndex = -1
onPopupVisibleChange(it)
},
name = "EditableListComboBoxPopup",
)
}

LaunchedEffect(itemKeys, selectedIndex, popupManager.isPopupVisible.value) {
if (!popupManager.isPopupVisible.value) {
Copy link
Collaborator Author

@DanielSouzaBertoldi DanielSouzaBertoldi Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really don't want to mess with the popup while its visible and the user is actively using it. This prevents any changes made programmatically from happening during this state.

val item = items.getOrNull(selectedIndex).orEmpty()
listState.selectedKeys = setOf(itemKeys(selectedIndex, item))
textFieldState.edit { replace(0, length, item) }
}
}

fun setSelectedItem(index: Int) {
Expand Down Expand Up @@ -506,16 +519,7 @@ public fun EditableListComboBox(
setSelectedItem(indexOfSelected)
}
},
popupManager =
remember {
PopupManager(
onPopupVisibleChange = {
hoveredItemIndex = -1
onPopupVisibleChange(it)
},
name = "EditableListComboBoxPopup",
)
},
popupManager = popupManager,
popupContent = {
PopupContent(
items = items,
Expand Down Expand Up @@ -676,15 +680,6 @@ internal fun <T : Any> ListComboBoxImpl(
),
itemContent: @Composable (index: Int, item: T, isSelected: Boolean, isActive: Boolean) -> Unit,
) {
LaunchedEffect(itemKeys) {
val item = items.getOrNull(selectedIndex)
if (item != null) {
listState.selectedKeys = setOf(itemKeys(selectedIndex, item))
} else {
listState.selectedKeys = emptySet()
}
}

val density = LocalDensity.current
var comboBoxSize by remember { mutableStateOf(DpSize.Zero) }
var currentComboBoxSize by remember { mutableStateOf(DpSize.Zero) }
Expand Down Expand Up @@ -761,6 +756,19 @@ internal fun <T : Any> ListComboBoxImpl(
)
}

// Sync external selectedIndex changes to listState, but only when popup is closed
// to avoid disrupting the user's active browsing session when popup is open
LaunchedEffect(itemKeys, selectedIndex, popupManager.isPopupVisible.value) {
if (!popupManager.isPopupVisible.value) {
val item = items.getOrNull(selectedIndex)
if (item != null) {
listState.selectedKeys = setOf(itemKeys(selectedIndex, item))
} else {
listState.selectedKeys = emptySet()
}
}
}

fun commitSelectionFromHoverOrMapped() {
val mappedIndex = listState.selectedItemIndex(items, itemKeys)
val targetIndex =
Expand Down
Loading