Fix combobox Enter submit precedence
- Prefer bound combobox submit handlers over Enter trigger activation. - Cover builder, markup, runtime, menu-selection, and command dispatch paths. - Document the TypeScript-first on-submit composition and keyboard behavior. Co-authored-by: Mohak Bajaj <77928693+MohakBajaj@users.noreply.github.com>
This commit is contained in:
@@ -5,6 +5,8 @@ import { AttrTable } from "@/components/attr-table";
|
||||
|
||||
`combobox` is a trigger-only primitive like [select](/docs/components/select), but the trigger is a text entry with a menu affordance: `on-input` names a Msg variant that receives every edit as a text-input event (`canvas.TextInputEvent` in a Zig core; the `TextInputEvent` union from `@native-sdk/core/text` in a TypeScript core), and the model filters the options as the user types. The options themselves are composed the same way as the select's — an anchored [dropdown-menu](/docs/components/dropdown-menu) of menu-items beside the trigger in a `stack`, rendered under an `if`, with `on-dismiss` clearing the model's open flag when Escape or a click outside closes the surface.
|
||||
|
||||
Enter submits when `on-submit` is bound; otherwise Enter opens the picker. Space and the open-arrow keys (Down/Up) always open it. Once focus moves into the open menu, Enter selects the focused `menu-item` as usual.
|
||||
|
||||
<ComponentPreview name="combobox" alt="A combobox rendered by the engine" caption="a combobox trigger with its search placeholder" />
|
||||
|
||||
## Markup
|
||||
@@ -13,8 +15,8 @@ The model owns the query and the open flag; the `for` source is the model-filter
|
||||
|
||||
```html
|
||||
<stack width="240">
|
||||
<combobox placeholder="Search frameworks" text="{framework_query}" on-input="framework_edited" on-press="open_framework_menu" />
|
||||
<if test="{framework_menu_open}">
|
||||
<combobox placeholder="Search frameworks" text="{frameworkQuery}" on-input="framework_edited" on-submit="commit_framework_query" on-press="open_framework_menu" />
|
||||
<if test="{frameworkMenuOpen}">
|
||||
<dropdown-menu anchor="below" anchor-alignment="stretch" on-dismiss="close_framework_menu">
|
||||
<for each="matchingFrameworks" key="id" as="f">
|
||||
<menu-item on-press="pick_framework:{f.id}">{f.name}</menu-item>
|
||||
@@ -24,6 +26,19 @@ The model owns the query and the open flag; the `for` source is the model-filter
|
||||
</stack>
|
||||
```
|
||||
|
||||
In the primary TypeScript core, the submit arm commits the current model-owned query. The open and submit messages remain separate, so Enter can commit without toggling the picker:
|
||||
|
||||
```ts
|
||||
export type Msg =
|
||||
| { readonly kind: "framework_edited"; readonly edit: TextInputEvent }
|
||||
| { readonly kind: "open_framework_menu" }
|
||||
| { readonly kind: "close_framework_menu" }
|
||||
| { readonly kind: "commit_framework_query" };
|
||||
|
||||
case "commit_framework_query":
|
||||
return { ...model, committedFrameworkQuery: model.frameworkQuery, frameworkMenuOpen: false };
|
||||
```
|
||||
|
||||
## Programmatic construction (Zig)
|
||||
|
||||
In a Zig view, the `canvas.Ui` builder constructs the same tree programmatically. `on_input` takes a comptime message constructor: `Ui.inputMsg(.tag)` builds `Msg{ .tag = edit }` for each `canvas.TextInputEvent`.
|
||||
@@ -34,6 +49,7 @@ ui.stack(.{ .width = 240 }, .{
|
||||
.placeholder = "Search frameworks",
|
||||
.text = model.framework_query,
|
||||
.on_input = Ui.inputMsg(.framework_edited),
|
||||
.on_submit = .commit_framework_query,
|
||||
.on_press = .open_framework_menu,
|
||||
}, .{}),
|
||||
if (model.framework_menu_open) ui.el(.dropdown_menu, .{
|
||||
@@ -56,6 +72,7 @@ ui.stack(.{ .width = 240 }, .{
|
||||
"disabled",
|
||||
"on-press",
|
||||
"on-input",
|
||||
"on-submit",
|
||||
"on-dismiss",
|
||||
]}
|
||||
/>
|
||||
|
||||
@@ -710,6 +710,9 @@ fn widgetSemanticPressControlIntent(widget: Widget, actions: WidgetActions) Widg
|
||||
}
|
||||
|
||||
pub fn isWidgetActivationKey(key: []const u8) bool {
|
||||
// Host adapters normalize the physical Return key to "enter" before
|
||||
// it reaches this canonical event vocabulary. AppKit maps both its
|
||||
// carriage-return key event and insertNewline: selector to that name.
|
||||
return std.ascii.eqlIgnoreCase(key, "space") or std.ascii.eqlIgnoreCase(key, "enter");
|
||||
}
|
||||
|
||||
|
||||
@@ -1471,13 +1471,14 @@ pub fn Ui(comptime Msg: type) type {
|
||||
if (widget.semantics.role == .treeitem and keyboard.focus_moved) {
|
||||
if (self.msgFor(target_id, .change)) |msg| return msg;
|
||||
}
|
||||
// A list row prefers a bound submit handler on plain
|
||||
// Enter: Enter is the row's PRIMARY action (open the
|
||||
// record, play the track — the desktop list convention),
|
||||
// while Space keeps the select activation below. Only
|
||||
// rows that bind `on_submit` take this branch; everything
|
||||
// else resolves exactly as before.
|
||||
if (widget.kind == .list_item and isSubmitKeyboard(widget, keyboard)) {
|
||||
// List rows and comboboxes prefer a bound submit handler
|
||||
// on plain Enter before their activation intent: Enter is
|
||||
// the row's PRIMARY action or commits the combobox text,
|
||||
// while Space and the combobox open arrows keep the
|
||||
// activation below. Only widgets that bind `on_submit`
|
||||
// return from this branch; everything else resolves
|
||||
// exactly as before.
|
||||
if ((widget.kind == .list_item or widget.kind == .combobox) and isSubmitKeyboard(widget, keyboard)) {
|
||||
if (self.msgFor(target_id, .submit)) |msg| return msg;
|
||||
}
|
||||
if (canvas.widgetKeyboardControlIntent(widget, keyboard)) |intent| {
|
||||
|
||||
@@ -993,6 +993,9 @@ test "compiled catalog elements match the interpreter and the hand-written view"
|
||||
interpreted.msgForKeyboard(input.id, submit).?,
|
||||
compiled.msgForKeyboard(input.id, submit).?,
|
||||
);
|
||||
const combobox = fixture.findByKind(compiled.root, .combobox).?;
|
||||
try testing.expectEqual(fixture.CatalogMsg.submit_query, compiled.msgForKeyboard(combobox.id, submit).?);
|
||||
try testing.expectEqual(fixture.CatalogMsg.open_picker, compiled.msgForKeyboard(combobox.id, .{ .phase = .key_down, .key = "space" }).?);
|
||||
}
|
||||
|
||||
test "compiled catalog stays in parity when conditional surfaces flip" {
|
||||
|
||||
@@ -2214,7 +2214,7 @@ pub const catalog_markup_source =
|
||||
\\ </row>
|
||||
\\ <row gap="8">
|
||||
\\ <input text="{query}" placeholder="Name" autofocus="true" on-input="query_edit" on-submit="submit_query" grow="1" />
|
||||
\\ <combobox text="{query}" placeholder="Search fruit" on-input="query_edit" />
|
||||
\\ <combobox text="{query}" placeholder="Search fruit" on-input="query_edit" on-press="open_picker" on-submit="submit_query" />
|
||||
\\ </row>
|
||||
\\ <radio-group gap="4" label="Formatting">
|
||||
\\ <radio checked="{bold}" on-change="toggle_bold" label="Bold" />
|
||||
@@ -2344,7 +2344,7 @@ pub fn handCatalogView(ui: *CatalogUi, model: *const CatalogModel) CatalogUi.Nod
|
||||
}),
|
||||
ui.row(.{ .gap = 8 }, .{
|
||||
ui.el(.input, .{ .text = model.query, .placeholder = "Name", .autofocus = true, .on_input = CatalogUi.inputMsg(.query_edit), .on_submit = .submit_query, .grow = 1 }, .{}),
|
||||
ui.el(.combobox, .{ .text = model.query, .placeholder = "Search fruit", .on_input = CatalogUi.inputMsg(.query_edit) }, .{}),
|
||||
ui.el(.combobox, .{ .text = model.query, .placeholder = "Search fruit", .on_input = CatalogUi.inputMsg(.query_edit), .on_press = .open_picker, .on_submit = .submit_query }, .{}),
|
||||
}),
|
||||
ui.el(.radio_group, .{ .gap = 4, .semantics = .{ .label = "Formatting" } }, .{
|
||||
ui.el(.radio, .{ .checked = model.bold, .on_change = .toggle_bold }, .{}),
|
||||
@@ -2489,6 +2489,10 @@ test "catalog elements build the hand-written tree and dispatch typed messages"
|
||||
try testing.expectEqualStrings("q", markup_tree.msgForKeyboard(input.id, typed).?.query_edit.insert_text);
|
||||
const submit = canvas.WidgetKeyboardEvent{ .phase = .key_down, .key = "enter" };
|
||||
try testing.expectEqual(CatalogMsg.submit_query, markup_tree.msgForKeyboard(input.id, submit).?);
|
||||
const combobox = findByKind(markup_tree.root, .combobox).?;
|
||||
try testing.expectEqual(CatalogMsg.submit_query, markup_tree.msgForKeyboard(combobox.id, submit).?);
|
||||
try testing.expectEqual(CatalogMsg.open_picker, markup_tree.msgForKeyboard(combobox.id, .{ .phase = .key_down, .key = "space" }).?);
|
||||
try testing.expectEqual(CatalogMsg.open_picker, markup_tree.msgForKeyboard(combobox.id, .{ .phase = .key_down, .key = "arrowdown" }).?);
|
||||
|
||||
// The whole catalog lays out through the canvas engine.
|
||||
var nodes: [256]canvas.WidgetLayoutNode = undefined;
|
||||
|
||||
@@ -349,6 +349,43 @@ test "keyboard events resolve activation and submit messages" {
|
||||
try testing.expectEqual(@as(?Msg, null), tree.msgForKeyboard(checkbox.id, letter));
|
||||
}
|
||||
|
||||
test "combobox Enter prefers submit while its other open keys still press" {
|
||||
var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
|
||||
defer arena_state.deinit();
|
||||
|
||||
var ui = InboxUi.init(arena_state.allocator());
|
||||
const tree = try ui.finalize(ui.column(.{}, .{
|
||||
ui.el(.combobox, .{
|
||||
.text = "both handlers",
|
||||
.on_press = .load_more,
|
||||
.on_submit = .add,
|
||||
}, .{}),
|
||||
ui.el(.combobox, .{
|
||||
.text = "press only",
|
||||
.on_press = .load_more,
|
||||
}, .{}),
|
||||
ui.el(.combobox, .{
|
||||
.text = "submit only",
|
||||
.on_submit = .add,
|
||||
}, .{}),
|
||||
}));
|
||||
|
||||
const both = tree.root.children[0];
|
||||
const press_only = tree.root.children[1];
|
||||
const submit_only = tree.root.children[2];
|
||||
const enter = canvas.WidgetKeyboardEvent{ .phase = .key_down, .key = "enter" };
|
||||
const space = canvas.WidgetKeyboardEvent{ .phase = .key_down, .key = "space" };
|
||||
const arrow_down = canvas.WidgetKeyboardEvent{ .phase = .key_down, .key = "arrowdown" };
|
||||
const arrow_up = canvas.WidgetKeyboardEvent{ .phase = .key_down, .key = "arrowup" };
|
||||
|
||||
try testing.expectEqual(Msg.add, tree.msgForKeyboard(both.id, enter).?);
|
||||
try testing.expectEqual(Msg.load_more, tree.msgForKeyboard(both.id, space).?);
|
||||
try testing.expectEqual(Msg.load_more, tree.msgForKeyboard(both.id, arrow_down).?);
|
||||
try testing.expectEqual(Msg.load_more, tree.msgForKeyboard(both.id, arrow_up).?);
|
||||
try testing.expectEqual(Msg.load_more, tree.msgForKeyboard(press_only.id, enter).?);
|
||||
try testing.expectEqual(Msg.add, tree.msgForKeyboard(submit_only.id, enter).?);
|
||||
}
|
||||
|
||||
test "tree keyboard navigation can select without dispatching pointer activation" {
|
||||
var arena_state = std.heap.ArenaAllocator.init(testing.allocator);
|
||||
defer arena_state.deinit();
|
||||
|
||||
@@ -2182,6 +2182,8 @@ test "runtime dispatches canvas widget commands from pointer and keyboard activa
|
||||
try std.testing.expectEqual(@as(u32, 6), app_state.command_count);
|
||||
|
||||
harness.runtime.views[0].canvas_widget_focused_id = 6;
|
||||
// Command-string widgets have no typed on-submit channel, so their
|
||||
// combobox Enter behavior intentionally remains trigger activation.
|
||||
try harness.runtime.dispatchPlatformEvent(app, .{ .gpu_surface_input = .{
|
||||
.window_id = 1,
|
||||
.label = "canvas",
|
||||
|
||||
@@ -3151,12 +3151,16 @@ const ComboMirrorModel = struct {
|
||||
note: canvas.TextBuffer(64) = .{},
|
||||
open: bool = false,
|
||||
opens: u32 = 0,
|
||||
submits: u32 = 0,
|
||||
picks: u32 = 0,
|
||||
query_edits: u32 = 0,
|
||||
};
|
||||
|
||||
const ComboMirrorMsg = union(enum) {
|
||||
open_picker,
|
||||
close_picker,
|
||||
submit_query,
|
||||
pick_query: []const u8,
|
||||
query_edit: canvas.TextInputEvent,
|
||||
note_edit: canvas.TextInputEvent,
|
||||
};
|
||||
@@ -3170,6 +3174,16 @@ fn comboMirrorUpdate(model: *ComboMirrorModel, msg: ComboMirrorMsg) void {
|
||||
model.opens += 1;
|
||||
},
|
||||
.close_picker => model.open = false,
|
||||
.submit_query => {
|
||||
model.open = false;
|
||||
model.submits += 1;
|
||||
},
|
||||
.pick_query => |query| {
|
||||
model.query.clear();
|
||||
model.query.apply(.{ .insert_text = query });
|
||||
model.open = false;
|
||||
model.picks += 1;
|
||||
},
|
||||
.query_edit => |edit| {
|
||||
model.query.apply(edit);
|
||||
model.query_edits += 1;
|
||||
@@ -3185,6 +3199,7 @@ fn comboMirrorView(ui: *ComboMirrorApp.Ui, model: *const ComboMirrorModel) Combo
|
||||
.width = 200,
|
||||
.expanded = model.open,
|
||||
.on_press = .open_picker,
|
||||
.on_submit = .submit_query,
|
||||
.on_input = ComboMirrorApp.Ui.inputMsg(.query_edit),
|
||||
}, .{});
|
||||
const picker = if (model.open) ui.stack(.{ .height = 28 }, .{
|
||||
@@ -3196,8 +3211,8 @@ fn comboMirrorView(ui: *ComboMirrorApp.Ui, model: *const ComboMirrorModel) Combo
|
||||
.height = 60,
|
||||
.on_dismiss = .close_picker,
|
||||
}, .{
|
||||
ui.el(.menu_item, .{ .key = .{ .int = 0 }, .text = "glass bead", .height = 26, .on_press = .close_picker }, .{}),
|
||||
ui.el(.menu_item, .{ .key = .{ .int = 1 }, .text = "glass jar", .height = 26, .on_press = .close_picker }, .{}),
|
||||
ui.el(.menu_item, .{ .key = .{ .int = 0 }, .text = "glass bead", .height = 26, .on_press = ComboMirrorMsg{ .pick_query = "glass bead" } }, .{}),
|
||||
ui.el(.menu_item, .{ .key = .{ .int = 1 }, .text = "glass jar", .height = 26, .on_press = ComboMirrorMsg{ .pick_query = "glass jar" } }, .{}),
|
||||
}),
|
||||
}) else ui.stack(.{ .height = 28 }, .{trigger});
|
||||
return ui.column(.{ .gap = 8, .padding = 12 }, .{
|
||||
@@ -3237,7 +3252,7 @@ fn comboMirrorRetainedSelection(harness: *core.TestHarness(), id: canvas.ObjectI
|
||||
return layout.findById(id).?.widget.text_selection;
|
||||
}
|
||||
|
||||
test "a closed combobox's open arrows move neither the retained caret nor the model mirror" {
|
||||
test "combobox submit precedence and open-menu selection keep the text mirror consistent" {
|
||||
// The split-brain escapee: a CLOSED combobox maps ArrowUp/Down to
|
||||
// BOTH its open press (`widgetKeyboardControlIntent`'s menu-open
|
||||
// keys) and — through the single-line caret derivation — a stamped
|
||||
@@ -3292,6 +3307,19 @@ test "a closed combobox's open arrows move neither the retained caret nor the mo
|
||||
try std.testing.expectEqualStrings("glass", app_state.model.query.text());
|
||||
try std.testing.expectEqualDeep(canvas.TextSelection.collapsed(3), app_state.model.query.selection);
|
||||
|
||||
// Enter resolves through on_submit BEFORE the trigger's open press.
|
||||
// The closed picker stays closed and neither side of the text mirror
|
||||
// changes while the submit message commits.
|
||||
const edits_before_submit = app_state.model.query_edits;
|
||||
try comboMirrorKey(harness, app, "enter");
|
||||
try std.testing.expect(!app_state.model.open);
|
||||
try std.testing.expectEqual(@as(u32, 0), app_state.model.opens);
|
||||
try std.testing.expectEqual(@as(u32, 1), app_state.model.submits);
|
||||
try std.testing.expectEqual(edits_before_submit, app_state.model.query_edits);
|
||||
try std.testing.expectEqualStrings("glass", app_state.model.query.text());
|
||||
try std.testing.expectEqualDeep(canvas.TextSelection.collapsed(3), app_state.model.query.selection);
|
||||
try std.testing.expectEqualDeep(@as(?canvas.TextSelection, canvas.TextSelection.collapsed(3)), try comboMirrorRetainedSelection(harness, combo_id));
|
||||
|
||||
// THE pin: ArrowDown on the closed trigger opens the picker and
|
||||
// both carets stay at 3 — no query edit is heard or applied.
|
||||
const edits_before_open = app_state.model.query_edits;
|
||||
@@ -3302,9 +3330,10 @@ test "a closed combobox's open arrows move neither the retained caret nor the mo
|
||||
try std.testing.expectEqualDeep(canvas.TextSelection.collapsed(3), app_state.model.query.selection);
|
||||
try std.testing.expectEqualDeep(@as(?canvas.TextSelection, canvas.TextSelection.collapsed(3)), try comboMirrorRetainedSelection(harness, combo_id));
|
||||
|
||||
// The OPEN-picker truth, pinned as-is: the next arrow walks the
|
||||
// keyboard INTO the mounted menu (the focus step consumes it before
|
||||
// routing reaches the trigger), so it is no caret edit either.
|
||||
// The OPEN-picker truth: the next arrow walks the keyboard INTO the
|
||||
// mounted menu (the focus step consumes it before routing reaches
|
||||
// the trigger), so Enter selects that menu item instead of reaching
|
||||
// the combobox submit handler again.
|
||||
const first_item_id = findWidgetIdByText(app_state.tree.?, .menu_item, "glass bead").?;
|
||||
try comboMirrorKey(harness, app, "arrowdown");
|
||||
try std.testing.expectEqual(first_item_id, harness.runtime.views[0].canvas_widget_focused_id);
|
||||
@@ -3312,12 +3341,12 @@ test "a closed combobox's open arrows move neither the retained caret nor the mo
|
||||
try std.testing.expectEqualDeep(canvas.TextSelection.collapsed(3), app_state.model.query.selection);
|
||||
try std.testing.expectEqualDeep(@as(?canvas.TextSelection, canvas.TextSelection.collapsed(3)), try comboMirrorRetainedSelection(harness, combo_id));
|
||||
|
||||
// Escape is consumed by the DISMISSAL pass while the menu floats:
|
||||
// the picker closes through `on_dismiss` and the combobox's
|
||||
// Escape-clear never runs — the query survives.
|
||||
try comboMirrorKey(harness, app, "escape");
|
||||
try comboMirrorKey(harness, app, "enter");
|
||||
try std.testing.expect(!app_state.model.open);
|
||||
try std.testing.expectEqualStrings("glass", app_state.model.query.text());
|
||||
try std.testing.expectEqual(@as(u32, 1), app_state.model.submits);
|
||||
try std.testing.expectEqual(@as(u32, 1), app_state.model.picks);
|
||||
try std.testing.expectEqualStrings("glass bead", app_state.model.query.text());
|
||||
try std.testing.expectEqualStrings("glass bead", (try harness.runtime.canvasWidgetLayout(1, combo_mirror_canvas_label)).findById(combo_id).?.widget.text);
|
||||
try std.testing.expectEqual(combo_id, harness.runtime.views[0].canvas_widget_focused_id);
|
||||
|
||||
// ArrowUp on the closed trigger is the same open key: opens, and
|
||||
@@ -3326,8 +3355,8 @@ test "a closed combobox's open arrows move neither the retained caret nor the mo
|
||||
try std.testing.expect(app_state.model.open);
|
||||
try std.testing.expectEqual(@as(u32, 2), app_state.model.opens);
|
||||
try std.testing.expectEqual(edits_before_open, app_state.model.query_edits);
|
||||
try std.testing.expectEqualDeep(canvas.TextSelection.collapsed(3), app_state.model.query.selection);
|
||||
try std.testing.expectEqualDeep(@as(?canvas.TextSelection, canvas.TextSelection.collapsed(3)), try comboMirrorRetainedSelection(harness, combo_id));
|
||||
try std.testing.expectEqualDeep(canvas.TextSelection.collapsed("glass bead".len), app_state.model.query.selection);
|
||||
try std.testing.expectEqualDeep(@as(?canvas.TextSelection, canvas.TextSelection.collapsed("glass bead".len)), try comboMirrorRetainedSelection(harness, combo_id));
|
||||
try comboMirrorKey(harness, app, "escape");
|
||||
try std.testing.expect(!app_state.model.open);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user