Skip to content

Commit cb693dd

Browse files
committed
ComboBox: reverse-lookup current-value writes into current-index
Implements Option A from #11970: a host write to current-value now means "select the row with this value" — the widget finds it in the model via array.index-of and moves current-index to match. A value that isn't in the model resolves to no selection (current-index -1, current-value ""), the same as setting current-index out of range — and, unlike the successful case, logs a debug() message naming the value, so a developer debugging an unexpectedly-cleared selection has something to go on. A successful reverse lookup stays silent. Uses index-of rather than find-index: ComboBox is the first built-in widget with any dependency on the array-predicates machinery, and index-of is the stable half of it (see the previous commit) — so this carries no dependency on the still-experimental find-index/any/all gate (#12777), unlike an inline find-index((value) => value == current-value) predicate would. This supersedes the non-reverting interim behavior from 7dfcfdf, which deliberately left an unmatched host write in place (with only a debug() warning) to avoid disturbing existing `<=>` two-way bindings while the real fix was blocked on iteration support. That blocker is gone for this narrow case: index-of needs no general loop, only a single-pass reverse lookup, so we no longer need the non-reverting compromise. `<=>` bindings that round-trip an arbitrary, not-in-model string through current-value will now see it replaced (with "" or the first match) instead of preserved — this is intentional, matching the issue's Option A design, not a regression. RadioGroup is intentionally left untouched: its public API (declared in builtins.slint) exposes current-value as `out` only and never exposes current-index at all, so the only host-writable selection path is `checked` on an individual RadioButton child, which already correctly stays in sync with current-index/current-value (see RadioButtonImplBase's `changed checked` handler) — there's no writable-but-unreliable surface left for a reverse lookup to fix. Known limitation, documented in combobox-base.slint and combobox.mdx: the reverse lookup runs from `changed current-value`, which only fires on an actual value transition. A write that doesn't change the property — writing the value it already holds, or a value equal to a later duplicate in `model` while an earlier duplicate should now be selected per the "first match" contract — is a no-op. Slint has no `.slint`-level hook for "run on every write regardless of value equality" (the Rust-internal equivalent, BindingCallable:: intercept_set, isn't usable from widget source), so this can't be closed without new core machinery; it equally affected the debug()-only interim mitigation this replaces. Covered by two regression-documenting assertions in tests/cases/widgets/combobox.slint so the behavior stays intentional and visible rather than silently drifting. changelog: ComboBox: writes to `current-value` from host code (Rust/C++/JS/Python) now update the selection via reverse lookup instead of being silently discarded. Fixes #11970 for ComboBox.
1 parent 8c04cce commit cb693dd

3 files changed

Lines changed: 72 additions & 14 deletions

File tree

docs/astro/src/content/docs/reference/std-widgets/basic-widgets/combobox.mdx

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,16 @@ ComboBox {
4242
<SlintProperty propName="current-value" typeName="string" defaultValue='""' propertyVisibility="in-out">
4343
The currently selected text.
4444

45-
:::caution[Note]
46-
To change the selection from host code (Rust, C++, JavaScript, Python), write to
47-
`current-index`. Writes to `current-value` are not supported — they don't change
48-
the selection. The visible state continues to reflect `model[current-index]`.
45+
:::note
46+
Writing to `current-value` from host code (Rust, C++, JavaScript, Python) looks up the
47+
written value in `model` and moves `current-index` to the first matching row. If the
48+
value isn't found in `model`, the selection is cleared (`current-index` becomes `-1`
49+
and `current-value` becomes `""`), the same as setting `current-index` out of range.
50+
Prefer writing `current-index` directly when the row's position is already known.
51+
52+
This lookup only runs when the write actually changes `current-value`. Writing the
53+
value it already holds — including when the model has duplicate entries and a later
54+
one is currently selected — is a no-op and does not move the selection.
4955
:::
5056
</SlintProperty>
5157

internal/compiler/widgets/common/combobox-base.slint

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,13 +80,20 @@ export component ComboBoxBase {
8080
root.update-current-value();
8181
}
8282

