From 5dadda01d514df6662e65718cbb86a7b9dcca285 Mon Sep 17 00:00:00 2001 From: foxnne Date: Fri, 16 Oct 2026 16:16:58 -0500 Subject: [PATCH] layout: the picker lists a view under the place showing it Follow-up to #466, from its review. Sections went by keywords alone, so a view the user had moved (Files dragged onto the Panel) was lit in the Panel's picker but listed under "Sidebar". A card now sits under the place holding it, and falls back to its keywords only when no place does. Places resolved by name (a split's leaf, a float) are each their own section rather than folded into a keyword group, so a view put in one reads under its name. The place being edited always leads with its own header. A split's new leaf accepts no view's keywords, so its picker used to open on other places' headers alone; it now says "Nothing here yet." under its name. Each card's section is worked out once per frame, not once per section. Co-Authored-By: Claude Opus 5.5 --- app/layout/Picker.zig | 92 +++++++++++++++++++++++++++++++------------ tests/integration.zig | 11 ++++++ 2 files changed, 77 insertions(+), 26 deletions(-) diff --git a/app/layout/Picker.zig b/app/layout/Picker.zig index 6553951e5..b46cfb932 100644 --- a/app/layout/Picker.zig +++ b/app/layout/Picker.zig @@ -11,9 +11,10 @@ //! Cards assign a surface here. That is how a sidebar view stays in the //! sidebar, and how a plugin readme lands in the center — keywords are the //! default guess, an assignment is the answer. "Back to defaults" undoes it. -//! The cards are listed under the place their keywords are for — this place -//! first, then each other place by its name, then "Other" — so a view made for -//! the panel reads as one, wherever the picker was opened. +//! The cards are listed under the place holding them, or else the place their +//! keywords are for — this place first, then each other place by its name, then +//! "Other" — so a view made for the panel reads as one, wherever the picker was +//! opened, and a view the user moved reads under where it went. //! //! A card is also a handle. Dragged out of the popup it becomes the same //! floating view the corner button lifts out of a place (`ViewDrag`), and @@ -275,16 +276,24 @@ pub fn draw(self: *Picker, f: *Layout) void { var col: usize = 0; const places = placesFor(f, ®ion); const ranked_all = offered(f, ®ion, &query); - // Each card under the place its keywords make it for, this one first, so what belongs in - // the explorer and what in the panel reads off the headers; a query ranks within each. + // Each card under the place holding it, or else the place its keywords make it for; this one + // first, so what belongs in the explorer and what in the panel reads off the headers. A + // query ranks within each. Sorted once, not per section. + const sections = f.arena.alloc(usize, ranked_all.len) catch return; + for (ranked_all, sections) |ranked, *section| { + const s = &f.host.surfaces.items[ranked.item]; + section.* = sectionOf(places, s.id, s.keywords) orelse places.len; + } var current: ?usize = null; - for (0..places.len + 1) |section| for (ranked_all) |ranked| { + for (0..places.len + 1) |section| for (ranked_all, sections) |ranked, card_section| { // `i` is the surface's place in the registry, not in this list: a card keeps its widget // id while the filter reorders what is around it. const i = ranked.item; const s = &f.host.surfaces.items[i]; - if ((sectionOf(places, s.keywords) orelse places.len) != section) continue; + if (card_section != section) continue; if (current != section) { + // This place's own header was drawn above, empty, when nothing else came first. + if (current == null and section != 0) emptyHome(®ion, &query); current = section; if (row) |r| r.deinit(); row = null; @@ -348,6 +357,7 @@ pub fn draw(self: *Picker, f: *Layout) void { state.markDirty(); dvui.refresh(null, @src(), null); }; + if (current == null) emptyHome(®ion, &query); if (state.store_catalog) |store| drawStoreSection(f, ®ion, store, &row, &col, &query); } @@ -367,34 +377,38 @@ fn offered(f: *Layout, region: *const Layout.Region, query: *const core.fuzzy.Qu return out.items; } -/// A place a card can be listed under: a region's name and the keywords it accepts. +/// A place a card can be listed under: a region's name, the keywords it accepts and the views it +/// holds now. const Place = struct { name: []const u8, keywords: []const []const u8, + holds: []const *sdk.Surface = &.{}, }; -/// The places the picker sorts cards under: `region` itself first, then each other kind of place -/// the shape declared, in its order. One per keyword group — the icon rail and the explorer it -/// chooses for are one place — and never a document pane, which offers only documents. +/// The places the picker sorts cards under: `region` itself first, then each other place the +/// shape declared, in its order — never a document pane, which offers only documents. Places +/// resolved by keywords are one per keyword group (the icon rail and the explorer it chooses for +/// are one place); a place resolved by name (a split's leaf, a float) is its own. fn placesFor(f: *Layout, region: *const Layout.Region) []const Place { var out: std.ArrayListUnmanaged(Place) = .empty; - out.append(f.arena, .{ .name = region.name, .keywords = region.keywords }) catch return &.{}; + out.append(f.arena, .{ .name = region.name, .keywords = region.keywords, .holds = f.matchingIn(region) }) catch return &.{}; for (f.state.regions.items) |*r| { if (r.name.len == 0 or r.kind_slot) continue; - const key = sdk.keywords.groupKey(r.keywords); const seen = for (out.items) |p| { - if (sdk.keywords.groupKey(p.keywords) == key) break true; + if (std.mem.eql(u8, p.name, r.name)) break true; + if (!r.by_name and sdk.keywords.groupKey(p.keywords) == sdk.keywords.groupKey(r.keywords)) break true; } else false; - if (!seen) out.append(f.arena, .{ .name = r.name, .keywords = r.keywords }) catch break; + if (!seen) out.append(f.arena, .{ .name = r.name, .keywords = r.keywords, .holds = f.matchingIn(r) }) catch break; } return out.items; } -/// The place a surface is made for: the one accepting its keywords most strongly, the first of -/// equals — `places[0]`, the place being edited, when it is among them. Null when none accepts it. -/// The same fit `Layout` binds by (`sdk.keywords.strength`), so the header a card sits under is -/// where its keywords would put it. -fn sectionOf(places: []const Place, keywords: []const []const u8) ?usize { +/// The place a card is listed under. The place holding the view, when one does: a view the user +/// moved reads under where it is, not where it came from. Otherwise the place its keywords are +/// for, by the fit `Layout` binds with (`sdk.keywords.strength`); between equals the first, +/// which puts `places[0]`, the place being edited, ahead of the rest. Null when neither. +fn sectionOf(places: []const Place, id: []const u8, keywords: []const []const u8) ?usize { + for (places, 0..) |p, i| if (contains(p.holds, id)) return i; var best: ?usize = null; var best_fit: sdk.keywords.Fit = .none; for (places, 0..) |p, i| { @@ -414,14 +428,40 @@ test "a view is listed under the place its keywords are for, the edited place fi .{ .name = "Sidebar", .keywords = kw.sidebar }, .{ .name = "Main", .keywords = kw.main }, }; - try std.testing.expectEqual(@as(?usize, 1), sectionOf(&places, &.{"explorer"})); - try std.testing.expectEqual(@as(?usize, 0), sectionOf(&places, &.{"output"})); + try std.testing.expectEqual(@as(?usize, 1), sectionOf(&places, "v", &.{"explorer"})); + try std.testing.expectEqual(@as(?usize, 0), sectionOf(&places, "v", &.{"output"})); // Made for both: the place being edited wins the tie. - try std.testing.expectEqual(@as(?usize, 0), sectionOf(&places, &.{ "sidebar", "panel" })); + try std.testing.expectEqual(@as(?usize, 0), sectionOf(&places, "v", &.{ "sidebar", "panel" })); // A specific sub-place still lands under the area that holds it. - try std.testing.expectEqual(@as(?usize, 2), sectionOf(&places, &.{"main.document"})); - try std.testing.expectEqual(@as(?usize, null), sectionOf(&places, &.{"inspector"})); - try std.testing.expectEqual(@as(?usize, null), sectionOf(&places, &.{})); + try std.testing.expectEqual(@as(?usize, 2), sectionOf(&places, "v", &.{"main.document"})); + try std.testing.expectEqual(@as(?usize, null), sectionOf(&places, "v", &.{"inspector"})); + try std.testing.expectEqual(@as(?usize, null), sectionOf(&places, "v", &.{})); +} + +test "a view a place holds is listed under that place, whatever its keywords" { + var files: sdk.Surface = .{ .id = "files", .title = "Files", .keywords = sdk.keywords.ide.sidebar, .draw = undefined }; + const holding = [_]*sdk.Surface{&files}; + const places = [_]Place{ + .{ .name = "Sidebar", .keywords = sdk.keywords.ide.sidebar }, + .{ .name = "Panel", .keywords = sdk.keywords.ide.panel, .holds = &holding }, + }; + try std.testing.expectEqual(@as(?usize, 1), sectionOf(&places, "files", files.keywords)); + // A split's new leaf accepts no view's keywords, and still takes the one put in it. + const leaf = [_]Place{ + .{ .name = "Panel/r1", .keywords = &.{"slot"}, .holds = &holding }, + .{ .name = "Sidebar", .keywords = sdk.keywords.ide.sidebar }, + }; + try std.testing.expectEqual(@as(?usize, 0), sectionOf(&leaf, "files", files.keywords)); +} + +/// The place being edited, when no card falls under it: its header still leads, so the popup +/// says which place it fills — a split's new leaf accepts no view's keywords until one is put in. +fn emptyHome(region: *const Layout.Region, query: *const core.fuzzy.Query) void { + sectionHeader(region.name, 0); + dvui.labelNoFmt(@src(), if (query.isEmpty()) "Nothing here yet." else "No matching views here.", .{}, .{ + .padding = .{ .x = 4, .h = 4 }, + .color_text = .{ .color = dvui.themeGet().color(.window, .text).opacity(0.45) }, + }); } /// A heading over a run of cards — a place's name, or the store. diff --git a/tests/integration.zig b/tests/integration.zig index 6389726de..c1583c5ef 100644 --- a/tests/integration.zig +++ b/tests/integration.zig @@ -2513,6 +2513,17 @@ test "the picker lists each view under the place it is made for, the open place for (0..4) |_| _ = try dvui.testing.step(PickerFrame.frame); try std.testing.expect(try y("test.files") < try y("test.output")); try std.testing.expect(try y("test.output") < try y("test.misc")); + + // Files moved into the panel reads under the panel, in one row with Output, from either + // picker. + try editor.app.layout.assign(editor.app.gpa, "Panel", &.{ "test.output", "test.files" }); + for (0..4) |_| _ = try dvui.testing.step(PickerFrame.frame); + try std.testing.expectEqual(try y("test.output"), try y("test.files")); + try std.testing.expect(try y("test.output") < try y("test.misc")); + editor.app.layout.openPicker(editor.app.gpa, "Panel", null); + for (0..4) |_| _ = try dvui.testing.step(PickerFrame.frame); + try std.testing.expectEqual(try y("test.output"), try y("test.files")); + try std.testing.expect(try y("test.output") < try y("test.misc")); } test "a hide_when_empty panel stays while its view is dragged onto main" {