Skip to content

Commit a0042af

Browse files
karkarlarturictbkudiessCopilot
authored
fix(chat): replace NativeElement escape hatch with FunctionalUI ComboBox primitive (supersedes #973) (#991)
* fix(chat): keep session picker stable during renders * fix(chat): stabilize native session picker host Keep the session picker ComboBox attached as a declarative native child across FunctionalUI renders, and harden native control lifecycle handling for mount, ownership, and event routing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(ui): clean up native controls across render paths * refactor(chat): replace NativeElement escape hatch with FunctionalUI ComboBox primitive The #970 fix originally reached around FunctionalUI with a raw NativeElement snapshot to keep the session picker open across status re-renders. Promote that into a first-class rich ComboBox primitive so re-render stability is a framework guarantee, not a per-call escape hatch. - Add ComboItem + ItemComboBoxElement with static ItemsEqual; ConfigureItemComboBox only rebuilds items when the list actually changes, preserving an open flyout. - Migrate both the session picker and the model picker (which had the same unfixed #970-class bug) onto the primitive. - Remove NativeElement and its tests entirely. - Add pure ItemComboBoxTests and source-contract ComposerSessionPickerTests guarding against escape-hatch regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: arturict <arturict@users.noreply.github.com> Co-authored-by: bakudies@microsoft.com <bakudies@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 0d1c8e3 commit a0042af

6 files changed

Lines changed: 280 additions & 107 deletions

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
namespace OpenClawTray.Chat;
2+
3+
public record ChannelGroup(string AgentLabel, (string Id, string Title)[] Sessions);

src/OpenClaw.Tray.WinUI/Chat/OpenClawComposer.cs

Lines changed: 58 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ namespace OpenClawTray.Chat;
3838
/// banner that <c>InputBar</c> used to render are preserved here above the
3939
/// composer.
4040
/// </summary>
41-
public record ChannelGroup(string AgentLabel, (string Id, string Title)[] Sessions);
42-
4341
public record OpenClawComposerProps(
4442
string ConnectionState,
4543
bool TurnActive,
@@ -93,6 +91,9 @@ public sealed class OpenClawComposer : Component<OpenClawComposerProps>
9391
// model id string. Selecting it routes to OnModelCleared (tri-state clear)
9492
// rather than OnModelChanged.
9593
private static readonly object ClearModelTag = new();
94+
// Reserved id used to represent the "default / clear" model row in the rich ComboBox
95+
// primitive (which keys selection by string id). Cannot collide with a real SelectionId.
96+
private const string ClearModelId = "\u0000__default__";
9697

9798
// Thinking levels matching the gateway's sessions.patch thinkingLevel values.
9899
// "medium" is the default when the session has no explicit thinkingLevel set.
@@ -246,69 +247,35 @@ public override Element Render()
246247
};
247248

248249
// ── Row 1: three compact dropdowns ─────────────────────────────
249-
// Build grouped session ComboBox directly (bypassing the FunctionalUI
250-
// ComboBox helper which only supports flat string[] items).
250+
// Grouped session picker via the reconciled rich ComboBox primitive. The primitive is
251+
// preserved by render path and only rebuilds its rows when the item set changes, so an
252+
// open dropdown survives unrelated status/thinking re-renders (the #970 regression).
251253
var groups = Props.AvailableChannels;
252-
var channelCombo = Border()
253-
.Set(border =>
254+
var multipleGroups = groups.Length > 1;
255+
var sessionItems = new List<ComboItem>();
256+
foreach (var group in groups)
257+
{
258+
if (multipleGroups)
259+
sessionItems.Add(new ComboItem("", group.AgentLabel, Enabled: false, IsHeader: true));
260+
foreach (var session in group.Sessions)
261+
sessionItems.Add(new ComboItem(session.Id, session.Title, Indent: multipleGroups ? 8 : 0));
262+
}
263+
264+
var onChannelChanged = Props.OnChannelChanged;
265+
var channelCombo = ComboBox(sessionItems, Props.ChannelId ?? "", id => onChannelChanged(id))
266+
.Set(cb =>
254267
{
255-
var cb = new ComboBox
256-
{
257-
MinWidth = 0,
258-
Width = double.NaN,
259-
Height = 28,
260-
FontSize = 11,
261-
Padding = new Thickness(8, 0, 4, 0),
262-
CornerRadius = composerCornerRadius,
263-
HorizontalAlignment = HorizontalAlignment.Stretch,
264-
VerticalAlignment = VerticalAlignment.Center,
265-
};
268+
cb.MinWidth = 0;
269+
cb.Width = double.NaN;
270+
cb.Height = 28;
271+
cb.FontSize = 11;
272+
cb.Padding = new Thickness(8, 0, 4, 0);
273+
cb.CornerRadius = composerCornerRadius;
274+
cb.HorizontalAlignment = HorizontalAlignment.Stretch;
275+
cb.VerticalAlignment = VerticalAlignment.Center;
266276
Microsoft.UI.Xaml.Automation.AutomationProperties.SetName(
267277
cb,
268278
LocalizationHelper.GetString("Chat_Composer_Accessibility_Session"));
269-
270-
ComboBoxItem? selectedItem = null;
271-
foreach (var group in groups)
272-
{
273-
if (groups.Length > 1)
274-
{
275-
cb.Items.Add(new ComboBoxItem
276-
{
277-
Content = group.AgentLabel,
278-
IsEnabled = false,
279-
FontWeight = Microsoft.UI.Text.FontWeights.SemiBold,
280-
FontSize = 10,
281-
Padding = new Thickness(4, 2, 4, 2),
282-
IsHitTestVisible = false,
283-
});
284-
}
285-
foreach (var session in group.Sessions)
286-
{
287-
var item = new ComboBoxItem
288-
{
289-
Content = session.Title,
290-
Tag = session.Id,
291-
Padding = groups.Length > 1
292-
? new Thickness(16, 4, 4, 4)
293-
: new Thickness(8, 4, 4, 4),
294-
};
295-
cb.Items.Add(item);
296-
if (session.Id == (Props.ChannelId ?? ""))
297-
selectedItem = item;
298-
}
299-
}
300-
301-
if (selectedItem != null)
302-
cb.SelectedItem = selectedItem;
303-
304-
var onChanged = Props.OnChannelChanged;
305-
cb.SelectionChanged += (_, _) =>
306-
{
307-
if (cb.SelectedItem is ComboBoxItem { Tag: string id })
308-
onChanged(id);
309-
};
310-
311-
border.Child = cb;
312279
});
313280

314281
// ── Model picker (provider-rich) ─────────────────────────────────
@@ -357,58 +324,42 @@ public override Element Render()
357324
modelEntries.Add((Props.CurrentModel ?? "model", Props.CurrentModel ?? "", false, true));
358325
}
359326

360-
var modelSelectedIndex = modelEntries.FindIndex(e => e.IsCurrent);
327+
// Provider-rich model picker via the same reconciled primitive. Unavailable rows stay
328+
// visible but disabled; the default/clear row maps to a reserved id.
329+
var modelItems = new List<ComboItem>(modelEntries.Count);
330+
string? modelSelectedId = null;
331+
foreach (var entry in modelEntries)
332+
{
333+
var id = ReferenceEquals(entry.Tag, ClearModelTag)
334+
? ClearModelId
335+
: entry.Tag as string ?? "";
336+
modelItems.Add(new ComboItem(id, entry.Label, Enabled: entry.Selectable));
337+
if (entry.IsCurrent) modelSelectedId ??= id;
338+
}
361339

362-
// Build directly so unavailable rows can be displayed but not selected.
363-
var modelCombo = Border()
364-
.Set(border =>
340+
var onModelChanged = Props.OnModelChanged;
341+
var onModelCleared = Props.OnModelCleared;
342+
var modelCombo = ComboBox(modelItems, modelSelectedId, id =>
365343
{
366-
var cb = new ComboBox
367-
{
368-
MinWidth = 0,
369-
Width = double.NaN,
370-
Height = 28,
371-
FontSize = 11,
372-
Padding = new Thickness(8, 0, 4, 0),
373-
CornerRadius = composerCornerRadius,
374-
HorizontalAlignment = HorizontalAlignment.Stretch,
375-
VerticalAlignment = VerticalAlignment.Center,
376-
IsEnabled = messageOptionControlsEnabled,
377-
};
344+
if (id == ClearModelId)
345+
onModelCleared?.Invoke();
346+
else if (!string.IsNullOrEmpty(id))
347+
onModelChanged(id);
348+
})
349+
.Set(cb =>
350+
{
351+
cb.MinWidth = 0;
352+
cb.Width = double.NaN;
353+
cb.Height = 28;
354+
cb.FontSize = 11;
355+
cb.Padding = new Thickness(8, 0, 4, 0);
356+
cb.CornerRadius = composerCornerRadius;
357+
cb.HorizontalAlignment = HorizontalAlignment.Stretch;
358+
cb.VerticalAlignment = VerticalAlignment.Center;
359+
cb.IsEnabled = messageOptionControlsEnabled;
378360
Microsoft.UI.Xaml.Automation.AutomationProperties.SetName(
379361
cb,
380362
LocalizationHelper.GetString("Chat_Composer_Accessibility_Model"));
381-
382-
ComboBoxItem? selectedItem = null;
383-
for (int i = 0; i < modelEntries.Count; i++)
384-
{
385-
var entry = modelEntries[i];
386-
var item = new ComboBoxItem
387-
{
388-
Content = entry.Label,
389-
Tag = entry.Tag,
390-
IsEnabled = entry.Selectable,
391-
Padding = new Thickness(8, 4, 4, 4),
392-
};
393-
cb.Items.Add(item);
394-
if (i == modelSelectedIndex) selectedItem = item;
395-
}
396-
397-
if (selectedItem != null)
398-
cb.SelectedItem = selectedItem;
399-
400-
var onModelChanged = Props.OnModelChanged;
401-
var onModelCleared = Props.OnModelCleared;
402-
cb.SelectionChanged += (_, _) =>
403-
{
404-
if (cb.SelectedItem is not ComboBoxItem { IsEnabled: true } sel) return;
405-
if (ReferenceEquals(sel.Tag, ClearModelTag))
406-
onModelCleared?.Invoke();
407-
else if (sel.Tag is string id && !string.IsNullOrEmpty(id))
408-
onModelChanged(id);
409-
};
410-
411-
border.Child = cb;
412363
})
413364
.VAlign(VerticalAlignment.Center);
414365

src/OpenClawTray.FunctionalUI/FunctionalUI.cs

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,30 @@ public sealed record ProgressRingElement(double? Value) : Element;
199199
public sealed record SliderElement(double Value, double Minimum, double Maximum, Action<double>? OnChanged) : Element;
200200
public sealed record ColorPickerElement(Color Value, Action<Color>? OnChanged) : Element;
201201
public sealed record ComboBoxElement(string[] Items, int SelectedIndex, Action<int>? OnSelectionChanged) : Element;
202+
/// <summary>
203+
/// A single row for the rich <see cref="ItemComboBoxElement"/> primitive. Pure data (no WinUI
204+
/// types) so it can be constructed and diffed off the UI thread and unit-tested directly.
205+
/// Headers render as non-selectable group labels; normal rows carry a stable <paramref name="Id"/>.
206+
/// </summary>
207+
public sealed record ComboItem(string Id, string Label, bool Enabled = true, bool IsHeader = false, double Indent = 0);
208+
/// <summary>
209+
/// A reconciled ComboBox that supports grouped/headered, indented, individually-enabled rows and
210+
/// selection by stable id. Unlike hand-rolling a native <c>ComboBox</c> inside a setter, this
211+
/// element is preserved by render path and only rebuilds its rows when the item set actually
212+
/// changes, so an open dropdown survives unrelated re-renders (the #970 regression).
213+
/// </summary>
214+
public sealed record ItemComboBoxElement(IReadOnlyList<ComboItem> Items, string? SelectedId, Action<string>? OnSelectionChanged) : Element
215+
{
216+
/// <summary>Pure, order-sensitive value comparison of two item lists. Testable off the UI thread.</summary>
217+
public static bool ItemsEqual(IReadOnlyList<ComboItem> left, IReadOnlyList<ComboItem> right)
218+
{
219+
if (ReferenceEquals(left, right)) return true;
220+
if (left is null || right is null || left.Count != right.Count) return false;
221+
for (var i = 0; i < left.Count; i++)
222+
if (left[i] != right[i]) return false;
223+
return true;
224+
}
225+
}
202226
public sealed record ImageElement(string Source) : Element;
203227
public sealed record BorderElement(Element? Child) : Element;
204228
public sealed record StackElement(Orientation Orientation, double Spacing, IReadOnlyList<Element?> Children) : Element;
@@ -591,6 +615,8 @@ public static ColorPickerElement ColorPicker(Color value, Action<Color>? onChang
591615
new(value, onChanged);
592616
public static ComboBoxElement ComboBox(string[] items, int selectedIndex = -1, Action<int>? onSelectionChanged = null) =>
593617
new(items, selectedIndex, onSelectionChanged);
618+
public static ItemComboBoxElement ComboBox(IReadOnlyList<ComboItem> items, string? selectedId = null, Action<string>? onSelectionChanged = null) =>
619+
new(items, selectedId, onSelectionChanged);
594620
public static ImageElement Image(string source) => new(source);
595621
public static BorderElement Border(Element? child = null) => new(child);
596622
public static FlexRowElement FlexRow(params Element?[] children) => new(children);
@@ -754,6 +780,7 @@ public static TextBlockElement SemiBold(this TextBlockElement element) =>
754780
public static SliderElement Set(this SliderElement element, Action<Slider> setter) => element.AddSetter(setter);
755781
public static ColorPickerElement Set(this ColorPickerElement element, Action<ColorPicker> setter) => element.AddSetter(setter);
756782
public static ComboBoxElement Set(this ComboBoxElement element, Action<ComboBox> setter) => element.AddSetter(setter);
783+
public static ItemComboBoxElement Set(this ItemComboBoxElement element, Action<ComboBox> setter) => element.AddSetter(setter);
757784
public static ImageElement Set(this ImageElement element, Action<Image> setter) => element.AddSetter(setter);
758785
public static BorderElement Set(this BorderElement element, Action<Border> setter) => element.AddSetter(setter);
759786
public static ProgressRingElement Set(this ProgressRingElement element, Action<ProgressRing> setter) => element.AddSetter(setter);
@@ -975,6 +1002,7 @@ private UIElement RenderElementCore(Element element, string path, List<Action> e
9751002
SliderElement e => ConfigureSlider(GetOrCreate<Slider>(path), e),
9761003
ColorPickerElement e => ConfigureColorPicker(GetOrCreate<ColorPicker>(path), e),
9771004
ComboBoxElement e => ConfigureComboBox(GetOrCreate<ComboBox>(path), e),
1005+
ItemComboBoxElement e => ConfigureItemComboBox(GetOrCreate<ComboBox>(path), e),
9781006
ImageElement e => ConfigureImage(GetOrCreate<Image>(path), e),
9791007
BorderElement e => ConfigureBorder(GetOrCreate<Border>(path), e, path, effects),
9801008
StackElement e => ConfigureStack(GetOrCreate<Border>(path), e, path, effects),
@@ -1262,6 +1290,68 @@ private ComboBox ConfigureComboBox(ComboBox control, ComboBoxElement element)
12621290
return control;
12631291
}
12641292

1293+
private ComboBox ConfigureItemComboBox(ComboBox control, ItemComboBoxElement element)
1294+
{
1295+
control.SelectionChanged -= ItemComboBoxSelectionChanged;
1296+
var previous = control.Tag as ItemComboBoxElement;
1297+
control.Tag = element;
1298+
1299+
// Rebuild the row containers only when the item set actually changes. Rebuilding on every
1300+
// render would dismiss an open dropdown, which is the #970 session-picker regression.
1301+
if (previous is null || !ItemComboBoxElement.ItemsEqual(previous.Items, element.Items))
1302+
{
1303+
control.Items.Clear();
1304+
foreach (var item in element.Items)
1305+
control.Items.Add(CreateComboBoxItem(item));
1306+
}
1307+
1308+
// Reconcile selection by stable id every render (a status re-render must not change it).
1309+
ComboBoxItem? target = null;
1310+
if (element.SelectedId is { } selectedId)
1311+
{
1312+
foreach (var candidate in control.Items)
1313+
{
1314+
if (candidate is ComboBoxItem { Tag: string id } container && id == selectedId)
1315+
{
1316+
target = container;
1317+
break;
1318+
}
1319+
}
1320+
}
1321+
if (!ReferenceEquals(control.SelectedItem, target))
1322+
control.SelectedItem = target;
1323+
1324+
// Reattach only after programmatic mutations so those don't fire the user callback.
1325+
control.SelectionChanged += ItemComboBoxSelectionChanged;
1326+
ApplyModifiers(control, element);
1327+
ApplySetters(control, element);
1328+
return control;
1329+
}
1330+
1331+
private static ComboBoxItem CreateComboBoxItem(ComboItem item)
1332+
{
1333+
if (item.IsHeader)
1334+
{
1335+
return new ComboBoxItem
1336+
{
1337+
Content = item.Label,
1338+
IsEnabled = false,
1339+
IsHitTestVisible = false,
1340+
FontWeight = Microsoft.UI.Text.FontWeights.SemiBold,
1341+
FontSize = 10,
1342+
Padding = new Thickness(4, 2, 4, 2),
1343+
};
1344+
}
1345+
1346+
return new ComboBoxItem
1347+
{
1348+
Content = item.Label,
1349+
Tag = item.Id,
1350+
IsEnabled = item.Enabled,
1351+
Padding = new Thickness(8 + item.Indent, 4, 4, 4),
1352+
};
1353+
}
1354+
12651355
private Image ConfigureImage(Image control, ImageElement element)
12661356
{
12671357
var sourceUri = new Uri(element.Source);
@@ -2103,6 +2193,12 @@ private static void ComboBoxSelectionChanged(object sender, SelectionChangedEven
21032193
element.OnSelectionChanged?.Invoke(combo.SelectedIndex);
21042194
}
21052195

2196+
private static void ItemComboBoxSelectionChanged(object sender, SelectionChangedEventArgs e)
2197+
{
2198+
if (sender is ComboBox { Tag: ItemComboBoxElement element, SelectedItem: ComboBoxItem { Tag: string id } })
2199+
element.OnSelectionChanged?.Invoke(id);
2200+
}
2201+
21062202
private static void MenuFlyoutItemClick(object sender, RoutedEventArgs e)
21072203
{
21082204
if (sender is MenuFlyoutItem { Tag: MenuFlyoutItemData data })
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
namespace OpenClaw.Tray.Tests;
2+
3+
/// <summary>
4+
/// Source-contract guards for the composer pickers. These assert the composer builds its session
5+
/// and model dropdowns through the reconciled FunctionalUI ComboBox primitive rather than
6+
/// hand-rolling a native <c>ComboBox</c> inside a setter — the imperative escape hatch that caused
7+
/// the #970 "dropdown slams shut on status render" regression.
8+
/// </summary>
9+
public sealed class ComposerSessionPickerTests
10+
{
11+
private static string ComposerSource() => File.ReadAllText(Path.Combine(
12+
TestRepositoryPaths.GetRepositoryRoot(),
13+
"src",
14+
"OpenClaw.Tray.WinUI",
15+
"Chat",
16+
"OpenClawComposer.cs"));
17+
18+
[Fact]
19+
public void SessionPicker_UsesReconciledItemComboBoxPrimitive()
20+
{
21+
var composer = ComposerSource();
22+
23+
Assert.Contains("var sessionItems = new List<ComboItem>();", composer);
24+
Assert.Contains("ComboBox(sessionItems, Props.ChannelId ?? \"\"", composer);
25+
}
26+
27+
[Fact]
28+
public void ModelPicker_UsesReconciledItemComboBoxPrimitive()
29+
{
30+
var composer = ComposerSource();
31+
32+
Assert.Contains("ComboBox(modelItems, modelSelectedId", composer);
33+
Assert.Contains("if (id == ClearModelId)", composer);
34+
}
35+
36+
[Fact]
37+
public void Composer_DoesNotHandRollNativePickersOrSnapshots()
38+
{
39+
var composer = ComposerSource();
40+
41+
// The escape-hatch patterns that produced #970 must not return.
42+
Assert.DoesNotContain("border.Child = cb;", composer);
43+
Assert.DoesNotContain("SessionPickerSnapshot", composer);
44+
Assert.DoesNotContain("Native(", composer);
45+
}
46+
}

tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
<Compile Include="..\..\src\OpenClaw.Tray.WinUI\Chat\DebugChatSurfaceOverrides.cs" Link="Chat\DebugChatSurfaceOverrides.cs" />
3838
<Compile Include="..\..\src\OpenClaw.Tray.WinUI\Chat\ChatEntryMetadata.cs" Link="Chat\ChatEntryMetadata.cs" />
3939
<Compile Include="..\..\src\OpenClaw.Tray.WinUI\Chat\ChatUsageFormatter.cs" Link="Chat\ChatUsageFormatter.cs" />
40+
<Compile Include="..\..\src\OpenClaw.Tray.WinUI\Chat\ChannelGroup.cs" Link="Chat\ChannelGroup.cs" />
4041
<Compile Include="..\..\src\OpenClaw.Tray.WinUI\Chat\ChatMarkdownSanitizer.cs" Link="Chat\ChatMarkdownSanitizer.cs" />
4142
<Compile Include="..\..\src\OpenClaw.Tray.WinUI\Chat\OpenClawChatDataProvider.cs" Link="Chat\OpenClawChatDataProvider.cs" />
4243
</ItemGroup>

0 commit comments

Comments
 (0)