83-
// Writes to `current-value` from host code don't change the selection;
84-
// the visible state still reflects `model[current-index]`. Warn on the
85-
// divergence so the developer learns to use `current-index` instead.
83+
// A host write to `current-value` expresses "select the row with this value": look it
84+
// up in `model` and let `changed current-index` (above) write the resolved value back,
85+
// or "" if not found. Uses the stable `index-of`, not the experimental `find-index` it
86+
// lowers to, so ComboBox doesn't depend on an experimental feature.
87+
// Only fires on an actual value change, so writing the value current-value already
88+
// holds is a no-op even when the "select the first match" contract says it shouldn't be.
8689
// See https://github.com/slint-ui/slint/issues/11970.
8790
changed current-value => {
8891
if root.current-value != root.model[root.current-index] {
89-
debug("ComboBox: `current-value` writes don't change the selection. Use `current-index` instead. See https://github.com/slint-ui/slint/issues/11970.");
92+
let found-index = root.model.index-of(root.current-value);
93+
if found-index < 0 {
94+
debug("ComboBox: `current-value` was set to \"" + root.current-value + "\", which is not in `model`; the selection was cleared. See https://github.com/slint-ui/slint/issues/11970.");
95+
}
96+
root.current-index = found-index;
9097
}
9198
}
9299

tests/cases/widgets/combobox.slint

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -256,14 +256,59 @@ instance.set_current_index(2);
256256
mock_elapsed_time(500);
257257
assert_selection(2, "!!!THIS!!!");
258258
259-
// Writes to `current-value` from host code are not a supported way to change
260-
// the selection — the widget emits a debug warning but doesn't revert the
261-
// host write. The current-index (and therefore the visible selection in the
262-
// popup) stays where it was. See https://github.com/slint-ui/slint/issues/11970.
259+
// A host write to `current-value` is a reverse lookup: it moves current-index to
260+
// match. A successful lookup stays silent. See https://github.com/slint-ui/slint/issues/11970.
261+
slint_testing::access_testing_window(instance.window(), |w| w.take_debug_log());
263262
instance.set_current_value(SharedString::from("not this3"));
264263
mock_elapsed_time(500);
265-
assert_eq!(instance.get_current_index(), 2);
266-
assert_eq!(instance.get_current_value(), SharedString::from("not this3"));
264+
assert_selection(3, "not this3");
265+
let logs = slint_testing::access_testing_window(instance.window(), |w| w.take_debug_log());
266+
assert!(logs.is_empty(), "expected no debug log on a successful reverse lookup, got: {logs:?}");
267+
268+
// A value not in the model resolves to no selection, and (unlike the case above) logs why.
269+
instance.set_current_value(SharedString::from("nope"));
270+
mock_elapsed_time(500);
271+
assert_selection(-1, "");
272+
let logs = slint_testing::access_testing_window(instance.window(), |w| w.take_debug_log());
273+
assert_eq!(logs.len(), 1, "expected one debug log for a not-found current-value write, got: {logs:?}");
274+
assert!(logs[0].contains("\"nope\""));
275+
assert!(logs[0].contains("not in `model`"));
276+
277+
// A model swap after a host-written current-value doesn't re-trigger the lookup above:
278+
// `changed model` already clamps current-index and re-derives current-value from it.
279+
instance.set_current_value(SharedString::from("not this1"));
280+
mock_elapsed_time(500);
281+
assert_selection(0, "not this1");
282+
283+
instance.set_model(Rc::new(VecModel::from_slice(&[SharedString::from("only"), SharedString::from("this")])).into());
284+
mock_elapsed_time(500);
285+
assert_selection(0, "only");
286+
287+
// Known limitation: the lookup only runs on an actual value change, so writing the value
288+
// current-value already holds is a no-op, even though "select the first match" says it
289+
// shouldn't be.
290+
instance.set_model(Rc::new(VecModel::from_slice(&[
291+
SharedString::from(""),
292+
SharedString::from("dup"),
293+
SharedString::from("dup"),
294+
])).into());
295+
mock_elapsed_time(500);
296+
assert_selection(0, "");
297+
instance.set_current_index(-1);
298+
mock_elapsed_time(500);
299+
assert_selection(-1, "");
300+
instance.set_current_value(SharedString::from(""));
301+
mock_elapsed_time(500);
302+
assert_selection(-1, "");
303+
304+
// Same limitation with a duplicate value: re-writing "dup" while the second occurrence
305+
// (index 2) is selected doesn't move to the first (index 1).
306+
instance.set_current_index(2);
307+
mock_elapsed_time(500);
308+
assert_selection(2, "dup");
309+
instance.set_current_value(SharedString::from("dup"));
310+
mock_elapsed_time(500);
311+
assert_selection(2, "dup");
267312
```
268313
269314
*/

0 commit comments

Comments
 (0)